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

Workspace's resolver = "2" leaks when crate is published, leading to MSRV breaks #11047

Closed
newpavlov opened this issue Sep 2, 2022 · 5 comments
Labels
C-bug Category: bug

Comments

@newpavlov
Copy link

newpavlov commented Sep 2, 2022

Problem

In RustCrypto we have strict MSRV guarantees. After new versions of several of our hash crates (with MSRV equal to 1.41) have been published, we got reports like this: RustCrypto/hashes#394. Initially we thought that #10954 is responsible for it (I usually use Nightly), but after I re-released the crates in RustCrypto/hashes#399 using the stable toolchain (1.63) the issue was not resolved.

It looks like that for some reason resolver = "2" used in the workspace's Cargo.toml gets leaked during publishing of the crates by the stable toolchain. Previously we did not have such issue (UPD: The PR which added resolver = "2" was after releases which I thought did not trigger the issue). Is this behavior intentional?

@epage
Copy link
Contributor

epage commented Sep 2, 2022

Potentially related PRs

Supposedly #10911 was only in 1.64.0 which is what we backported #10961 to in #10970.

Initially we thought that #10954 is responsible for it (I usually use Nightly), but after I re-released the crates in RustCrypto/hashes#399 using the stable toolchain (1.63) the issue was not resolved.

Could you narrow down which version introduced this by running cargo package and inspecting the results?

@newpavlov
Copy link
Author

newpavlov commented Sep 2, 2022

Hm, it looks like this behavior can be observed as far as 1.52, so it looks like the issue is on our side (i.e. we should not use resolver = "2" for workspace if it contains crates with pre-1.51 MSRV) and the described behavior is intentional (albeit I find it somewhat surprising). Feel free to close this issue.

@ehuss
Copy link
Contributor

ehuss commented Sep 26, 2022

Yea, the resolver setting will introduce a higher MSRV requirement. I see that it is a bit unfortunate, since the resolver setting is only relevant for cargo install of binaries. I'm going to close since this is generally working as intended.

@ehuss ehuss closed this as not planned Won't fix, can't repro, duplicate, stale Sep 26, 2022
@epage
Copy link
Contributor

epage commented Sep 1, 2023

This came up in serde-rs/serde#2603. Moving the conversation here for wider visibility and ensuring the conversation is in one place

@dtolnay

I'd prefer if it did not do that for packages not containing any bin crates.

@epage

At this time, the code responsible does not have that knowledge. However, I've been wanting to change our publish code to to explicitly list all auto targets at which point it would be possible to determine that. There still might be enough nuance to be annoying because examples are considered installable.

@dtolnay

As an easier fix that still eliminates resolver-related breakage for libraries, could we make sure Cargo disregards any value of resolver in the manifest of library dependencies?

I tried editing $CARGO_HOME/registry/src/index.crates.io-6f17d22bba15001f/serde-1.0.188/Cargo.toml to add resolver = "100", and that currently makes crates depending on serde as a library fail with this error, on current nightly:

error: failed to download `serde v1.0.188`

Caused by:
  unable to get packages from source

Caused by:
  failed to download replaced source registry `crates-io`

Caused by:
  failed to parse manifest at `$CARGO_HOME/registry/src/index.crates.io-6f17d22bba15001f/serde-1.0.188/Cargo.toml`

Caused by:
  `resolver` setting `100` is not valid, valid options are "1" or "2"

@epage
Copy link
Contributor

epage commented Sep 1, 2023

That is an intriguing idea for us to delay the validation of the resolver value until we need it.

In thinking about this, I guess you could say I'm a little less sympathetic to this use case because there isn't really a way to validate the MSRV but it has to be taken upon trust / care and planning (yes, you could do a cargo package and then build that .crate file but that is high friction as well). While there might be advanced users who can do a better job of tracking this (which serde-rs/serde#2603 shows it isn't perfect), I'm worried about the common users attempting this, not just with resolver = "2" but "3" and with other features.

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

No branches or pull requests

3 participants