diff --git a/crate_universe/src/lockfile.rs b/crate_universe/src/lockfile.rs index b0d76428ab..a3ff040c34 100644 --- a/crate_universe/src/lockfile.rs +++ b/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,7 +83,7 @@ impl Digest { cargo_bazel_version, &cargo_version, &rustc_version, - ), + )?, None => Self::compute( context, config, @@ -89,7 +91,7 @@ impl Digest { 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 { // 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::(); tracing::debug!("Digest hash: {}", hash); - Self(hash) + Ok(Self(hash)) } pub(crate) fn bin_version(binary: &Path) -> Result { @@ -209,6 +217,82 @@ impl Digest { } } +#[derive(Serialize)] +struct DigestSplicingMetadata<'a> { + direct_packages: &'a BTreeMap, + // 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, +} + +impl<'a> DigestSplicingMetadata<'a> { + fn new(splicing_metadata: &'a SplicingMetadata) -> Result { + let mut manifests: BTreeMap> = 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>); + +impl Serialize for DigestManifestMap<'_> { + fn serialize(&self, serializer: S) -> Result + 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 for Digest { fn eq(&self, other: &str) -> bool { self.0 == other @@ -225,11 +309,13 @@ impl PartialEq 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,7 +458,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("45ccf7109db2d274420fac521f4736a1fb55450ec60e6df698e1be4dc2c89fad".to_owned()), @@ -377,6 +467,132 @@ mod test { ); } + #[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, diff --git a/examples/crate_universe/cargo_aliases/cargo-bazel-lock.json b/examples/crate_universe/cargo_aliases/cargo-bazel-lock.json index 1be1df3a7d..d2c6821a11 100644 --- a/examples/crate_universe/cargo_aliases/cargo-bazel-lock.json +++ b/examples/crate_universe/cargo_aliases/cargo-bazel-lock.json @@ -1,5 +1,5 @@ { - "checksum": "3b3510130287a3e37d9fa11cfdb1072e6be1d64310cfe4557e28c48fa3ee5ecb", + "checksum": "d58590f873d7601e159f1c7e829f6d96842f82716a3c15ea34618eb83f21cc7b", "crates": { "aho-corasick 0.7.20": { "name": "aho-corasick", diff --git a/examples/crate_universe/cargo_conditional_deps/cargo-bazel-lock.json b/examples/crate_universe/cargo_conditional_deps/cargo-bazel-lock.json index 855272ba04..3f1c41d4ab 100644 --- a/examples/crate_universe/cargo_conditional_deps/cargo-bazel-lock.json +++ b/examples/crate_universe/cargo_conditional_deps/cargo-bazel-lock.json @@ -1,5 +1,5 @@ { - "checksum": "55534d4df8d908a61408bb80977a2cdb87a44636933ee9d23b6d1173e427c097", + "checksum": "7571ea617ab448f1b3426c07ba5c50778a0020684f3825cad276e93572907a38", "crates": { "autocfg 1.1.0": { "name": "autocfg", diff --git a/examples/crate_universe/cargo_workspace/cargo-bazel-lock.json b/examples/crate_universe/cargo_workspace/cargo-bazel-lock.json index 76a6a7cfc0..f74e831273 100644 --- a/examples/crate_universe/cargo_workspace/cargo-bazel-lock.json +++ b/examples/crate_universe/cargo_workspace/cargo-bazel-lock.json @@ -1,5 +1,5 @@ { - "checksum": "7b3cac450781db2b258674934f9d41fc149d2e90294815e68711eeb926d8eca0", + "checksum": "b8a27f916b92c4f395b6915e08861939ce0a6354c0d91d1713d909bd1f2e9174", "crates": { "ansi_term 0.12.1": { "name": "ansi_term", diff --git a/examples/crate_universe/multi_package/cargo-bazel-lock.json b/examples/crate_universe/multi_package/cargo-bazel-lock.json index af0d9e0023..6aa8b4808b 100644 --- a/examples/crate_universe/multi_package/cargo-bazel-lock.json +++ b/examples/crate_universe/multi_package/cargo-bazel-lock.json @@ -1,5 +1,5 @@ { - "checksum": "0eea66f6f1238cd243a4656ffdc715aa2218d45b9d5c6fa47a981711334d0a42", + "checksum": "5d3efb3a8b7aecb82aa61a5488597ca7cb9342560de40eb7037cd79e7445b9aa", "crates": { "aho-corasick 0.7.20": { "name": "aho-corasick", diff --git a/examples/crate_universe/using_cxx/cxxbridge-cmd.cargo-bazel-lock.json b/examples/crate_universe/using_cxx/cxxbridge-cmd.cargo-bazel-lock.json index 63f93579e2..170cd2c66b 100644 --- a/examples/crate_universe/using_cxx/cxxbridge-cmd.cargo-bazel-lock.json +++ b/examples/crate_universe/using_cxx/cxxbridge-cmd.cargo-bazel-lock.json @@ -1,5 +1,5 @@ { - "checksum": "ab8b093d4ba7e3bca34a3fc81413c70271914e1b01ed42d22db41c6ea5e9dab6", + "checksum": "1eeb80f1d0fa732cc8155ca4d18d9338651532ba8437ce17eb9422bf99d437f4", "crates": { "anstyle 1.0.1": { "name": "anstyle",