From 0bef063cd2edaae073859a0dd9075f88b8a80498 Mon Sep 17 00:00:00 2001 From: ericmcbride Date: Thu, 3 Apr 2025 14:32:31 -0500 Subject: [PATCH 01/16] Add new cargo creds feature --- crate_universe/extensions.bzl | 4 + crate_universe/private/common_utils.bzl | 9 ++- crate_universe/private/crates_repository.bzl | 4 + crate_universe/private/crates_vendor.bzl | 11 ++- crate_universe/private/splicing_utils.bzl | 9 ++- crate_universe/src/splicing.rs | 13 ++++ crate_universe/src/splicing/splicer.rs | 78 +++++++++++++++----- 7 files changed, 105 insertions(+), 23 deletions(-) diff --git a/crate_universe/extensions.bzl b/crate_universe/extensions.bzl index aa48cc849b..b672e01395 100644 --- a/crate_universe/extensions.bzl +++ b/crate_universe/extensions.bzl @@ -606,6 +606,7 @@ def _generate_hub_and_spokes( packages = packages, splicing_config = splicing_config, cargo_config = cfg.cargo_config, + cargo_creds = cfg.cargo_creds, manifests = manifests, manifest_to_path = module_ctx.path, ), @@ -965,6 +966,8 @@ def _crate_impl(module_ctx): module_ctx.watch(cfg.lockfile) if cfg.cargo_config: module_ctx.watch(cfg.cargo_config) + if cfg.cargo_creds: + module_ctx.watch(cfg.cargo_creds) if hasattr(cfg, "manifests"): for m in cfg.manifests: module_ctx.watch(m) @@ -1031,6 +1034,7 @@ def _crate_impl(module_ctx): _FROM_COMMON_ATTRS = { "cargo_config": CRATES_VENDOR_ATTRS["cargo_config"], + "cargo_creds": CRATES_VENDOR_ATTRS["cargo_creds"], "cargo_lockfile": CRATES_VENDOR_ATTRS["cargo_lockfile"], "generate_binaries": CRATES_VENDOR_ATTRS["generate_binaries"], "generate_build_scripts": CRATES_VENDOR_ATTRS["generate_build_scripts"], diff --git a/crate_universe/private/common_utils.bzl b/crate_universe/private/common_utils.bzl index 816deddcff..7ae56e8cb8 100644 --- a/crate_universe/private/common_utils.bzl +++ b/crate_universe/private/common_utils.bzl @@ -115,7 +115,7 @@ def get_rust_tools(repository_ctx, host_triple): Returns: struct: A struct containing the expected rust tools """ - + # This is a bit hidden but to ensure Cargo behaves consistently based # on the user provided config file, the config file is installed into # the `CARGO_HOME` path. This is done so here since fetching tools @@ -125,6 +125,13 @@ def get_rust_tools(repository_ctx, host_triple): cargo_home_config = repository_ctx.path("{}/config.toml".format(cargo_home)) cargo_config = repository_ctx.path(repository_ctx.attr.cargo_config) repository_ctx.symlink(cargo_config, cargo_home_config) + + # Workspace backwards compat for cargo credentia + if repository_ctx.attr.isolated and repository_ctx.attr.cargo_creds: + cargo_home = _cargo_home_path(repository_ctx) + cargo_home_config = repository_ctx.path("{}/credentials.toml".format(cargo_home)) + cargo_creds = repository_ctx.path(repository_ctx.attr.cargo_creds) + repository_ctx.symlink(cargo_creds, cargo_home_config) if repository_ctx.attr.rust_version.startswith(("beta", "nightly")): channel, _, version = repository_ctx.attr.rust_version.partition("/") diff --git a/crate_universe/private/crates_repository.bzl b/crate_universe/private/crates_repository.bzl index e92241d8a5..a4e42462a9 100644 --- a/crate_universe/private/crates_repository.bzl +++ b/crate_universe/private/crates_repository.bzl @@ -245,6 +245,10 @@ CARGO_BAZEL_REPIN=1 CARGO_BAZEL_REPIN_ONLY=crate_index bazel sync --only=crate_i "cargo_config": attr.label( doc = "A [Cargo configuration](https://doc.rust-lang.org/cargo/reference/config.html) file", ), + "cargo_creds": attr.label( + doc = "A [Cargo credentials](https://doc.rust-lang.org/cargo/reference/config.html#credentials) file.", + allow_single_file = True, + ), "cargo_lockfile": attr.label( doc = ( "The path used to store the `crates_repository` specific " + diff --git a/crate_universe/private/crates_vendor.bzl b/crate_universe/private/crates_vendor.bzl index 27cd5fff86..5208b30ee5 100644 --- a/crate_universe/private/crates_vendor.bzl +++ b/crate_universe/private/crates_vendor.bzl @@ -223,6 +223,7 @@ def _write_splicing_manifest(ctx): packages = ctx.attr.packages, splicing_config = splicing_config, cargo_config = ctx.attr.cargo_config, + cargo_creds = ctx.attr.cargo_creds, manifests = manifests, manifest_to_path = _prepare_manifest_path, ), @@ -232,10 +233,11 @@ def _write_splicing_manifest(ctx): env = [_sys_runfile_env(ctx, "SPLICING_MANIFEST", manifest, is_windows)] args = ["--splicing-manifest", _expand_env("SPLICING_MANIFEST", is_windows)] - runfiles = [manifest] + ctx.files.manifests + ([ctx.file.cargo_config] if ctx.attr.cargo_config else []) + runfiles = [manifest] + ctx.files.manifests + ([ctx.file.cargo_config] if ctx.attr.cargo_config else []) + ([ctx.file.cargo_creds] if ctx.attr.cargo_creds else []) + return args, env, runfiles -def generate_splicing_manifest(*, packages, splicing_config, cargo_config, manifests, manifest_to_path): +def generate_splicing_manifest(*, packages, splicing_config, cargo_config, cargo_creds, manifests, manifest_to_path): # Deserialize information about direct packages direct_packages_info = { # Ensure the data is using kebab-case as that's what `cargo_toml::DependencyDetail` expects. @@ -245,6 +247,7 @@ def generate_splicing_manifest(*, packages, splicing_config, cargo_config, manif splicing_manifest_content = { "cargo_config": str(manifest_to_path(cargo_config)) if cargo_config else None, + "cargo_creds": str(manifest_to_path(cargo_creds)) if cargo_creds else None, "direct_packages": direct_packages_info, "manifests": manifests, } @@ -510,6 +513,10 @@ CRATES_VENDOR_ATTRS = { doc = "A [Cargo configuration](https://doc.rust-lang.org/cargo/reference/config.html) file.", allow_single_file = True, ), + "cargo_creds": attr.label( + doc = "A [Cargo credentials](https://doc.rust-lang.org/cargo/reference/config.html#credentials) file.", + allow_single_file = True, + ), "cargo_lockfile": attr.label( doc = "The path to an existing `Cargo.lock` file", allow_single_file = True, diff --git a/crate_universe/private/splicing_utils.bzl b/crate_universe/private/splicing_utils.bzl index c799647a23..226b112980 100644 --- a/crate_universe/private/splicing_utils.bzl +++ b/crate_universe/private/splicing_utils.bzl @@ -33,7 +33,7 @@ def kebab_case_keys(data): for (key, val) in data.items() } -def compile_splicing_manifest(splicing_config, manifests, cargo_config_path, packages): +def compile_splicing_manifest(splicing_config, manifests, cargo_config_path, cargo_creds_path, packages): """Produce a manifest containing required components for splicing a new Cargo workspace [cargo_config]: https://doc.rust-lang.org/cargo/reference/config.html @@ -59,6 +59,7 @@ def compile_splicing_manifest(splicing_config, manifests, cargo_config_path, pac # Auto-generated splicer manifest values splicing_manifest_content = { "cargo_config": cargo_config_path, + "cargo_creds": cargo_creds_path, "direct_packages": direct_packages_info, "manifests": manifests, } @@ -91,6 +92,11 @@ def create_splicing_manifest(repository_ctx): else: cargo_config = None + if repository_ctx.attr.cargo_creds: + cargo_creds = str(repository_ctx.path(repository_ctx.attr.cargo_creds)) + else: + cargo_creds = None + # Load user configurable splicing settings config = json.decode(repository_ctx.attr.splicing_config or splicing_config()) @@ -100,6 +106,7 @@ def create_splicing_manifest(repository_ctx): splicing_config = config, manifests = manifests, cargo_config_path = cargo_config, + cargo_creds = cargo_creds, packages = repository_ctx.attr.packages, ) diff --git a/crate_universe/src/splicing.rs b/crate_universe/src/splicing.rs index 4417386bea..6c02ce058a 100644 --- a/crate_universe/src/splicing.rs +++ b/crate_universe/src/splicing.rs @@ -39,6 +39,9 @@ pub(crate) struct SplicingManifest { /// The path of a Cargo config file pub(crate) cargo_config: Option, + /// The path of a Cargo cred file + pub(crate) cargo_creds: Option, + /// The Cargo resolver version to use for splicing pub(crate) resolver_version: cargo_toml::Resolver, } @@ -61,6 +64,7 @@ impl SplicingManifest { let Self { manifests, cargo_config, + cargo_creds, .. } = self; @@ -88,9 +92,18 @@ impl SplicingManifest { Utf8PathBuf::from(resolved_path) }); + let cargo_creds = cargo_creds.map(|path| { + let resolved_path = path + .to_string() + .replace("${build_workspace_directory}", &workspace_dir_str) + .replace("${output_base}", &output_base_str); + Utf8PathBuf::from(resolved_path) + }); + Self { manifests, cargo_config, + cargo_creds, ..self } } diff --git a/crate_universe/src/splicing/splicer.rs b/crate_universe/src/splicing/splicer.rs index cb657519ca..b677b67410 100644 --- a/crate_universe/src/splicing/splicer.rs +++ b/crate_universe/src/splicing/splicer.rs @@ -155,8 +155,16 @@ impl<'a> SplicerKind<'a> { Some(IGNORE_LIST), )?; - // Optionally install the cargo config after contents have been symlinked - Self::setup_cargo_config(&splicing_manifest.cargo_config, workspace_dir.as_std_path())?; + Self::setup_cargo_dot_files( + &splicing_manifest.cargo_config, + workspace_dir.as_std_path(), + "config", + )?; + Self::setup_cargo_dot_files( + &splicing_manifest.cargo_creds, + workspace_dir.as_std_path(), + "credentials", + )?; // Add any additional depeendencies to the root package if !splicing_manifest.direct_packages.is_empty() { @@ -196,7 +204,16 @@ impl<'a> SplicerKind<'a> { )?; // Optionally install the cargo config after contents have been symlinked - Self::setup_cargo_config(&splicing_manifest.cargo_config, workspace_dir.as_std_path())?; + Self::setup_cargo_dot_files( + &splicing_manifest.cargo_config, + workspace_dir.as_std_path(), + "config", + )?; + Self::setup_cargo_dot_files( + &splicing_manifest.cargo_creds, + workspace_dir.as_std_path(), + "credentials", + )?; // Ensure the root package manifest has a populated `workspace` member let mut manifest = (*manifest).clone(); @@ -233,7 +250,16 @@ impl<'a> SplicerKind<'a> { let mut manifest = default_cargo_workspace_manifest(&splicing_manifest.resolver_version); // Optionally install a cargo config file into the workspace root. - Self::setup_cargo_config(&splicing_manifest.cargo_config, workspace_dir.as_std_path())?; + Self::setup_cargo_dot_files( + &splicing_manifest.cargo_config, + workspace_dir.as_std_path(), + "config", + )?; + Self::setup_cargo_dot_files( + &splicing_manifest.cargo_creds, + workspace_dir.as_std_path(), + "credentials", + )?; let installations = Self::inject_workspace_members(&mut manifest, manifests, workspace_dir.as_std_path())?; @@ -269,13 +295,16 @@ impl<'a> SplicerKind<'a> { /// 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( + fn setup_cargo_dot_files( cargo_config_path: &Option, workspace_dir: &Path, + dot_file_root: &str, ) -> Result<()> { // 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"); + let dot_file_toml = &format!("{}.toml", dot_file_root); + if dot_cargo_dir.exists() { let is_symlink = dot_cargo_dir .symlink_metadata() @@ -290,16 +319,28 @@ impl<'a> SplicerKind<'a> { ) })?; fs::create_dir(&dot_cargo_dir)?; - symlink_roots(&real_path, &dot_cargo_dir, Some(&["config", "config.toml"]))?; + + symlink_roots( + &real_path, + &dot_cargo_dir, + Some(&[dot_file_root, dot_file_toml]), + )?; + + // if cargo home set we symlink to cargo home. + // This is so credentials.toml works. + if let Ok(cargo_home) = std::env::var("CARGO_HOME") { + let path = Path::new(&cargo_home); + symlink_roots(&real_path, &path, Some(&[dot_file_root, dot_file_toml]))?; + } } else { for config in [ - dot_cargo_dir.join("config"), - dot_cargo_dir.join("config.toml"), + dot_cargo_dir.join(dot_file_root), + dot_cargo_dir.join(dot_file_toml), ] { if config.exists() { remove_symlink(&config).with_context(|| { format!( - "Failed to delete existing cargo config: {}", + "Failed to delete existing cargo dot file: {}", config.display() ) })?; @@ -310,10 +351,10 @@ impl<'a> SplicerKind<'a> { // Make sure no other config files exist for config in [ - workspace_dir.join("config"), - workspace_dir.join("config.toml"), - dot_cargo_dir.join("config"), - dot_cargo_dir.join("config.toml"), + workspace_dir.join(dot_file_root), + workspace_dir.join(dot_file_toml), + dot_cargo_dir.join(dot_file_root), + dot_cargo_dir.join(dot_file_toml), ] { if config.exists() { remove_symlink(&config).with_context(|| { @@ -330,12 +371,12 @@ impl<'a> SplicerKind<'a> { while let Some(parent) = current_parent { let dot_cargo_dir = parent.join(".cargo"); for config in [ - dot_cargo_dir.join("config.toml"), - dot_cargo_dir.join("config"), + dot_cargo_dir.join(dot_file_toml), + dot_cargo_dir.join(dot_file_root), ] { 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 = {}", + "A Cargo dot 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 dot file = {}", workspace_dir.display(), config.display(), ) @@ -349,9 +390,8 @@ impl<'a> SplicerKind<'a> { if !dot_cargo_dir.exists() { fs::create_dir_all(&dot_cargo_dir)?; } - - debug!("Using Cargo config: {}", cargo_config_path); - fs::copy(cargo_config_path, dot_cargo_dir.join("config.toml"))?; + debug!("Using Cargo dot file: {}", cargo_config_path); + fs::copy(cargo_config_path, dot_cargo_dir.join(dot_file_toml))?; } Ok(()) From 4d2168493a4f22fed2c71ba85708c6f99df39bcd Mon Sep 17 00:00:00 2001 From: ericmcbride Date: Thu, 3 Apr 2025 14:50:19 -0500 Subject: [PATCH 02/16] Update formatters --- crate_universe/private/common_utils.bzl | 4 ++-- crate_universe/private/splicing_utils.bzl | 2 ++ crate_universe/src/splicing/splicer.rs | 4 +++- crate_universe/test_data/serialized_configs/BUILD.bazel | 1 + 4 files changed, 8 insertions(+), 3 deletions(-) diff --git a/crate_universe/private/common_utils.bzl b/crate_universe/private/common_utils.bzl index 7ae56e8cb8..84c0fee592 100644 --- a/crate_universe/private/common_utils.bzl +++ b/crate_universe/private/common_utils.bzl @@ -115,7 +115,7 @@ def get_rust_tools(repository_ctx, host_triple): Returns: struct: A struct containing the expected rust tools """ - + # This is a bit hidden but to ensure Cargo behaves consistently based # on the user provided config file, the config file is installed into # the `CARGO_HOME` path. This is done so here since fetching tools @@ -125,7 +125,7 @@ def get_rust_tools(repository_ctx, host_triple): cargo_home_config = repository_ctx.path("{}/config.toml".format(cargo_home)) cargo_config = repository_ctx.path(repository_ctx.attr.cargo_config) repository_ctx.symlink(cargo_config, cargo_home_config) - + # Workspace backwards compat for cargo credentia if repository_ctx.attr.isolated and repository_ctx.attr.cargo_creds: cargo_home = _cargo_home_path(repository_ctx) diff --git a/crate_universe/private/splicing_utils.bzl b/crate_universe/private/splicing_utils.bzl index 226b112980..7e27715ffa 100644 --- a/crate_universe/private/splicing_utils.bzl +++ b/crate_universe/private/splicing_utils.bzl @@ -37,12 +37,14 @@ def compile_splicing_manifest(splicing_config, manifests, cargo_config_path, car """Produce a manifest containing required components for splicing a new Cargo workspace [cargo_config]: https://doc.rust-lang.org/cargo/reference/config.html + [cargo_creds]: https://doc.rust-lang.org/cargo/reference/config.html#credentials [cargo_toml]: https://doc.rust-lang.org/cargo/reference/manifest.html Args: splicing_config (dict): A deserialized `splicing_config` manifests (dict): A mapping of paths to Bazel labels which represent [Cargo manifests][cargo_toml]. cargo_config_path (str): The absolute path to a [Cargo config][cargo_config]. + cargo_creds_path (str): The absolute path to a [Cargo creds File][cargo_creds]. packages (dict): A set of crates (packages) specifications to depend on Returns: diff --git a/crate_universe/src/splicing/splicer.rs b/crate_universe/src/splicing/splicer.rs index b677b67410..2fffc9fb05 100644 --- a/crate_universe/src/splicing/splicer.rs +++ b/crate_universe/src/splicing/splicer.rs @@ -327,7 +327,9 @@ impl<'a> SplicerKind<'a> { )?; // if cargo home set we symlink to cargo home. - // This is so credentials.toml works. + // This is so credentials.toml works. Do i need to do an addtional check + // here somehow to make sure that we aren't stomping on someones non + // isolated build?! if let Ok(cargo_home) = std::env::var("CARGO_HOME") { let path = Path::new(&cargo_home); symlink_roots(&real_path, &path, Some(&[dot_file_root, dot_file_toml]))?; diff --git a/crate_universe/test_data/serialized_configs/BUILD.bazel b/crate_universe/test_data/serialized_configs/BUILD.bazel index c68bc48936..f2887a81c1 100644 --- a/crate_universe/test_data/serialized_configs/BUILD.bazel +++ b/crate_universe/test_data/serialized_configs/BUILD.bazel @@ -42,6 +42,7 @@ write_file( out = "splicing_manifest.json", content = [json.encode(compile_splicing_manifest( cargo_config_path = "/tmp/abs/path/workspace/.cargo/config.toml", + cargo_creds_path = "/tmp/abs/path/workspace/.cargo/credentials.toml", manifests = { "${build_workspace_directory}/submod/Cargo.toml": "//submod:Cargo.toml", "${output_base}/external_crate/Cargo.toml": "@external_crate//:Cargo.toml", From cea7e778f78a1e80eb14bf387f78cf31542f8cc0 Mon Sep 17 00:00:00 2001 From: ericmcbride Date: Thu, 3 Apr 2025 14:59:57 -0500 Subject: [PATCH 03/16] updatE --- crate_universe/src/splicing.rs | 1 + crate_universe/src/splicing/splicer.rs | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/crate_universe/src/splicing.rs b/crate_universe/src/splicing.rs index 6c02ce058a..f6983d2913 100644 --- a/crate_universe/src/splicing.rs +++ b/crate_universe/src/splicing.rs @@ -678,6 +678,7 @@ mod test { ), ]), cargo_config: None, + cargo_creds: None, resolver_version: cargo_toml::Resolver::V2, }; let metadata = SplicingMetadata::try_from(manifest).unwrap(); diff --git a/crate_universe/src/splicing/splicer.rs b/crate_universe/src/splicing/splicer.rs index 2fffc9fb05..64306f8397 100644 --- a/crate_universe/src/splicing/splicer.rs +++ b/crate_universe/src/splicing/splicer.rs @@ -1697,9 +1697,9 @@ mod test { // 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.starts_with("A Cargo dot file was found in a parent directory")); assert!(err_str.contains(&format!("Workspace = {}", workspace_root))); - assert!(err_str.contains(&format!("Cargo config = {}", external_config))); + assert!(err_str.contains(&format!("Cargo dot file = {}", external_config))); } fn syn_dependency_detail() -> cargo_toml::DependencyDetail { From ebcbb1369f61f53d22fba67e714b58d263375192 Mon Sep 17 00:00:00 2001 From: ericmcbride Date: Thu, 3 Apr 2025 15:03:30 -0500 Subject: [PATCH 04/16] fix clippy --- crate_universe/src/splicing/splicer.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crate_universe/src/splicing/splicer.rs b/crate_universe/src/splicing/splicer.rs index 64306f8397..9d3062c033 100644 --- a/crate_universe/src/splicing/splicer.rs +++ b/crate_universe/src/splicing/splicer.rs @@ -332,7 +332,7 @@ impl<'a> SplicerKind<'a> { // isolated build?! if let Ok(cargo_home) = std::env::var("CARGO_HOME") { let path = Path::new(&cargo_home); - symlink_roots(&real_path, &path, Some(&[dot_file_root, dot_file_toml]))?; + symlink_roots(&real_path, path, Some(&[dot_file_root, dot_file_toml]))?; } } else { for config in [ From fcd09967d0cd2772a33c879c80f0ae510b9f0ce3 Mon Sep 17 00:00:00 2001 From: ericmcbride Date: Thu, 3 Apr 2025 15:10:15 -0500 Subject: [PATCH 05/16] oops --- crate_universe/private/splicing_utils.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crate_universe/private/splicing_utils.bzl b/crate_universe/private/splicing_utils.bzl index 7e27715ffa..ff9cffa4eb 100644 --- a/crate_universe/private/splicing_utils.bzl +++ b/crate_universe/private/splicing_utils.bzl @@ -108,7 +108,7 @@ def create_splicing_manifest(repository_ctx): splicing_config = config, manifests = manifests, cargo_config_path = cargo_config, - cargo_creds = cargo_creds, + cargo_creds_path = cargo_creds, packages = repository_ctx.attr.packages, ) From 4b4ca3f6441d6d4812a1ae48b6c287225722b22d Mon Sep 17 00:00:00 2001 From: ericmcbride Date: Mon, 7 Apr 2025 09:22:04 -0500 Subject: [PATCH 06/16] Update to pass isolated thru --- crate_universe/private/splicing_utils.bzl | 6 ++++-- crate_universe/src/splicing.rs | 17 +++++++++++++++++ crate_universe/src/splicing/splicer.rs | 9 --------- 3 files changed, 21 insertions(+), 11 deletions(-) diff --git a/crate_universe/private/splicing_utils.bzl b/crate_universe/private/splicing_utils.bzl index ff9cffa4eb..8ca00672c2 100644 --- a/crate_universe/private/splicing_utils.bzl +++ b/crate_universe/private/splicing_utils.bzl @@ -33,7 +33,7 @@ def kebab_case_keys(data): for (key, val) in data.items() } -def compile_splicing_manifest(splicing_config, manifests, cargo_config_path, cargo_creds_path, packages): +def compile_splicing_manifest(splicing_config, manifests, cargo_config_path, cargo_creds_path, packages, isolated): """Produce a manifest containing required components for splicing a new Cargo workspace [cargo_config]: https://doc.rust-lang.org/cargo/reference/config.html @@ -63,6 +63,7 @@ def compile_splicing_manifest(splicing_config, manifests, cargo_config_path, car "cargo_config": cargo_config_path, "cargo_creds": cargo_creds_path, "direct_packages": direct_packages_info, + "isolated": isolated, "manifests": manifests, } @@ -103,12 +104,13 @@ def create_splicing_manifest(repository_ctx): config = json.decode(repository_ctx.attr.splicing_config or splicing_config()) splicing_manifest = repository_ctx.path("splicing_manifest.json") - + data = compile_splicing_manifest( splicing_config = config, manifests = manifests, cargo_config_path = cargo_config, cargo_creds_path = cargo_creds, + isolated = repository_ctx.attr.isolated, packages = repository_ctx.attr.packages, ) diff --git a/crate_universe/src/splicing.rs b/crate_universe/src/splicing.rs index f6983d2913..6bc9a45fca 100644 --- a/crate_universe/src/splicing.rs +++ b/crate_universe/src/splicing.rs @@ -42,6 +42,10 @@ pub(crate) struct SplicingManifest { /// The path of a Cargo cred file pub(crate) cargo_creds: Option, + /// Defines if `CARGO_HOME` isolated from the host. Defaults to true in starlark. + /// Overridden with the environmental `CARGO_BAZEL_ISOLATED` + pub(crate) isolated: bool, + /// The Cargo resolver version to use for splicing pub(crate) resolver_version: cargo_toml::Resolver, } @@ -65,6 +69,7 @@ impl SplicingManifest { manifests, cargo_config, cargo_creds, + isolated, .. } = self; @@ -97,6 +102,17 @@ impl SplicingManifest { .to_string() .replace("${build_workspace_directory}", &workspace_dir_str) .replace("${output_base}", &output_base_str); + + // if isolated we need to symlink the cargo creds to `CARGO_HOME`. + // https://doc.rust-lang.org/cargo/reference/config.html#credentials + if isolated { + // find the isolated cargo home + if let Ok(cargo_home) = std::env::var("CARGO_HOME") { + let path = Path::new(&cargo_home); + let cargo_creds = Path::new(&resolved_path); + splicer::symlink_roots(&cargo_creds, path, None).unwrap_or_default(); + } + } Utf8PathBuf::from(resolved_path) }); @@ -104,6 +120,7 @@ impl SplicingManifest { manifests, cargo_config, cargo_creds, + isolated, ..self } } diff --git a/crate_universe/src/splicing/splicer.rs b/crate_universe/src/splicing/splicer.rs index 9d3062c033..b296c0db6e 100644 --- a/crate_universe/src/splicing/splicer.rs +++ b/crate_universe/src/splicing/splicer.rs @@ -325,15 +325,6 @@ impl<'a> SplicerKind<'a> { &dot_cargo_dir, Some(&[dot_file_root, dot_file_toml]), )?; - - // if cargo home set we symlink to cargo home. - // This is so credentials.toml works. Do i need to do an addtional check - // here somehow to make sure that we aren't stomping on someones non - // isolated build?! - if let Ok(cargo_home) = std::env::var("CARGO_HOME") { - let path = Path::new(&cargo_home); - symlink_roots(&real_path, path, Some(&[dot_file_root, dot_file_toml]))?; - } } else { for config in [ dot_cargo_dir.join(dot_file_root), From f2a135e8e3bf2db926e25b9cf753de61c1ea608c Mon Sep 17 00:00:00 2001 From: ericmcbride Date: Mon, 7 Apr 2025 09:24:36 -0500 Subject: [PATCH 07/16] format --- crate_universe/private/splicing_utils.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crate_universe/private/splicing_utils.bzl b/crate_universe/private/splicing_utils.bzl index 8ca00672c2..ebba5bb610 100644 --- a/crate_universe/private/splicing_utils.bzl +++ b/crate_universe/private/splicing_utils.bzl @@ -104,7 +104,7 @@ def create_splicing_manifest(repository_ctx): config = json.decode(repository_ctx.attr.splicing_config or splicing_config()) splicing_manifest = repository_ctx.path("splicing_manifest.json") - + data = compile_splicing_manifest( splicing_config = config, manifests = manifests, From 490f83f9ad49ff038ce2dd8ab22aad49a36d347a Mon Sep 17 00:00:00 2001 From: ericmcbride Date: Mon, 7 Apr 2025 09:30:43 -0500 Subject: [PATCH 08/16] update --- crate_universe/private/crates_vendor.bzl | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crate_universe/private/crates_vendor.bzl b/crate_universe/private/crates_vendor.bzl index 5208b30ee5..938793e7cd 100644 --- a/crate_universe/private/crates_vendor.bzl +++ b/crate_universe/private/crates_vendor.bzl @@ -224,6 +224,7 @@ def _write_splicing_manifest(ctx): splicing_config = splicing_config, cargo_config = ctx.attr.cargo_config, cargo_creds = ctx.attr.cargo_creds, + isolated = ctx.attr.isolated, manifests = manifests, manifest_to_path = _prepare_manifest_path, ), @@ -237,7 +238,7 @@ def _write_splicing_manifest(ctx): return args, env, runfiles -def generate_splicing_manifest(*, packages, splicing_config, cargo_config, cargo_creds, manifests, manifest_to_path): +def generate_splicing_manifest(*, packages, splicing_config, cargo_config, cargo_creds, manifests, manifest_to_path, isolated): # Deserialize information about direct packages direct_packages_info = { # Ensure the data is using kebab-case as that's what `cargo_toml::DependencyDetail` expects. @@ -248,6 +249,7 @@ def generate_splicing_manifest(*, packages, splicing_config, cargo_config, cargo splicing_manifest_content = { "cargo_config": str(manifest_to_path(cargo_config)) if cargo_config else None, "cargo_creds": str(manifest_to_path(cargo_creds)) if cargo_creds else None, + "isolated": isolated, "direct_packages": direct_packages_info, "manifests": manifests, } From b4a72ad710e6cfe98aae9bdf2a6c399cb9d680f7 Mon Sep 17 00:00:00 2001 From: ericmcbride Date: Mon, 7 Apr 2025 09:33:27 -0500 Subject: [PATCH 09/16] Mistake --- crate_universe/private/crates_vendor.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crate_universe/private/crates_vendor.bzl b/crate_universe/private/crates_vendor.bzl index 938793e7cd..eb381facb0 100644 --- a/crate_universe/private/crates_vendor.bzl +++ b/crate_universe/private/crates_vendor.bzl @@ -224,9 +224,9 @@ def _write_splicing_manifest(ctx): splicing_config = splicing_config, cargo_config = ctx.attr.cargo_config, cargo_creds = ctx.attr.cargo_creds, - isolated = ctx.attr.isolated, manifests = manifests, manifest_to_path = _prepare_manifest_path, + isolated = ctx.attr.isolated, ), ) From 82986d7182b60182e7d1bc6a143130b1c533d8f2 Mon Sep 17 00:00:00 2001 From: ericmcbride Date: Mon, 7 Apr 2025 09:35:23 -0500 Subject: [PATCH 10/16] forgot --- crate_universe/extensions.bzl | 1 + 1 file changed, 1 insertion(+) diff --git a/crate_universe/extensions.bzl b/crate_universe/extensions.bzl index b672e01395..d5867a46c4 100644 --- a/crate_universe/extensions.bzl +++ b/crate_universe/extensions.bzl @@ -609,6 +609,7 @@ def _generate_hub_and_spokes( cargo_creds = cfg.cargo_creds, manifests = manifests, manifest_to_path = module_ctx.path, + isolated = cfg.isolated, ), ) From 0f1d76e0f8dfbc0759e315cefc40af578ee05a06 Mon Sep 17 00:00:00 2001 From: ericmcbride Date: Mon, 7 Apr 2025 09:41:40 -0500 Subject: [PATCH 11/16] update lints --- crate_universe/private/crates_vendor.bzl | 2 +- crate_universe/private/splicing_utils.bzl | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/crate_universe/private/crates_vendor.bzl b/crate_universe/private/crates_vendor.bzl index eb381facb0..fd84639296 100644 --- a/crate_universe/private/crates_vendor.bzl +++ b/crate_universe/private/crates_vendor.bzl @@ -249,8 +249,8 @@ def generate_splicing_manifest(*, packages, splicing_config, cargo_config, cargo splicing_manifest_content = { "cargo_config": str(manifest_to_path(cargo_config)) if cargo_config else None, "cargo_creds": str(manifest_to_path(cargo_creds)) if cargo_creds else None, - "isolated": isolated, "direct_packages": direct_packages_info, + "isolated": isolated, "manifests": manifests, } diff --git a/crate_universe/private/splicing_utils.bzl b/crate_universe/private/splicing_utils.bzl index ebba5bb610..4556898beb 100644 --- a/crate_universe/private/splicing_utils.bzl +++ b/crate_universe/private/splicing_utils.bzl @@ -46,6 +46,7 @@ def compile_splicing_manifest(splicing_config, manifests, cargo_config_path, car cargo_config_path (str): The absolute path to a [Cargo config][cargo_config]. cargo_creds_path (str): The absolute path to a [Cargo creds File][cargo_creds]. packages (dict): A set of crates (packages) specifications to depend on + isolated (bool): whether or not the `CARGO_HOME` environment variable should be isolated from the host configuration Returns: dict: A dictionary representation of a `cargo_bazel::splicing::SplicingManifest` From 34d7fe7279eae0fbbe017e87f233b48ac9dc00e3 Mon Sep 17 00:00:00 2001 From: ericmcbride Date: Mon, 7 Apr 2025 09:45:09 -0500 Subject: [PATCH 12/16] fix weird spacing error --- crate_universe/private/splicing_utils.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crate_universe/private/splicing_utils.bzl b/crate_universe/private/splicing_utils.bzl index 4556898beb..e52e8d3fab 100644 --- a/crate_universe/private/splicing_utils.bzl +++ b/crate_universe/private/splicing_utils.bzl @@ -46,7 +46,7 @@ def compile_splicing_manifest(splicing_config, manifests, cargo_config_path, car cargo_config_path (str): The absolute path to a [Cargo config][cargo_config]. cargo_creds_path (str): The absolute path to a [Cargo creds File][cargo_creds]. packages (dict): A set of crates (packages) specifications to depend on - isolated (bool): whether or not the `CARGO_HOME` environment variable should be isolated from the host configuration + isolated (bool): Is the `CARGO_HOME` environment variable should be isolated from the host. Returns: dict: A dictionary representation of a `cargo_bazel::splicing::SplicingManifest` From 4b19431c5a8e92e791b433e944dbbcd8a434d287 Mon Sep 17 00:00:00 2001 From: ericmcbride Date: Mon, 7 Apr 2025 09:46:50 -0500 Subject: [PATCH 13/16] fix unit test --- crate_universe/test_data/serialized_configs/BUILD.bazel | 1 + 1 file changed, 1 insertion(+) diff --git a/crate_universe/test_data/serialized_configs/BUILD.bazel b/crate_universe/test_data/serialized_configs/BUILD.bazel index f2887a81c1..1c508d8e02 100644 --- a/crate_universe/test_data/serialized_configs/BUILD.bazel +++ b/crate_universe/test_data/serialized_configs/BUILD.bazel @@ -43,6 +43,7 @@ write_file( content = [json.encode(compile_splicing_manifest( cargo_config_path = "/tmp/abs/path/workspace/.cargo/config.toml", cargo_creds_path = "/tmp/abs/path/workspace/.cargo/credentials.toml", + isolated = "True", manifests = { "${build_workspace_directory}/submod/Cargo.toml": "//submod:Cargo.toml", "${output_base}/external_crate/Cargo.toml": "@external_crate//:Cargo.toml", From c5ec3bed5726cac76dbe7d6798466dc83421ddaa Mon Sep 17 00:00:00 2001 From: ericmcbride Date: Mon, 7 Apr 2025 09:54:08 -0500 Subject: [PATCH 14/16] Fix cargo integration test --- crate_universe/src/splicing.rs | 1 + crate_universe/test_data/serialized_configs/BUILD.bazel | 2 +- crate_universe/tests/cargo_integration_test.rs | 1 + 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/crate_universe/src/splicing.rs b/crate_universe/src/splicing.rs index 6bc9a45fca..d540d69f7f 100644 --- a/crate_universe/src/splicing.rs +++ b/crate_universe/src/splicing.rs @@ -680,6 +680,7 @@ mod test { .unwrap(); let manifest = SplicingManifest { direct_packages: BTreeMap::new(), + isolated: false, manifests: BTreeMap::from([ ( Utf8PathBuf::try_from(workspace_manifest_path).unwrap(), diff --git a/crate_universe/test_data/serialized_configs/BUILD.bazel b/crate_universe/test_data/serialized_configs/BUILD.bazel index 1c508d8e02..5e7051244b 100644 --- a/crate_universe/test_data/serialized_configs/BUILD.bazel +++ b/crate_universe/test_data/serialized_configs/BUILD.bazel @@ -43,7 +43,7 @@ write_file( content = [json.encode(compile_splicing_manifest( cargo_config_path = "/tmp/abs/path/workspace/.cargo/config.toml", cargo_creds_path = "/tmp/abs/path/workspace/.cargo/credentials.toml", - isolated = "True", + isolated = True, manifests = { "${build_workspace_directory}/submod/Cargo.toml": "//submod:Cargo.toml", "${output_base}/external_crate/Cargo.toml": "@external_crate//:Cargo.toml", diff --git a/crate_universe/tests/cargo_integration_test.rs b/crate_universe/tests/cargo_integration_test.rs index 3b863aa7bd..cab650f6e6 100644 --- a/crate_universe/tests/cargo_integration_test.rs +++ b/crate_universe/tests/cargo_integration_test.rs @@ -77,6 +77,7 @@ fn run(repository_name: &str, manifests: HashMap, lockfile: &str serde_json::to_string(&json!({ "manifests": manifests, "direct_packages": {}, + "isolated": true, "resolver_version": "2" })) .unwrap(), From 92c8aea7f6b39cb3c74c208a1c2a8189d8f63079 Mon Sep 17 00:00:00 2001 From: ericmcbride Date: Mon, 7 Apr 2025 10:03:27 -0500 Subject: [PATCH 15/16] update --- crate_universe/private/crates_vendor.bzl | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/crate_universe/private/crates_vendor.bzl b/crate_universe/private/crates_vendor.bzl index fd84639296..d67f54172f 100644 --- a/crate_universe/private/crates_vendor.bzl +++ b/crate_universe/private/crates_vendor.bzl @@ -545,6 +545,16 @@ CRATES_VENDOR_ATTRS = { doc = "DEPRECATED: Moved to `render_config`.", default = True, ), + "isolated": attr.bool( + doc = ( + "If true, `CARGO_HOME` will be overwritten to a directory within the generated repository in " + + "order to prevent other uses of Cargo from impacting having any effect on the generated targets " + + "produced by this rule. For users who either have multiple `crate_repository` definitions in a " + + "WORKSPACE or rapidly re-pin dependencies, setting this to false may improve build times. This " + + "variable is also controled by `CARGO_BAZEL_ISOLATED` environment variable." + ), + default = True, + ), "manifests": attr.label_list( doc = "A list of Cargo manifests (`Cargo.toml` files).", allow_files = ["Cargo.toml"], From adb4eaf3bc2c54a862414c4464e6ebcd81a36e1b Mon Sep 17 00:00:00 2001 From: ericmcbride Date: Mon, 7 Apr 2025 10:08:32 -0500 Subject: [PATCH 16/16] update --- crate_universe/src/splicing.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crate_universe/src/splicing.rs b/crate_universe/src/splicing.rs index d540d69f7f..aea7407431 100644 --- a/crate_universe/src/splicing.rs +++ b/crate_universe/src/splicing.rs @@ -110,7 +110,7 @@ impl SplicingManifest { if let Ok(cargo_home) = std::env::var("CARGO_HOME") { let path = Path::new(&cargo_home); let cargo_creds = Path::new(&resolved_path); - splicer::symlink_roots(&cargo_creds, path, None).unwrap_or_default(); + splicer::symlink_roots(cargo_creds, path, None).unwrap_or_default(); } } Utf8PathBuf::from(resolved_path)