Skip to content

Add new cargo creds feature #3385

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
Open
5 changes: 5 additions & 0 deletions crate_universe/extensions.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -606,8 +606,10 @@ 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,
isolated = cfg.isolated,
),
)

Expand Down Expand Up @@ -965,6 +967,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)
Expand Down Expand Up @@ -1031,6 +1035,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"],
Expand Down
7 changes: 7 additions & 0 deletions crate_universe/private/common_utils.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,13 @@ def get_rust_tools(repository_ctx, host_triple):
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("/")
else:
Expand Down
4 changes: 4 additions & 0 deletions crate_universe/private/crates_repository.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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 " +
Expand Down
23 changes: 21 additions & 2 deletions crate_universe/private/crates_vendor.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -223,19 +223,22 @@ 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,
isolated = ctx.attr.isolated,
),
)

is_windows = _is_windows(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, isolated):
# Deserialize information about direct packages
direct_packages_info = {
# Ensure the data is using kebab-case as that's what `cargo_toml::DependencyDetail` expects.
Expand All @@ -245,7 +248,9 @@ 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,
"isolated": isolated,
"manifests": manifests,
}

Expand Down Expand Up @@ -510,6 +515,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,
Expand All @@ -536,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"],
Expand Down
14 changes: 13 additions & 1 deletion crate_universe/private/splicing_utils.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -33,17 +33,20 @@ 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, isolated):
"""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
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`
Expand All @@ -59,7 +62,9 @@ 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,
"isolated": isolated,
"manifests": manifests,
}

Expand Down Expand Up @@ -91,6 +96,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())

Expand All @@ -100,6 +110,8 @@ def create_splicing_manifest(repository_ctx):
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,
)

Expand Down
32 changes: 32 additions & 0 deletions crate_universe/src/splicing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,13 @@ pub(crate) struct SplicingManifest {
/// The path of a Cargo config file
pub(crate) cargo_config: Option<Utf8PathBuf>,

/// The path of a Cargo cred file
pub(crate) cargo_creds: Option<Utf8PathBuf>,

/// 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,
}
Expand All @@ -61,6 +68,8 @@ impl SplicingManifest {
let Self {
manifests,
cargo_config,
cargo_creds,
isolated,
..
} = self;

Expand Down Expand Up @@ -88,9 +97,30 @@ 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);

// 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)
});

Self {
manifests,
cargo_config,
cargo_creds,
isolated,
..self
}
}
Expand Down Expand Up @@ -650,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(),
Expand All @@ -665,6 +696,7 @@ mod test {
),
]),
cargo_config: None,
cargo_creds: None,
resolver_version: cargo_toml::Resolver::V2,
};
let metadata = SplicingMetadata::try_from(manifest).unwrap();
Expand Down
75 changes: 54 additions & 21 deletions crate_universe/src/splicing/splicer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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())?;
Expand Down Expand Up @@ -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<Utf8PathBuf>,
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()
Expand All @@ -290,16 +319,21 @@ 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]),
)?;
} 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()
)
})?;
Expand All @@ -310,10 +344,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(|| {
Expand All @@ -330,12 +364,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(),
)
Expand All @@ -349,9 +383,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(())
Expand Down Expand Up @@ -1655,9 +1688,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 {
Expand Down
Loading