search

dalek-cryptography / curve25519-dalek

A pure-Rust implementation of group operations on Ristretto and Curve25519
Other
850 stars 416 forks source link

Address `nightly-2024-05-05` warns #652

Closed pinkforest closed 2 months ago

pinkforest commented 2 months ago

1.80 supposedly will and nightly (2024-05-05) has introduced cfg-check spitting out some warnings.

The problem is if we declare the cfg's as intended - this will then complain about MSRV despite where we would only print these flags when rustc is >= 1.77 a.k.a this feature hard-checks MSRV and then asks to bump MSRV 1.77 despite build.rs never using the feature as gated in build.rs for below <= 1.77 as checked - I've asked clarification whether this is intended.

In order to mitigate this we can just temporarily allow the status-quo and when MSRV jumps to 1.77 we can declare them when MSRV bumps to 1.77 - it seems we can still use them just fine without making MSRV check failing.

Also need to allow two occassions unnecessary qualification (group::ff) where when only group feature is used when ff is available directrly where as it's always available via group::ff - leading to lint complaining.

EDIT: The derive conditional one is cursed - need to check that later but it seems that lint there cannot be disabled for some reason - so temporarily just made it not complain as it was in test - need to double-check it

tarcieri commented 2 months ago

Is there an issue with emitting e.g. cargo::rustc-check-cfg=cfg(curve25519_dalek_backend) from build.rs?

pinkforest commented 2 months ago

Yes it will result to MSRV check and it has a hard complaint to require bumping to 1.77 regardless if this is gated only to >= 1.77- fwiw I flagged this and asked whether it was intentional given bumping to MSRV 1.77 at 1.80 isn't always doable

tarcieri commented 2 months ago

How does that actually manifest? Do older rusts complain about it?

pinkforest commented 2 months ago

No the new versions complain that is the problem - where we actually would declare them and where it takes effect.

tarcieri commented 2 months ago

Aah, I see now:

error: the cargo:: syntax for build script output instructions was added in Rust 1.77.0, but the minimum supported Rust version of curve25519-dalek v4.1.2 (/Users/tony/src/curve25519-dalek/curve25519-dalek) is 1.60.0. See https://doc.rust-lang.org/cargo/reference/build-scripts.html#outputs-of-the-build-script for more information about build script outputs.