diff --git a/src/cargo/core/resolver/encode.rs b/src/cargo/core/resolver/encode.rs index ce3ade40b19..d2414801ed9 100644 --- a/src/cargo/core/resolver/encode.rs +++ b/src/cargo/core/resolver/encode.rs @@ -19,17 +19,32 @@ //! We do, however, want to change `Cargo.lock` over time. (and we have!). To do //! this the rules that we currently have are: //! -//! * Add support for the new format to Cargo -//! * Continue to, by default, generate the old format -//! * Preserve the new format if found -//! * Wait a "long time" (e.g. 6 months or so) -//! * Change Cargo to by default emit the new format +//! * Add support for the new format to Cargo. This involves code changes in +//! Cargo itself, likely by adding a new variant of `ResolveVersion` and +//! branching on that where necessary. This is accompanied with tests in the +//! `lockfile_compat` module. +//! +//! * Do not update `ResolveVersion::default()`. The new lockfile format will +//! not be used yet. +//! +//! * Preserve the new format if found. This means that if Cargo finds the new +//! version it'll keep using it, but otherwise it continues to use whatever +//! format it previously found. +//! +//! * Wait a "long time". This is at least until the changes here hit stable +//! Rust. Often though we wait a little longer to let the changes percolate +//! into one or two older stable releases. +//! +//! * Change the return value of `ResolveVersion::default()` to the new format. +//! This will cause new lock files to use the latest encoding as well as +//! causing any operation which updates the lock file to update to the new +//! format. //! //! This migration scheme in general means that Cargo we'll get *support* for a -//! new format into Cargo ASAP, but it won't really be exercised yet (except in -//! Cargo's own tests really). Eventually when stable/beta/nightly all have -//! support for the new format (and maybe a few previous stable versions) we -//! flip the switch. Projects on nightly will quickly start seeing changes, but +//! new format into Cargo ASAP, but it won't be exercised yet (except in Cargo's +//! own tests). Eventually when stable/beta/nightly all have support for the new +//! format (and maybe a few previous stable versions) we flip the switch. +//! Projects on nightly will quickly start seeing changes, but //! stable/beta/nightly will all understand this new format and will preserve //! it. //! diff --git a/src/cargo/core/resolver/mod.rs b/src/cargo/core/resolver/mod.rs index dbc4fe8be8d..094c64065b1 100644 --- a/src/cargo/core/resolver/mod.rs +++ b/src/cargo/core/resolver/mod.rs @@ -161,7 +161,7 @@ pub fn resolve( cksums, BTreeMap::new(), Vec::new(), - ResolveVersion::default_for_new_lockfiles(), + ResolveVersion::default(), summaries, ); diff --git a/src/cargo/core/resolver/resolve.rs b/src/cargo/core/resolver/resolve.rs index c9903d0503e..9345aaf9e55 100644 --- a/src/cargo/core/resolver/resolve.rs +++ b/src/cargo/core/resolver/resolve.rs @@ -5,7 +5,6 @@ use crate::util::errors::CargoResult; use crate::util::interning::InternedString; use crate::util::Graph; use std::borrow::Borrow; -use std::cmp; use std::collections::{HashMap, HashSet}; use std::fmt; @@ -219,35 +218,8 @@ unable to verify that `{0}` is the same as when the lockfile was generated // Be sure to just copy over any unknown metadata. self.metadata = previous.metadata.clone(); - // The goal of Cargo is largely to preserve the encoding of `Cargo.lock` - // that it finds on the filesystem. Sometimes `Cargo.lock` changes are - // in the works where they haven't been set as the default yet but will - // become the default soon. - // - // The scenarios we could be in are: - // - // * This is a brand new lock file with nothing previous. In that case - // this method isn't actually called at all, but instead - // `default_for_new_lockfiles` called below was encoded during the - // resolution step, so that's what we're gonna use. - // - // * We have an old lock file. In this case we want to switch the - // version to `default_for_old_lockfiles`. That puts us in one of - // three cases: - // - // * Our version is older than the default. This means that we're - // migrating someone forward, so we switch the encoding. - // * Our version is equal to the default, nothing to do! - // * Our version is *newer* than the default. This is where we - // critically keep the new version of encoding. - // - // This strategy should get new lockfiles into the pipeline more quickly - // while ensuring that any time an old cargo sees a future lock file it - // keeps the future lockfile encoding. - self.version = cmp::max( - previous.version, - ResolveVersion::default_for_old_lockfiles(), - ); + // Preserve the lockfile encoding where possible to avoid lockfile churn + self.version = previous.version; Ok(()) } @@ -383,6 +355,10 @@ unable to verify that `{0}` is the same as when the lockfile was generated self.version } + pub fn set_version(&mut self, version: ResolveVersion) { + self.version = version; + } + pub fn summary(&self, pkg_id: PackageId) -> &Summary { &self.summaries[&pkg_id] } @@ -418,27 +394,21 @@ impl fmt::Debug for Resolve { } } -impl ResolveVersion { - /// The default way to encode new `Cargo.lock` files. +impl Default for ResolveVersion { + /// The default way to encode new or updated `Cargo.lock` files. /// /// It's important that if a new version of `ResolveVersion` is added that /// this is not updated until *at least* the support for the version is in - /// the stable release of Rust. It's ok for this to be newer than - /// `default_for_old_lockfiles` below. - pub fn default_for_new_lockfiles() -> ResolveVersion { - ResolveVersion::V2 - } - - /// The default way to encode old preexisting `Cargo.lock` files. This is - /// often trailing the new lockfiles one above to give older projects a - /// longer time to catch up. + /// the stable release of Rust. /// - /// It's important that this trails behind `default_for_new_lockfiles` for - /// quite some time. This gives projects a quite large window to update in - /// where we don't force updates, so if projects span many versions of Cargo - /// all those versions of Cargo will have support for a new version of the - /// lock file. - pub fn default_for_old_lockfiles() -> ResolveVersion { - ResolveVersion::V1 + /// This resolve version will be used for all new lock files, for example + /// those generated by `cargo update` (update everything) or building after + /// a `cargo new` (where no lock file previously existed). This is also used + /// for *updated* lock files such as when a dependency is added or when a + /// version requirement changes. In this situation Cargo's updating the lock + /// file anyway so it takes the opportunity to bump the lock file version + /// forward. + fn default() -> ResolveVersion { + ResolveVersion::V2 } } diff --git a/src/cargo/ops/cargo_generate_lockfile.rs b/src/cargo/ops/cargo_generate_lockfile.rs index feccede48be..a86b34e3ae0 100644 --- a/src/cargo/ops/cargo_generate_lockfile.rs +++ b/src/cargo/ops/cargo_generate_lockfile.rs @@ -21,7 +21,7 @@ pub struct UpdateOptions<'a> { pub fn generate_lockfile(ws: &Workspace<'_>) -> CargoResult<()> { let mut registry = PackageRegistry::new(ws.config())?; - let resolve = ops::resolve_with_previous( + let mut resolve = ops::resolve_with_previous( &mut registry, ws, &ResolveOpts::everything(), @@ -30,7 +30,7 @@ pub fn generate_lockfile(ws: &Workspace<'_>) -> CargoResult<()> { &[], true, )?; - ops::write_pkg_lockfile(ws, &resolve)?; + ops::write_pkg_lockfile(ws, &mut resolve)?; Ok(()) } @@ -113,7 +113,7 @@ pub fn update_lockfile(ws: &Workspace<'_>, opts: &UpdateOptions<'_>) -> CargoRes registry.add_sources(sources)?; } - let resolve = ops::resolve_with_previous( + let mut resolve = ops::resolve_with_previous( &mut registry, ws, &ResolveOpts::everything(), @@ -153,7 +153,7 @@ pub fn update_lockfile(ws: &Workspace<'_>, opts: &UpdateOptions<'_>) -> CargoRes .shell() .warn("not updating lockfile due to dry run")?; } else { - ops::write_pkg_lockfile(ws, &resolve)?; + ops::write_pkg_lockfile(ws, &mut resolve)?; } return Ok(()); diff --git a/src/cargo/ops/cargo_package.rs b/src/cargo/ops/cargo_package.rs index fc98f5de66a..5dc077c4b4d 100644 --- a/src/cargo/ops/cargo_package.rs +++ b/src/cargo/ops/cargo_package.rs @@ -284,14 +284,14 @@ fn build_lock(ws: &Workspace<'_>) -> CargoResult { // Regenerate Cargo.lock using the old one as a guide. let tmp_ws = Workspace::ephemeral(new_pkg, ws.config(), None, true)?; - let (pkg_set, new_resolve) = ops::resolve_ws(&tmp_ws)?; + let (pkg_set, mut new_resolve) = ops::resolve_ws(&tmp_ws)?; if let Some(orig_resolve) = orig_resolve { compare_resolve(config, tmp_ws.current()?, &orig_resolve, &new_resolve)?; } check_yanked(config, &pkg_set, &new_resolve)?; - ops::resolve_to_string(&tmp_ws, &new_resolve) + ops::resolve_to_string(&tmp_ws, &mut new_resolve) } // Checks that the package has some piece of metadata that a human can diff --git a/src/cargo/ops/lockfile.rs b/src/cargo/ops/lockfile.rs index 05ffc0bdaa7..9ed0c76d27b 100644 --- a/src/cargo/ops/lockfile.rs +++ b/src/cargo/ops/lockfile.rs @@ -27,17 +27,17 @@ pub fn load_pkg_lockfile(ws: &Workspace<'_>) -> CargoResult> { } /// Generate a toml String of Cargo.lock from a Resolve. -pub fn resolve_to_string(ws: &Workspace<'_>, resolve: &Resolve) -> CargoResult { +pub fn resolve_to_string(ws: &Workspace<'_>, resolve: &mut Resolve) -> CargoResult { let (_orig, out, _ws_root) = resolve_to_string_orig(ws, resolve)?; Ok(out) } -pub fn write_pkg_lockfile(ws: &Workspace<'_>, resolve: &Resolve) -> CargoResult<()> { - let (orig, out, ws_root) = resolve_to_string_orig(ws, resolve)?; +pub fn write_pkg_lockfile(ws: &Workspace<'_>, resolve: &mut Resolve) -> CargoResult<()> { + let (orig, mut out, ws_root) = resolve_to_string_orig(ws, resolve)?; // If the lock file contents haven't changed so don't rewrite it. This is // helpful on read-only filesystems. - if let Some(orig) = orig { + if let Some(orig) = &orig { if are_equal_lockfiles(orig, &out, ws) { return Ok(()); } @@ -62,6 +62,16 @@ pub fn write_pkg_lockfile(ws: &Workspace<'_>, resolve: &Resolve) -> CargoResult< ); } + // While we're updating the lock file anyway go ahead and update its + // encoding to whatever the latest default is. That way we can slowly roll + // 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. + if resolve.version() < ResolveVersion::default() { + resolve.set_version(ResolveVersion::default()); + out = serialize_resolve(resolve, orig.as_deref()); + } + // Ok, if that didn't work just write it out ws_root .open_rw("Cargo.lock", ws.config(), "Cargo.lock file") @@ -76,7 +86,7 @@ pub fn write_pkg_lockfile(ws: &Workspace<'_>, resolve: &Resolve) -> CargoResult< fn resolve_to_string_orig( ws: &Workspace<'_>, - resolve: &Resolve, + resolve: &mut Resolve, ) -> CargoResult<(Option, String, Filesystem)> { // Load the original lock file if it exists. let ws_root = Filesystem::new(ws.root().to_path_buf()); @@ -86,7 +96,11 @@ fn resolve_to_string_orig( f.read_to_string(&mut s)?; Ok(s) }); + let out = serialize_resolve(resolve, orig.as_ref().ok().map(|s| &**s)); + Ok((orig.ok(), out, ws_root)) +} +fn serialize_resolve(resolve: &Resolve, orig: Option<&str>) -> String { let toml = toml::Value::try_from(resolve).unwrap(); let mut out = String::new(); @@ -100,7 +114,7 @@ fn resolve_to_string_orig( out.push_str(extra_line); out.push('\n'); // and preserve any other top comments - if let Ok(orig) = &orig { + if let Some(orig) = orig { let mut comments = orig.lines().take_while(|line| line.starts_with('#')); if let Some(first) = comments.next() { if first != marker_line { @@ -156,11 +170,11 @@ fn resolve_to_string_orig( out.pop(); } } - - Ok((orig.ok(), out, ws_root)) + out } -fn are_equal_lockfiles(mut orig: String, current: &str, ws: &Workspace<'_>) -> bool { +fn are_equal_lockfiles(orig: &str, current: &str, ws: &Workspace<'_>) -> bool { + let mut orig = orig.to_string(); if has_crlf_line_endings(&orig) { orig = orig.replace("\r\n", "\n"); } diff --git a/src/cargo/ops/resolve.rs b/src/cargo/ops/resolve.rs index 7d6065374c9..05b3f5961e3 100644 --- a/src/cargo/ops/resolve.rs +++ b/src/cargo/ops/resolve.rs @@ -165,7 +165,7 @@ fn resolve_with_registry<'cfg>( registry: &mut PackageRegistry<'cfg>, ) -> CargoResult { let prev = ops::load_pkg_lockfile(ws)?; - let resolve = resolve_with_previous( + let mut resolve = resolve_with_previous( registry, ws, &ResolveOpts::everything(), @@ -176,7 +176,7 @@ fn resolve_with_registry<'cfg>( )?; if !ws.is_ephemeral() && ws.require_optional_deps() { - ops::write_pkg_lockfile(ws, &resolve)?; + ops::write_pkg_lockfile(ws, &mut resolve)?; } Ok(resolve) } diff --git a/tests/testsuite/lockfile_compat.rs b/tests/testsuite/lockfile_compat.rs index 7dec021058f..dbda8e0466a 100644 --- a/tests/testsuite/lockfile_compat.rs +++ b/tests/testsuite/lockfile_compat.rs @@ -29,16 +29,14 @@ fn oldest_lockfile_still_works_with_command(cargo_command: &str) { name = "bar" version = "0.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "[..]" [[package]] name = "foo" version = "0.0.1" dependencies = [ - "bar [..]", + "bar", ] - -[metadata] -"[..]" = "[..]" "#; let old_lockfile = r#" @@ -132,14 +130,14 @@ fn totally_wild_checksums_works() { .file( "Cargo.toml", r#" - [project] - name = "foo" - version = "0.0.1" - authors = [] + [project] + name = "foo" + version = "0.0.1" + authors = [] - [dependencies] - bar = "0.1.0" - "#, + [dependencies] + bar = "0.1.0" + "#, ) .file("src/lib.rs", "") .file( @@ -175,16 +173,14 @@ source = "registry+https://github.com/rust-lang/crates.io-index" name = "bar" version = "0.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "[..]" [[package]] name = "foo" version = "0.0.1" dependencies = [ - "bar [..]", + "bar", ] - -[metadata] -"[..]" = "[..]" "#, &lock, ); @@ -722,3 +718,47 @@ Caused by: .with_status(101) .run(); } + +#[cargo_test] +fn preserve_old_format_if_no_update_needed() { + let cksum = Package::new("bar", "0.1.0").publish(); + let 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#" + [project] + name = "foo" + version = "0.0.1" + authors = [] + + [dependencies] + bar = "0.1.0" + "#, + ) + .file("src/lib.rs", "") + .file("Cargo.lock", &lockfile) + .build(); + + p.cargo("build --locked").run(); +}