Ignore path deps in metadata hash

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.

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