fix(why-miss): join the cascade walk by compilation unit, not crate name (#627)

src/args.rs

@@ -59,6 +59,48 @@ pub fn format_crate_output_stem(crate_name: &str, extra_filename: &str) -> Strin
     format!("{crate_name}{extra_filename}")
 }
 
+/// Compilation-unit identity for diagnostics: cargo's `-C extra-filename` hash,
+/// without its leading dash (kunobi-ninja/kache#627).
+///
+/// `crate_name` is not a unit identity — two versions of a package, a host and
+/// a target build of the same crate, and different feature sets all collapse
+/// onto one name. Cargo's `extra-filename` is exactly the disambiguator that
+/// keeps those units' artifacts from colliding in one `deps/` directory, so
+/// within a build tree it identifies the unit precisely.
+///
+/// Deliberately NOT `-C metadata`: cargo sets the two to different hashes
+/// (`-C metadata=04bad873faff484a -C extra-filename=-843f02d6a46ebef1` in one
+/// observed invocation), and it is `extra-filename` that appears in the
+/// artifact filename a consumer sees on its `--extern` path. Matching the two
+/// sides needs the one that is visible from both.
+///
+/// This is recorded on events, never folded into a cache key: keying on it
+/// would tie the key to cargo's unit hashing and break cross-machine sharing.
+pub fn unit_id_from_extra_filename(extra_filename: &str) -> Option<String> {
+    let id = extra_filename.strip_prefix('-').unwrap_or(extra_filename);
+    (!id.is_empty()).then(|| id.to_string())
+}
+
+/// The producing unit's identity, recovered from an `--extern` artifact path.
+///
+/// Cargo names dependency artifacts `lib{crate_name}-{hash}.rlib` (`.rmeta`,
+/// `.dylib`, `.so`, `.dll`), where `-{hash}` is the producer's
+/// `-C extra-filename`. Taking the tail after the LAST dash is safe because a
+/// rustc crate name cannot contain one.
+///
+/// Returns `None` for anything not carrying that suffix — a sysroot crate, a
+/// hand-rolled rustc invocation, an artifact built without `extra-filename` —
+/// so callers fall back to name matching rather than inventing an identity.
+pub fn unit_id_from_artifact_path(path: &Path) -> Option<String> {
+    let stem = path.file_stem()?.to_str()?;
+    let (_, suffix) = stem.rsplit_once('-')?;
+    // Cargo's hash is lowercase hex. Requiring that shape keeps a crate whose
+    // *file* name happens to carry a dash (`libfoo-bar.rlib`, built outside
+    // cargo) from being read as a unit id.
+    let hex = suffix.len() >= 8 && suffix.bytes().all(|b| b.is_ascii_hexdigit());
+    hex.then(|| suffix.to_string())
+}
+
 /// Parsed rustc invocation arguments relevant to caching.
 #[derive(Debug, Clone, Default)]
 pub struct RustcArgs {
@@ -537,6 +579,15 @@ impl RustcArgs {
         ))
     }
 
+    /// This compile's own unit identity, for diagnostics only
+    /// (kunobi-ninja/kache#627). `None` when cargo passed no `-C extra-filename`,
+    /// which is the same case [`unit_id_from_artifact_path`] cannot resolve from
+    /// the consumer side — so the two sides go unidentified together, and
+    /// `why-miss` falls back to matching by crate name.
+    pub fn unit_id(&self) -> Option<String> {
+        unit_id_from_extra_filename(self.extra_filename.as_deref()?)
+    }
+
     /// Whether this compilation has coverage instrumentation enabled (-C instrument-coverage).
     /// When active, path remapping must be skipped so coverage tools (tarpaulin, llvm-cov)
     /// can map profraw data back to source files.
