From 2218245a6dd8be002f5ebe2ec724568982bd44b2 Mon Sep 17 00:00:00 2001 From: JP Sugarbroad Date: Mon, 4 Apr 2016 19:07:08 -0700 Subject: [PATCH] Deduplicate libraries on hash instead of filename. --- src/librustc/hir/svh.rs | 2 +- src/librustc_metadata/loader.rs | 98 ++++++++----------- .../run-make/compiler-lookup-paths/Makefile | 8 ++ src/test/run-make/compiler-lookup-paths/e2.rs | 14 +++ src/test/run-make/extern-flag-fun/Makefile | 5 +- src/test/run-make/extern-flag-fun/bar-alt.rs | 11 +++ src/test/run-make/issue-11908/Makefile | 10 +- 7 files changed, 84 insertions(+), 64 deletions(-) create mode 100644 src/test/run-make/compiler-lookup-paths/e2.rs create mode 100644 src/test/run-make/extern-flag-fun/bar-alt.rs diff --git a/src/librustc/hir/svh.rs b/src/librustc/hir/svh.rs index 08c3d70034a..1536f884b09 100644 --- a/src/librustc/hir/svh.rs +++ b/src/librustc/hir/svh.rs @@ -48,7 +48,7 @@ use std::fmt; -#[derive(Clone, PartialEq, Debug)] +#[derive(Clone, Eq, Hash, PartialEq, Debug)] pub struct Svh { hash: String, } diff --git a/src/librustc_metadata/loader.rs b/src/librustc_metadata/loader.rs index be1e194efc3..28e0e5746a3 100644 --- a/src/librustc_metadata/loader.rs +++ b/src/librustc_metadata/loader.rs @@ -470,20 +470,17 @@ impl<'a> Context<'a> { // A Library candidate is created if the metadata for the set of // libraries corresponds to the crate id and hash criteria that this // search is being performed for. - let mut libraries = Vec::new(); + let mut libraries = HashMap::new(); for (_hash, (rlibs, dylibs)) in candidates { - let mut metadata = None; - let rlib = self.extract_one(rlibs, CrateFlavor::Rlib, &mut metadata); - let dylib = self.extract_one(dylibs, CrateFlavor::Dylib, &mut metadata); - match metadata { - Some(metadata) => { - libraries.push(Library { - dylib: dylib, - rlib: rlib, - metadata: metadata, - }) - } - None => {} + let mut slot = None; + let rlib = self.extract_one(rlibs, CrateFlavor::Rlib, &mut slot); + let dylib = self.extract_one(dylibs, CrateFlavor::Dylib, &mut slot); + if let Some((h, m)) = slot { + libraries.insert(h, Library { + dylib: dylib, + rlib: rlib, + metadata: m, + }); } } @@ -492,13 +489,13 @@ impl<'a> Context<'a> { // libraries or not. match libraries.len() { 0 => None, - 1 => Some(libraries.into_iter().next().unwrap()), + 1 => Some(libraries.into_iter().next().unwrap().1), _ => { let mut err = struct_span_err!(self.sess, self.span, E0464, "multiple matching crates for `{}`", self.crate_name); err.note("candidates:"); - for lib in &libraries { + for (_, lib) in libraries { match lib.dylib { Some((ref p, _)) => { err.note(&format!("path: {}", @@ -532,13 +529,13 @@ impl<'a> Context<'a> { // be read, it is assumed that the file isn't a valid rust library (no // errors are emitted). fn extract_one(&mut self, m: HashMap, flavor: CrateFlavor, - slot: &mut Option) -> Option<(PathBuf, PathKind)> { - let mut ret = None::<(PathBuf, PathKind)>; + slot: &mut Option<(Svh, MetadataBlob)>) -> Option<(PathBuf, PathKind)> { + let mut ret: Option<(PathBuf, PathKind)> = None; let mut error = 0; if slot.is_some() { // FIXME(#10786): for an optimization, we only read one of the - // library's metadata sections. In theory we should + // libraries' metadata sections. In theory we should // read both, but reading dylib metadata is quite // slow. if m.is_empty() { @@ -551,10 +548,10 @@ impl<'a> Context<'a> { let mut err: Option = None; for (lib, kind) in m { info!("{} reading metadata from: {}", flavor, lib.display()); - let metadata = match get_metadata_section(self.target, flavor, &lib) { + let (hash, metadata) = match get_metadata_section(self.target, flavor, &lib) { Ok(blob) => { - if self.crate_matches(blob.as_slice(), &lib) { - blob + if let Some(h) = self.crate_matches(blob.as_slice(), &lib) { + (h, blob) } else { info!("metadata mismatch"); continue @@ -565,12 +562,8 @@ impl<'a> Context<'a> { continue } }; - // If we've already found a candidate and we're not matching hashes, - // emit an error about duplicate candidates found. If we're matching - // based on a hash, however, then if we've gotten this far both - // candidates have the same hash, so they're not actually - // duplicates that we should warn about. - if ret.is_some() && self.hash.is_none() { + // If we see multiple hashes, emit an error about duplicate candidates. + if slot.as_ref().map_or(false, |s| s.0 != hash) { let mut e = struct_span_err!(self.sess, self.span, E0465, "multiple {} candidates for `{}` found", flavor, self.crate_name); @@ -583,7 +576,7 @@ impl<'a> Context<'a> { } err = Some(e); error = 1; - ret = None; + *slot = None; } if error > 0 { error += 1; @@ -592,7 +585,7 @@ impl<'a> Context<'a> { lib.display())); continue } - *slot = Some(metadata); + *slot = Some((hash, metadata)); ret = Some((lib, kind)); } @@ -604,22 +597,20 @@ impl<'a> Context<'a> { } } - fn crate_matches(&mut self, crate_data: &[u8], libpath: &Path) -> bool { + fn crate_matches(&mut self, crate_data: &[u8], libpath: &Path) -> Option { if self.should_match_name { match decoder::maybe_get_crate_name(crate_data) { Some(ref name) if self.crate_name == *name => {} - _ => { info!("Rejecting via crate name"); return false } + _ => { info!("Rejecting via crate name"); return None } } } let hash = match decoder::maybe_get_crate_hash(crate_data) { - Some(hash) => hash, None => { - info!("Rejecting via lack of crate hash"); - return false; - } + None => { info!("Rejecting via lack of crate hash"); return None; } + Some(h) => h, }; let triple = match decoder::get_crate_triple(crate_data) { - None => { debug!("triple not present"); return false } + None => { debug!("triple not present"); return None } Some(t) => t, }; if triple != self.triple { @@ -628,24 +619,21 @@ impl<'a> Context<'a> { path: libpath.to_path_buf(), got: triple.to_string() }); - return false; + return None; } - match self.hash { - None => true, - Some(myhash) => { - if *myhash != hash { - info!("Rejecting via hash: expected {} got {}", *myhash, hash); - self.rejected_via_hash.push(CrateMismatch { - path: libpath.to_path_buf(), - got: myhash.as_str().to_string() - }); - false - } else { - true - } + if let Some(myhash) = self.hash { + if *myhash != hash { + info!("Rejecting via hash: expected {} got {}", *myhash, hash); + self.rejected_via_hash.push(CrateMismatch { + path: libpath.to_path_buf(), + got: myhash.as_str().to_string() + }); + return None; } } + + Some(hash) } @@ -717,13 +705,13 @@ impl<'a> Context<'a> { }; // Extract the rlib/dylib pair. - let mut metadata = None; - let rlib = self.extract_one(rlibs, CrateFlavor::Rlib, &mut metadata); - let dylib = self.extract_one(dylibs, CrateFlavor::Dylib, &mut metadata); + let mut slot = None; + let rlib = self.extract_one(rlibs, CrateFlavor::Rlib, &mut slot); + let dylib = self.extract_one(dylibs, CrateFlavor::Dylib, &mut slot); if rlib.is_none() && dylib.is_none() { return None } - match metadata { - Some(metadata) => Some(Library { + match slot { + Some((_, metadata)) => Some(Library { dylib: dylib, rlib: rlib, metadata: metadata, diff --git a/src/test/run-make/compiler-lookup-paths/Makefile b/src/test/run-make/compiler-lookup-paths/Makefile index 154e46c0edc..e22b937a087 100644 --- a/src/test/run-make/compiler-lookup-paths/Makefile +++ b/src/test/run-make/compiler-lookup-paths/Makefile @@ -18,13 +18,21 @@ all: $(TMPDIR)/libnative.a $(RUSTC) d.rs -L crate=$(TMPDIR)/native && exit 1 || exit 0 $(RUSTC) d.rs -L native=$(TMPDIR)/native $(RUSTC) d.rs -L all=$(TMPDIR)/native + # Deduplication tests: + # Same hash, no errors. mkdir -p $(TMPDIR)/e1 mkdir -p $(TMPDIR)/e2 $(RUSTC) e.rs -o $(TMPDIR)/e1/libe.rlib $(RUSTC) e.rs -o $(TMPDIR)/e2/libe.rlib + $(RUSTC) f.rs -L $(TMPDIR)/e1 -L $(TMPDIR)/e2 + $(RUSTC) f.rs -L crate=$(TMPDIR)/e1 -L $(TMPDIR)/e2 + $(RUSTC) f.rs -L crate=$(TMPDIR)/e1 -L crate=$(TMPDIR)/e2 + # Different hash, errors. + $(RUSTC) e2.rs -o $(TMPDIR)/e2/libe.rlib $(RUSTC) f.rs -L $(TMPDIR)/e1 -L $(TMPDIR)/e2 && exit 1 || exit 0 $(RUSTC) f.rs -L crate=$(TMPDIR)/e1 -L $(TMPDIR)/e2 && exit 1 || exit 0 $(RUSTC) f.rs -L crate=$(TMPDIR)/e1 -L crate=$(TMPDIR)/e2 && exit 1 || exit 0 + # Native/dependency paths don't cause errors. $(RUSTC) f.rs -L native=$(TMPDIR)/e1 -L $(TMPDIR)/e2 $(RUSTC) f.rs -L dependency=$(TMPDIR)/e1 -L $(TMPDIR)/e2 $(RUSTC) f.rs -L dependency=$(TMPDIR)/e1 -L crate=$(TMPDIR)/e2 diff --git a/src/test/run-make/compiler-lookup-paths/e2.rs b/src/test/run-make/compiler-lookup-paths/e2.rs new file mode 100644 index 00000000000..f8c8c029c0b --- /dev/null +++ b/src/test/run-make/compiler-lookup-paths/e2.rs @@ -0,0 +1,14 @@ +// Copyright 2016 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![crate_name = "e"] +#![crate_type = "rlib"] + +pub fn f() {} diff --git a/src/test/run-make/extern-flag-fun/Makefile b/src/test/run-make/extern-flag-fun/Makefile index ca5aa052a7b..a9f25853350 100644 --- a/src/test/run-make/extern-flag-fun/Makefile +++ b/src/test/run-make/extern-flag-fun/Makefile @@ -3,14 +3,15 @@ all: $(RUSTC) bar.rs --crate-type=rlib $(RUSTC) bar.rs --crate-type=rlib -C extra-filename=-a + $(RUSTC) bar-alt.rs --crate-type=rlib $(RUSTC) foo.rs --extern hello && exit 1 || exit 0 $(RUSTC) foo.rs --extern bar=no-exist && exit 1 || exit 0 $(RUSTC) foo.rs --extern bar=foo.rs && exit 1 || exit 0 $(RUSTC) foo.rs \ --extern bar=$(TMPDIR)/libbar.rlib \ - --extern bar=$(TMPDIR)/libbar-a.rlib \ + --extern bar=$(TMPDIR)/libbar-alt.rlib \ && exit 1 || exit 0 $(RUSTC) foo.rs \ --extern bar=$(TMPDIR)/libbar.rlib \ - --extern bar=$(TMPDIR)/libbar.rlib + --extern bar=$(TMPDIR)/libbar-a.rlib $(RUSTC) foo.rs --extern bar=$(TMPDIR)/libbar.rlib diff --git a/src/test/run-make/extern-flag-fun/bar-alt.rs b/src/test/run-make/extern-flag-fun/bar-alt.rs new file mode 100644 index 00000000000..d6ebd9d896f --- /dev/null +++ b/src/test/run-make/extern-flag-fun/bar-alt.rs @@ -0,0 +1,11 @@ +// Copyright 2014 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +pub fn f() {} diff --git a/src/test/run-make/issue-11908/Makefile b/src/test/run-make/issue-11908/Makefile index 663a9f7125e..cf6572c27ad 100644 --- a/src/test/run-make/issue-11908/Makefile +++ b/src/test/run-make/issue-11908/Makefile @@ -9,15 +9,13 @@ all: mkdir $(TMPDIR)/other - $(RUSTC) foo.rs --crate-type=dylib + $(RUSTC) foo.rs --crate-type=dylib -C prefer-dynamic mv $(call DYLIB,foo) $(TMPDIR)/other - $(RUSTC) foo.rs --crate-type=dylib - $(RUSTC) bar.rs -L $(TMPDIR)/other 2>&1 | \ - grep "multiple dylib candidates" + $(RUSTC) foo.rs --crate-type=dylib -C prefer-dynamic + $(RUSTC) bar.rs -L $(TMPDIR)/other rm -rf $(TMPDIR) mkdir -p $(TMPDIR)/other $(RUSTC) foo.rs --crate-type=rlib mv $(TMPDIR)/libfoo.rlib $(TMPDIR)/other $(RUSTC) foo.rs --crate-type=rlib - $(RUSTC) bar.rs -L $(TMPDIR)/other 2>&1 | \ - grep "multiple rlib candidates" + $(RUSTC) bar.rs -L $(TMPDIR)/other