From b265c47b46b2ad011a41f6c2ceff80d2c696cb9d Mon Sep 17 00:00:00 2001 From: David Zbarsky Date: Thu, 4 Sep 2025 16:38:15 -0400 Subject: [PATCH] [cargo-bazel] Avoid regenerating cargo lockfile when splicing if not needed --- crate_universe/extensions.bzl | 1 + crate_universe/private/crates_repository.bzl | 1 + crate_universe/private/generate_utils.bzl | 4 +-- crate_universe/private/splicing_utils.bzl | 7 +++++ crate_universe/src/cli/splice.rs | 30 ++++++++++++++----- .../tests/cargo_integration_test.rs | 1 + 6 files changed, 33 insertions(+), 11 deletions(-) diff --git a/crate_universe/extensions.bzl b/crate_universe/extensions.bzl index 2f105af3a2..b6053e1774 100644 --- a/crate_universe/extensions.bzl +++ b/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, ) diff --git a/crate_universe/private/crates_repository.bzl b/crate_universe/private/crates_repository.bzl index 659a4f2694..4ed8a32ab0 100644 --- a/crate_universe/private/crates_repository.bzl +++ b/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, ) diff --git a/crate_universe/private/generate_utils.bzl b/crate_universe/private/generate_utils.bzl index a59f4a96cb..adb1f6f909 100644 --- a/crate_universe/private/generate_utils.bzl +++ b/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: diff --git a/crate_universe/private/splicing_utils.bzl b/crate_universe/private/splicing_utils.bzl index c0aac189d3..7fe1e68461 100644 --- a/crate_universe/private/splicing_utils.bzl +++ b/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: diff --git a/crate_universe/src/cli/splice.rs b/crate_universe/src/cli/splice.rs index f245b7ad07..d07d4dd802 100644 --- a/crate_universe/src/cli/splice.rs +++ b/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")?; diff --git a/crate_universe/tests/cargo_integration_test.rs b/crate_universe/tests/cargo_integration_test.rs index 7ce82d68d1..7b74101fa7 100644 --- a/crate_universe/tests/cargo_integration_test.rs +++ b/crate_universe/tests/cargo_integration_test.rs @@ -117,6 +117,7 @@ fn run(repository_name: &str, manifests: HashMap, lockfile: &str cargo, rustc, repository_name: String::from("crates_index"), + skip_cargo_lockfile_overwrite: false, }) .unwrap();