Update lock file encodings on changes

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.
 //!

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,
     );
 

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
     }
 }

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(());
 

src/cargo/ops/cargo_package.rs

@@ -284,14 +284,14 @@ fn build_lock(ws: &Workspace<'_>) -> CargoResult<String> {
 
     // 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

src/cargo/ops/lockfile.rs

@@ -27,17 +27,17 @@ pub fn load_pkg_lockfile(ws: &Workspace<'_>) -> CargoResult<Option<Resolve>> {
 }
 
 /// Generate a toml String of Cargo.lock from a Resolve.
-pub fn resolve_to_string(ws: &Workspace<'_>, resolve: &Resolve) -> CargoResult<String> {
+pub fn resolve_to_string(ws: &Workspace<'_>, resolve: &mut Resolve) -> CargoResult<String> {
     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>, 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");
     }

src/cargo/ops/resolve.rs

@@ -165,7 +165,7 @@ fn resolve_with_registry<'cfg>(
     registry: &mut PackageRegistry<'cfg>,
 ) -> CargoResult<Resolve> {
     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)
 }