Merge #337

337: Add '--to-lockfile' flag to cargo-upgrade r=ordian a=tofay

When this flag is present, all dependencies in 'Cargo.toml'
are upgraded to the locked version as recorded in 'Cargo.lock'.

Fixes #297

Co-authored-by: Tom Fay <[email protected]>
Co-authored-by: Tom Fay <[email protected]>

Author: bors[bot]

src/bin/upgrade/main.rs

@@ -55,7 +55,12 @@ Dev, build, and all target dependencies will also be upgraded. Only dependencies
 supported. Git/path dependencies will be ignored.
 
 All packages in the workspace will be upgraded if the `--all` flag is supplied. The `--all` flag may
-be supplied in the presence of a virtual manifest."
+be supplied in the presence of a virtual manifest.
+
+If the '--to-lockfile' flag is supplied, all dependencies will be upgraded to the currently locked
+version as recorded in the Cargo.lock file. This flag requires that the Cargo.lock file is
+up-to-date. If the lock file is missing, or it needs to be updated, cargo-upgrade will exit with an
+error. If the '--to-lockfile' flag is supplied then the network won't be accessed."
     )]
     Upgrade(Args),
 }
@@ -84,11 +89,42 @@ struct Args {
     /// Run without accessing the network
     #[structopt(long = "offline")]
     pub offline: bool,
+
+    /// Upgrade all packages to the version in the lockfile.
+    #[structopt(long = "to-lockfile", conflicts_with = "dependency")]
+    pub to_lockfile: bool,
 }
 
 /// A collection of manifests.
 struct Manifests(Vec<(LocalManifest, cargo_metadata::Package)>);
 
