From 013c1afb9ac899f1cd0706bc61c998497e2b6d93 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 12 Nov 2019 07:19:27 -0800 Subject: [PATCH] Add support for new lockfile formats on new projects This commit adds support to Cargo and refactors the lockfile versioning slightly. The goal here is that Cargo continually has two thresholds of lockfile formats to create: * One is used for new lock files at all times * The other is used to update old lock files The logic for these two thresholds is appropriately updated throughout Cargo, and then this commit also preserves the previous update where new lock files will get the new format, but old lockfiles will continue to retain the old format by default. --- src/cargo/core/resolver/mod.rs | 2 +- src/cargo/core/resolver/resolve.rs | 77 +++++++++++++++++++----------- tests/testsuite/lockfile_compat.rs | 12 +++-- tests/testsuite/update.rs | 2 +- 4 files changed, 59 insertions(+), 34 deletions(-) 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);