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

Consider doing a forced lock of the standard library. #38

Open
ehuss opened this issue Sep 6, 2019 · 11 comments · May be fixed by rust-lang/cargo#14589
Open

Consider doing a forced lock of the standard library. #38

ehuss opened this issue Sep 6, 2019 · 11 comments · May be fixed by rust-lang/cargo#14589
Labels
implementation Implementation exploration and tracking issues stabilization blocker This needs a resolution before stabilization

Comments

@ehuss
Copy link
Contributor

ehuss commented Sep 6, 2019

It might be a good idea to do the equivalent of --locked for the standard library to ensure that the Cargo.lock isn't modified, and that it is correct.

@ehuss ehuss added the implementation Implementation exploration and tracking issues label Sep 6, 2019
@alexcrichton
Copy link
Member

I agree with this in spirit, but --locked probably won't work "as is" because we are actually technically vastly changing the previous lock file by deleting most nodes from it (e.g. everything related to the compiler/tools/etc). In that sense we want an assertion that the final crate graph is a subset of the original crate graph, but that's not precisely what --locked is doing.

Either way, agreed would be good to implement!

@ehuss ehuss added the stabilization blocker This needs a resolution before stabilization label May 3, 2023
@adamgemmell
Copy link

I think it might make more sense to do this in a cargo test. I'm not sure what user configuration could break this behaviour (patching dependencies, and ignoring the lockfile with cargo install don't work). The only thing that might change this is if the same workspace was used and the lockfiles combined, but then Alex's proposal wouldn't work.

@weihanglo
Copy link
Member

With rust-lang/rust#128534 and rust-lang/cargo#14358, it is possible to implement the --locked behavior in -Zbuild-std.

@adamgemmell
Copy link

This would remove the need for rust-lang/cargo#13916, correct? My understanding is that we'd still end up with a subset of the original lockfile but can actually use the lockfile on disk now, making the checks unnecessary.

@weihanglo
Copy link
Member

Yes. I'll close that one. Thank you for taking care of it.
We still need to implement the actual --locked behavior to -Zbuild-std. See rust-lang/cargo#14358 (comment). I am think that Workspace can have a field additionally check if Cargo is about to write a new lockfile, it bails out.
https://github.com/rust-lang/cargo/blob/2cd657cc33a46ac026154534d5b6fac264acd15b/src/cargo/ops/lockfile.rs#L40-L65

@adamgemmell adamgemmell linked a pull request Sep 24, 2024 that will close this issue
@weihanglo
Copy link
Member

I think it might make more sense to do this in a cargo test. I'm not sure what user configuration could break this behaviour (patching dependencies, and ignoring the lockfile with cargo install don't work). The only thing that might change this is if the same workspace was used and the lockfiles combined, but then Alex's proposal wouldn't work.

It indeed breaks [patch]. You can pull rust-lang/cargo@4e7dfb1 and check with the reproduction in rust-lang/cargo#14003.
One can argue that if patches are needed, clone rust-lang/rust repo and do patches in the source, though that is kinda inconvenient. So I think unconditionally banning lockfile writes might not be the best solution.

Cargo recently got the --lockfile-path unstable flag. We could potentially combine this and patch if people want to patch std dependencies. I am not sure what the UI would look like though.

@bjorn3
Copy link
Member

bjorn3 commented Oct 15, 2024

Cargo has an internal env var to specify the location to search for the standard library sources. Maybe everyone who wants to patch the standard library or it's dependencies could be made to use this env var and only force --frozen when not using this env var. That would also allow keeping the [patch] section for the standard library in the standard library workspace rather than mixing it into the user project workspace, which also should fix rust-lang/cargo#14003. In any case I don't think stable cargo should ever allow [patch] in the user workspace to apply to the standard library as no guarantees whatsoever are provided on what dependencies (and versions of said dependencies) the standard library uses and the patch may break things with future versions of the standard library.

@glandium
Copy link

vendoring is another thing that makes things go wrong with build-std.

@bjorn3
Copy link
Member

bjorn3 commented Oct 16, 2024

In what way do you mean?

@glandium
Copy link

Cargo fails to find the std dependencies, starting with compiler_builtins.

@adamgemmell
Copy link

If it's not related to --locked behaviour then that sounds like its covered by #23

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
implementation Implementation exploration and tracking issues stabilization blocker This needs a resolution before stabilization
Projects
None yet
6 participants