diff --git a/src/cargo/core/resolver/mod.rs b/src/cargo/core/resolver/mod.rs index 5e6592258..85d107430 100644 --- a/src/cargo/core/resolver/mod.rs +++ b/src/cargo/core/resolver/mod.rs @@ -151,7 +151,7 @@ pub fn resolve( cksums, BTreeMap::new(), Vec::new(), - ResolveVersion::default(), + ResolveVersion::default_for_new_lockfiles(), ); check_cycles(&resolve)?; diff --git a/src/cargo/core/resolver/resolve.rs b/src/cargo/core/resolver/resolve.rs index 00d7eb5f0..ffd169a88 100644 --- a/src/cargo/core/resolver/resolve.rs +++ b/src/cargo/core/resolver/resolve.rs @@ -1,4 +1,5 @@ use std::borrow::Borrow; +use std::cmp; use std::collections::{HashMap, HashSet}; use std::fmt; use std::iter::FromIterator; @@ -58,9 +59,12 @@ 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(PartialEq, Clone, Debug)] +#[derive(PartialEq, Eq, Clone, Copy, Debug, PartialOrd, Ord)] pub enum ResolveVersion { + // Historical baseline for when this abstraction was added. V1, + // Update around 2019 where `dependencies` arrays got compressed and + // checksums are listed inline. V2, } @@ -209,21 +213,35 @@ 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. We want to preserve those - // features if we find them. + // 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. // - // For this reason if the previous `Cargo.lock` is from the future, or - // otherwise it looks like it's produced with future features we - // understand, then the new resolve will be encoded with the same - // version. Note that new instances of `Resolve` always use the default - // encoding, and this is where we switch it to a future encoding if the - // future encoding isn't yet the default. - if previous.version.from_the_future() { - self.version = previous.version.clone(); - } + // 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(), + ); Ok(()) } @@ -389,23 +407,26 @@ impl fmt::Debug for Resolve { } impl ResolveVersion { - /// The default way to encode `Cargo.lock`. + /// The default way to encode new `Cargo.lock` files. /// - /// This is used for new `Cargo.lock` files that are generated without a - /// previous `Cargo.lock` files, and generally matches with what we want to - /// encode. - pub fn default() -> ResolveVersion { + /// 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 } - /// Returns whether this encoding version is "from the future". + /// 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. /// - /// This means that this encoding version is not currently the default but - /// intended to become the default "soon". - pub fn from_the_future(&self) -> bool { - match self { - ResolveVersion::V2 => false, - ResolveVersion::V1 => false, - } + /// 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 } } diff --git a/tests/testsuite/lockfile_compat.rs b/tests/testsuite/lockfile_compat.rs index faf35e435..2f0c34c4d 100644 --- a/tests/testsuite/lockfile_compat.rs +++ b/tests/testsuite/lockfile_compat.rs @@ -27,14 +27,16 @@ 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#" @@ -171,14 +173,16 @@ 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, ); diff --git a/tests/testsuite/update.rs b/tests/testsuite/update.rs index 1f7a2ed59..93f27e34f 100644 --- a/tests/testsuite/update.rs +++ b/tests/testsuite/update.rs @@ -572,7 +572,7 @@ fn preserve_top_comment() { let mut lines = lockfile.lines().collect::>(); lines.insert(2, "# some other comment"); let mut lockfile = lines.join("\n"); - lockfile.push_str("\n"); // .lines/.join loses the last newline + lockfile.push_str("\n\n"); // .lines/.join loses the last newline println!("saving Cargo.lock contents:\n{}", lockfile); p.change_file("Cargo.lock", &lockfile);