sys-info-rs icon indicating copy to clipboard operation
sys-info-rs copied to clipboard

Build fail for riscv64 due to test case with cpu_speed()

Open yuzibo opened this issue 2 years ago • 2 comments

Hi, From Debian rust sys-info package which FBTFS fail: https://buildd.debian.org/status/package.php?p=rust-sys-info&suite=sid I submit a patch to pass the tests case: https://salsa.debian.org/rust-team/debcargo-conf/-/merge_requests/295 But It seems very suboptimally. So what is the best solution for such case? The /proc/cpuinfo is below:

root@unmatched-local:~/git/debcargo-conf/src/sys-info/debian# cat /proc/cpuinfo processor       : 0 hart            : 1 isa             : rv64imafdc mmu             : sv39 uarch           : sifive,bullet0  processor       : 1 hart            : 2 isa             : rv64imafdc mmu             : sv39 uarch           : sifive,bullet0  processor       : 2 hart            : 3 isa             : rv64imafdc mmu             : sv39 uarch           : sifive,bullet0  processor       : 3 hart            : 4 isa             : rv64imafdc mmu             : sv39 uarch           : sifive,bullet0

If no other options, I will send RP to pass the case that is the same patch for Debian.

yuzibo avatar Apr 21 '22 05:04 yuzibo

The /proc/cpuinfo file is very variable across Linux architectures and even varies at runtime, as an install moves between CPUs (such as with a Linux live image) or even as you switch between Linux kernel versions (since newer ones might expose more info on very new hardware such as RISC-V). So the CPU speed test should probably not be run at build time. Instead the API should indicate that the function can return nothing/error at runtime and users of the API should be expected to handle that at runtime.

The CPU speed feature also isn't very useful, since at least on my Intel system, the cpu MHz field is different for each core and different each time that I read the /proc/cpuinfo file. So perhaps the CPU speed feature should be deprecated and later removed.

pabs3 avatar Apr 21 '22 06:04 pabs3

+1 for @pabs3 comment. That field does not provide anything useful as clocks will change rapidly, and they are different per core. This part should be modified to incl. riscv64: https://github.com/FillZpp/sys-info-rs/blob/60ecf1470a5b7c90242f429934a3bacb6023ec4d/lib.rs#L600C1-L603C6 and it will return: Err(Error::UnsupportedSystem)

davidlt avatar Feb 25 '24 11:02 davidlt