[cargo-bazel] Avoid regenerating cargo lockfile when splicing if not needed

Author: dzbarsky

crate_universe/extensions.bzl

@@ -645,6 +645,7 @@ def _generate_hub_and_spokes(
             config_path = config_file,
             output_dir = tag_path.get_child("splicing-output"),
             debug_workspace_dir = tag_path.get_child("splicing-workspace"),
+            skip_cargo_lockfile_overwrite = cfg.skip_cargo_lockfile_overwrite,
             repository_name = cfg.name,
         )
 

crate_universe/private/crates_repository.bzl

@@ -91,6 +91,7 @@ def _crates_repository_impl(repository_ctx):
             splicing_manifest = splicing_manifest,
             config_path = config_path,
             output_dir = repository_ctx.path("splicing-output"),
+            skip_cargo_lockfile_overwrite = repository_ctx.attr.skip_cargo_lockfile_overwrite,
             repository_name = repository_ctx.name,
         )
 

crate_universe/private/generate_utils.bzl

@@ -497,9 +497,7 @@ def execute_generator(
         ])
 
     if skip_cargo_lockfile_overwrite:
-        args.extend([
-            "--skip-cargo-lockfile-overwrite",
-        ])
+        args.append("--skip-cargo-lockfile-overwrite")
 
     # Some components are not required unless re-pinning is enabled
     if metadata:

crate_universe/private/splicing_utils.bzl

@@ -123,6 +123,7 @@ def splice_workspace_manifest(
         config_path,
         output_dir,
         repository_name,
+        skip_cargo_lockfile_overwrite,
         debug_workspace_dir = None):
     """Splice together a Cargo workspace from various other manifests and package definitions
 
@@ -134,6 +135,9 @@ def splice_workspace_manifest(
         config_path (path): The path to the config file (containing `cargo_bazel::config::Config`.)
         output_dir (path): THe location in which to write splicing outputs.
         repository_name (str): Name of the repository being generated.
+        skip_cargo_lockfile_overwrite (bool): Whether to skip writing the cargo lockfile back after resolving.
+            You may want to set this if your dependency versions are maintained externally through a non-trivial set-up.
+            But you probably don't want to set this.
         debug_workspace_dir (path): The location in which to save splicing outputs for future review.
 
     Returns:
@@ -159,6 +163,9 @@ def splice_workspace_manifest(
             cargo_lockfile,
         ])
 
+    if skip_cargo_lockfile_overwrite:
+        arguments.append("--skip-cargo-lockfile-overwrite")
+
     # Optionally set the splicing workspace directory to somewhere within the repository directory
     # to improve the debugging experience.
     if CARGO_BAZEL_DEBUG in repository_ctx.os.environ:

crate_universe/src/cli/splice.rs

@@ -64,6 +64,12 @@ pub struct SpliceOptions {
     /// The name of the repository being generated.
     #[clap(long)]
     pub repository_name: String,
+
+    /// Whether to skip writing the cargo lockfile back after resolving.
+    /// You may want to set this if your dependency versions are maintained externally through a non-trivial set-up.
+    /// But you probably don't want to set this.
+    #[clap(long)]
+    pub skip_cargo_lockfile_overwrite: bool,
 }
 
 /// Combine a set of disjoint manifests into a single workspace.
@@ -94,14 +100,22 @@ pub fn splice(opt: SpliceOptions) -> Result<()> {
         .splice(&splicing_dir)
         .with_context(|| format!("Failed to splice workspace {}", opt.repository_name))?;
 
-    // Generate a lockfile
-    let cargo_lockfile = generate_lockfile(
-        &manifest_path,
-        &opt.cargo_lockfile,
-        cargo.clone(),
-        &opt.repin,
-    )
-    .context("Failed to generate lockfile")?;
+    // Use the existing lockfile if possible, otherwise generate a new one.
+    let cargo_lockfile = if opt.cargo_lockfile.is_some() && opt.skip_cargo_lockfile_overwrite {
+        let cargo_lockfile_path = opt.cargo_lockfile.unwrap();
+        cargo_lock::Lockfile::load(&cargo_lockfile_path).context(format!(
+            "Failed to load lockfile: {}",
+            cargo_lockfile_path.display()
+        ))?
+    } else {
+        generate_lockfile(
+            &manifest_path,
+            &opt.cargo_lockfile,
+            cargo.clone(),
+            &opt.repin,
+        )
+        .context("Failed to generate lockfile")?
+    };
 
     let config = Config::try_from_path(&opt.config).context("Failed to parse config")?;
 

crate_universe/tests/cargo_integration_test.rs

@@ -117,6 +117,7 @@ fn run(repository_name: &str, manifests: HashMap<String, String>, lockfile: &str
         cargo,
         rustc,
         repository_name: String::from("crates_index"),
+        skip_cargo_lockfile_overwrite: false,
     })
     .unwrap();