Fixed `crates_repository` deleting `.cargo/config.toml` files. (#1227)
* Fixed `crates_repository` deleting `.cargo/config.toml` files.
* Added example `config.toml`
* Updated lockfile.
* Ensure parent directories do not have cargo configs
* Added tests
* Update crate_universe/src/splicing/splicer.rs
Co-authored-by: Daniel Wagner-Hall <dawagner@gmail.com>
* Run splicing in temp directories to avoid configs in home directories
* Update crate_universe/src/cli/splice.rs
Co-authored-by: Daniel Wagner-Hall <dawagner@gmail.com>
* rustfmt
Co-authored-by: Daniel Wagner-Hall <dawagner@gmail.com>
diff --git a/crate_universe/private/splicing_utils.bzl b/crate_universe/private/splicing_utils.bzl
index a419c75..249e7a4 100644
--- a/crate_universe/private/splicing_utils.bzl
+++ b/crate_universe/private/splicing_utils.bzl
@@ -2,6 +2,8 @@
load(":common_utils.bzl", "cargo_environ", "execute")
+CARGO_BAZEL_DEBUG = "CARGO_BAZEL_DEBUG"
+
def splicing_config(resolver_version = "1"):
"""arious settings used to configure Cargo manifest splicing behavior.
@@ -170,14 +172,14 @@
), indent = " " * 4),
)
- cargo_workspace = repository_ctx.path("{}/cargo-bazel-splicing".format(repo_dir))
+ splicing_output_dir = repository_ctx.path("splicing-output")
# Generate a workspace root which contains all workspace members
arguments = [
generator,
"splice",
- "--workspace-dir",
- cargo_workspace,
+ "--output-dir",
+ splicing_output_dir,
"--splicing-manifest",
splicing_manifest,
"--extra-manifests-manifest",
@@ -188,6 +190,14 @@
rustc,
]
+ # 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:
+ arguments.extend([
+ "--workspace-dir",
+ repository_ctx.path("{}/splicing-workspace".format(repo_dir)),
+ ])
+
# Splicing accepts a Cargo.lock file in some scenarios. Ensure it's passed
# if the lockfile is a actually a Cargo lockfile.
if lockfile.kind == "cargo":
@@ -211,13 +221,12 @@
env = env,
)
- root_manifest = repository_ctx.path("{}/Cargo.toml".format(cargo_workspace))
- if not root_manifest.exists:
- fail("Root manifest does not exist: {}".format(root_manifest))
+ # This file must have been produced by the execution above.
+ spliced_lockfile = repository_ctx.path("{}/Cargo.lock".format(splicing_output_dir))
+ if not spliced_lockfile.exists:
+ fail("Lockfile file does not exist: {}".format(spliced_lockfile))
+ spliced_metadata = repository_ctx.path("{}/metadata.json".format(splicing_output_dir))
+ if not spliced_metadata.exists:
+ fail("Metadata file does not exist: {}".format(spliced_metadata))
- # This file must match the one generated in splicing
- metadata_path = repository_ctx.path("{}/cargo-bazel-spliced-metadata.json".format(cargo_workspace))
- if not metadata_path.exists:
- fail("Root metadata file does not exist: {}".format(metadata_path))
-
- return metadata_path
+ return spliced_metadata
diff --git a/crate_universe/src/cli/splice.rs b/crate_universe/src/cli/splice.rs
index cb8ba20..e8e72ee 100644
--- a/crate_universe/src/cli/splice.rs
+++ b/crate_universe/src/cli/splice.rs
@@ -2,6 +2,7 @@
use std::path::PathBuf;
+use anyhow::Context;
use clap::Parser;
use crate::cli::Result;
@@ -26,10 +27,14 @@
#[clap(long)]
pub cargo_lockfile: Option<PathBuf>,
- /// The directory in which to build the workspace. A `Cargo.toml` file
- /// should always be produced within this directory.
+ /// The directory in which to build the workspace. If this argument is not
+ /// passed, a temporary directory will be generated.
#[clap(long)]
- pub workspace_dir: PathBuf,
+ pub workspace_dir: Option<PathBuf>,
+
+ /// The location where the results of splicing are written.
+ #[clap(long)]
+ pub output_dir: PathBuf,
/// If true, outputs will be printed instead of written to disk.
#[clap(long)]
@@ -55,12 +60,18 @@
let extra_manifests_manifest =
ExtraManifestsManifest::try_from_path(opt.extra_manifests_manifest)?;
+ // Determine the splicing workspace
+ let temp_dir;
+ let splicing_dir = match &opt.workspace_dir {
+ Some(dir) => dir.clone(),
+ None => {
+ temp_dir = tempfile::tempdir().context("Failed to generate temporary directory")?;
+ temp_dir.as_ref().to_path_buf()
+ }
+ };
+
// Generate a splicer for creating a Cargo workspace manifest
- let splicer = Splicer::new(
- opt.workspace_dir,
- splicing_manifest,
- extra_manifests_manifest,
- )?;
+ let splicer = Splicer::new(splicing_dir, splicing_manifest, extra_manifests_manifest)?;
// Splice together the manifest
let manifest_path = splicer.splice_workspace()?;
@@ -72,19 +83,33 @@
// Write the registry url info to the manifest now that a lockfile has been generated
WorkspaceMetadata::write_registry_urls(&cargo_lockfile, &manifest_path)?;
+ let output_dir = opt.output_dir.clone();
+
// Write metadata to the workspace for future reuse
let (cargo_metadata, _) = Generator::new()
.with_cargo(opt.cargo)
.with_rustc(opt.rustc)
.generate(&manifest_path.as_path_buf())?;
- // Write metadata next to the manifest
- let metadata_path = manifest_path
+ let cargo_lockfile_path = manifest_path
.as_path_buf()
.parent()
- .expect("Newly spliced cargo manifest has no parent directory")
- .join("cargo-bazel-spliced-metadata.json");
- write_metadata(&metadata_path, &cargo_metadata)?;
+ .with_context(|| {
+ format!(
+ "The path {} is expected to have a parent directory",
+ manifest_path.as_path_buf().display()
+ )
+ })?
+ .join("Cargo.lock");
+
+ // Generate the consumable outputs of the splicing process
+ std::fs::create_dir_all(&output_dir)
+ .with_context(|| format!("Failed to create directories for {}", &output_dir.display()))?;
+
+ write_metadata(&opt.output_dir.join("metadata.json"), &cargo_metadata)?;
+
+ std::fs::copy(cargo_lockfile_path, output_dir.join("Cargo.lock"))
+ .context("Failed to copy lockfile")?;
Ok(())
}
diff --git a/crate_universe/src/splicing/splicer.rs b/crate_universe/src/splicing/splicer.rs
index f4afde7..747f3ef 100644
--- a/crate_universe/src/splicing/splicer.rs
+++ b/crate_universe/src/splicing/splicer.rs
@@ -344,21 +344,6 @@
/// A helper for installing Cargo config files into the spliced workspace while also
/// ensuring no other linked config file is available
fn setup_cargo_config(cargo_config_path: &Option<PathBuf>, workspace_dir: &Path) -> Result<()> {
- // Make sure no other config files exist
- for config in vec![
- workspace_dir.join("config"),
- workspace_dir.join("config.toml"),
- ] {
- if config.exists() {
- fs::remove_file(&config).with_context(|| {
- format!(
- "Failed to delete existing cargo config: {}",
- config.display()
- )
- })?;
- }
- }
-
// If the `.cargo` dir is a symlink, we'll need to relink it and ensure
// a Cargo config file is omitted
let dot_cargo_dir = workspace_dir.join(".cargo");
@@ -383,12 +368,53 @@
dot_cargo_dir.join("config.toml"),
] {
if config.exists() {
- fs::remove_file(&config)?;
+ remove_symlink(&config).with_context(|| {
+ format!(
+ "Failed to delete existing cargo config: {}",
+ config.display()
+ )
+ })?;
}
}
}
}
+ // Make sure no other config files exist
+ for config in vec![
+ workspace_dir.join("config"),
+ workspace_dir.join("config.toml"),
+ dot_cargo_dir.join("config"),
+ dot_cargo_dir.join("config.toml"),
+ ] {
+ if config.exists() {
+ remove_symlink(&config).with_context(|| {
+ format!(
+ "Failed to delete existing cargo config: {}",
+ config.display()
+ )
+ })?;
+ }
+ }
+
+ // Ensure no parent directory also has a cargo config
+ let mut current_parent = workspace_dir.parent();
+ while let Some(parent) = current_parent {
+ let dot_cargo_dir = parent.join(".cargo");
+ for config in vec![
+ dot_cargo_dir.join("config.toml"),
+ dot_cargo_dir.join("config"),
+ ] {
+ if config.exists() {
+ bail!(
+ "A Cargo config file was found in a parent directory to the current workspace. This is not allowed because these settings will leak into your Bazel build but will not be reproducible on other machines.\nWorkspace = {}\nCargo config = {}",
+ workspace_dir.display(),
+ config.display(),
+ )
+ }
+ }
+ current_parent = parent.parent()
+ }
+
// Install the new config file after having removed all others
if let Some(cargo_config_path) = cargo_config_path {
if !dot_cargo_dir.exists() {
@@ -1440,4 +1466,100 @@
// Ensure lockfile was successfully spliced
cargo_lock::Lockfile::load(workspace_root.as_ref().join("Cargo.lock")).unwrap();
}
+
+ #[test]
+ fn cargo_config_setup() {
+ let (mut splicing_manifest, _cache_dir) = mock_splicing_manifest_with_workspace_in_root();
+
+ // Write a cargo config
+ let temp_dir = tempfile::tempdir().unwrap();
+ let external_config = temp_dir.as_ref().join("config.toml");
+ fs::write(&external_config, "# Cargo configuration file").unwrap();
+ splicing_manifest.cargo_config = Some(external_config);
+
+ // Splice the workspace
+ let workspace_root = tempfile::tempdir().unwrap();
+ Splicer::new(
+ workspace_root.as_ref().to_path_buf(),
+ splicing_manifest,
+ ExtraManifestsManifest::default(),
+ )
+ .unwrap()
+ .splice_workspace()
+ .unwrap();
+
+ let cargo_config = workspace_root.as_ref().join(".cargo").join("config.toml");
+ assert!(cargo_config.exists());
+ assert_eq!(
+ fs::read_to_string(cargo_config).unwrap().trim(),
+ "# Cargo configuration file"
+ );
+ }
+
+ #[test]
+ fn unregistered_cargo_config_replaced() {
+ let (mut splicing_manifest, cache_dir) = mock_splicing_manifest_with_workspace_in_root();
+
+ // Generate a cargo config that is not tracked by the splicing manifest
+ fs::create_dir_all(cache_dir.as_ref().join(".cargo")).unwrap();
+ fs::write(
+ cache_dir.as_ref().join(".cargo").join("config.toml"),
+ "# Untracked Cargo configuration file",
+ )
+ .unwrap();
+
+ // Write a cargo config
+ let temp_dir = tempfile::tempdir().unwrap();
+ let external_config = temp_dir.as_ref().join("config.toml");
+ fs::write(&external_config, "# Cargo configuration file").unwrap();
+ splicing_manifest.cargo_config = Some(external_config);
+
+ // Splice the workspace
+ let workspace_root = tempfile::tempdir().unwrap();
+ Splicer::new(
+ workspace_root.as_ref().to_path_buf(),
+ splicing_manifest,
+ ExtraManifestsManifest::default(),
+ )
+ .unwrap()
+ .splice_workspace()
+ .unwrap();
+
+ let cargo_config = workspace_root.as_ref().join(".cargo").join("config.toml");
+ assert!(cargo_config.exists());
+ assert_eq!(
+ fs::read_to_string(cargo_config).unwrap().trim(),
+ "# Cargo configuration file"
+ );
+ }
+
+ #[test]
+ fn error_on_cargo_config_in_parent() {
+ let (mut splicing_manifest, _cache_dir) = mock_splicing_manifest_with_workspace_in_root();
+
+ // Write a cargo config
+ let temp_dir = tempfile::tempdir().unwrap();
+ let dot_cargo_dir = temp_dir.as_ref().join(".cargo");
+ fs::create_dir_all(&dot_cargo_dir).unwrap();
+ let external_config = dot_cargo_dir.join("config.toml");
+ fs::write(&external_config, "# Cargo configuration file").unwrap();
+ splicing_manifest.cargo_config = Some(external_config.clone());
+
+ // Splice the workspace
+ let workspace_root = temp_dir.as_ref().join("workspace_root");
+ let splicing_result = Splicer::new(
+ workspace_root.clone(),
+ splicing_manifest,
+ ExtraManifestsManifest::default(),
+ )
+ .unwrap()
+ .splice_workspace();
+
+ // Ensure cargo config files in parent directories lead to errors
+ assert!(splicing_result.is_err());
+ let err_str = splicing_result.err().unwrap().to_string();
+ assert!(err_str.starts_with("A Cargo config file was found in a parent directory"));
+ assert!(err_str.contains(&format!("Workspace = {}", workspace_root.display())));
+ assert!(err_str.contains(&format!("Cargo config = {}", external_config.display())));
+ }
}
diff --git a/examples/crate_universe/WORKSPACE.bazel b/examples/crate_universe/WORKSPACE.bazel
index f78b777..2ae229a 100644
--- a/examples/crate_universe/WORKSPACE.bazel
+++ b/examples/crate_universe/WORKSPACE.bazel
@@ -105,6 +105,7 @@
crates_repository(
name = "crate_index_cargo_workspace",
+ cargo_config = "//cargo_workspace:.cargo/config.toml",
# `generator` is not necessary in official releases.
# See load satement for `cargo_bazel_bootstrap`.
generator = "@cargo_bazel_bootstrap//:cargo-bazel",
diff --git a/examples/crate_universe/cargo_workspace/.cargo/config.toml b/examples/crate_universe/cargo_workspace/.cargo/config.toml
new file mode 100644
index 0000000..1e04f5c
--- /dev/null
+++ b/examples/crate_universe/cargo_workspace/.cargo/config.toml
@@ -0,0 +1,2 @@
+# This file can contain cargo settings. For more details see:
+# https://doc.rust-lang.org/cargo/reference/config.html
diff --git a/examples/crate_universe/cargo_workspace/BUILD.bazel b/examples/crate_universe/cargo_workspace/BUILD.bazel
index e69de29..22293b9 100644
--- a/examples/crate_universe/cargo_workspace/BUILD.bazel
+++ b/examples/crate_universe/cargo_workspace/BUILD.bazel
@@ -0,0 +1,4 @@
+exports_files(
+ [".cargo/config.toml"],
+ visibility = ["//visibility:public"],
+)
diff --git a/examples/crate_universe/cargo_workspace/Cargo.Bazel.lock b/examples/crate_universe/cargo_workspace/Cargo.Bazel.lock
index 87a70c0..b95ed75 100644
--- a/examples/crate_universe/cargo_workspace/Cargo.Bazel.lock
+++ b/examples/crate_universe/cargo_workspace/Cargo.Bazel.lock
@@ -1,5 +1,5 @@
{
- "checksum": "19de3c063492d2c19858f606defde13feda1682fb84231a931e4c7958bb6bcd0",
+ "checksum": "ed5260c74ff6f8ccb2f28cd0e31b248aab3e110c7fd0321a9e697cf96ab659eb",
"crates": {
"ansi_term 0.12.1": {
"name": "ansi_term",
@@ -84,7 +84,7 @@
],
"cfg(unix)": [
{
- "id": "libc 0.2.120",
+ "id": "libc 0.2.121",
"target": "libc"
}
],
@@ -389,7 +389,7 @@
],
"cfg(unix)": [
{
- "id": "libc 0.2.120",
+ "id": "libc 0.2.121",
"target": "libc"
}
]
@@ -439,7 +439,7 @@
"deps": {
"common": [
{
- "id": "libc 0.2.120",
+ "id": "libc 0.2.121",
"target": "libc"
}
],
@@ -450,13 +450,13 @@
},
"license": "MIT/Apache-2.0"
},
- "libc 0.2.120": {
+ "libc 0.2.121": {
"name": "libc",
- "version": "0.2.120",
+ "version": "0.2.121",
"repository": {
"Http": {
- "url": "https://crates.io/api/v1/crates/libc/0.2.120/download",
- "sha256": "ad5c14e80759d0939d013e6ca49930e59fc53dd8e5009132f76240c179380c09"
+ "url": "https://crates.io/api/v1/crates/libc/0.2.121/download",
+ "sha256": "efaa7b300f3b5fe8eb6bf21ce3895e1751d9665086af2d64b42f19701015ff4f"
}
},
"targets": [
@@ -493,14 +493,14 @@
"deps": {
"common": [
{
- "id": "libc 0.2.120",
+ "id": "libc 0.2.121",
"target": "build_script_build"
}
],
"selects": {}
},
"edition": "2015",
- "version": "0.2.120"
+ "version": "0.2.121"
},
"build_script_attrs": {
"data_glob": [
@@ -683,7 +683,7 @@
],
"cfg(unix)": [
{
- "id": "libc 0.2.120",
+ "id": "libc 0.2.121",
"target": "libc"
}
]
