Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fiat backend MSRV is 1.57.0 #462

Closed
pinkforest opened this issue Dec 9, 2022 · 6 comments
Closed

fiat backend MSRV is 1.57.0 #462

pinkforest opened this issue Dec 9, 2022 · 6 comments

Comments

@pinkforest
Copy link
Contributor

When testing cfg(curve25519_dalek_serial) addressing #437

foobar@foobar-MS-7C77:~/cc/curve25519-dalek$ env RUSTFLAGS="--cfg curve25519_dalek_serial="fiat"" cargo test

error: package `os_str_bytes v6.4.1` cannot be built because it requires rustc 1.57.0 or newer, while the currently active rustc version is 1.56.1
@tarcieri
Copy link
Contributor

tarcieri commented Dec 9, 2022

That appears to be unrelated to fiat-crypto which has no dependencies

For starters, --cfg curve25519_dalek_serial="fiat" is not the correct name for the cfg directive (and this is a good example of a downside of using cfg, namely we get no checks for invalid cfg attributes). It should be curve25519_dalek_backend="fiat". nm, I guess you were trying to add that in #463

os_str_bytes is being pulled in as:

dev-dependencies -> criterion -> clap v3 -> clap_lex -> os_str_bytes

...so this should only affect the MSRV when running cargo test, not the MSRV of the overall crate.

We also use -Z minimal-versions to test MSRV.

@pinkforest
Copy link
Contributor Author

Ah that makes sense, closing.

@pinkforest
Copy link
Contributor Author

pinkforest commented Dec 9, 2022

We also use -Z minimal-versions to test MSRV.

CI still hardcodes 1.56.1 - -Z minimal is only used for getting the Cargo.lock w/ nightly ?

It doesn't actually test MSRV .. maybe it should ? -Z minimal-versions was stabilised in 1.56 me thinks ?

    # First run `cargo +nightly -Z minimal-verisons check` in order to get a
    # Cargo.lock with the oldest possible deps
    - uses: dtolnay/rust-toolchain@nightly
    - run: cargo -Z minimal-versions check --no-default-features --features serde
    # Now check that `cargo build` works with respect to the oldest possible
    # deps and the stated MSRV
    - uses: dtolnay/[email protected]
    - run: cargo build --no-default-features --features serde

@tarcieri
Copy link
Contributor

tarcieri commented Dec 9, 2022

These parts of the job:

 - uses: dtolnay/[email protected]
 - run: cargo build --no-default-features --features serde

...check that it builds successfully with 1.56.1, the advertised MSRV.

@pinkforest
Copy link
Contributor Author

pinkforest commented Dec 9, 2022

yes - build yes but not test plus 1.56.1 is hardcoded there - I always wondered how to avoid hardcoding it in CI :)

@tarcieri
Copy link
Contributor

tarcieri commented Dec 9, 2022

Aah, yeah it should probably test MSRV.

That’s easy enough to do by adding MSRV and stable to the test matrix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants