feat: respect `rust-version` when generating lockfile #12861

weihanglo · 2023-10-20T04:37:21Z

What does this PR try to resolve?

Respect package.rust-version when generating lockfile, so that
a package with an old MSRV will never get an incompatible lockfile,
even when using the latest Cargo.

Users are still able to edit the version field in the lockfile
manually, if they intend to switch to a specific lockfile version.

How should we test and review this PR?

There is a test matrix of different combinations of msrv and lockfile, which demonstrates that only when missing a lockfile it generates a lockfile compatible with MSRV.

Unless people are doing weird things to lockfile, this is a "more correct" behavior for a project with rust-version set.

Additional information

This is a request from #12852 (comment)

Blocked on #12860.

rustbot · 2023-10-20T04:37:27Z

r? @ehuss

(rustbot has picked a reviewer for you, use r? to override)

epage · 2023-10-20T12:12:31Z

src/cargo/core/resolver/resolve.rs

+    /// Gets the default lockfile version for the given Rust version.
+    pub fn with_rust_version(rust_version: Option<&RustVersion>) -> Self {
+        let Some(rust_version) = rust_version else {
+            return ResolveVersion::default();


Should we remove Default impl to help force people to use with_rust_version?

A even terrible idea: we rename it to fn default(…) so people never misuse 🙈.

addressed this in an awkward way but easier to refer to: 991663c. Let me know if there are other strategies to communicate this.

src/cargo/core/resolver/resolve.rs

epage · 2023-10-20T12:13:59Z

src/cargo/ops/lockfile.rs

+    let default_version = ResolveVersion::with_rust_version(ws.rust_version());
    let current_version = resolve.version();
    let next_lockfile_bump = ws.config().cli_unstable().next_lockfile_bump;
+    tracing::debug!("lockfile - current: {current_version:?}, default: {default_version:?}");


I believe the callers of this have the rust version. Should we use that?

The difference is whether --ignore-rust-version was used or not.

Did you mean the caller of write_pkg_lockfile? Yep we probably need to take care of that route.

epage · 2023-10-20T12:16:20Z

src/cargo/core/resolver/resolve.rs

@@ -97,6 +98,21 @@ impl ResolveVersion {
    pub fn max_stable() -> ResolveVersion {
        ResolveVersion::V3
    }
+
+    /// Gets the default lockfile version for the given Rust version.
+    pub fn with_rust_version(rust_version: Option<&RustVersion>) -> Self {


Should we put this behind -Zmsrv-policy or insta-stable?

I'm fine with insta-stable. The main policy questions would be around

Whether this should respect --ignore-rust-version, where present

A workspace has no rust-version, so this is determined by finding the minimum package.rust-version among all workspace members

Don't feel like it's a thing -Zmsrv-policy wants to address. If we had got MSRV-aware resolver when we started versioning lockfile, I believe we would have added this.

Regarding --ignore-rust-version, I am pretty sold on respecting that, given the semantic is like "hey I want Cargo to work as if I never set rust-version".

I'm fine with that.

After looking into the current codebase, I am now more unsure about supporting --ignore-rust-version for generating lockfile. If we aim to do that, we may need to expose --ignore-rust-version to every command that generates lockfiles.

cargo build and other commands involving build

cargo fix

cargo generate-lockfile

cargo tree

cargo metadata

cargo update

It's a cognitive overload for users to learn they have the control over lockfile versions for all these commands. The user experience would be better if we just provide it out-of-the-box. Lockfile versions are somehow opaque to users to me.

I cannot think of any situation people may opt-in --ignore-rust-version for controlling the lockfile version. It's incompatible when you want to support old versions but requesting for a new lockfile version. For people want old lockfiles, unless there are bugs in newer versions, otherwise we don't want people to continue using old versions. And they are alwasy able to set rust-version for older lockfiles.

Moreover, the workaround is always there — manually changing the version field in the lockfile.

Is there any use case I am not aware of for respecting --ignore-rust-version for lockfile generating?

We could start respecting rust-version from v4, so rust-version older than 1.75 won't affect lockfile versions. This would be less controversial.

This is reminding me of an idea I had for MSRV-aware resolver of putting the MSRV in the lockfile and making it sticky. Having to pass flags along for every call can be pretty frustrating.

bors · 2023-11-10T02:37:58Z

☔ The latest upstream changes (presumably #12930) made this pull request unmergeable. Please resolve the merge conflicts.

ehuss · 2024-01-18T19:02:39Z

r? @epage
If you're ok with that.

epage · 2024-01-19T21:43:29Z

note: this was also discussed at https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/Cargo.2Elock.20and.20--ignore-rust-version

and it was discussed in a team meeting on 2023-10-31

rfcbot · 2024-02-06T16:08:12Z

🔔 This is now entering its final comment period, as per the review above. 🔔

Respect `package.rust-version` when generating lockfile, so that a package with an old MSRV will never get an incompatible lockfile, even when using the latest Cargo. Users are still able to edit the `version` field in the lockfile manually, if they intend to switch to a specific lockfile version.

This prevents a misuse of it.

rfcbot · 2024-02-16T16:08:31Z

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

ehuss · 2024-02-19T18:42:34Z

@weihanglo you can r=ehuss if and when you are ready.

weihanglo · 2024-02-19T19:03:57Z

@bors r=ehuss

Thanks!

bors · 2024-02-19T19:03:59Z

📌 Commit b82910f has been approved by ehuss

It is now in the queue for this repository.

bors · 2024-02-19T19:05:05Z

⌛ Testing commit b82910f with merge e7ff7a6...

bors · 2024-02-19T19:39:02Z

☀️ Test successful - checks-actions
Approved by: ehuss
Pushing e7ff7a6 to master...

Update cargo 9 commits in 7b7af3077bff8d60b7f124189bc9de227d3063a9..194a60b2952bd5d12ba15dd2577a97eed7d3c587 2024-02-17 14:13:00 +0000 to 2024-02-21 01:53:45 +0000 - fix: remove unused `sysroot_host_libdir` (rust-lang/cargo#13468) - feat: support `target.<triple>.rustdocflags` officially (rust-lang/cargo#13197) - Fix unused imports on Windows. (rust-lang/cargo#13469) - Fix more redundant imports. (rust-lang/cargo#13466) - test: Remove empty snapshots (rust-lang/cargo#13465) - chore: Rename `Config` to `GlobalContext` (rust-lang/cargo#13409) - Fix redundant imports. (rust-lang/cargo#13464) - feat: respect `rust-version` when generating lockfile (rust-lang/cargo#12861) - chore(ci): bump CI tools (rust-lang/cargo#13459) r? ghost

Pkgsrc changes: * Adapt checksums and patches, some have beene intregrated upstream. Upstream chnages: Version 1.78.0 (2024-05-02) =========================== Language -------- - [Stabilize `#[cfg(target_abi = ...)]`] (rust-lang/rust#119590) - [Stabilize the `#[diagnostic]` namespace and `#[diagnostic::on_unimplemented]` attribute] (rust-lang/rust#119888) - [Make async-fn-in-trait implementable with concrete signatures] (rust-lang/rust#120103) - [Make matching on NaN a hard error, and remove the rest of `illegal_floating_point_literal_pattern`] (rust-lang/rust#116284) - [static mut: allow mutable reference to arbitrary types, not just slices and arrays] (rust-lang/rust#117614) - [Extend `invalid_reference_casting` to include references casting to bigger memory layout] (rust-lang/rust#118983) - [Add `non_contiguous_range_endpoints` lint for singleton gaps after exclusive ranges] (rust-lang/rust#118879) - [Add `wasm_c_abi` lint for use of older wasm-bindgen versions] (rust-lang/rust#117918) This lint currently only works when using Cargo. - [Update `indirect_structural_match` and `pointer_structural_match` lints to match RFC] (rust-lang/rust#120423) - [Make non-`PartialEq`-typed consts as patterns a hard error] (rust-lang/rust#120805) - [Split `refining_impl_trait` lint into `_reachable`, `_internal` variants] (rust-lang/rust#121720) - [Remove unnecessary type inference when using associated types inside of higher ranked `where`-bounds] (rust-lang/rust#119849) - [Weaken eager detection of cyclic types during type inference] (rust-lang/rust#119989) - [`trait Trait: Auto {}`: allow upcasting from `dyn Trait` to `dyn Auto`] (rust-lang/rust#119338) Compiler -------- - [Made `INVALID_DOC_ATTRIBUTES` lint deny by default] (rust-lang/rust#111505) - [Increase accuracy of redundant `use` checking] (rust-lang/rust#117772) - [Suggest moving definition if non-found macro_rules! is defined later] (rust-lang/rust#121130) - [Lower transmutes from int to pointer type as gep on null] (rust-lang/rust#121282) Target changes: - [Windows tier 1 targets now require at least Windows 10] (rust-lang/rust#115141) - [Enable CMPXCHG16B, SSE3, SAHF/LAHF and 128-bit Atomics in tier 1 Windows] (rust-lang/rust#120820) - [Add `wasm32-wasip1` tier 2 (without host tools) target] (rust-lang/rust#120468) - [Add `wasm32-wasip2` tier 3 target] (rust-lang/rust#119616) - [Rename `wasm32-wasi-preview1-threads` to `wasm32-wasip1-threads`] (rust-lang/rust#122170) - [Add `arm64ec-pc-windows-msvc` tier 3 target] (rust-lang/rust#119199) - [Add `armv8r-none-eabihf` tier 3 target for the Cortex-R52] (rust-lang/rust#110482) - [Add `loongarch64-unknown-linux-musl` tier 3 target] (rust-lang/rust#121832) Refer to Rust's [platform support page][platform-support-doc] for more information on Rust's tiered platform support. Libraries --------- - [Bump Unicode to version 15.1.0, regenerate tables] (rust-lang/rust#120777) - [Make align_offset, align_to well-behaved in all cases] (rust-lang/rust#121201) - [PartialEq, PartialOrd: document expectations for transitive chains] (rust-lang/rust#115386) - [Optimize away poison guards when std is built with panic=abort] (rust-lang/rust#100603) - [Replace pthread `RwLock` with custom implementation] (rust-lang/rust#110211) - [Implement unwind safety for Condvar on all platforms] (rust-lang/rust#121768) - [Add ASCII fast-path for `char::is_grapheme_extended`] (rust-lang/rust#121138) Stabilized APIs --------------- - [`impl Read for &Stdin`] (https://doc.rust-lang.org/stable/std/io/struct.Stdin.html#impl-Read-for-%26Stdin) - [Accept non `'static` lifetimes for several `std::error::Error` related implementations] (rust-lang/rust#113833) - [Make `impl<Fd: AsFd>` impl take `?Sized`] (rust-lang/rust#114655) - [`impl From<TryReserveError> for io::Error`] (https://doc.rust-lang.org/stable/std/io/struct.Error.html#impl-From%3CTryReserveError%3E-for-Error) These APIs are now stable in const contexts: - [`Barrier::new()`] (https://doc.rust-lang.org/stable/std/sync/struct.Barrier.html#method.new) Cargo ----- - [Stabilize lockfile v4](rust-lang/cargo#12852) - [Respect `rust-version` when generating lockfile] (rust-lang/cargo#12861) - [Control `--charset` via auto-detecting config value] (rust-lang/cargo#13337) - [Support `target.<triple>.rustdocflags` officially] (rust-lang/cargo#13197) - [Stabilize global cache data tracking] (rust-lang/cargo#13492) Misc ---- - [rustdoc: add `--test-builder-wrapper` arg to support wrappers such as RUSTC_WRAPPER when building doctests] (rust-lang/rust#114651) Compatibility Notes ------------------- - [Many unsafe precondition checks now run for user code with debug assertions enabled] (rust-lang/rust#120594) This change helps users catch undefined behavior in their code, though the details of how much is checked are generally not stable. - [riscv only supports split_debuginfo=off for now] (rust-lang/rust#120518) - [Consistently check bounds on hidden types of `impl Trait`] (rust-lang/rust#121679) - [Change equality of higher ranked types to not rely on subtyping] (rust-lang/rust#118247) - [When called, additionally check bounds on normalized function return type] (rust-lang/rust#118882) - [Expand coverage for `arithmetic_overflow` lint] (rust-lang/rust#119432) Internal Changes ---------------- These changes do not affect any public interfaces of Rust, but they represent significant improvements to the performance or internals of rustc and related tools. - [Update to LLVM 18](rust-lang/rust#120055) - [Build `rustc` with 1CGU on `x86_64-pc-windows-msvc`] (rust-lang/rust#112267) - [Build `rustc` with 1CGU on `x86_64-apple-darwin`] (rust-lang/rust#112268) - [Introduce `run-make` V2 infrastructure, a `run_make_support` library and port over 2 tests as example] (rust-lang/rust#113026) - [Windows: Implement condvar, mutex and rwlock using futex] (rust-lang/rust#121956)

rustbot assigned ehuss Oct 20, 2023

rustbot added A-dependency-resolution Area: dependency resolution and the resolver A-lockfile Area: Cargo.lock issues S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 20, 2023

weihanglo mentioned this pull request Oct 20, 2023

feat: stabilize lockfile v4 #12852

Merged

epage reviewed Oct 20, 2023

View reviewed changes

src/cargo/core/resolver/resolve.rs Outdated Show resolved Hide resolved

epage reviewed Oct 20, 2023

View reviewed changes

weihanglo force-pushed the msrv-aware-lockfile branch from 888077f to 7e14aec Compare October 23, 2023 19:52

rustbot added the A-semver Area: semver specifications, version matching, etc. label Oct 23, 2023

epage mentioned this pull request Nov 16, 2023

MSRV-dependent dependency version resolution #9930

Open

25 tasks

rustbot assigned epage and unassigned ehuss Jan 18, 2024

weihanglo force-pushed the msrv-aware-lockfile branch from 7e14aec to db1b1f9 Compare January 20, 2024 17:08

rustbot added A-manifest Area: Cargo.toml issues A-testing-cargo-itself Area: cargo's tests Command-fix Command-generate-lockfile Command-metadata Command-package Command-tree labels Jan 20, 2024

weihanglo force-pushed the msrv-aware-lockfile branch 2 times, most recently from d4b590a to 2e79c14 Compare January 30, 2024 21:22

weihanglo removed Command-package Command-tree labels Jan 30, 2024

rfcbot removed the proposed-final-comment-period An FCP proposal has started, but not yet signed off. label Feb 6, 2024

weihanglo added 7 commits February 16, 2024 10:05

refactor: always get max rust-version from workspace

10b8e7b

feat(cargo-util-schemas): TryFrom<PartialVersion> for RustVersion

7b09193

test: demonstrate old lockfile compat matrix

92b82d6

test(lockfile): v4 isn't the default for rust-version 1.77

9ae485d

refactor: don't derive Default for ResolveVersion

cd7cca3

This prevents a misuse of it.

docs(lockfile): make bullet points for version history

b82910f

weihanglo force-pushed the msrv-aware-lockfile branch from 89a86c5 to b82910f Compare February 16, 2024 15:08

rfcbot added finished-final-comment-period FCP complete to-announce and removed final-comment-period FCP — a period for last comments before action is taken labels Feb 16, 2024

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 19, 2024

bors merged commit e7ff7a6 into rust-lang:master Feb 19, 2024
21 checks passed

bors mentioned this pull request Feb 19, 2024

chore: Rename Config to GlobalContext #13409

Merged

weihanglo deleted the msrv-aware-lockfile branch February 19, 2024 22:52

weihanglo mentioned this pull request Feb 21, 2024

Update cargo rust-lang/rust#121380

Merged

rustbot added this to the 1.78.0 milestone Feb 21, 2024

This was referenced Feb 29, 2024

Generate "Cargo.lock" for old Rust compiler #13503

Closed

chore(ci): Ensure lockfile is respected during MSRV testing #13523

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: respect `rust-version` when generating lockfile #12861

feat: respect `rust-version` when generating lockfile #12861

weihanglo commented Oct 20, 2023 •

edited

Loading

rustbot commented Oct 20, 2023

epage Oct 20, 2023

weihanglo Oct 20, 2023

weihanglo Jan 30, 2024

epage Oct 20, 2023

weihanglo Oct 20, 2023

epage Oct 20, 2023

weihanglo Oct 20, 2023

epage Oct 22, 2023

weihanglo Oct 23, 2023

weihanglo Oct 23, 2023

epage Oct 24, 2023

bors commented Nov 10, 2023

ehuss commented Jan 18, 2024

epage commented Jan 19, 2024 •

edited

Loading

rfcbot commented Feb 6, 2024

rfcbot commented Feb 16, 2024

ehuss commented Feb 19, 2024

weihanglo commented Feb 19, 2024

bors commented Feb 19, 2024

bors commented Feb 19, 2024

bors commented Feb 19, 2024

feat: respect rust-version when generating lockfile #12861

feat: respect rust-version when generating lockfile #12861

Conversation

weihanglo commented Oct 20, 2023 • edited Loading

What does this PR try to resolve?

How should we test and review this PR?

Additional information

rustbot commented Oct 20, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bors commented Nov 10, 2023

ehuss commented Jan 18, 2024

epage commented Jan 19, 2024 • edited Loading

rfcbot commented Feb 6, 2024

rfcbot commented Feb 16, 2024

ehuss commented Feb 19, 2024

weihanglo commented Feb 19, 2024

bors commented Feb 19, 2024

bors commented Feb 19, 2024

bors commented Feb 19, 2024

feat: respect `rust-version` when generating lockfile #12861

feat: respect `rust-version` when generating lockfile #12861

weihanglo commented Oct 20, 2023 •

edited

Loading

epage commented Jan 19, 2024 •

edited

Loading