Ignore path deps in metadata hash

crate_universe/src/lockfile.rs

@@ -93,7 +93,7 @@ impl Digest {
         })
     }
 
-    /// A helper for generating a hash and logging it's contents.
+    /// A helper for generating a hash and logging its contents.
     fn compute_single_hash(data: &str, id: &str) -> String {
         let mut hasher = Sha256::new();
         hasher.update(data.as_bytes());
@@ -141,7 +141,7 @@ 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.
         hasher.update(Digest::compute_single_hash(
-            &serde_json::to_string(splicing_metadata).unwrap(),
+            &splicing_metadata.as_digestable_str(),
             "splicing manifest",
         ));
         hasher.update(b"\0");

crate_universe/src/splicing.rs

@@ -96,7 +96,7 @@ impl SplicingManifest {
 }
 
 /// The result of fully resolving a [SplicingManifest] in preparation for splicing.
-#[derive(Debug, Serialize, Default)]
+#[derive(Clone, Debug, Serialize, Default)]
 pub(crate) struct SplicingMetadata {
     /// A set of all packages directly written to the rule
     pub(crate) direct_packages: DirectPackageManifest,
@@ -148,6 +148,27 @@ impl TryFrom<SplicingManifest> for SplicingMetadata {
     }
 }
 
+impl SplicingMetadata {
+    // Calculates a string which can be digested to represent the splicing metadata.
+    // This allows us to strip out actions which may create noise when hashing otherwise causing spurious rebuilds.
+    // Things should only be stripped out if we are very confident they aren't used as part of resolving or rendering.
+    pub(crate) fn as_digestable_str(&self) -> String {
+        let mut copy = self.clone();
+        use cargo_toml::Dependency::*;
+        for (_, v) in copy.manifests.iter_mut() {
+            // Strip path dependencies.
+            // They don't end up factoring into what we generate, so we pretend they don't exist.
+            // This avoids people who are using both Cargo and Bazel needing to spuriously repin just because they edited Cargo-only dependency information.
+            v.dependencies.retain(|_, dep| match dep {
+                Detailed(details) => details.path.is_none(),
+                Simple(..) => true,
+                Inherited(..) => true,
+            })
+        }
+        serde_json::to_string(&copy).unwrap()
+    }
+}
+
 #[derive(Debug, Default, Serialize, Deserialize, Clone)]
 pub(crate) struct SourceInfo {
     /// A url where to a `.crate` file.
@@ -227,7 +248,7 @@ impl WorkspaceMetadata {
             })
             .collect();
 
-        // It is invald for toml maps to use empty strings as keys. In the case
+        // It is invalid for toml maps to use empty strings as keys. In the case
         // the empty key is expected to be the root package. If the root package
         // has a prefix, then all other packages will as well (even if no other
         // manifest represents them). The value is then saved as a separate value
@@ -653,4 +674,56 @@ mod test {
             "serialized metadata should not contain absolute path"
         );
     }
+
+    #[test]
+    fn splicing_metadata_digest_ignores_path_deps() {
+        let cargo_toml_manifest_with_path_dep_str = r#"[package]
+name = "pkg"
+version = "0.1.0"
+edition = "2021"
+
+[dependencies]
+local_pkg = { path = "local_pkg" }
+termcolor = "1.4.1"
+"#;
+
+        let cargo_toml_manifest_without_path_dep_str = r#"[package]
+name = "pkg"
+version = "0.1.0"
+edition = "2021"
+
+[dependencies]
+termcolor = "1.4.1"
+"#;
+
+        let label = Label::from_str("//pkg:Cargo.toml").unwrap();
+
+        let mut manifests_with_path_dep = BTreeMap::new();
+        manifests_with_path_dep.insert(
+            label.clone(),
+            cargo_toml::Manifest::from_str(cargo_toml_manifest_with_path_dep_str).unwrap(),
+        );
+
+        let mut manifests_without_path_dep = BTreeMap::new();
+        manifests_without_path_dep.insert(
+            label,
+            cargo_toml::Manifest::from_str(cargo_toml_manifest_without_path_dep_str).unwrap(),
+        );
+
+        let metadata_with_path_dep = SplicingMetadata {
+            direct_packages: DirectPackageManifest::new(),
+            manifests: manifests_with_path_dep,
+            cargo_config: None,
+        };
+        let metadata_without_path_dep = SplicingMetadata {
+            direct_packages: DirectPackageManifest::new(),
+            manifests: manifests_without_path_dep,
+            cargo_config: None,
+        };
+
+        assert_eq!(
+            metadata_with_path_dep.as_digestable_str(),
+            metadata_without_path_dep.as_digestable_str()
+        );
+    }
 }

crate_universe/src/splicing/cargo_config.rs

@@ -11,7 +11,7 @@ use serde::{Deserialize, Serialize};
 
 /// The [`[registry]`](https://doc.rust-lang.org/cargo/reference/config.html#registry)
 /// table controls the default registry used when one is not specified.
-#[derive(Debug, Deserialize, Serialize, PartialEq, Eq)]
+#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq)]
 pub(crate) struct Registry {
     /// name of the default registry
     pub(crate) default: String,
@@ -22,7 +22,7 @@ pub(crate) struct Registry {
 
 /// The [`[source]`](https://doc.rust-lang.org/cargo/reference/config.html#source)
 /// table defines the registry sources available.
-#[derive(Debug, Deserialize, Serialize, PartialEq, Eq)]
+#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq)]
 pub(crate) struct Source {
     /// replace this source with the given named source
     #[serde(rename = "replace-with")]
@@ -38,7 +38,7 @@ fn default_registry_url() -> String {
     utils::CRATES_IO_INDEX_URL.to_owned()
 }
 
-#[derive(Debug, Deserialize, Serialize, PartialEq, Eq)]
+#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq)]
 /// registries other than crates.io
 pub(crate) struct AdditionalRegistry {
     /// URL of the registry index
@@ -52,7 +52,7 @@ pub(crate) struct AdditionalRegistry {
 /// is required for parsing registry information.
 /// See [cargo docs](https://doc.rust-lang.org/cargo/reference/config.html#configuration-format)
 /// for more details.
-#[derive(Debug, Deserialize, Serialize, PartialEq, Eq)]
+#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq)]
 pub(crate) struct CargoConfig {
     /// registries other than crates.io
     #[serde(default = "default_registries")]

crate_universe/src/splicing/splicer.rs

@@ -224,7 +224,7 @@ impl<'a> SplicerKind<'a> {
         // Optionally install the cargo config after contents have been symlinked
         Self::setup_cargo_config(&splicing_manifest.cargo_config, workspace_dir)?;
 
-        // Add any additional depeendencies to the root package
+        // Add any additional dependencies to the root package
         if !splicing_manifest.direct_packages.is_empty() {
             Self::inject_direct_packages(&mut manifest, &splicing_manifest.direct_packages)?;
         }