rust-lang / cargo

The Rust package manager
https://doc.rust-lang.org/cargo
Apache License 2.0
12.58k stars 2.39k forks source link

Hard to work with crates that need versions kept in-sync #12734

Open nwalfield opened 11 months ago

nwalfield commented 11 months ago

Problem

Version 1.16.0 of sequoia-openpgp depends on win-crypto-ng (version 0.4 or 0.5), and ed25519-dalek (version 1).

ed25519-dalek depends on version 0.5 of rand_core and 0.7 of rand.

Version 0.5.1 of win-crypto-ng depends on rand_core 0.5 or 0.6. (0.5.0 depended only on version 0.5 of rand_core.)

We call ed25519_dalek::Keypair::generate. It takes a parameter, which has to implement rand::CryptoRng and rand::RngCore. These traits are taken from rand 0.7, which just reexports the symbols from rand_core 0.5.

We provide ed25519_dalek::Keypair::generate a win_crypto_ng::RandomNumberGenerator. RandomNumberGenerator, which implements rand_core::CryptoRng and rand_core::RngCore. When it uses rand_core 0.6, these traits of course come from rand_core 0.6.

As reported in this issue, cargo pulls in rand_core 0.5 for ed25519, and rand_core 0.6 for win-crypto-ng.

This results in errors like:

error[E0277]: the trait bound `RandomNumberGenerator: rand_core::CryptoRng` is not satisfied
   --> C:\Users\runneradmin\.cargo\registry\src\index.crates.io-6f17d22bba15001f\sequoia-openpgp-1.16.0\src\crypto\backend\cng\asymmetric.rs:917:68
    |
917 |                 let Keypair { public, secret } = Keypair::generate(&mut rng);
    |                                                  ----------------- ^^^^^^^^ the trait `rand_core::CryptoRng` is not implemented for `RandomNumberGenerator`
    |                                                  |
    |                                                  required by a bound introduced by this call
    |

Is there a way to express to cargo that ed25519-dalek and win-crypto-ng need to use the same version of rand_core?

Do you have any other recommendations on how we / our users should work around this issue?

Thanks!

Steps

Build sequoia-openpgp on Windows with the Windows CNG crypto backend:

$ git clone https://gitlab.com/sequoia-pgp/sequoia.git
$ cd sequoia
$ git checkout wiktor/test-1.16
# commit 20cb6b32
$ cargo run --manifest-path openpgp/Cargo.toml --no-default-features --features crypto-cng,compression --example supported-algorithms

The results can be seen in this CI job.

Possible Solution(s)

No response

Notes

No response

Version

https://gitlab.com/sequoia-pgp/sequoia/-/jobs/5155713023#L40

$ cargo --version --verbose
cargo 1.67.0 (8ecd4f20a 2023-01-10)
release: 1.67.0
commit-hash: 8ecd4f20a9efb626975ac18a016d480dc7183d9b
commit-date: 2023-01-10
host: x86_64-pc-windows-msvc
libgit2: 1.5.0 (sys:0.16.0 vendored)
libcurl: 7.86.0-DEV (sys:0.4.59+curl-7.86.0 vendored ssl:Schannel)
os: Windows 10.0.17763 (Windows Server 2019 Datacenter) [64-bit]
epage commented 11 months ago

FYI I've renamed this issue in an attempt to capture the problem, rather than a solution, especially since I at least feel that that solution doesn't quite work for your problem.

Normally asserting that a single package exists is because of symbols (sys packages) or because of some globals.

cargo deny is a third-party subcommand that can lint for duplicate packages with an allowlist (iirc).

This is more about keeping packages in-sync with each other which is more of rust-lang/rust#44663 though we need to re-think the automatically keeping packages in-sync part because we found that to have a lot of problems and is what has stalled that from being implemented for years when there would be benefits from the other parts of that RFC.

nwalfield commented 11 months ago

Thanks for the quick and thoughtful response.

cargo deny is a third-party subcommand that can lint for duplicate packages with an allowlist (iirc).

This doesn't work well in our case as we (sequoia-openpgp) are a library, but the dependency resolution is done by the application. We could add this to our CI to detect these types of problems early.

This is more about keeping packages in-sync with each other which is more of #44663 though we need to re-think the automatically keeping packages in-sync part because we found that to have a lot of problems and is what has stalled that from being implemented for years when there would be benefits from the other parts of that RFC.

I think you mean to link to https://github.com/rust-lang/rust/issues/44663, and that does look relevant. Thanks for pointing that out.

epage commented 11 months ago

Ah, the part that I missed was win-crypto-ng's Cargo.toml

rand_core = { version = ">= 0.5, <0.7", optional = true }

I think we have an issue about doing a better job of resolving these ranges but not finding it atm.