Skip to content

After a hash mismatch error, emit file-system paths of crates involved. #13284

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Apr 5, 2014
Merged

After a hash mismatch error, emit file-system paths of crates involved. #13284

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3ccbffa
After a hash mismatch error, emit file-system paths of crates involved.
pnkfelix Apr 3, 2014
1599d22
Added session.fileline_note() method and support infrastucture for it.
pnkfelix Apr 5, 2014
4afd060
Accumulate list of paths for crate hash mismatch.
pnkfelix Apr 5, 2014
f4cde4e
Rough regression test for #13266.
pnkfelix Apr 5, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 26 additions & 18 deletions src/librustc/metadata/creader.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ use metadata::cstore;
use metadata::decoder;
use metadata::loader;
use metadata::loader::Os;
use metadata::loader::CratePaths;

use std::cell::RefCell;
use std::rc::Rc;
Expand Down Expand Up @@ -141,7 +142,7 @@ fn visit_view_item(e: &mut Env, i: &ast::ViewItem) {

match extract_crate_info(e, i) {
Some(info) => {
let cnum = resolve_crate(e, None, info.ident, &info.crate_id, None,
let cnum = resolve_crate(e, &None, info.ident, &info.crate_id, None,
i.span);
e.sess.cstore.add_extern_mod_stmt_cnum(info.id, cnum);
}
Expand Down Expand Up @@ -278,13 +279,13 @@ fn existing_match(e: &Env, crate_id: &CrateId,
None
}

fn resolve_crate(e: &mut Env,
root_ident: Option<&str>,
ident: &str,
crate_id: &CrateId,
hash: Option<&Svh>,
span: Span)
-> ast::CrateNum {
fn resolve_crate<'a>(e: &mut Env,
root: &Option<CratePaths>,
ident: &str,
crate_id: &CrateId,
hash: Option<&Svh>,
span: Span)
-> ast::CrateNum {
match existing_match(e, crate_id, hash) {
None => {
let id_hash = link::crate_id_hash(crate_id);
Expand All @@ -297,11 +298,11 @@ fn resolve_crate(e: &mut Env,
hash: hash.map(|a| &*a),
os: e.os,
intr: e.intr.clone(),
rejected_via_hash: false,
rejected_via_hash: None,
};
let loader::Library {
dylib, rlib, metadata
} = load_ctxt.load_library_crate(root_ident);
} = load_ctxt.load_library_crate(root);

let crate_id = decoder::get_crate_id(metadata.as_slice());
let hash = decoder::get_crate_hash(metadata.as_slice());
Expand All @@ -316,15 +317,22 @@ fn resolve_crate(e: &mut Env,
});
e.next_crate_num += 1;

// Maintain a reference to the top most crate.
let root_crate = match root_ident {
Some(c) => c,
None => load_ctxt.ident.clone()
// Stash paths for top-most crate locally if necessary.
let crate_paths = if root.is_none() {
Some(CratePaths {
ident: load_ctxt.ident.to_owned(),
dylib: dylib.clone(),
rlib: rlib.clone(),
})
} else {
None
};
// Maintain a reference to the top most crate.
let root = if root.is_some() { root } else { &crate_paths };

// Now resolve the crates referenced by this crate
let cnum_map = resolve_crate_deps(e,
Some(root_crate),
root,
metadata.as_slice(),
span);

Expand All @@ -349,7 +357,7 @@ fn resolve_crate(e: &mut Env,

// Go through the crate metadata and load any crates that it references
fn resolve_crate_deps(e: &mut Env,
root_ident: Option<&str>,
root: &Option<CratePaths>,
cdata: &[u8], span : Span)
-> cstore::cnum_map {
debug!("resolving deps of external crate");
Expand All @@ -360,7 +368,7 @@ fn resolve_crate_deps(e: &mut Env,
for dep in r.iter() {
let extrn_cnum = dep.cnum;
debug!("resolving dep crate {} hash: `{}`", dep.crate_id, dep.hash);
let local_cnum = resolve_crate(e, root_ident,
let local_cnum = resolve_crate(e, root,
dep.crate_id.name.as_slice(),
&dep.crate_id,
Some(&dep.hash),
Expand Down Expand Up @@ -393,7 +401,7 @@ impl<'a> Loader<'a> {
impl<'a> CrateLoader for Loader<'a> {
fn load_crate(&mut self, krate: &ast::ViewItem) -> MacroCrate {
let info = extract_crate_info(&self.env, krate).unwrap();
let cnum = resolve_crate(&mut self.env, None, info.ident,
let cnum = resolve_crate(&mut self.env, &None, info.ident,
&info.crate_id, None, krate.span);
let library = self.env.sess.cstore.get_used_crate_source(cnum).unwrap();
MacroCrate {
Expand Down
54 changes: 44 additions & 10 deletions src/librustc/metadata/loader.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ pub enum Os {
OsFreebsd
}

pub struct ViaHash {
path: Path,
}

pub struct Context<'a> {
pub sess: &'a Session,
pub span: Span,
Expand All @@ -54,7 +58,8 @@ pub struct Context<'a> {
pub hash: Option<&'a Svh>,
pub os: Os,
pub intr: Rc<IdentInterner>,
pub rejected_via_hash: bool,
/// Some if rejected
pub rejected_via_hash: Option<ViaHash>
}

pub struct Library {
Expand All @@ -69,6 +74,25 @@ pub struct ArchiveMetadata {
data: &'static [u8],
}

pub struct CratePaths {
pub ident: ~str,
pub dylib: Option<Path>,
pub rlib: Option<Path>
}

impl CratePaths {
fn describe_paths(&self) -> ~str {
match (&self.dylib, &self.rlib) {
(&None, &None)
=> ~"",
(&Some(ref p), &None) | (&None, &Some(ref p))
=> format!("{}", p.display()),
(&Some(ref p1), &Some(ref p2))
=> format!("{}, {}", p1.display(), p2.display()),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These paths are quite long usually, so it may be best to display them on separate lines with separate note messages on the session.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay I can fix that.

}
}
}

// FIXME(#11857) this should be a "real" realpath
fn realpath(p: &Path) -> Path {
use std::os;
Expand All @@ -82,26 +106,35 @@ fn realpath(p: &Path) -> Path {
}

impl<'a> Context<'a> {
pub fn load_library_crate(&mut self, root_ident: Option<&str>) -> Library {
pub fn load_library_crate(&mut self, root: &Option<CratePaths>) -> Library {
match self.find_library_crate() {
Some(t) => t,
None => {
self.sess.abort_if_errors();
let message = if self.rejected_via_hash {
let message = if self.rejected_via_hash.is_some() {
format!("found possibly newer version of crate `{}`",
self.ident)
} else {
format!("can't find crate for `{}`", self.ident)
};
let message = match root_ident {
None => message,
Some(c) => format!("{} which `{}` depends on", message, c),
let message = match root {
&None => message,
&Some(ref r) => format!("{} which `{}` depends on",
message, r.ident)
};
self.sess.span_err(self.span, message);

if self.rejected_via_hash {
if self.rejected_via_hash.is_some() {
self.sess.span_note(self.span, "perhaps this crate needs \
to be recompiled?");
self.rejected_via_hash.as_ref().map(
|r| self.sess.note(format!(
"crate `{}` at path: {}",
self.ident, r.path.display())));
root.as_ref().map(
|r| self.sess.note(format!(
"crate `{}` at path(s): {}",
r.ident, r.describe_paths())));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps these could be span_note? Or was that left out intentionally to have some more screen real estate?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah I think the spans are already reported in the error report above?

}
self.sess.abort_if_errors();
unreachable!()
Expand Down Expand Up @@ -291,7 +324,7 @@ impl<'a> Context<'a> {
info!("{} reading metadata from: {}", flavor, lib.display());
let metadata = match get_metadata_section(self.os, &lib) {
Ok(blob) => {
if self.crate_matches(blob.as_slice()) {
if self.crate_matches(blob.as_slice(), &lib) {
blob
} else {
info!("metadata mismatch");
Expand Down Expand Up @@ -326,7 +359,7 @@ impl<'a> Context<'a> {
return if error > 0 {None} else {ret}
}

fn crate_matches(&mut self, crate_data: &[u8]) -> bool {
fn crate_matches(&mut self, crate_data: &[u8], libpath: &Path) -> bool {
match decoder::maybe_get_crate_id(crate_data) {
Some(ref id) if self.crate_id.matches(id) => {}
_ => return false
Expand All @@ -338,7 +371,8 @@ impl<'a> Context<'a> {
None => true,
Some(myhash) => {
if *myhash != hash {
self.rejected_via_hash = true;
self.rejected_via_hash =
Some(ViaHash{ path: libpath.clone(), });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could also be a vector to collect all rejected candidates rather than just one (although most of the time there will only be one)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll look into doing that

false
} else {
true
Expand Down