fix: canonicalize manifest labels in lockfile digests

Ignore repository names when hashing splicing metadata so cargo-bazel lockfiles remain portable between root and dependency contexts. Reject canonicalization collisions explicitly and cover both behaviors with unit tests.

Author: brianduff

crate_universe/src/lockfile.rs

@@ -8,13 +8,15 @@ use std::process::Command;
 
 use anyhow::{bail, Context as AnyhowContext, Result};
 use hex::ToHex;
-use serde::{Deserialize, Serialize};
+use serde::ser::SerializeMap;
+use serde::{Deserialize, Serialize, Serializer};
 use sha2::{Digest as Sha2Digest, Sha256};
 
 use crate::config::Config;
 use crate::context::Context;
 use crate::metadata::Cargo;
 use crate::splicing::{SplicingManifest, SplicingMetadata};
+use crate::utils::starlark::{Label, Repository};
 
 pub(crate) fn lock_context(
     mut context: Context,
@@ -81,15 +83,15 @@ impl Digest {
                 cargo_bazel_version,
                 &cargo_version,
                 &rustc_version,
-            ),
+            )?,
             None => Self::compute(
                 context,
                 config,
                 &splicing_metadata,
                 cargo_bazel_version,
                 &cargo_version,
                 &rustc_version,
-            ),
+            )?,
         })
     }
 
