rust-embedded / riscv

Low level access to RISC-V processors
818 stars 160 forks source link

riscv: build: make `cfg` variables more robust #205

Closed rmsyn closed 3 months ago

rmsyn commented 3 months ago

Change which Cargo CFG environment variables are used to select the custom cfg variables.

See https://github.com/rust-embedded/riscv/pull/204#issuecomment-2094089195 for discussion.

rmsyn commented 3 months ago

I don't know why the clippy checks are suddenly failing now. I run the checks locally from CI runner:

cargo clippy --package riscv-rt --all --features=s-mode -- -D warnings

The checks pass. :thinking:

romancardenas commented 3 months ago

In nightly, clippy now complains about "custom" compiler features (check this).

So, as suggested by the CI, now we should add to build.rs something like:

println!("cargo::rustc-check-cfg=cfg(riscv)");
println!("cargo::rustc-check-cfg=cfg(riscv32)");
println!("cargo::rustc-check-cfg=cfg(riscv64)");
rmsyn commented 3 months ago

Build checks on 1.60 machines are failing because of bumped MSRV for the cfg checks.

Without an MSRV bump, the clippy checks have the following error:

cargo +nightly clippy --all --no-default-features --target riscv64imac-unknown-none-elf -- -D warnings
   Compiling riscv v0.11.1 (/riscv/riscv)
   Compiling riscv-rt v0.13.0 (/riscv/riscv-rt)
error: the `cargo::` syntax for build script output instructions was added in Rust 1.77.0, but the minimum supported Rust version of `riscv v0.11.1 (/riscv/riscv)` is 1.60.
See https://doc.rust-lang.org/cargo/reference/build-scripts.html#outputs-of-the-build-script for more information about build script outputs.
romancardenas commented 3 months ago

I don't like the idea of bumping MSRV from 1.60 to 1.77 due to a nightly clippy issue... I looked at the line provided by your post and:

Note: The old invocation prefix cargo: (one colon only) is deprecated and won’t get any new features. To migrate, use two-colons prefix cargo::, which was added in Rust 1.77. If you were using cargo:KEY=VALUE for arbitrary links manifest key-value pairs, it is encouraged to switch to cargo::metadata=KEY=VALUE. Stick to cargo: only if the support of Rust version older than 1.77 is required.

I suggest using just : instead of :: until we face this issue in stable clippy.

rmsyn commented 3 months ago

I don't like the idea of bumping MSRV from 1.60 to 1.77 due to a nightly clippy issue

I didn't like it either. Looks like the single colon fixed the warning/error. Weird that the compiler would suggest the non-MSRV compatible fix... Probably pretty involved for it to do otherwise.

Either way, the CI looks like it's fixed now. Thank you for the suggestion @romancardenas.