diff --git a/crate_universe/src/cli/generate.rs b/crate_universe/src/cli/generate.rs index f4e4c1f773..eb965de82f 100644 --- a/crate_universe/src/cli/generate.rs +++ b/crate_universe/src/cli/generate.rs @@ -113,13 +113,21 @@ pub struct GenerateOptions { } pub fn generate(opt: GenerateOptions) -> Result<()> { - // Load the config + // Load the config. `config.label_injection_mapping` is the per-session + // apparent -> canonical map reflecting the root module's current + // `single_version_override` / `multiple_version_override` choices. It is + // applied to the Context just before rendering and is sanitized out of + // the digest hash in `Digest::new`, so consumer-side overrides don't + // force a producer-side repin (the producer's lockfile may live in a + // read-only bzlmod cache). let config = Config::try_from_path(&opt.config)?; // Go straight to rendering if there is no need to repin if !opt.repin { if let Some(lockfile) = &opt.lockfile { - let context = Context::try_from_path(lockfile)?; + // Lockfile stores apparent labels; rewrite to canonical here. + let context = Context::try_from_path(lockfile)? + .apply_label_injection_mapping(&config.label_injection_mapping)?; // Render build files let outputs = Renderer::new( @@ -205,15 +213,23 @@ pub fn generate(opt: GenerateOptions) -> Result<()> { &opt.nonhermetic_root_bazel_workspace_dir, )?; - // Generate renderable contexts for each package + // Generate renderable contexts for each package. The Context here holds + // the user's APPARENT labels (e.g. `@openssl//:install`) because the + // label_injection mapping was detached at config load. let context = Context::new(annotations, config.rendering.are_sources_present())?; - // Render build files + // Render build files. Apply the apparent -> canonical mapping just for + // rendering — the lockfile written below uses the original apparent + // Context, so consumer-side overrides stay sound without producer-side + // repin. + let render_context = context + .clone() + .apply_label_injection_mapping(&config.label_injection_mapping)?; let outputs = Renderer::new( Arc::new(config.rendering.clone()), Arc::new(config.supported_platform_triples.clone()), ) - .render(&context, opt.generator)?; + .render(&render_context, opt.generator)?; // make file paths compatible with bazel labels let normalized_outputs = normalize_cargo_file_paths(outputs, &opt.repository_dir); diff --git a/crate_universe/src/cli/query.rs b/crate_universe/src/cli/query.rs index e6506712d6..ef9d658e72 100644 --- a/crate_universe/src/cli/query.rs +++ b/crate_universe/src/cli/query.rs @@ -61,7 +61,11 @@ pub fn query(opt: QueryOptions) -> Result<()> { None => bail!("No digest provided in lockfile"), }; - // Load the config file + // Load the config file. `config.label_injection_mapping` is populated + // but unused for the digest check — `Digest::new` sanitizes it out + // before hashing so this comparison stays stable across consumer-side + // overrides (e.g., `single_version_override` on an injected dep); + // otherwise every override would force a producer-side repin. let config = Config::try_from_path(&opt.config)?; let splicing_manifest = SplicingManifest::try_from_path(&opt.splicing_manifest)?; diff --git a/crate_universe/src/cli/splice.rs b/crate_universe/src/cli/splice.rs index 0be90c733f..6c23889b38 100644 --- a/crate_universe/src/cli/splice.rs +++ b/crate_universe/src/cli/splice.rs @@ -127,6 +127,8 @@ pub fn splice(opt: SpliceOptions) -> Result<()> { .context("Failed to generate lockfile")? }; + // Splice doesn't render BUILD files; `config.label_injection_mapping` is + // populated but unused here. The substitution happens in `generate`. let config = Config::try_from_path(&opt.config).context("Failed to parse config")?; let resolver_data = TreeResolver::new(cargo.clone()) diff --git a/crate_universe/src/cli/vendor.rs b/crate_universe/src/cli/vendor.rs index e721da69bb..1349e4b909 100644 --- a/crate_universe/src/cli/vendor.rs +++ b/crate_universe/src/cli/vendor.rs @@ -251,7 +251,9 @@ pub fn vendor(opt: VendorOptions) -> anyhow::Result<()> { &opt.repin, )?; - // Load the config from disk + // Load the config from disk. `config.label_injection_mapping` is applied + // to the Context just before render (see near `Renderer::new` below) and + // is sanitized out of the digest hash by `Digest::new`. let config = Config::try_from_path(&opt.config)?; let resolver_data = TreeResolver::new(cargo.clone()).generate( @@ -288,6 +290,13 @@ pub fn vendor(opt: VendorOptions) -> anyhow::Result<()> { // Generate renderable contexts for search package let context = Context::new(annotations, config.rendering.are_sources_present())?; + // Apply label_injection just before render. The Context at this point + // contains the user's apparent labels (e.g. `@openssl//:install`); the + // mapping rewrites them to the per-session canonical (e.g. + // `@@openssl+v3.5.5//:install`), reflecting whatever the root module's + // current `single_version_override` etc. resolved to. + let context = context.apply_label_injection_mapping(&config.label_injection_mapping)?; + // Render build files let outputs = Renderer::new( Arc::new(config.rendering.clone()), diff --git a/crate_universe/src/config.rs b/crate_universe/src/config.rs index 6b4ed7ca9c..e7345ea016 100644 --- a/crate_universe/src/config.rs +++ b/crate_universe/src/config.rs @@ -1,6 +1,6 @@ //! A module for configuration information -mod label_injection; +pub(crate) mod label_injection; use std::cmp::Ordering; use std::collections::{BTreeMap, BTreeSet}; @@ -719,6 +719,18 @@ pub(crate) struct Config { /// A set of platform triples to use in generated select statements #[serde(default, skip_serializing_if = "BTreeSet::is_empty")] pub(crate) supported_platform_triples: BTreeSet, + + /// Apparent -> canonical label_injection map extracted from each + /// annotation's `label_injections` field at load time. Populated by + /// `Config::try_from_path`; not present in config.json itself + /// (`deny_unknown_fields` is fine because `extract_global_mapping` + /// strips `label_injections` before deserialization). Sanitized to + /// `Default::default()` in `Digest::new` before hashing — same trick + /// as `Context.checksum = None` in `lockfile.rs` — so consumer-side + /// `single_version_override` shifts (which change the canonical names + /// here) don't perturb the digest and force a producer-side repin. + #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] + pub(crate) label_injection_mapping: LabelInjectionMapping, } // rules_rust/crate_universe/private/generate_utils.bzl:generate_config @@ -744,14 +756,34 @@ where } impl Config { + /// Load a config JSON and extract its per-annotation `label_injections` + /// into a single `label_injection_mapping` on the Config. The Config + /// itself keeps the user's APPARENT labels in annotation strings — the + /// rewrite to canonical happens via + /// [`label_injection::apply_mapping_to_value`] just before each render + /// (see [`crate::context::Context::apply_label_injection_mapping`]), + /// using whatever mapping the current bazel session resolved (reflecting + /// any root-level `single_version_override` / `multiple_version_override`). + /// + /// The mapping is excluded from the digest hash via + /// [`crate::lockfile::Digest::new`], mirroring the + /// `Context.checksum = None` clear in the same file. That's what keeps + /// the digest stable across consumer-side overrides. pub(crate) fn try_from_path>(path: T) -> Result { let data = fs::read_to_string(path)?; let mut value: serde_json::Value = serde_json::from_str(&data)?; - label_injection::apply(&mut value); - Ok(serde_json::from_value(value)?) + let mapping = label_injection::extract_global_mapping(&mut value)?; + let mut config: Self = serde_json::from_value(value)?; + config.label_injection_mapping = mapping; + Ok(config) } } +/// Apparent-repo-prefix -> canonical-repo-prefix map carried on +/// [`Config::label_injection_mapping`]. See [`label_injection`] for the WHY +/// of the deferred-substitution design. +pub(crate) type LabelInjectionMapping = std::collections::BTreeMap; + #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)] pub struct CrateNameAndVersionReq { /// The name of the crate diff --git a/crate_universe/src/config/label_injection.rs b/crate_universe/src/config/label_injection.rs index 4ad4cb170e..abb04879fb 100644 --- a/crate_universe/src/config/label_injection.rs +++ b/crate_universe/src/config/label_injection.rs @@ -1,55 +1,118 @@ -//! Apply per-annotation `label_injections` to the raw config JSON before it is -//! deserialized into typed structures. +//! Per-annotation `label_injections` handling, deferred to render time so the +//! lockfile stays apparent-only. //! //! Each annotation may carry a `label_injections` object that maps a canonical -//! repository prefix (e.g. `@@xz~v1.2.3`) to the apparent repository prefix -//! the user wrote in the annotation (e.g. `@xz`). Keys and values are bare -//! repo prefixes — no `//pkg:target` suffix — because the rewrite below is a -//! plain substring `replace`, and appending a target would cause double-`//` -//! mangling on user references like `@xz//:lzma`. The map is produced by -//! `sanitize_label_injections` in `crate_universe/private/common_utils.bzl`, -//! which is the only place the apparent → canonical resolution is available. +//! repository prefix (e.g. `@@openssl+v3.5.5`) to the apparent repository +//! prefix the user wrote in the annotation (e.g. `@openssl`). Keys and values +//! are bare repo prefixes — no `//pkg:target` suffix — because the rewrite is +//! a plain substring `replace`, and appending a target would cause double-`//` +//! mangling on user references like `@openssl//:install`. The map is produced +//! by `sanitize_label_injections` in `crate_universe/private/common_utils.bzl`, +//! which is the only place the apparent -> canonical resolution is available. //! -//! Here we apply that map by walking every string under each annotation and -//! replacing apparent occurrences with their canonical form. Any `//pkg:target` -//! the user wrote after the apparent repo prefix is preserved verbatim. The -//! `label_injections` field is consumed and removed before typed -//! deserialization sees the annotation. +//! Resolution timing matters: the apparent -> canonical mapping is computed +//! fresh in Starlark on every extension evaluation, so it reflects whatever +//! `bazel_dep` / `single_version_override` / `multiple_version_override` the +//! ROOT module is currently asking for — not whatever was in effect when the +//! producing module last repinned. If we baked the canonical form into the +//! lockfile (or hashed it into the digest), root-level overrides would force +//! every consumer to repin the producer's lockfile to recover. That's +//! impossible for proper bzlmod consumers — the producer's lockfile lives in +//! a read-only registry cache. +//! +//! The flow this module enables: +//! +//! 1. [`extract_global_mapping`] removes each annotation's +//! `label_injections` field at config-load time and merges them into a +//! single global apparent -> canonical map. The Config keeps the user's +//! apparent labels intact. +//! 2. The Context built from that Config — and then serialized into the +//! lockfile — therefore contains apparent labels and is stable across +//! consumer-side overrides. +//! 3. Just before rendering BUILD files, [`apply_mapping_to_value`] walks +//! the (per-session, freshly-loaded) Context and rewrites every string +//! using the current session's mapping. +//! +//! Conflicts (same apparent prefix mapping to two different canonicals across +//! annotations) are rejected — they only arise under +//! `multiple_version_override` with crossing `label_injections`, which has no +//! coherent single-substitution form. use std::collections::BTreeMap; +use anyhow::{bail, Result}; use serde_json::{Map, Value}; -/// Locates `annotations` in the parsed config JSON and applies each -/// annotation's `label_injections` to itself, removing the field in the -/// process. No-op if `annotations` is absent or empty. -pub(super) fn apply(config: &mut Value) { +/// Walk `config.annotations` and: +/// +/// * strip each annotation's `label_injections` field (so typed +/// deserialization downstream doesn't see it); +/// * merge every mapping into one global apparent -> canonical map. +/// +/// Returns the merged map (canonical-repo-prefix -> apparent-repo-prefix, the +/// same orientation [`apply_mapping_to_value`] expects). Empty if `annotations` +/// is absent or no annotation declared injections. +/// +/// Fails if two annotations declare different canonicals for the same apparent +/// — that only happens when a root module is in `multiple_version_override` +/// territory AND has two crate annotations injecting different versions of the +/// same apparent name. There's no sound way to substitute one string into +/// both forms; the user must split the offending crates into separate hubs. +pub(super) fn extract_global_mapping(config: &mut Value) -> Result> { let Some(annotations) = config.get_mut("annotations").and_then(Value::as_object_mut) else { - return; + return Ok(BTreeMap::new()); }; + // Inverse map (apparent -> canonical) used for conflict detection; the + // returned map is canonical -> apparent, matching `apply_mapping_to_value`'s + // iteration shape. + let mut by_apparent: BTreeMap = BTreeMap::new(); + let mut by_canonical: BTreeMap = BTreeMap::new(); + for annotation in annotations.values_mut() { - apply_to_annotation(annotation); + let Some(obj) = annotation.as_object_mut() else { + continue; + }; + let Some(injections) = obj.remove("label_injections") else { + continue; + }; + for (canonical, apparent) in into_mapping(injections) { + if let Some(existing) = by_apparent.get(&apparent) { + if existing != &canonical { + bail!( + "conflicting `label_injections` across annotations for \ + apparent label `{apparent}`: `{existing}` vs `{canonical}`. \ + This occurs when the root module pulls two versions of the \ + same bazel_dep (e.g. via `multiple_version_override`) and \ + crate_universe annotations from different modules each inject \ + their own version. Move the conflicting crate into its own \ + `crate.from_specs`/`crate.from_cargo` hub so each hub only \ + sees one canonical." + ); + } + } + by_apparent.insert(apparent.clone(), canonical.clone()); + by_canonical.insert(canonical, apparent); + } } -} - -fn apply_to_annotation(annotation: &mut Value) { - let Some(obj) = annotation.as_object_mut() else { - return; - }; - let Some(injections) = obj.remove("label_injections") else { - return; - }; + Ok(by_canonical) +} - let mapping = into_mapping(injections); +/// Substitute every apparent-label prefix in `value` with its canonical form, +/// using `mapping` (canonical -> apparent — the same shape returned by +/// [`extract_global_mapping`]). Recurses into objects and arrays; rewrites +/// both keys and values of objects (annotation fields like `extra_aliased_targets` +/// use labels as keys). +/// +/// Idempotent if the mapping is stable — applying it a second time with the +/// same canonical strings on the LHS finds nothing to replace because the +/// apparent prefix no longer appears. +pub(crate) fn apply_mapping_to_value(value: &mut Value, mapping: &BTreeMap) { if mapping.is_empty() { return; } - - for value in obj.values_mut() { - rewrite(value, &mapping); - } + rewrite(value, mapping); } fn into_mapping(value: Value) -> BTreeMap { @@ -106,40 +169,44 @@ fn replace_all(input: &str, mapping: &BTreeMap) -> String { #[cfg(test)] mod tests { - // The mapping keys in these tests come from `sanitize_label_injections` - // in `crate_universe/private/common_utils.bzl`. That helper takes - // `{Label(canonical): apparent_string}` and produces - // `{canonical_repo_prefix: apparent_string}`. The shape of the keys is - // therefore "canonical repo with optional package/target", and the values - // are the apparent labels the user wrote in their annotations. The Rust - // pass below substitutes apparent → canonical. + // Mapping keys come from `sanitize_label_injections` in + // `crate_universe/private/common_utils.bzl`. That helper takes + // `{Label(apparent): apparent_string}` and produces + // `{canonical_repo_prefix: apparent_repo_prefix}` (the Label coercion is + // where Starlark resolves apparent -> canonical for the current session). + // Substitution here rewrites apparent -> canonical. use super::*; use serde_json::json; - fn run(mut config: Value) -> Value { - apply(&mut config); - config - } + // ---- extract_global_mapping ---- #[test] - fn no_annotations_is_noop() { - let input = json!({"other": 1}); - assert_eq!(run(input.clone()), input); + fn returns_empty_when_no_annotations() { + let mut input = json!({"other": 1}); + let mapping = extract_global_mapping(&mut input).unwrap(); + assert!(mapping.is_empty()); + assert_eq!(input, json!({"other": 1})); } #[test] - fn missing_label_injections_is_noop() { - let input = json!({ + fn returns_empty_when_no_annotation_declares_injections() { + let mut input = json!({ "annotations": { "tokio 1.0.0": {"deps": ["@crate_index//:foo"]} } }); - assert_eq!(run(input.clone()), input); + let mapping = extract_global_mapping(&mut input).unwrap(); + assert!(mapping.is_empty()); + // Input untouched — nothing to strip. + assert_eq!( + input, + json!({"annotations": {"tokio 1.0.0": {"deps": ["@crate_index//:foo"]}}}), + ); } #[test] - fn empty_label_injections_is_stripped() { - let input = json!({ + fn strips_empty_label_injections_field() { + let mut input = json!({ "annotations": { "tokio 1.0.0": { "label_injections": {}, @@ -147,222 +214,206 @@ mod tests { } } }); - let expected = json!({ - "annotations": { - "tokio 1.0.0": {"deps": ["@crate_index//:foo"]} - } - }); - assert_eq!(run(input), expected); + let mapping = extract_global_mapping(&mut input).unwrap(); + assert!(mapping.is_empty()); + // The (empty) label_injections field is stripped so typed deserialization + // doesn't trip over it; remaining strings are NOT substituted (deferred). + assert_eq!( + input, + json!({"annotations": {"tokio 1.0.0": {"deps": ["@crate_index//:foo"]}}}), + ); } #[test] - fn substitutes_in_list_strings() { - let input = json!({ + fn merges_compatible_per_annotation_maps_into_one_global_map() { + let mut input = json!({ "annotations": { "tokio 1.0.0": { "label_injections": {"@@xz~v1.2.3": "@xz"}, - "build_script_data": ["@xz//:lzma"], + "deps": ["@xz//:lzma"], + }, + "rustls 0.21.0": { + "label_injections": {"@@xz~v1.2.3": "@xz"}, + "deps": ["@xz//:liblzma"], } } }); - let expected = json!({ - "annotations": { - "tokio 1.0.0": { - "build_script_data": ["@@xz~v1.2.3//:lzma"], + let mapping = extract_global_mapping(&mut input).unwrap(); + assert_eq!( + mapping, + BTreeMap::from([("@@xz~v1.2.3".to_owned(), "@xz".to_owned())]), + ); + // label_injections stripped from both annotations; per-annotation strings + // are NOT rewritten here — that's apply_mapping_to_value's job at render + // time so the lockfile stays apparent-only. + assert_eq!( + input, + json!({ + "annotations": { + "tokio 1.0.0": {"deps": ["@xz//:lzma"]}, + "rustls 0.21.0": {"deps": ["@xz//:liblzma"]}, } - } - }); - assert_eq!(run(input), expected); + }), + ); } #[test] - fn substitutes_in_dict_values() { - let input = json!({ - "annotations": { - "tokio 1.0.0": { - "label_injections": {"@@xz~v1.2.3": "@xz"}, - "build_script_env": {"LZMA_BIN": "$(execpath @xz//:lzma)"}, - } - } - }); - let expected = json!({ + fn rejects_two_annotations_pointing_one_apparent_at_different_canonicals() { + // The only scenario this triggers is a root module in + // `multiple_version_override` territory where two crate annotations + // (from different bzlmod modules) each inject their own version of + // the same apparent label. There's no coherent single substitution. + let mut input = json!({ "annotations": { "tokio 1.0.0": { - "build_script_env": {"LZMA_BIN": "$(execpath @@xz~v1.2.3//:lzma)"}, + "label_injections": {"@@openssl+v3.5.5": "@openssl"}, + "deps": ["@openssl//:install"], + }, + "rustls 0.21.0": { + "label_injections": {"@@openssl+v3.3.1": "@openssl"}, + "deps": ["@openssl//:install"], } } }); - assert_eq!(run(input), expected); + let err = extract_global_mapping(&mut input).unwrap_err(); + let msg = format!("{err:#}"); + assert!(msg.contains("conflicting `label_injections`"), "{msg}"); + assert!(msg.contains("@openssl"), "{msg}"); + } + + // ---- apply_mapping_to_value ---- + + #[test] + fn noop_when_mapping_empty() { + let mut v = json!({"deps": ["@openssl//:install"]}); + apply_mapping_to_value(&mut v, &BTreeMap::new()); + assert_eq!(v, json!({"deps": ["@openssl//:install"]})); + } + + #[test] + fn substitutes_in_list_strings() { + let mut v = json!(["@xz//:lzma"]); + let mapping = BTreeMap::from([("@@xz~v1.2.3".to_owned(), "@xz".to_owned())]); + apply_mapping_to_value(&mut v, &mapping); + assert_eq!(v, json!(["@@xz~v1.2.3//:lzma"])); + } + + #[test] + fn substitutes_in_dict_values() { + let mut v = json!({"LZMA_BIN": "$(execpath @xz//:lzma)"}); + let mapping = BTreeMap::from([("@@xz~v1.2.3".to_owned(), "@xz".to_owned())]); + apply_mapping_to_value(&mut v, &mapping); + assert_eq!(v, json!({"LZMA_BIN": "$(execpath @@xz~v1.2.3//:lzma)"})); } #[test] fn substitutes_in_dict_keys() { - let input = json!({ - "annotations": { - "tokio 1.0.0": { - "label_injections": {"@@xz~v1.2.3": "@xz"}, - "extra_aliased_targets": {"@xz//:lzma": "lzma_alias"}, - } - } - }); - let expected = json!({ - "annotations": { - "tokio 1.0.0": { - "extra_aliased_targets": {"@@xz~v1.2.3//:lzma": "lzma_alias"}, - } - } - }); - assert_eq!(run(input), expected); + // `extra_aliased_targets` keys, build_script_env keys, etc. are labels + // — the rewrite walks both keys and values. + let mut v = json!({"@xz//:lzma": "lzma_alias"}); + let mapping = BTreeMap::from([("@@xz~v1.2.3".to_owned(), "@xz".to_owned())]); + apply_mapping_to_value(&mut v, &mapping); + assert_eq!(v, json!({"@@xz~v1.2.3//:lzma": "lzma_alias"})); } #[test] - fn substitutes_in_additive_build_file_content() { - let input = json!({ - "annotations": { - "tokio 1.0.0": { - "label_injections": {"@@xz~v1.2.3": "@xz"}, - "additive_build_file_content": "rust_library(deps = [\"@xz//:lzma\"])", - } - } - }); - let expected = json!({ - "annotations": { - "tokio 1.0.0": { - "additive_build_file_content": - "rust_library(deps = [\"@@xz~v1.2.3//:lzma\"])", - } - } - }); - assert_eq!(run(input), expected); + fn substitutes_in_long_string_payloads() { + // additive_build_file_content is a raw Starlark blob; substring + // substitution still applies (and must not double-mangle the `//`). + let mut v = json!("rust_library(deps = [\"@xz//:lzma\"])"); + let mapping = BTreeMap::from([("@@xz~v1.2.3".to_owned(), "@xz".to_owned())]); + apply_mapping_to_value(&mut v, &mapping); + assert_eq!(v, json!("rust_library(deps = [\"@@xz~v1.2.3//:lzma\"])")); } #[test] - fn substitutes_in_nested_select() { - let input = json!({ - "annotations": { - "tokio 1.0.0": { - "label_injections": {"@@xz~v1.2.3": "@xz"}, - "deps": { - "common": ["@xz//:lzma"], - "selects": { - "@platforms//os:linux": ["@xz//:linux_only"] - } + fn substitutes_under_deep_nesting_with_selects() { + // The headline shape: build_script_env carrying `$(execpath ...)` / + // `$(rlocationpath ...)` expansions of apparent labels, both at the + // `common` level and inside platform `selects`. Every nested string + // (including those nested via Select dict keys) must be rewritten. + let mut v = json!({ + "build_script_data": { + "common": ["@xz//:lzma"], + "selects": { + "@platforms//os:linux": ["@xz//:liblzma"] + } + }, + "build_script_env": { + "common": {"LZMA_BIN": "$(execpath @xz//:lzma)"}, + "selects": { + "@platforms//os:linux": { + "LZMA_LIB": "$(rlocationpath @xz//:liblzma)" } } } }); - let expected = json!({ - "annotations": { - "tokio 1.0.0": { - "deps": { - "common": ["@@xz~v1.2.3//:lzma"], - "selects": { - "@platforms//os:linux": ["@@xz~v1.2.3//:linux_only"] + let mapping = BTreeMap::from([("@@xz~v1.2.3".to_owned(), "@xz".to_owned())]); + apply_mapping_to_value(&mut v, &mapping); + assert_eq!( + v, + json!({ + "build_script_data": { + "common": ["@@xz~v1.2.3//:lzma"], + "selects": { + "@platforms//os:linux": ["@@xz~v1.2.3//:liblzma"] + } + }, + "build_script_env": { + "common": {"LZMA_BIN": "$(execpath @@xz~v1.2.3//:lzma)"}, + "selects": { + "@platforms//os:linux": { + "LZMA_LIB": "$(rlocationpath @@xz~v1.2.3//:liblzma)" } } } - } - }); - assert_eq!(run(input), expected); + }), + ); } #[test] - fn applies_multiple_mappings() { - let input = json!({ - "annotations": { - "tokio 1.0.0": { - "label_injections": { - "@@xz~v1.2.3": "@xz", - "@@bzip2~v0.5": "@bzip2", - }, - "build_script_data": ["@xz//:lzma", "@bzip2//:bz2"], - } - } - }); - let expected = json!({ - "annotations": { - "tokio 1.0.0": { - "build_script_data": ["@@xz~v1.2.3//:lzma", "@@bzip2~v0.5//:bz2"], - } - } - }); - assert_eq!(run(input), expected); + fn applies_multiple_mapping_entries_in_one_pass() { + let mut v = json!(["@xz//:lzma", "@bzip2//:bz2"]); + let mapping = BTreeMap::from([ + ("@@xz~v1.2.3".to_owned(), "@xz".to_owned()), + ("@@bzip2~v0.5".to_owned(), "@bzip2".to_owned()), + ]); + apply_mapping_to_value(&mut v, &mapping); + assert_eq!(v, json!(["@@xz~v1.2.3//:lzma", "@@bzip2~v0.5//:bz2"])); } #[test] - fn substitutes_in_build_script_env_with_select_and_location_expansion() { - // The headline use case: a `build_script_env` that carries a - // `$(execpath ...)` expansion of an apparent label, both at the - // `common` level and under a platform `select`. Every nested string - // must be rewritten. - let input = json!({ - "annotations": { - "lzma-sys 0.1.0": { - "label_injections": {"@@xz~v1.2.3": "@xz"}, - "build_script_data": { - "common": ["@xz//:lzma"], - "selects": { - "@platforms//os:linux": ["@xz//:liblzma"] - } - }, - "build_script_env": { - "common": {"LZMA_BIN": "$(execpath @xz//:lzma)"}, - "selects": { - "@platforms//os:linux": { - "LZMA_LIB": "$(rlocationpath @xz//:liblzma)" - } - } + fn rewrites_context_like_payload() { + // Approximates how generate.rs uses the helper: substitution applied + // to a Context JSON's per-crate fields just before rendering. + let mut context = json!({ + "crates": { + "openssl-sys 0.9.116": { + "common_attrs": { + "build_script_data": ["@openssl//:install"], + "build_script_env": { + "common": {"OPENSSL_DIR": "$(execpath @openssl//:install)"} + }, } } } }); - let expected = json!({ - "annotations": { - "lzma-sys 0.1.0": { - "build_script_data": { - "common": ["@@xz~v1.2.3//:lzma"], - "selects": { - "@platforms//os:linux": ["@@xz~v1.2.3//:liblzma"] - } - }, - "build_script_env": { - "common": {"LZMA_BIN": "$(execpath @@xz~v1.2.3//:lzma)"}, - "selects": { - "@platforms//os:linux": { - "LZMA_LIB": "$(rlocationpath @@xz~v1.2.3//:liblzma)" - } + let mapping = BTreeMap::from([("@@openssl+v3.5.5".to_owned(), "@openssl".to_owned())]); + apply_mapping_to_value(&mut context, &mapping); + assert_eq!( + context, + json!({ + "crates": { + "openssl-sys 0.9.116": { + "common_attrs": { + "build_script_data": ["@@openssl+v3.5.5//:install"], + "build_script_env": { + "common": {"OPENSSL_DIR": "$(execpath @@openssl+v3.5.5//:install)"} + }, } } } - } - }); - assert_eq!(run(input), expected); - } - - #[test] - fn per_annotation_isolation() { - // Mapping under one annotation must not leak into another. - let input = json!({ - "annotations": { - "tokio 1.0.0": { - "label_injections": {"@@xz~v1.2.3": "@xz"}, - "deps": ["@xz//:lzma"], - }, - "serde 1.0.0": { - "deps": ["@xz//:lzma"], - } - } - }); - let expected = json!({ - "annotations": { - "tokio 1.0.0": { - "deps": ["@@xz~v1.2.3//:lzma"], - }, - "serde 1.0.0": { - "deps": ["@xz//:lzma"], - } - } - }); - assert_eq!(run(input), expected); + }), + ); } } diff --git a/crate_universe/src/context.rs b/crate_universe/src/context.rs index 3ec2a9e4d3..2c2ec03c9a 100644 --- a/crate_universe/src/context.rs +++ b/crate_universe/src/context.rs @@ -11,7 +11,8 @@ use std::sync::Arc; use anyhow::Result; use serde::{Deserialize, Serialize}; -use crate::config::{CrateId, RenderConfig}; +use crate::config::label_injection; +use crate::config::{CrateId, LabelInjectionMapping, RenderConfig}; use crate::context::platforms::resolve_cfg_platforms; use crate::lockfile::Digest; use crate::metadata::{Annotations, Dependency}; @@ -162,6 +163,30 @@ impl Context { }) } + /// Rewrite apparent-label prefixes in every string under this Context to + /// their canonical form using the freshly-resolved mapping passed from + /// Starlark via [`Config::try_from_path`]. Done by a JSON serde + /// round-trip — the substitution surface is wide (deps, build_script_data, + /// build_script_env, additive_build_file_content, etc.) and a typed walk + /// would have to be re-derived whenever new string fields are added; the + /// round-trip naturally tracks the schema. + /// + /// Called just before rendering BUILD files; the Context written to the + /// lockfile is the pre-rewrite form, so the lockfile and digest stay + /// stable across `single_version_override` / `multiple_version_override` + /// changes the root module makes. + pub(crate) fn apply_label_injection_mapping( + self, + mapping: &LabelInjectionMapping, + ) -> Result { + if mapping.is_empty() { + return Ok(self); + } + let mut value = serde_json::to_value(&self)?; + label_injection::apply_mapping_to_value(&mut value, mapping); + Ok(serde_json::from_value(value)?) + } + // A helper function for locating the unique path in a workspace to a workspace member fn get_package_path_id( package: &cargo_metadata::Package, diff --git a/crate_universe/src/lockfile.rs b/crate_universe/src/lockfile.rs index b0d76428ab..aa57d5ce82 100644 --- a/crate_universe/src/lockfile.rs +++ b/crate_universe/src/lockfile.rs @@ -69,6 +69,19 @@ impl Digest { let rustc_version = Self::bin_version(rustc_bin)?; let cargo_bazel_version = env!("CARGO_PKG_VERSION"); + // Mirror the Context.checksum sanitization below for Config's + // `label_injection_mapping`: that field is a per-session derived + // artifact (apparent -> canonical labels resolved through the consumer + // module's repo_mapping). If it entered the hash, a consumer-side + // `single_version_override` would shift the canonical names, change + // the digest, and force a producer-side repin to recover — which is + // impossible for registry-distributed producers whose lockfile lives + // in a read-only bzlmod cache. + let config_for_hash = Config { + label_injection_mapping: Default::default(), + ..config.clone() + }; + // Ensure the checksum of a digest is not present before computing one Ok(match context.checksum { Some(_) => Self::compute( @@ -76,7 +89,7 @@ impl Digest { checksum: None, ..context.clone() }, - config, + &config_for_hash, &splicing_metadata, cargo_bazel_version, &cargo_version, @@ -84,7 +97,7 @@ impl Digest { ), None => Self::compute( context, - config, + &config_for_hash, &splicing_metadata, cargo_bazel_version, &cargo_version, diff --git a/crate_universe/tests/integration/.gitignore b/crate_universe/tests/integration/.gitignore new file mode 100644 index 0000000000..23a133416e --- /dev/null +++ b/crate_universe/tests/integration/.gitignore @@ -0,0 +1,2 @@ +bazel-* +user.bazelrc diff --git a/crate_universe/tests/integration/cargo_local/BUILD.bazel b/crate_universe/tests/integration/cargo_local/BUILD.bazel index cc9bd5cfee..86da58413b 100644 --- a/crate_universe/tests/integration/cargo_local/BUILD.bazel +++ b/crate_universe/tests/integration/cargo_local/BUILD.bazel @@ -4,7 +4,7 @@ load("@rules_rust//rust:defs.bzl", "rust_library", "rust_test") rust_library( name = "cargo_local", - srcs = glob(["**/*.rs"]), + srcs = ["src/lib.rs"], aliases = aliases(), edition = "2018", proc_macro_deps = all_crate_deps(proc_macro = True), @@ -26,3 +26,28 @@ build_test( "@crate_index_cargo_local//:aws-lc-rs", ], ) + +# Exercise transitive consumption of another module's crate_universe +# extension: `cu_test_complicated_dependencies` declares +# `crate.from_specs(... annotation(label_injections = ...))` for openssl, +# and this root module never calls `use_extension` on it — yet building + +# running its exposed `:openssl_version` binary forces the hub repo to be +# fetched, the annotation-injected `@openssl` label to resolve, and the +# resulting executable to link and run. +rust_test( + name = "transitive_module_extension_test", + srcs = ["transitive_module_extension_test.rs"], + data = ["@cu_test_complicated_dependencies//:openssl_version"], + edition = "2018", + rustc_env = { + # rlocationpath emits a workspace-rooted runfiles path + # (`cu_test_complicated_dependencies+/openssl_version` etc.) that the + # runfiles library resolves at runtime via the test's RUNFILES_DIR / + # RUNFILES_MANIFEST_FILE. Using rootpath would only work when the + # test runs from the exec root, which isn't guaranteed (e.g. with + # `--enable_runfiles=false` on Windows or when invoked from a sh_test + # wrapper). + "EXECUTABLE_RLOCATIONPATH": "$(rlocationpath @cu_test_complicated_dependencies//:openssl_version)", + }, + deps = ["@rules_rust//rust/runfiles"], +) diff --git a/crate_universe/tests/integration/cargo_local/MODULE.bazel b/crate_universe/tests/integration/cargo_local/MODULE.bazel index 9688548a66..27aa572b7c 100644 --- a/crate_universe/tests/integration/cargo_local/MODULE.bazel +++ b/crate_universe/tests/integration/cargo_local/MODULE.bazel @@ -11,6 +11,36 @@ local_path_override( bazel_dep(name = "bazel_skylib", version = "1.8.2") +# Pull in `cu_test_complicated_dependencies` as a transitive bzlmod dep so +# that its `crate.from_specs(name = "complicated_dependencies", ...)` module +# extension — which exercises crate_universe annotations and label_injection +# — is invoked here even though this root module never calls `use_extension` +# on it itself. The `:transitive_module_extension_test` rust_test below +# spawns the binary that module exposes and verifies it runs end-to-end. +bazel_dep(name = "cu_test_complicated_dependencies", version = "0.0.0") +local_path_override( + module_name = "cu_test_complicated_dependencies", + path = "../complicated_dependencies", +) + +# Force a different revision of the `openssl` bazel module than the one +# `cu_test_complicated_dependencies` requested (3.5.5.bcr.4). This proves +# that complicated_dependencies' `crate.annotation(label_injections = {"@openssl": ...})` +# on openssl-sys is resilient to a root-driven swap: Bzlmod MVS / override +# replaces the `@openssl` repo the build script consumes, and the injected +# label must re-resolve through the new module instance for the openssl-sys +# build to still link. If label_injection silently captured the original +# canonical name at extension-evaluation time, this would break. +# +# 3.5.5.bcr.3 is the lowest BCR revision of openssl 3.5.5 that still exposes +# the `:install` collate_into_directory target that openssl-sys' build script +# needs (`OPENSSL_DIR = $(execpath @openssl//:install)`). Earlier revisions +# only expose `:openssl`/`:crypto`/`:ssl` individually. +single_version_override( + module_name = "openssl", + version = "3.5.5.bcr.3", +) + rust = use_extension("@rules_rust//rust:extensions.bzl", "rust") use_repo(rust, "rust_toolchains") diff --git a/crate_universe/tests/integration/cargo_local/transitive_module_extension_test.rs b/crate_universe/tests/integration/cargo_local/transitive_module_extension_test.rs new file mode 100644 index 0000000000..b7f69c273b --- /dev/null +++ b/crate_universe/tests/integration/cargo_local/transitive_module_extension_test.rs @@ -0,0 +1,51 @@ +//! Spawn the binary defined in the transitively-depended +//! `cu_test_complicated_dependencies` module. That module declares +//! `crate.from_specs(name = "complicated_dependencies", ...)` with +//! `crate.annotation(label_injections = ...)` for openssl-sys — but this +//! root module never calls `use_extension` on it. A successful spawn with +//! an OpenSSL version banner on stdout proves the transitively-declared +//! crate_universe extension was invoked, its hub repo rendered, the +//! annotation-injected `@openssl` label propagated to the build script, and +//! the resulting binary linked and ran end-to-end. +//! +//! This root module also `single_version_override`s `openssl` to a different +//! BCR revision than `complicated_dependencies` declared. The build must +//! still succeed without re-pinning `complicated_dependencies`'s lockfile — +//! label_injection's apparent -> canonical substitution is deferred from +//! repin-time to per-session render-time (see +//! `crate_universe/src/config/label_injection.rs`), so the lockfile carries +//! apparent labels and adapts to whatever the current bazel session resolved. +//! +//! Note: this test lives in `cargo_local` rather than in its own module so +//! that one CI job exercises both crate_universe's `from_cargo` resolution +//! (used by `cargo_local`'s own root extension) and a transitively-fetched +//! `from_specs` extension that survives a root-driven override. + +use std::process::Command; + +use runfiles::{rlocation, Runfiles}; + +#[test] +fn transitive_binary_prints_openssl_version() { + let r = Runfiles::create().expect("failed to initialize runfiles"); + let exe = rlocation!(r, env!("EXECUTABLE_RLOCATIONPATH")) + .expect("failed to resolve transitive binary via runfiles"); + + let output = Command::new(&exe) + .output() + .expect("failed to spawn transitively-fetched binary"); + + assert!( + output.status.success(), + "binary exited non-zero: {:?}\nstderr: {}", + output.status, + String::from_utf8_lossy(&output.stderr), + ); + + let stdout = String::from_utf8(output.stdout).expect("stdout was not valid UTF-8"); + assert!( + stdout.starts_with("OpenSSL "), + "expected OpenSSL version banner, got: {:?}", + stdout, + ); +} diff --git a/crate_universe/tests/integration/complicated_dependencies/BUILD.bazel b/crate_universe/tests/integration/complicated_dependencies/BUILD.bazel index 97f026a29f..a3980e5f74 100644 --- a/crate_universe/tests/integration/complicated_dependencies/BUILD.bazel +++ b/crate_universe/tests/integration/complicated_dependencies/BUILD.bazel @@ -1,8 +1,21 @@ load("@bazel_skylib//rules:build_test.bzl", "build_test") +load("@rules_rust//rust:defs.bzl", "rust_binary") + +# Binary that links the `openssl` crate fetched by this module's +# `crate.from_specs` extension. Exposed publicly so dependent modules can +# spawn it and prove the transitively-resolved hub repo produced a runnable +# artifact (not just a buildable one). +rust_binary( + name = "openssl_version", + srcs = ["src/main.rs"], + edition = "2021", + visibility = ["//visibility:public"], + deps = ["@complicated_dependencies//:openssl"], +) build_test( name = "build_test", targets = [ - "@complicated_dependencies//:openssl", + ":openssl_version", ], ) diff --git a/crate_universe/tests/integration/complicated_dependencies/MODULE.bazel b/crate_universe/tests/integration/complicated_dependencies/MODULE.bazel index a36fe711b7..5893f7ddb9 100644 --- a/crate_universe/tests/integration/complicated_dependencies/MODULE.bazel +++ b/crate_universe/tests/integration/complicated_dependencies/MODULE.bazel @@ -20,9 +20,6 @@ use_repo(rust, "rust_toolchains") register_toolchains("@rust_toolchains//:all") crate = use_extension("@rules_rust//crate_universe:extensions.bzl", "crate") - -inject_repo(crate, "openssl") - crate.annotation( build_script_data = [ "@openssl//:install", diff --git a/crate_universe/tests/integration/complicated_dependencies/cargo-bazel-lock.json b/crate_universe/tests/integration/complicated_dependencies/cargo-bazel-lock.json index 103bf3f654..f26ffd3ae6 100644 --- a/crate_universe/tests/integration/complicated_dependencies/cargo-bazel-lock.json +++ b/crate_universe/tests/integration/complicated_dependencies/cargo-bazel-lock.json @@ -1,5 +1,5 @@ { - "checksum": "a3b09e201ba3f88ca87f85078016cce27309cebfaa1b7602df394b928a6a0d8d", + "checksum": "19334385e62eab33f99e63eda21c73d7d5e676c4bf292e8abea1ee6471da3b19", "crates": { "bitflags 2.11.1": { "name": "bitflags", @@ -587,7 +587,7 @@ "common_attrs": { "compile_data": { "common": [ - "@@openssl+//:install" + "@openssl//:install" ], "selects": {} }, @@ -619,7 +619,7 @@ ], "data": { "common": [ - "@@openssl+//:install" + "@openssl//:install" ], "selects": {} }, @@ -645,7 +645,7 @@ }, "build_script_env": { "common": { - "OPENSSL_DIR": "$(execpath @@openssl+//:install)", + "OPENSSL_DIR": "$(execpath @openssl//:install)", "OPENSSL_NO_VENDOR": "1", "OPENSSL_STATIC": "1" }, diff --git a/crate_universe/tests/integration/complicated_dependencies/src/main.rs b/crate_universe/tests/integration/complicated_dependencies/src/main.rs new file mode 100644 index 0000000000..1e25ad4e41 --- /dev/null +++ b/crate_universe/tests/integration/complicated_dependencies/src/main.rs @@ -0,0 +1,8 @@ +//! Minimal binary that links against the openssl crate fetched by this +//! module's `crate.from_specs` extension and prints the linked OpenSSL +//! version. Consuming modules execute it to verify the transitively-fetched +//! crate is reachable and runnable end-to-end. + +fn main() { + println!("{}", openssl::version::version()); +}