Skip to content

Commit

Permalink
Read metadata from rmeta exclusively, if possible
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Mark-Simulacrum committed Jan 20, 2020
1 parent 4884061 commit 4bb6882
Showing 1 changed file with 38 additions and 6 deletions.
44 changes: 38 additions & 6 deletions src/librustc_metadata/locator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -656,14 +656,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.
Expand All @@ -681,12 +703,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());
Expand Down

0 comments on commit 4bb6882

Please sign in to comment.