From 10b8e7bdac1e01fedce0d9707d4c1c29fe2e7090 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Sat, 20 Jan 2024 10:54:56 -0500 Subject: [PATCH 1/7] refactor: always get max `rust-version` from workspace --- benches/benchsuite/benches/resolve.rs | 4 ---- src/cargo/core/compiler/standard_lib.rs | 2 -- src/cargo/ops/cargo_compile/mod.rs | 2 -- src/cargo/ops/cargo_generate_lockfile.rs | 6 ------ src/cargo/ops/cargo_output_metadata.rs | 3 --- src/cargo/ops/cargo_package.rs | 3 --- src/cargo/ops/fix.rs | 3 --- src/cargo/ops/resolve.rs | 14 +++----------- src/cargo/ops/tree/mod.rs | 2 -- 9 files changed, 3 insertions(+), 36 deletions(-) diff --git a/benches/benchsuite/benches/resolve.rs b/benches/benchsuite/benches/resolve.rs index f5e28322e24..e235441e1e2 100644 --- a/benches/benchsuite/benches/resolve.rs +++ b/benches/benchsuite/benches/resolve.rs @@ -31,7 +31,6 @@ fn do_resolve<'cfg>(config: &'cfg Config, ws_root: &Path) -> ResolveInfo<'cfg> { let specs = pkgs.to_package_id_specs(&ws).unwrap(); let has_dev_units = HasDevUnits::Yes; let force_all_targets = ForceAllTargets::No; - let max_rust_version = None; // Do an initial run to download anything necessary so that it does // not confuse criterion's warmup. let ws_resolve = cargo::ops::resolve_ws_with_opts( @@ -42,7 +41,6 @@ fn do_resolve<'cfg>(config: &'cfg Config, ws_root: &Path) -> ResolveInfo<'cfg> { &specs, has_dev_units, force_all_targets, - max_rust_version, ) .unwrap(); ResolveInfo { @@ -84,7 +82,6 @@ fn resolve_ws(c: &mut Criterion) { force_all_targets, .. } = lazy_info.get_or_insert_with(|| do_resolve(&config, &ws_root)); - let max_rust_version = None; b.iter(|| { cargo::ops::resolve_ws_with_opts( ws, @@ -94,7 +91,6 @@ fn resolve_ws(c: &mut Criterion) { specs, *has_dev_units, *force_all_targets, - max_rust_version, ) .unwrap(); }) diff --git a/src/cargo/core/compiler/standard_lib.rs b/src/cargo/core/compiler/standard_lib.rs index 5272c211bd7..b76c395b87e 100644 --- a/src/cargo/core/compiler/standard_lib.rs +++ b/src/cargo/core/compiler/standard_lib.rs @@ -145,7 +145,6 @@ pub fn resolve_std<'cfg>( let cli_features = CliFeatures::from_command_line( &features, /*all_features*/ false, /*uses_default_features*/ false, )?; - let max_rust_version = ws.rust_version(); let resolve = ops::resolve_ws_with_opts( &std_ws, target_data, @@ -154,7 +153,6 @@ pub fn resolve_std<'cfg>( &specs, HasDevUnits::No, crate::core::resolver::features::ForceAllTargets::No, - max_rust_version, )?; Ok(( resolve.pkg_set, diff --git a/src/cargo/ops/cargo_compile/mod.rs b/src/cargo/ops/cargo_compile/mod.rs index 04ebe7db1e4..d458b940e27 100644 --- a/src/cargo/ops/cargo_compile/mod.rs +++ b/src/cargo/ops/cargo_compile/mod.rs @@ -262,7 +262,6 @@ pub fn create_bcx<'a, 'cfg>( HasDevUnits::No } }; - let max_rust_version = ws.rust_version(); let resolve = ops::resolve_ws_with_opts( ws, &mut target_data, @@ -271,7 +270,6 @@ pub fn create_bcx<'a, 'cfg>( &specs, has_dev_units, crate::core::resolver::features::ForceAllTargets::No, - max_rust_version, )?; let WorkspaceResolve { mut pkg_set, diff --git a/src/cargo/ops/cargo_generate_lockfile.rs b/src/cargo/ops/cargo_generate_lockfile.rs index fa75ad1bb72..30825af7e7b 100644 --- a/src/cargo/ops/cargo_generate_lockfile.rs +++ b/src/cargo/ops/cargo_generate_lockfile.rs @@ -26,7 +26,6 @@ pub struct UpdateOptions<'a> { pub fn generate_lockfile(ws: &Workspace<'_>) -> CargoResult<()> { let mut registry = PackageRegistry::new(ws.config())?; - let max_rust_version = ws.rust_version(); let mut resolve = ops::resolve_with_previous( &mut registry, ws, @@ -36,7 +35,6 @@ pub fn generate_lockfile(ws: &Workspace<'_>) -> CargoResult<()> { None, &[], true, - max_rust_version, )?; ops::write_pkg_lockfile(ws, &mut resolve)?; Ok(()) @@ -57,8 +55,6 @@ pub fn update_lockfile(ws: &Workspace<'_>, opts: &UpdateOptions<'_>) -> CargoRes .config() .acquire_package_cache_lock(CacheLockMode::DownloadExclusive)?; - let max_rust_version = ws.rust_version(); - let previous_resolve = match ops::load_pkg_lockfile(ws)? { Some(resolve) => resolve, None => { @@ -78,7 +74,6 @@ pub fn update_lockfile(ws: &Workspace<'_>, opts: &UpdateOptions<'_>) -> CargoRes None, &[], true, - max_rust_version, )? } } @@ -157,7 +152,6 @@ pub fn update_lockfile(ws: &Workspace<'_>, opts: &UpdateOptions<'_>) -> CargoRes Some(&to_avoid), &[], true, - max_rust_version, )?; // Summarize what is changing for the user. diff --git a/src/cargo/ops/cargo_output_metadata.rs b/src/cargo/ops/cargo_output_metadata.rs index c62275578bc..3ae0d35779e 100644 --- a/src/cargo/ops/cargo_output_metadata.rs +++ b/src/cargo/ops/cargo_output_metadata.rs @@ -140,8 +140,6 @@ fn build_resolve_graph( crate::core::resolver::features::ForceAllTargets::No }; - let max_rust_version = ws.rust_version(); - // Note that even with --filter-platform we end up downloading host dependencies as well, // as that is the behavior of download_accessible. let ws_resolve = ops::resolve_ws_with_opts( @@ -152,7 +150,6 @@ fn build_resolve_graph( &specs, HasDevUnits::Yes, force_all, - max_rust_version, )?; let package_map: BTreeMap = ws_resolve diff --git a/src/cargo/ops/cargo_package.rs b/src/cargo/ops/cargo_package.rs index b3d0cc495d2..7dcf295289c 100644 --- a/src/cargo/ops/cargo_package.rs +++ b/src/cargo/ops/cargo_package.rs @@ -461,8 +461,6 @@ fn build_lock(ws: &Workspace<'_>, orig_pkg: &Package) -> CargoResult { to_real_manifest(toml_manifest, false, source_id, package_root, config)?; let new_pkg = Package::new(manifest, orig_pkg.manifest_path()); - let max_rust_version = new_pkg.rust_version().cloned(); - // Regenerate Cargo.lock using the old one as a guide. let tmp_ws = Workspace::ephemeral(new_pkg, ws.config(), None, true)?; let mut tmp_reg = PackageRegistry::new(ws.config())?; @@ -475,7 +473,6 @@ fn build_lock(ws: &Workspace<'_>, orig_pkg: &Package) -> CargoResult { None, &[], true, - max_rust_version.as_ref(), )?; let pkg_set = ops::get_resolved_packages(&new_resolve, tmp_reg)?; diff --git a/src/cargo/ops/fix.rs b/src/cargo/ops/fix.rs index d57b94e0184..4dd53d3a9eb 100644 --- a/src/cargo/ops/fix.rs +++ b/src/cargo/ops/fix.rs @@ -245,8 +245,6 @@ fn check_resolver_change(ws: &Workspace<'_>, opts: &FixOptions) -> CargoResult<( let mut target_data = RustcTargetData::new(ws, &opts.compile_opts.build_config.requested_kinds)?; let mut resolve_differences = |has_dev_units| -> CargoResult<(WorkspaceResolve<'_>, DiffMap)> { - let max_rust_version = ws.rust_version(); - let ws_resolve = ops::resolve_ws_with_opts( ws, &mut target_data, @@ -255,7 +253,6 @@ fn check_resolver_change(ws: &Workspace<'_>, opts: &FixOptions) -> CargoResult<( &specs, has_dev_units, crate::core::resolver::features::ForceAllTargets::No, - max_rust_version, )?; let feature_opts = FeatureOpts::new_behavior(ResolveBehavior::V2, has_dev_units); diff --git a/src/cargo/ops/resolve.rs b/src/cargo/ops/resolve.rs index b00dc15004e..502ad5da3c4 100644 --- a/src/cargo/ops/resolve.rs +++ b/src/cargo/ops/resolve.rs @@ -73,7 +73,6 @@ use crate::util::cache_lock::CacheLockMode; use crate::util::errors::CargoResult; use crate::util::{profile, CanonicalUrl}; use anyhow::Context as _; -use cargo_util_schemas::manifest::RustVersion; use std::collections::{HashMap, HashSet}; use tracing::{debug, trace}; @@ -109,10 +108,8 @@ version. This may also occur with an optional dependency that is not enabled."; /// This is a simple interface used by commands like `clean`, `fetch`, and /// `package`, which don't specify any options or features. pub fn resolve_ws<'a>(ws: &Workspace<'a>) -> CargoResult<(PackageSet<'a>, Resolve)> { - let max_rust_version = ws.rust_version(); - let mut registry = PackageRegistry::new(ws.config())?; - let resolve = resolve_with_registry(ws, &mut registry, max_rust_version)?; + let resolve = resolve_with_registry(ws, &mut registry)?; let packages = get_resolved_packages(&resolve, registry)?; Ok((packages, resolve)) } @@ -135,7 +132,6 @@ pub fn resolve_ws_with_opts<'cfg>( specs: &[PackageIdSpec], has_dev_units: HasDevUnits, force_all_targets: ForceAllTargets, - max_rust_version: Option<&RustVersion>, ) -> CargoResult> { let mut registry = PackageRegistry::new(ws.config())?; let mut add_patches = true; @@ -144,7 +140,7 @@ pub fn resolve_ws_with_opts<'cfg>( } else if ws.require_optional_deps() { // First, resolve the root_package's *listed* dependencies, as well as // downloading and updating all remotes and such. - let resolve = resolve_with_registry(ws, &mut registry, max_rust_version)?; + let resolve = resolve_with_registry(ws, &mut registry)?; // No need to add patches again, `resolve_with_registry` has done it. add_patches = false; @@ -190,7 +186,6 @@ pub fn resolve_ws_with_opts<'cfg>( None, specs, add_patches, - max_rust_version, )?; let pkg_set = get_resolved_packages(&resolved_with_overrides, registry)?; @@ -242,7 +237,6 @@ pub fn resolve_ws_with_opts<'cfg>( fn resolve_with_registry<'cfg>( ws: &Workspace<'cfg>, registry: &mut PackageRegistry<'cfg>, - max_rust_version: Option<&RustVersion>, ) -> CargoResult { let prev = ops::load_pkg_lockfile(ws)?; let mut resolve = resolve_with_previous( @@ -254,7 +248,6 @@ fn resolve_with_registry<'cfg>( None, &[], true, - max_rust_version, )?; if !ws.is_ephemeral() && ws.require_optional_deps() { @@ -287,7 +280,6 @@ pub fn resolve_with_previous<'cfg>( to_avoid: Option<&HashSet>, specs: &[PackageIdSpec], register_patches: bool, - max_rust_version: Option<&RustVersion>, ) -> CargoResult { // We only want one Cargo at a time resolving a crate graph since this can // involve a lot of frobbing of the global caches. @@ -326,7 +318,7 @@ pub fn resolve_with_previous<'cfg>( version_prefs.version_ordering(VersionOrdering::MinimumVersionsFirst) } if ws.config().cli_unstable().msrv_policy { - version_prefs.max_rust_version(max_rust_version.cloned()); + version_prefs.max_rust_version(ws.rust_version().cloned()); } // This is a set of PackageIds of `[patch]` entries, and some related locked PackageIds, for diff --git a/src/cargo/ops/tree/mod.rs b/src/cargo/ops/tree/mod.rs index 79a69a66a4f..c2f63d9823e 100644 --- a/src/cargo/ops/tree/mod.rs +++ b/src/cargo/ops/tree/mod.rs @@ -150,7 +150,6 @@ pub fn build_and_print(ws: &Workspace<'_>, opts: &TreeOptions) -> CargoResult<() } else { ForceAllTargets::No }; - let max_rust_version = ws.rust_version(); let ws_resolve = ops::resolve_ws_with_opts( ws, &mut target_data, @@ -159,7 +158,6 @@ pub fn build_and_print(ws: &Workspace<'_>, opts: &TreeOptions) -> CargoResult<() &specs, has_dev, force_all, - max_rust_version, )?; let package_map: HashMap = ws_resolve From 7b0919399d9a7ccde9ce17632d00806921454c9e Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Sat, 20 Jan 2024 11:57:07 -0500 Subject: [PATCH 2/7] feat(cargo-util-schemas): `TryFrom` for `RustVersion` --- crates/cargo-util-schemas/src/manifest.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/crates/cargo-util-schemas/src/manifest.rs b/crates/cargo-util-schemas/src/manifest.rs index 1591ec30197..f6d131f533d 100644 --- a/crates/cargo-util-schemas/src/manifest.rs +++ b/crates/cargo-util-schemas/src/manifest.rs @@ -1417,6 +1417,14 @@ impl std::str::FromStr for RustVersion { fn from_str(value: &str) -> Result { let partial = value.parse::(); let partial = partial.map_err(RustVersionErrorKind::PartialVersion)?; + partial.try_into() + } +} + +impl TryFrom for RustVersion { + type Error = RustVersionError; + + fn try_from(partial: PartialVersion) -> Result { if partial.pre.is_some() { return Err(RustVersionErrorKind::Prerelease.into()); } From 92b82d6134dc1de300dbc949affd981d3a191b5a Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Thu, 19 Oct 2023 21:16:46 -0400 Subject: [PATCH 3/7] test: demonstrate old lockfile compat matrix --- tests/testsuite/lockfile_compat.rs | 132 +++++++++++++++++++++++++++++ 1 file changed, 132 insertions(+) diff --git a/tests/testsuite/lockfile_compat.rs b/tests/testsuite/lockfile_compat.rs index f0d85037911..98d1ee8386c 100644 --- a/tests/testsuite/lockfile_compat.rs +++ b/tests/testsuite/lockfile_compat.rs @@ -1157,3 +1157,135 @@ fn v4_and_git_url_encoded_tag() { fn v4_and_git_url_encoded_rev() { v4_and_git_url_encoded("rev", create_tag) } + +#[cargo_test] +fn with_msrv() { + let cksum = Package::new("bar", "0.1.0").publish(); + let v2_lockfile = format!( + r#"# This file is automatically @generated by Cargo. +# It is not intended for manual editing. +[[package]] +name = "bar" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "{cksum}" + +[[package]] +name = "foo" +version = "0.0.1" +dependencies = [ + "bar", +] +"# + ); + + let v1_lockfile = format!( + r#"# This file is automatically @generated by Cargo. +# It is not intended for manual editing. +[[package]] +name = "bar" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" + +[[package]] +name = "foo" +version = "0.0.1" +dependencies = [ + "bar 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)", +] + +[metadata] +"checksum bar 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)" = "{cksum}" +"# + ); + + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.0.1" + + [dependencies] + bar = "0.1.0" + "#, + ) + .file("src/lib.rs", "") + .build(); + + let cases = [ + // v1 is the default + ("1.37", None, 3), + ("1.37", Some(1), 1), + ("1.37", Some(2), 2), + ("1.37", Some(3), 3), + // v2 introduced + ("1.38", None, 3), + // last version of v1 as the default + ("1.40", None, 3), + // v2 is the default + ("1.41", None, 3), + ("1.41", Some(1), 1), + ("1.41", Some(2), 2), + ("1.41", Some(3), 3), + // v3 introduced + ("1.47", None, 3), + // last version of v2 as the default + ("1.48", None, 3), + // v3 is the default + ("1.53", None, 3), + ("1.53", Some(1), 1), + ("1.53", Some(2), 2), + ("1.53", Some(3), 3), + ]; + + for (msrv, existing_lockfile, expected_version) in cases { + // Clean previous lockfile. + _ = std::fs::remove_file(p.root().join("Cargo.lock")); + + p.change_file( + "Cargo.toml", + &format!( + r#" + [package] + name = "foo" + version = "0.0.1" + rust-version = "{msrv}" + + [dependencies] + bar = "0.1.0" + "#, + ), + ); + + if let Some(existing_lockfile) = existing_lockfile { + let existing_lockfile = match existing_lockfile { + 1 => v1_lockfile.as_str().into(), + 2 => v2_lockfile.as_str().into(), + v => std::borrow::Cow::from(format!("version = {v}")), + }; + p.change_file("Cargo.lock", &existing_lockfile); + } + + p.cargo("fetch").run(); + + let lock = p.read_lockfile(); + let toml = lock.parse::().unwrap(); + // get `version = ` from Cargo.lock + let version_field = toml.get("version").and_then(|v| v.as_integer()); + + let actual_version = if let Some(ver) = version_field { + ver + } else if lock.find("\nchecksum = ").is_some() { + 2 + } else { + 1 + }; + + assert_eq!( + expected_version, actual_version, + "msrv: {msrv}, existing lockfile: {existing_lockfile:?}" + ); + } +} From fb95ac4887766221665f8c6ff4b720255ee69ad7 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Thu, 19 Oct 2023 22:31:34 -0400 Subject: [PATCH 4/7] feat: respect `rust-version` when generating lockfile 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. --- crates/resolver-tests/src/lib.rs | 2 ++ src/cargo/core/resolver/mod.rs | 5 +++- src/cargo/core/resolver/resolve.rs | 37 ++++++++++++++++++++++++++++++ src/cargo/ops/lockfile.rs | 3 ++- src/cargo/ops/resolve.rs | 1 + tests/testsuite/lockfile_compat.rs | 12 +++++----- 6 files changed, 52 insertions(+), 8 deletions(-) diff --git a/crates/resolver-tests/src/lib.rs b/crates/resolver-tests/src/lib.rs index 00d64894ef5..af6a2c2524e 100644 --- a/crates/resolver-tests/src/lib.rs +++ b/crates/resolver-tests/src/lib.rs @@ -14,6 +14,7 @@ use std::time::Instant; use cargo::core::dependency::DepKind; use cargo::core::resolver::{self, ResolveOpts, VersionOrdering, VersionPreferences}; use cargo::core::Resolve; +use cargo::core::ResolveVersion; use cargo::core::{Dependency, PackageId, Registry, Summary}; use cargo::core::{GitReference, SourceId}; use cargo::sources::source::QueryKind; @@ -174,6 +175,7 @@ pub fn resolve_with_config_raw( &[], &mut registry, &version_prefs, + ResolveVersion::default(), Some(config), ); diff --git a/src/cargo/core/resolver/mod.rs b/src/cargo/core/resolver/mod.rs index 4b12f2cf32c..5a444bbf274 100644 --- a/src/cargo/core/resolver/mod.rs +++ b/src/cargo/core/resolver/mod.rs @@ -118,6 +118,8 @@ mod version_prefs; /// * `version_prefs` - this represents a preference for some versions over others, /// based on the lock file or other reasons such as `[patch]`es. /// +/// * `resolve_version` - this controls how the lockfile will be serialized. +/// /// * `config` - a location to print warnings and such, or `None` if no warnings /// should be printed pub fn resolve( @@ -125,6 +127,7 @@ pub fn resolve( replacements: &[(PackageIdSpec, Dependency)], registry: &mut dyn Registry, version_prefs: &VersionPreferences, + resolve_version: ResolveVersion, config: Option<&Config>, ) -> CargoResult { let _p = profile::start("resolving"); @@ -169,7 +172,7 @@ pub fn resolve( cksums, BTreeMap::new(), Vec::new(), - ResolveVersion::default(), + resolve_version, summaries, ); diff --git a/src/cargo/core/resolver/resolve.rs b/src/cargo/core/resolver/resolve.rs index f7027dcaceb..141721d188c 100644 --- a/src/cargo/core/resolver/resolve.rs +++ b/src/cargo/core/resolver/resolve.rs @@ -1,3 +1,6 @@ +use cargo_util_schemas::core::PartialVersion; +use cargo_util_schemas::manifest::RustVersion; + use super::encode::Metadata; use crate::core::dependency::DepKind; use crate::core::{Dependency, PackageId, PackageIdSpec, PackageIdSpecQuery, Summary, Target}; @@ -99,6 +102,40 @@ impl ResolveVersion { pub fn max_stable() -> ResolveVersion { ResolveVersion::V4 } + + /// 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(); + }; + + let rust_1_41 = PartialVersion { + major: 1, + minor: Some(41), + patch: None, + pre: None, + build: None, + } + .try_into() + .expect("PartialVersion 1.41"); + let rust_1_53 = PartialVersion { + major: 1, + minor: Some(53), + patch: None, + pre: None, + build: None, + } + .try_into() + .expect("PartialVersion 1.53"); + + if rust_version >= &rust_1_53 { + ResolveVersion::V3 + } else if rust_version >= &rust_1_41 { + ResolveVersion::V2 + } else { + ResolveVersion::V1 + } + } } impl Resolve { diff --git a/src/cargo/ops/lockfile.rs b/src/cargo/ops/lockfile.rs index 2160b6f0154..f16a1f0d4a8 100644 --- a/src/cargo/ops/lockfile.rs +++ b/src/cargo/ops/lockfile.rs @@ -64,9 +64,10 @@ pub fn write_pkg_lockfile(ws: &Workspace<'_>, resolve: &mut Resolve) -> CargoRes // out lock file updates as they're otherwise already updated, and changes // which don't touch dependencies won't seemingly spuriously update the lock // file. - let default_version = ResolveVersion::default(); + 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:?}"); if current_version < default_version { resolve.set_version(default_version); diff --git a/src/cargo/ops/resolve.rs b/src/cargo/ops/resolve.rs index 502ad5da3c4..bd1b68b9fd9 100644 --- a/src/cargo/ops/resolve.rs +++ b/src/cargo/ops/resolve.rs @@ -504,6 +504,7 @@ pub fn resolve_with_previous<'cfg>( &replace, registry, &version_prefs, + ResolveVersion::with_rust_version(ws.rust_version()), Some(ws.config()), )?; let patches: Vec<_> = registry diff --git a/tests/testsuite/lockfile_compat.rs b/tests/testsuite/lockfile_compat.rs index 98d1ee8386c..ded9ef29e33 100644 --- a/tests/testsuite/lockfile_compat.rs +++ b/tests/testsuite/lockfile_compat.rs @@ -1216,23 +1216,23 @@ dependencies = [ let cases = [ // v1 is the default - ("1.37", None, 3), + ("1.37", None, 1), ("1.37", Some(1), 1), ("1.37", Some(2), 2), ("1.37", Some(3), 3), // v2 introduced - ("1.38", None, 3), + ("1.38", None, 1), // last version of v1 as the default - ("1.40", None, 3), + ("1.40", None, 1), // v2 is the default - ("1.41", None, 3), + ("1.41", None, 2), ("1.41", Some(1), 1), ("1.41", Some(2), 2), ("1.41", Some(3), 3), // v3 introduced - ("1.47", None, 3), + ("1.47", None, 2), // last version of v2 as the default - ("1.48", None, 3), + ("1.48", None, 2), // v3 is the default ("1.53", None, 3), ("1.53", Some(1), 1), From 9ae485df28ad9357ed1583dcfce7ddc9511e359f Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Tue, 30 Jan 2024 16:21:56 -0500 Subject: [PATCH 5/7] test(lockfile): v4 isn't the default for rust-version 1.77 --- src/cargo/core/resolver/resolve.rs | 2 +- tests/testsuite/lockfile_compat.rs | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/cargo/core/resolver/resolve.rs b/src/cargo/core/resolver/resolve.rs index 141721d188c..431b3c83998 100644 --- a/src/cargo/core/resolver/resolve.rs +++ b/src/cargo/core/resolver/resolve.rs @@ -85,7 +85,7 @@ pub enum ResolveVersion { V3, /// SourceId URL serialization is aware of URL encoding. For example, /// `?branch=foo bar` is now encoded as `?branch=foo+bar` and can be decoded - /// back and forth correctly. Introduced in 2024 in version 1.77. + /// back and forth correctly. Introduced in 2024 in version 1.78. V4, /// Unstable. Will collect a certain amount of changes and then go. /// diff --git a/tests/testsuite/lockfile_compat.rs b/tests/testsuite/lockfile_compat.rs index ded9ef29e33..d47c284a5b7 100644 --- a/tests/testsuite/lockfile_compat.rs +++ b/tests/testsuite/lockfile_compat.rs @@ -1220,6 +1220,7 @@ dependencies = [ ("1.37", Some(1), 1), ("1.37", Some(2), 2), ("1.37", Some(3), 3), + ("1.37", Some(4), 4), // v2 introduced ("1.38", None, 1), // last version of v1 as the default @@ -1229,6 +1230,7 @@ dependencies = [ ("1.41", Some(1), 1), ("1.41", Some(2), 2), ("1.41", Some(3), 3), + ("1.41", Some(4), 4), // v3 introduced ("1.47", None, 2), // last version of v2 as the default @@ -1238,6 +1240,9 @@ dependencies = [ ("1.53", Some(1), 1), ("1.53", Some(2), 2), ("1.53", Some(3), 3), + ("1.53", Some(4), 4), + // v4 introduced + ("1.78", None, 3), ]; for (msrv, existing_lockfile, expected_version) in cases { From cd7cca369e9e33d49dde6e3de8f0bea899ddc7bc Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Tue, 30 Jan 2024 16:56:19 -0500 Subject: [PATCH 6/7] refactor: don't derive Default for `ResolveVersion` This prevents a misuse of it. --- crates/resolver-tests/src/lib.rs | 2 +- src/cargo/core/resolver/resolve.rs | 16 +++++++++++++--- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/crates/resolver-tests/src/lib.rs b/crates/resolver-tests/src/lib.rs index af6a2c2524e..efd9cebc6c6 100644 --- a/crates/resolver-tests/src/lib.rs +++ b/crates/resolver-tests/src/lib.rs @@ -175,7 +175,7 @@ pub fn resolve_with_config_raw( &[], &mut registry, &version_prefs, - ResolveVersion::default(), + ResolveVersion::with_rust_version(None), Some(config), ); diff --git a/src/cargo/core/resolver/resolve.rs b/src/cargo/core/resolver/resolve.rs index 431b3c83998..0ac4b2d2c0f 100644 --- a/src/cargo/core/resolver/resolve.rs +++ b/src/cargo/core/resolver/resolve.rs @@ -51,7 +51,7 @@ pub struct Resolve { /// A version to indicate how a `Cargo.lock` should be serialized. /// -/// When creating a new lockfile, the version with `#[default]` is used. +/// When creating a new lockfile, the version in [`ResolveVersion::default`] is used. /// If an old version of lockfile already exists, it will stay as-is. /// /// It's important that if a new version is added that this is not updated @@ -67,7 +67,7 @@ pub struct Resolve { /// /// It's theorized that we can add more here over time to track larger changes /// to the `Cargo.lock` format, but we've yet to see how that strategy pans out. -#[derive(Default, PartialEq, Eq, Clone, Copy, Debug, PartialOrd, Ord)] +#[derive(PartialEq, Eq, Clone, Copy, Debug, PartialOrd, Ord)] pub enum ResolveVersion { /// Historical baseline for when this abstraction was added. V1, @@ -81,7 +81,6 @@ pub enum ResolveVersion { /// `branch = "master"` are no longer encoded the same way as those without /// branch specifiers. Introduced in 2020 in version 1.47. New lockfiles use /// V3 by default staring in 1.53. - #[default] V3, /// SourceId URL serialization is aware of URL encoding. For example, /// `?branch=foo bar` is now encoded as `?branch=foo+bar` and can be decoded @@ -94,6 +93,17 @@ pub enum ResolveVersion { } impl ResolveVersion { + /// Gets the default lockfile version. + /// + /// This is intended to be private. + /// You shall use [`ResolveVersion::with_rust_version`] always. + /// + /// Update this and the description of enum variants of [`ResolveVersion`] + /// when we're changing the default lockfile version. + fn default() -> ResolveVersion { + ResolveVersion::V3 + } + /// The maximum version of lockfile made into the stable channel. /// /// Any version larger than this needs `-Znext-lockfile-bump` to enable. From b82910fdbaef9f0a153512d2ce639ec53194a062 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Tue, 30 Jan 2024 17:03:35 -0500 Subject: [PATCH 7/7] docs(lockfile): make bullet points for version history --- src/cargo/core/resolver/resolve.rs | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/cargo/core/resolver/resolve.rs b/src/cargo/core/resolver/resolve.rs index 0ac4b2d2c0f..d77ff9f3590 100644 --- a/src/cargo/core/resolver/resolve.rs +++ b/src/cargo/core/resolver/resolve.rs @@ -73,18 +73,24 @@ pub enum ResolveVersion { V1, /// A more compact format, more amenable to avoiding source-control merge /// conflicts. The `dependencies` arrays are compressed and checksums are - /// listed inline. Introduced in 2019 in version 1.38. New lockfiles use - /// V2 by default from 1.41 to 1.52. + /// listed inline. + /// + /// * Introduced in 2019 in version 1.38. + /// * New lockfiles use V2 by default from 1.41 to 1.52. V2, /// A format that explicitly lists a `version` at the top of the file as /// well as changing how git dependencies are encoded. Dependencies with /// `branch = "master"` are no longer encoded the same way as those without - /// branch specifiers. Introduced in 2020 in version 1.47. New lockfiles use - /// V3 by default staring in 1.53. + /// branch specifiers. + /// + /// * Introduced in 2020 in version 1.47. + /// * New lockfiles use V3 by default starting in 1.53. V3, /// SourceId URL serialization is aware of URL encoding. For example, /// `?branch=foo bar` is now encoded as `?branch=foo+bar` and can be decoded - /// back and forth correctly. Introduced in 2024 in version 1.78. + /// back and forth correctly. + /// + /// * Introduced in 2024 in version 1.78. V4, /// Unstable. Will collect a certain amount of changes and then go. ///