+/// Helper function to check whether a `cargo_metadata::Dependency` is a version dependency.
+fn is_version_dep(dependency: &cargo_metadata::Dependency) -> bool {
+    match dependency.source {
+        // This is the criterion cargo uses (in `SourceId::from_url`) to decide whether a
+        // dependency has the 'registry' kind.
+        Some(ref s) => s.splitn(2, '+').next() == Some("registry"),
+        _ => false,
+    }
+}
+
+fn dry_run_message() -> Result<()> {
+    let bufwtr = BufferWriter::stdout(ColorChoice::Always);
+    let mut buffer = bufwtr.buffer();
+    buffer
+        .set_color(ColorSpec::new().set_fg(Some(Color::Cyan)).set_bold(true))
+        .chain_err(|| "Failed to set output colour")?;
+    write!(&mut buffer, "Starting dry run. ").chain_err(|| "Failed to write dry run message")?;
+    buffer
+        .set_color(&ColorSpec::new())
+        .chain_err(|| "Failed to clear output colour")?;
+    writeln!(&mut buffer, "Changes will not be saved.")
+        .chain_err(|| "Failed to write dry run message")?;
+    bufwtr
+        .print(&buffer)
+        .chain_err(|| "Failed to print dry run message")
+}
+
 impl Manifests {
     /// Get all manifests in the workspace.
     fn get_all(manifest_path: &Option<PathBuf>) -> Result<Self> {
@@ -145,16 +181,6 @@ impl Manifests {
     /// Get the the combined set of dependencies to upgrade. If the user has specified
     /// per-dependency desired versions, extract those here.
     fn get_dependencies(&self, only_update: Vec<String>) -> Result<DesiredUpgrades> {
-        /// Helper function to check whether a `cargo_metadata::Dependency` is a version dependency.
-        fn is_version_dep(dependency: &cargo_metadata::Dependency) -> bool {
-            match dependency.source {
-                // This is the criterion cargo uses (in `SourceId::from_url`) to decide whether a
-                // dependency has the 'registry' kind.
-                Some(ref s) => s.splitn(2, '+').next() == Some("registry"),
-                _ => false,
-            }
-        }
-
         // Map the names of user-specified dependencies to the (optionally) requested version.
         let selected_dependencies = only_update
             .into_iter()
@@ -203,21 +229,7 @@ impl Manifests {
     /// Upgrade the manifests on disk following the previously-determined upgrade schema.
     fn upgrade(self, upgraded_deps: &ActualUpgrades, dry_run: bool) -> Result<()> {
         if dry_run {
-            let bufwtr = BufferWriter::stdout(ColorChoice::Always);
-            let mut buffer = bufwtr.buffer();
-            buffer
-                .set_color(ColorSpec::new().set_fg(Some(Color::Cyan)).set_bold(true))
-                .chain_err(|| "Failed to set output colour")?;
-            write!(&mut buffer, "Starting dry run. ")
-                .chain_err(|| "Failed to write dry run message")?;
-            buffer
-                .set_color(&ColorSpec::new())
-                .chain_err(|| "Failed to clear output colour")?;
-            writeln!(&mut buffer, "Changes will not be saved.")
-                .chain_err(|| "Failed to write dry run message")?;
-            bufwtr
-                .print(&buffer)
-                .chain_err(|| "Failed to print dry run message")?;
+            dry_run_message()?;
         }
 
         for (mut manifest, package) in self.0 {
@@ -234,6 +246,61 @@ impl Manifests {
 
         Ok(())
     }
+
+    /// Update dependencies in Cargo.toml file(s) to match the corresponding
+    /// version in Cargo.lock.
+    fn sync_to_lockfile(self, dry_run: bool) -> Result<()> {
+        // Get locked dependencies. For workspaces with multiple Cargo.toml
+        // files, there is only a single lockfile, so it suffices to get
+        // metadata for any one of Cargo.toml files.
+        let (manifest, _package) =
+            self.0.iter().next().ok_or_else(|| {
+                ErrorKind::CargoEditLib(::cargo_edit::ErrorKind::InvalidCargoConfig)
+            })?;
+        let mut cmd = cargo_metadata::MetadataCommand::new();
+        cmd.manifest_path(manifest.path.clone());
+        cmd.other_options(vec!["--locked".to_string()]);
+
+        let result = cmd
+            .exec()
+            .map_err(|e| Error::from(e.compat()).chain_err(|| "Invalid manifest"))?;
+
+        let locked = result
+            .packages
+            .into_iter()
+            .filter(|p| p.source.is_some()) // Source is none for local packages
+            .collect::<Vec<_>>();
+
+        if dry_run {
+            dry_run_message()?;
+        }
+
+        for (mut manifest, package) in self.0 {
+            println!("{}:", package.name);
+
+            // Upgrade the manifests one at a time, as multiple manifests may
+            // request the same dependency at differing versions.
+            for (name, version) in package
+                .dependencies
+                .clone()
+                .into_iter()
+                .filter(is_version_dep)
+                .filter_map(|d| {
+                    for p in &locked {
+                        // The requested dependency may be present in the lock file with different versions,
+                        // but only one will be semver-compatible with the requested version.
+                        if d.name == p.name && d.req.matches(&p.version) {
+                            return Some((d.name, p.version.to_string()));
+                        }
+                    }
+                    None
+                })
+            {
+                manifest.upgrade(&Dependency::new(&name).set_version(&version), dry_run)?;
+            }
+        }
+        Ok(())
+    }
 }
 
 /// The set of dependencies to be upgraded, alongside the registries returned from cargo metadata, and
@@ -287,10 +354,11 @@ fn process(args: Args) -> Result<()> {
         all,
         allow_prerelease,
         dry_run,
+        to_lockfile,
         ..
     } = args;
 
-    if !args.offline {
+    if !args.offline && !to_lockfile {
         let url = registry_url(&find(&manifest_path)?, None)?;
         update_registry_index(&url)?;
     }
@@ -301,27 +369,31 @@ fn process(args: Args) -> Result<()> {
         Manifests::get_local_one(&manifest_path)
     }?;
 
-    let existing_dependencies = manifests.get_dependencies(dependency)?;
-
-    // Update indices for any alternative registries, unless
-    // we're offline.
-    if !args.offline {
-        for registry_url in existing_dependencies
-            .0
-            .values()
-            .filter_map(|(registry, _)| registry.as_ref())
-            .collect::<HashSet<_>>()
-        {
-            update_registry_index(&Url::parse(registry_url).map_err(|_| {
-                ErrorKind::CargoEditLib(::cargo_edit::ErrorKind::InvalidCargoConfig)
-            })?)?;
+    if to_lockfile {
+        manifests.sync_to_lockfile(dry_run)
+    } else {
+        let existing_dependencies = manifests.get_dependencies(dependency)?;
+
+        // Update indices for any alternative registries, unless
+        // we're offline.
+        if !args.offline {
+            for registry_url in existing_dependencies
+                .0
+                .values()
+                .filter_map(|(registry, _)| registry.as_ref())
+                .collect::<HashSet<_>>()
+            {
+                update_registry_index(&Url::parse(registry_url).map_err(|_| {
+                    ErrorKind::CargoEditLib(::cargo_edit::ErrorKind::InvalidCargoConfig)
+                })?)?;
+            }
         }
-    }
 
-    let upgraded_dependencies =
-        existing_dependencies.get_upgraded(allow_prerelease, &find(&manifest_path)?)?;
+        let upgraded_dependencies =
+            existing_dependencies.get_upgraded(allow_prerelease, &find(&manifest_path)?)?;
 
-    manifests.upgrade(&upgraded_dependencies, dry_run)
+        manifests.upgrade(&upgraded_dependencies, dry_run)
+    }
 }
 
 fn main() {

src/manifest.rs

@@ -366,7 +366,7 @@ impl str::FromStr for Manifest {
 #[derive(Debug)]
 pub struct LocalManifest {
     /// Path to the manifest
-    path: PathBuf,
+    pub path: PathBuf,
     /// Manifest contents
     manifest: Manifest,
 }

tests/cargo-upgrade.rs

@@ -1,7 +1,7 @@
 #[macro_use]
 extern crate pretty_assertions;
 
-use std::fs;
+use std::{fs, path::Path};
 
 mod utils;
 use crate::utils::{
@@ -41,6 +41,7 @@ pub fn copy_workspace_test() -> (tempdir::TempDir, String, Vec<String>) {
 
         let root_manifest_path = copy_in(".", "Cargo.toml");
         copy_in(".", "dummy.rs");
+        copy_in(".", "Cargo.lock");
 
         let workspace_manifest_paths = ["one", "two", "implicit/three", "explicit/four"]
             .iter()
@@ -415,6 +416,40 @@ For more information try --help ",
     .unwrap();
 }
 
+// Verify that an upgraded Cargo.toml matches what we expect.
+#[test]
+fn upgrade_to_lockfile() {
+    let (tmpdir, manifest) = clone_out_test("tests/fixtures/upgrade/Cargo.toml.lockfile_source");
+    fs::copy(
+        Path::new("tests/fixtures/upgrade/Cargo.lock"),
+        tmpdir.path().join("Cargo.lock"),
+    )
+    .unwrap_or_else(|err| panic!("could not copy test lock file: {}", err));;
+    execute_command(&["upgrade", "--to-lockfile"], &manifest);
+
+    let upgraded = get_toml(&manifest);
+    let target = get_toml("tests/fixtures/upgrade/Cargo.toml.lockfile_target");
+
+    assert_eq!(target.to_string(), upgraded.to_string());
+}
+
+#[test]
+fn upgrade_workspace_to_lockfile() {
+    let (tmpdir, root_manifest, _workspace_manifests) = copy_workspace_test();
+
+    execute_command(&["upgrade", "--all", "--to-lockfile"], &root_manifest);
+
+    // The members one and two both request different, semver incompatible
+    // versions of rand. Test that both were upgraded correctly.
+    let one_upgraded = get_toml(tmpdir.path().join("one/Cargo.toml").to_str().unwrap());
+    let one_target = get_toml("tests/fixtures/workspace/one/Cargo.toml.lockfile_target");
+    assert_eq!(one_target.to_string(), one_upgraded.to_string());
+
+    let two_upgraded = get_toml(tmpdir.path().join("two/Cargo.toml").to_str().unwrap());
+    let two_target = get_toml("tests/fixtures/workspace/two/Cargo.toml.lockfile_target");
+    assert_eq!(two_target.to_string(), two_upgraded.to_string());
+}
+
 #[test]
 #[cfg(feature = "test-external-apis")]
 fn upgrade_prints_messages() {

tests/fixtures/upgrade/Cargo.lock

@@ -0,0 +1,10 @@
+[package]
+name = "one"
+version = "0.1.0"
+
+[lib]
+path = "../dummy.rs"
+
+[dependencies]
+libc = "0.2.28"
+rand = "0.3"

tests/fixtures/upgrade/Cargo.toml.lockfile_source

@@ -0,0 +1,10 @@
+[package]
+name = "one"
+version = "0.1.0"
+
+[lib]
+path = "../dummy.rs"
+
+[dependencies]
+libc = "0.2.65"
+rand = "0.3.10"