Auto merge of #68298 - Mark-Simulacrum:binary-depdep-fix, r=petrochenkov

Avoid declaring a fake dependency edge

When we're producing an rlib, we do not need anything more than an rmeta file
for each of our dependencies (this is indeed utilized by Cargo for pipelining).
Previously, we were still storing the paths of possible rlib/dylib crates, which
meant that they could still plausibly be accessed. With -Zbinary-dep-depinfo,
that meant that Cargo thought that rustc was using both the rlib and an (earlier
emitted) rmeta, and so needed a recompile, as the rlib may have finished writing
*after* compilation started (for more detail, see issue 68149).

This commit changes metadata loading to not store the filepaths of dylib/rlib if
we're going to end up creating an rlib only.

Fixes #68149.

2 files changed, 45 insertions, 8 deletions

diff --git a/src/librustc_metadata/locator.rs b/src/librustc_metadata/locator.rs
index 578216454f9..2157b8ce159 100644
--- a/src/librustc_metadata/locator.rs
+++ b/src/librustc_metadata/locator.rs
@@ -654,14 +654,36 @@ impl<'a> CrateLocator<'a> {
         dylibs: FxHashMap<PathBuf, PathKind>,
     ) -> Option<(Svh, Library)> {
         let mut slot = None;
+        // Order here matters, rmeta should come first. See comment in
+        // `extract_one` below.
         let source = CrateSource {
-            rlib: self.extract_one(rlibs, CrateFlavor::Rlib, &mut slot),
             rmeta: self.extract_one(rmetas, CrateFlavor::Rmeta, &mut slot),
+            rlib: self.extract_one(rlibs, CrateFlavor::Rlib, &mut slot),
             dylib: self.extract_one(dylibs, CrateFlavor::Dylib, &mut slot),
         };
         slot.map(|(svh, metadata)| (svh, Library { source, metadata }))
     }
 
+    fn needs_crate_flavor(&self, flavor: CrateFlavor) -> bool {
+        if flavor == CrateFlavor::Dylib && self.is_proc_macro == Some(true) {
+            return true;
+        }
+
+        // The all loop is because `--crate-type=rlib --crate-type=rlib` is
+        // legal and produces both inside this type.
+        let is_rlib = self.sess.crate_types.borrow().iter().all(|c| *c == config::CrateType::Rlib);
+        let needs_object_code = self.sess.opts.output_types.should_codegen();
+        // If we're producing an rlib, then we don't need object code.
+        // Or, if we're not producing object code, then we don't need it either
+        // (e.g., if we're a cdylib but emitting just metadata).
+        if is_rlib || !needs_object_code {
+            flavor == CrateFlavor::Rmeta
+        } else {
+            // we need all flavors (perhaps not true, but what we do for now)
+            true
+        }
+    }
+
     // Attempts to extract *one* library from the set `m`. If the set has no
     // elements, `None` is returned. If the set has more than one element, then
     // the errors and notes are emitted about the set of libraries.
@@ -679,12 +701,22 @@ impl<'a> CrateLocator<'a> {
         let mut ret: Option<(PathBuf, PathKind)> = None;
         let mut error = 0;
 
+        // If we are producing an rlib, and we've already loaded metadata, then
+        // we should not attempt to discover further crate sources (unless we're
+        // locating a proc macro; exact logic is in needs_crate_flavor). This means
+        // that under -Zbinary-dep-depinfo we will not emit a dependency edge on
+        // the *unused* rlib, and by returning `None` here immediately we
+        // guarantee that we do indeed not use it.
+        //
+        // See also #68149 which provides more detail on why emitting the
+        // dependency on the rlib is a bad thing.
+        //
+        // We currenty do not verify that these other sources are even in sync,
+        // and this is arguably a bug (see #10786), but because reading metadata
+        // is quite slow (especially from dylibs) we currently do not read it
+        // from the other crate sources.
         if slot.is_some() {
-            // FIXME(#10786): for an optimization, we only read one of the
-            //                libraries' metadata sections. In theory we should
-            //                read both, but reading dylib metadata is quite
-            //                slow.
-            if m.is_empty() {
+            if m.is_empty() || !self.needs_crate_flavor(flavor) {
                 return None;
             } else if m.len() == 1 {
                 return Some(m.into_iter().next().unwrap());
diff --git a/src/test/ui/rmeta-rpass.rs b/src/test/ui/rmeta-rpass.rs
index 5a63b5b8598..173a6a394eb 100644
--- a/src/test/ui/rmeta-rpass.rs
+++ b/src/test/ui/rmeta-rpass.rs
@@ -1,6 +1,11 @@
 // run-pass
 // Test that using rlibs and rmeta dep crates work together. Specifically, that
-// there can be both an rmeta and an rlib file and rustc will prefer the rlib.
+// there can be both an rmeta and an rlib file and rustc will prefer the rmeta
+// file.
+//
+// This behavior is simply making sure this doesn't accidentally change; in this
+// case we want to make sure that the rlib isn't being used as that would cause
+// bugs in -Zbinary-dep-depinfo (see #68298).
 
 // aux-build:rmeta-rmeta.rs
 // aux-build:rmeta-rlib-rpass.rs
@@ -9,5 +14,5 @@ extern crate rmeta_aux;
 use rmeta_aux::Foo;
 
 pub fn main() {
-    let _ = Foo { field: 42 };
+    let _ = Foo { field2: 42 };
 }