@@ -110,10 +112,9 @@ impl Digest {
         cargo_bazel_version: &str,
         cargo_version: &str,
         rustc_version: &str,
-    ) -> Self {
+    ) -> Result<Self> {
         // Since this method is private, it should be expected that context is
-        // always None. This then allows us to have this method not return a
-        // Result.
+        // always checksum-free.
         debug_assert!(context.checksum.is_none());
 
         let mut hasher = Sha256::new();
@@ -140,8 +141,15 @@ impl Digest {
 
         // Data collected about Cargo manifests and configs that feed into dependency generation. This file
         // is also generated by Bazel behind the scenes based on user inputs.
+        //
+        // Manifest labels can differ between root and dependency contexts
+        // (`//pkg:Cargo.toml` vs `@@repo+//pkg:Cargo.toml`) even when the
+        // Cargo inputs are otherwise identical. Canonicalize labels only for
+        // hashing so a checked-in cargo-bazel lockfile stays portable across
+        // those contexts without changing runtime splicing behavior.
+        let digest_splicing_metadata = DigestSplicingMetadata::new(splicing_metadata)?;
         hasher.update(Digest::compute_single_hash(
-            &serde_json::to_string(splicing_metadata).unwrap(),
+            &serde_json::to_string(&digest_splicing_metadata).unwrap(),
             "splicing manifest",
         ));
         hasher.update(b"\0");
@@ -155,7 +163,7 @@ impl Digest {
         let hash = hasher.finalize().encode_hex::<String>();
         tracing::debug!("Digest hash: {}", hash);
 
-        Self(hash)
+        Ok(Self(hash))
     }
 
     pub(crate) fn bin_version(binary: &Path) -> Result<String> {
@@ -209,6 +217,82 @@ impl Digest {
     }
 }
 
+#[derive(Serialize)]
+struct DigestSplicingMetadata<'a> {
+    direct_packages: &'a BTreeMap<String, cargo_toml::DependencyDetail>,
+    // Keep the serialized shape aligned with SplicingMetadata so existing
+    // digests remain stable when no manifest labels require canonicalization.
+    // If multiple manifests collapse to the same repo-neutral label, serialize
+    // them as a stable list for that key instead of discarding any entries.
+    manifests: DigestManifestMap<'a>,
+    cargo_config: &'a Option<crate::splicing::cargo_config::CargoConfig>,
+}
+
+impl<'a> DigestSplicingMetadata<'a> {
+    fn new(splicing_metadata: &'a SplicingMetadata) -> Result<Self> {
+        let mut manifests: BTreeMap<Label, Vec<&'a cargo_toml::Manifest>> = BTreeMap::new();
+
+        for (label, manifest) in &splicing_metadata.manifests {
+            // Repository identity is not semantically relevant to the Cargo
+            // graph encoded by the lockfile, but package/target paths still
+            // are, so only drop the repo portion here.
+            let canonical_label = canonical_manifest_label(label);
+            manifests.entry(canonical_label).or_default().push(manifest);
+        }
+
+        Ok(Self {
+            direct_packages: &splicing_metadata.direct_packages,
+            manifests: DigestManifestMap(manifests),
+            cargo_config: &splicing_metadata.cargo_config,
+        })
+    }
+}
+
+struct DigestManifestMap<'a>(BTreeMap<Label, Vec<&'a cargo_toml::Manifest>>);
+
+impl Serialize for DigestManifestMap<'_> {
+    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: Serializer,
+    {
+        let mut map = serializer.serialize_map(Some(self.0.len()))?;
+
+        for (label, manifests) in &self.0 {
+            if manifests.len() == 1 {
+                map.serialize_entry(label, manifests[0])?;
+                continue;
+            }
+
+            let mut sorted_manifests = manifests.clone();
+            sorted_manifests.sort_by(|left, right| {
+                serde_json::to_string(left)
+                    .unwrap()
+                    .cmp(&serde_json::to_string(right).unwrap())
+            });
+            map.serialize_entry(label, &sorted_manifests)?;
+        }
+
+        map.end()
+    }
+}
+
+fn canonical_manifest_label(label: &Label) -> Label {
+    match label {
+        Label::Relative { target } => Label::Relative {
+            target: target.clone(),
+        },
+        Label::Absolute {
+            package, target, ..
+        } => Label::Absolute {
+            // Preserve package and target so distinct workspace/package paths
+            // still hash differently; only canonical repo names are removed.
+            repository: Repository::Local,
+            package: package.clone(),
+            target: target.clone(),
+        },
+    }
+}
+
 impl PartialEq<str> for Digest {
     fn eq(&self, other: &str) -> bool {
         self.0 == other
@@ -225,11 +309,13 @@ impl PartialEq<String> for Digest {
 mod test {
     use crate::config::{CrateAnnotations, CrateNameAndVersionReq};
     use crate::splicing::cargo_config::{AdditionalRegistry, CargoConfig, Registry};
+    use crate::utils::starlark::Label;
     use crate::utils::target_triple::TargetTriple;
 
     use super::*;
 
     use std::collections::BTreeSet;
+    use std::str::FromStr;
 
     #[test]
     fn simple_digest() {
@@ -244,7 +330,8 @@ mod test {
             "0.1.0",
             "cargo 1.57.0 (b2e52d7ca 2021-10-21)",
             "rustc 1.57.0 (f1edd0429 2021-11-29)",
-        );
+        )
+        .unwrap();
 
         assert_eq!(
             Digest("edd73970897c01af3bb0e6c9d62f572203dd38a03c189dcca555d463990aa086".to_owned()),
@@ -289,7 +376,8 @@ mod test {
             "0.1.0",
             "cargo 1.57.0 (b2e52d7ca 2021-10-21)",
             "rustc 1.57.0 (f1edd0429 2021-11-29)",
-        );
+        )
+        .unwrap();
 
         assert_eq!(
             Digest("8a4c1b3bb4c2d6c36e27565e71a13d54cff9490696a492c66a3a37bdd3893edf".to_owned()),
@@ -320,7 +408,8 @@ mod test {
             "0.1.0",
             "cargo 1.57.0 (b2e52d7ca 2021-10-21)",
             "rustc 1.57.0 (f1edd0429 2021-11-29)",
-        );
+        )
+        .unwrap();
 
         assert_eq!(
             Digest("1e01331686ba1f26f707dc098cd9d21c39d6ccd8e46be03329bb2470d3833e15".to_owned()),
@@ -369,14 +458,141 @@ mod test {
             "0.1.0",
             "cargo 1.57.0 (b2e52d7ca 2021-10-21)",
             "rustc 1.57.0 (f1edd0429 2021-11-29)",
-        );
+        )
+        .unwrap();
 
         assert_eq!(
             Digest("45ccf7109db2d274420fac521f4736a1fb55450ec60e6df698e1be4dc2c89fad".to_owned()),
             digest,
         );
     }
 
+    #[test]
+    fn digest_ignores_manifest_repository_names() {
+        let context = Context::default();
+        let config = Config::default();
+        let manifest = cargo_toml::Manifest::from_str(
+            r#"
+[package]
+name = "lockfile-repro"
+version = "0.0.1"
+edition = "2021"
+"#,
+        )
+        .unwrap();
+
+        // Same manifest content, but one label is evaluated from the root
+        // workspace and the other from a dependent module's external repo.
+        let local = SplicingMetadata {
+            manifests: BTreeMap::from([(
+                Label::from_str("//:Cargo.toml").unwrap(),
+                manifest.clone(),
+            )]),
+            ..SplicingMetadata::default()
+        };
+        let dependency = SplicingMetadata {
+            manifests: BTreeMap::from([(
+                Label::from_str("@@published_ruleset+//:Cargo.toml").unwrap(),
+                manifest,
+            )]),
+            ..SplicingMetadata::default()
+        };
+
+        let local_digest = Digest::compute(
+            &context,
+            &config,
+            &local,
+            "0.1.0",
+            "cargo 1.57.0 (b2e52d7ca 2021-10-21)",
+            "rustc 1.57.0 (f1edd0429 2021-11-29)",
+        )
+        .unwrap();
+
+        let dependency_digest = Digest::compute(
+            &context,
+            &config,
+            &dependency,
+            "0.1.0",
+            "cargo 1.57.0 (b2e52d7ca 2021-10-21)",
+            "rustc 1.57.0 (f1edd0429 2021-11-29)",
+        )
+        .unwrap();
+
+        assert_eq!(local_digest, dependency_digest);
+    }
+
+    #[test]
+    fn digest_stable_with_manifest_label_collisions() {
+        let context = Context::default();
+        let config = Config::default();
+        let first_manifest = cargo_toml::Manifest::from_str(
+            r#"
+[package]
+name = "collision-check-a"
+version = "0.0.1"
+edition = "2021"
+"#,
+        )
+        .unwrap();
+        let second_manifest = cargo_toml::Manifest::from_str(
+            r#"
+[package]
+name = "collision-check-b"
+version = "0.0.2"
+edition = "2021"
+"#,
+        )
+        .unwrap();
+
+        let splicing_metadata = SplicingMetadata {
+            manifests: BTreeMap::from([
+                (
+                    Label::from_str("@first_workspace//pkg:Cargo.toml").unwrap(),
+                    first_manifest.clone(),
+                ),
+                (
+                    Label::from_str("@@second_workspace+//pkg:Cargo.toml").unwrap(),
+                    second_manifest.clone(),
+                ),
+            ]),
+            ..SplicingMetadata::default()
+        };
+        let reversed_splicing_metadata = SplicingMetadata {
+            manifests: BTreeMap::from([
+                (
+                    Label::from_str("@@second_workspace+//pkg:Cargo.toml").unwrap(),
+                    second_manifest,
+                ),
+                (
+                    Label::from_str("@first_workspace//pkg:Cargo.toml").unwrap(),
+                    first_manifest,
+                ),
+            ]),
+            ..SplicingMetadata::default()
+        };
+
+        let digest = Digest::compute(
+            &context,
+            &config,
+            &splicing_metadata,
+            "0.1.0",
+            "cargo 1.57.0 (b2e52d7ca 2021-10-21)",
+            "rustc 1.57.0 (f1edd0429 2021-11-29)",
+        )
+        .unwrap();
+        let reversed_digest = Digest::compute(
+            &context,
+            &config,
+            &reversed_splicing_metadata,
+            "0.1.0",
+            "cargo 1.57.0 (b2e52d7ca 2021-10-21)",
+            "rustc 1.57.0 (f1edd0429 2021-11-29)",
+        )
+        .unwrap();
+
+        assert_eq!(digest, reversed_digest);
+    }
+
     #[test]
     fn digest_stable_with_crlf_cargo_config() {
         let context = Context::default();
@@ -413,7 +629,8 @@ mod test {
             "0.1.0",
             "cargo 1.57.0 (b2e52d7ca 2021-10-21)",
             "rustc 1.57.0 (f1edd0429 2021-11-29)",
-        );
+        )
+        .unwrap();
 
         let digest_lf = Digest::compute(
             &context,
@@ -422,7 +639,8 @@ mod test {
             "0.1.0",
             "cargo 1.57.0 (b2e52d7ca 2021-10-21)",
             "rustc 1.57.0 (f1edd0429 2021-11-29)",
-        );
+        )
+        .unwrap();
 
         assert_eq!(
             digest_crlf, digest_lf,

examples/crate_universe/cargo_aliases/cargo-bazel-lock.json

@@ -1,5 +1,5 @@
 {
-  "checksum": "ab8b093d4ba7e3bca34a3fc81413c70271914e1b01ed42d22db41c6ea5e9dab6",
+  "checksum": "1eeb80f1d0fa732cc8155ca4d18d9338651532ba8437ce17eb9422bf99d437f4",
   "crates": {
     "anstyle 1.0.1": {
       "name": "anstyle",