@@ -619,6 +670,59 @@ fn record_codegen_opt(parsed: &mut RustcArgs, value: &str) {
 mod tests {
     use super::*;
 
+    /// The two sides of the #627 join have to agree: what a producer records
+    /// for itself (`-C extra-filename`) must equal what a consumer recovers
+    /// from the artifact filename cargo built with that flag.
+    #[test]
+    fn unit_id_round_trips_between_the_producer_and_its_artifact() {
+        let producer = unit_id_from_extra_filename("-843f02d6a46ebef1").unwrap();
+        for artifact in [
+            "/w/target/debug/deps/librust_check-843f02d6a46ebef1.rmeta",
+            "/w/target/debug/deps/librust_check-843f02d6a46ebef1.rlib",
+            "/w/target/debug/deps/librust_check-843f02d6a46ebef1.dylib",
+            r"C:\w\target\debug\deps\librust_check-843f02d6a46ebef1.rlib",
+        ] {
+            assert_eq!(
+                unit_id_from_artifact_path(Path::new(artifact)).as_deref(),
+                Some(producer.as_str()),
+                "{artifact}"
+            );
+        }
+    }
+
+    #[test]
+    fn unit_id_declines_paths_without_a_cargo_hash_suffix() {
+        // A sysroot crate, and a crate whose file name merely contains a dash:
+        // inventing an identity for either would be worse than falling back to
+        // matching by name.
+        for artifact in [
+            "/toolchain/lib/rustlib/x86_64/lib/libstd.rlib",
+            "/w/target/debug/deps/libfoo-bar.rlib",
+            "/w/target/debug/deps/libfoo-123.rlib",
+        ] {
+            assert_eq!(
+                unit_id_from_artifact_path(Path::new(artifact)),
+                None,
+                "{artifact}"
+            );
+        }
+        assert_eq!(unit_id_from_extra_filename(""), None);
+        assert_eq!(unit_id_from_extra_filename("-"), None);
+    }
+
+    #[test]
+    fn unit_id_reads_the_parsed_extra_filename() {
+        let args: Vec<String> = ["rustc", "rustc", "--crate-name", "foo", "src/lib.rs"]
+            .iter()
+            .map(|s| s.to_string())
+            .collect();
+        let mut parsed = RustcArgs::parse(&args).unwrap();
+        assert_eq!(parsed.unit_id(), None, "no -C extra-filename, no identity");
+
+        parsed.extra_filename = Some("-d44c553abc12".to_string());
+        assert_eq!(parsed.unit_id().as_deref(), Some("d44c553abc12"));
+    }
+
     #[test]
     fn test_parse_basic_lib() {
         let args: Vec<String> = vec![

src/cache_key.rs

@@ -461,6 +461,39 @@ pub fn take_last_key_externs() -> Option<std::collections::BTreeMap<String, Stri
     LAST_KEY_EXTERNS.lock().ok().and_then(|mut g| g.take())
 }
 
+/// Producing-unit identity per extern, teed off the same loop that computes
+/// [`LAST_KEY_EXTERNS`] (kunobi-ninja/kache#627).
+///
+/// Keyed by the name the CONSUMER used, which under Cargo's `package = "..."`
+/// renaming is an alias (`foo_old` for a crate whose own events say `foo`). The
+/// value is the producer's `-C extra-filename`, recovered from the artifact path
+/// — the one identity visible from both sides, so `why-miss` can join a changed
+/// dependency to the exact unit that produced it instead of guessing by name.
+///
+/// Absent for an extern whose path carries no such suffix (sysroot crates,
+/// non-cargo invocations); the walk then falls back to matching by name.
+static LAST_KEY_EXTERN_UNITS: std::sync::Mutex<Option<std::collections::BTreeMap<String, String>>> =
+    std::sync::Mutex::new(None);
+
+/// Take (consume) the per-extern producing-unit ids of the last computed rustc
+/// key. Always taken alongside [`take_last_key_externs`] so a stale map cannot
+/// outlive its digests.
+pub fn take_last_key_extern_units() -> Option<std::collections::BTreeMap<String, String>> {
+    LAST_KEY_EXTERN_UNITS.lock().ok().and_then(|mut g| g.take())
+}
+
+/// The compiling unit's own identity, stashed at key computation so the event
+/// writer needs no extra plumbing — the same pattern the per-group digests use
+/// (kunobi-ninja/kache#131). Set unconditionally at the top of
+/// [`compute_cache_key`], including to `None` when cargo passed no
+/// `-C extra-filename`, so it can never carry over from a previous compile.
+static LAST_KEY_UNIT_ID: std::sync::Mutex<Option<String>> = std::sync::Mutex::new(None);
+
+/// Take (consume) the unit id of the last computed rustc key.
+pub fn take_last_key_unit_id() -> Option<String> {
+    LAST_KEY_UNIT_ID.lock().ok().and_then(|mut g| g.take())
+}
+
 /// Compute the blake3 cache key for a rustc invocation.
 ///
 /// The key captures everything that affects compilation output:
@@ -492,6 +525,15 @@ pub fn compute_cache_key(
     if let Ok(mut stash) = LAST_KEY_EXTERNS.lock() {
         *stash = None;
     }
+    // Same reasoning for the unit ids (#627); both are cleared and written
+    // together so the walk can never pair one compile's digests with another's
+    // identities.
+    if let Ok(mut stash) = LAST_KEY_EXTERN_UNITS.lock() {
+        *stash = None;
+    }
+    if let Ok(mut stash) = LAST_KEY_UNIT_ID.lock() {
+        *stash = args.unit_id();
+    }
 
     // key version — bump CACHE_KEY_VERSION to invalidate all prior entries
     hasher.update(b"key_version:");
@@ -771,8 +813,15 @@ pub fn compute_cache_key(
     // already in hand — while the decision to PERSIST it stays with the
     // wrapper's `explain_miss` gate.
     let mut extern_digests = std::collections::BTreeMap::new();
+    // Producing-unit ids, from the artifact filename rather than the extern
+    // name, so a renamed or duplicated dependency still joins to its producer
+    // (kunobi-ninja/kache#627).
+    let mut extern_units = std::collections::BTreeMap::new();
     for ext in &externs {
         if let Some(path) = &ext.path {
+            if let Some(unit) = crate::args::unit_id_from_artifact_path(path) {
+                extern_units.insert(ext.name.clone(), unit);
+            }
             match file_hasher.hash(path) {
                 Ok(dep_hash) => {
                     hasher.update(b"extern:");
@@ -810,6 +859,9 @@ pub fn compute_cache_key(
     if let Ok(mut stash) = LAST_KEY_EXTERNS.lock() {
         *stash = Some(extern_digests);
     }
+    if let Ok(mut stash) = LAST_KEY_EXTERN_UNITS.lock() {
+        *stash = Some(extern_units);
+    }
 
     // RUSTFLAGS — normalize via PathNormalizer (canonical-prefix
     // sentinel substitution; supersedes the older CWD-only
@@ -5217,6 +5269,51 @@ pub const OUT_DIR_AT_COMPILE_TIME: &str = env!("OUT_DIR");
         ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner())
     }
 
+    /// Key computation stashes this compile's unit identity and its per-extern
+    /// producer ids, and each is TAKEN — a second read yields `None`, so a
+    /// compile that computes no rustc key cannot inherit the previous one's
+    /// identities from the same process (kunobi-ninja/kache#627).
+    #[test]
+    fn key_computation_stashes_unit_identity_and_yields_it_once() {
+        let _lock = key_test_lock();
+        let args: Vec<String> = [
+            "rustc",
+            "--crate-name",
+            "app",
+            "src/lib.rs",
+            "-C",
+            "extra-filename=-843f02d6a46ebef1",
+            "--extern",
+            "foo_old=/w/target/debug/deps/libfoo-0532daf0ee3516f0.rlib",
+        ]
+        .iter()
+        .map(|s| s.to_string())
+        .collect();
+        let mut parsed = RustcArgs::parse(&args).unwrap();
+        // Skip the dep-info pre-pass: this is about what the externs loop
+        // records, not source discovery.
+        parsed.source_file = None;
+
+        compute_cache_key(&parsed, &FileHasher::new(), &PathNormalizer::empty()).unwrap();
+
+        assert_eq!(
+            take_last_key_unit_id().as_deref(),
+            Some("843f02d6a46ebef1"),
+            "the compile's own `-C extra-filename`"
+        );
+        assert_eq!(take_last_key_unit_id(), None, "taken, not copied");
+
+        let units = take_last_key_extern_units().expect("recorded with the digests");
+        assert_eq!(
+            units.get("foo_old").map(String::as_str),
+            // Keyed by the alias the consumer used; the value is the producer's
+            // identity, recovered from the artifact filename even though that
+            // file does not exist here.
+            Some("0532daf0ee3516f0")
+        );
+        assert_eq!(take_last_key_extern_units(), None, "taken, not copied");
+    }
+
     /// True if a bare `rustc` is invocable. `compute_cache_key` forks
     /// rustc for the version probe and dep-info pre-pass; without it
     /// the key still computes (the pre-pass falls back) but the

src/cli.rs

@@ -5365,6 +5365,8 @@ mod tests {
             key_diff: Vec::new(),
             key_externs: Default::default(),
             key_externs_recorded: false,
+            unit_id: String::new(),
+            extern_units: Default::default(),
         }
     }
 

src/events.rs

@@ -37,7 +37,8 @@ pub struct BuildEvent {
     /// 9 = event root, 10 = build session id (#583),
     /// 11 = key field hashes + miss key diff (#131),
     /// 12 = per-extern artifact digests (#609),
-    /// 13 = store-failure reason (#629).
+    /// 13 = store-failure reason (#629),
+    /// 14 = compilation-unit identity, own and per-extern (#627).
     #[serde(default)]
     pub schema: u32,
     /// Build session this event belongs to (kunobi-ninja/kache#583 P0.5).
@@ -182,6 +183,35 @@ pub struct BuildEvent {
     /// default (non-`explain_miss`) path.
     #[serde(default, skip_serializing_if = "is_false")]
     pub key_externs_recorded: bool,
+    /// This compile's own compilation-unit identity — cargo's
+    /// `-C extra-filename` hash (kunobi-ninja/kache#627).
+    ///
+    /// `crate_name` cannot carry this: two versions of a package, a host and a
+    /// target build of the same crate, and two feature sets of it all share one
+    /// name, so pairing a compile with "the previous event of the same crate"
+    /// can pair two unrelated units. This is the disambiguator cargo itself uses
+    /// to keep those units' artifacts apart in one `deps/` directory.
+    ///
+    /// Diagnostic only — never folded into a cache key, which would tie the key
+    /// to cargo's unit hashing and break cross-machine sharing. Written under
+    /// the same `[cache] explain_miss` gate as `key_externs`; empty for cc
+    /// compiles, passthroughs, non-cargo rustc invocations, and pre-#627
+    /// wrappers.
+    #[serde(default, skip_serializing_if = "String::is_empty")]
+    pub unit_id: String,
+    /// Producing unit per extern, as `name the consumer used -> the producer's
+    /// unit id` (kunobi-ninja/kache#627).
+    ///
+    /// The companion to `key_externs`: that map says which dependency's artifact
+    /// moved, this one says which compilation unit produced it. It is what lets
+    /// the cascade walk follow a renamed dependency (`foo_old = { package =
+    /// "foo" }` records the alias in the consumer's key but `foo` in the
+    /// producer's events) and pick the right one of several same-named units.
+    ///
+    /// Recovered from the `--extern` artifact filename, so an extern without
+    /// that suffix simply has no entry and the walk falls back to name matching.
+    #[serde(default, skip_serializing_if = "std::collections::BTreeMap::is_empty")]
+    pub extern_units: std::collections::BTreeMap<String, String>,
 }
 
 fn is_false(value: &bool) -> bool {
@@ -1083,6 +1113,8 @@ impl BuildEvent {
             key_diff: Vec::new(),
             key_externs: Default::default(),
             key_externs_recorded: false,
+            unit_id: String::new(),
+            extern_units: Default::default(),
         }
     }
 }
@@ -1153,6 +1185,8 @@ mod tests {
             key_diff: Vec::new(),
             key_externs: Default::default(),
             key_externs_recorded: false,
+            unit_id: String::new(),
+            extern_units: Default::default(),
         };
 
         log_event(&log_path, &event).unwrap();