Skip to content

Commit

Permalink
Auto merge of #1164 - alexcrichton:issue-209, r=huonw
Browse files Browse the repository at this point in the history
All paths printed will now be absolute paths unless the path is a descendant of
the current directory. This should keep error messages and warnings of a
reasonable length when working with the local project while still allowing
errors in registry/git dependencies to be tracked down.

Special care is taken in these situations to ensure that the error message from
the compiler prints a reasonable path.

Closes #209
Closes #694
  • Loading branch information
bors committed Jan 14, 2015
2 parents 6c22ca1 + 4a91d01 commit 25fa147
Show file tree
Hide file tree
Showing 11 changed files with 82 additions and 36 deletions.
8 changes: 7 additions & 1 deletion src/cargo/ops/cargo_rustc/context.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use std::collections::hash_map::HashMap;
use std::collections::hash_map::Entry::{Occupied, Vacant};
use std::collections::hash_map::HashMap;
use std::os;
use std::str;
use std::sync::Arc;

Expand Down Expand Up @@ -29,6 +30,7 @@ pub struct Context<'a, 'b: 'a> {
pub compilation: Compilation,
pub build_state: Arc<BuildState>,
pub exec_engine: Arc<Box<ExecEngine>>,
pub cwd: Path,

env: &'a str,
host: Layout,
Expand Down Expand Up @@ -60,6 +62,9 @@ impl<'a, 'b: 'a> Context<'a, 'b> {
};
let target_triple = config.target().map(|s| s.to_string());
let target_triple = target_triple.unwrap_or(config.rustc_host().to_string());
let cwd = try!(os::getcwd().chain_error(|| {
human("failed to get the current directory")
}));
Ok(Context {
target_triple: target_triple,
env: env,
Expand All @@ -78,6 +83,7 @@ impl<'a, 'b: 'a> Context<'a, 'b> {
build_state: Arc::new(BuildState::new(build_config.clone(), deps)),
build_config: build_config,
exec_engine: Arc::new(Box::new(ProcessEngine) as Box<ExecEngine>),
cwd: cwd,
})
}

Expand Down
30 changes: 26 additions & 4 deletions src/cargo/ops/cargo_rustc/fingerprint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ pub fn prepare_target(cx: &mut Context, pkg: &Package, target: &Target,
// indicates that the target is fresh.
let dep_info = dep_info_loc(cx, pkg, target, kind);
let mut are_files_fresh = use_pkg ||
try!(calculate_target_fresh(pkg, &dep_info));
try!(calculate_target_fresh(&dep_info));

// Second bit of the freshness calculation, whether rustc itself, the
// target are fresh, and the enabled set of features are all fresh.
Expand Down Expand Up @@ -226,8 +226,15 @@ fn mk_fingerprint<T: Hash<SipHasher>>(cx: &Context, data: &T) -> String {
util::to_hex(hasher.finish())
}

fn calculate_target_fresh(pkg: &Package, dep_info: &Path) -> CargoResult<bool> {
let line = match BufferedReader::new(File::open(dep_info)).lines().next() {
fn calculate_target_fresh(dep_info: &Path) -> CargoResult<bool> {
macro_rules! fs_try {
($e:expr) => (match $e { Ok(e) => e, Err(..) => return Ok(false) })
}
let mut f = BufferedReader::new(fs_try!(File::open(dep_info)));
// see comments in append_current_dir for where this cwd is manifested from.
let cwd = fs_try!(f.read_until(0));
let cwd = Path::new(&cwd[..cwd.len()-1]);
let line = match f.lines().next() {
Some(Ok(line)) => line,
_ => return Ok(false),
};
Expand All @@ -250,7 +257,7 @@ fn calculate_target_fresh(pkg: &Package, dep_info: &Path) -> CargoResult<bool> {
file.push(' ');
file.push_str(deps.next().unwrap())
}
match fs::stat(&pkg.get_root().join(file.as_slice())) {
match fs::stat(&cwd.join(file.as_slice())) {
Ok(stat) if stat.modified <= mtime => {}
Ok(stat) => {
info!("stale: {} -- {} vs {}", file, stat.modified, mtime);
Expand Down Expand Up @@ -289,3 +296,18 @@ fn filename(target: &Target) -> String {
};
format!("{}{}-{}", flavor, kind, target.get_name())
}

// The dep-info files emitted by the compiler all have their listed paths
// relative to whatever the current directory was at the time that the compiler
// was invoked. As the current directory may change over time, we need to record
// what that directory was at the beginning of the file so we can know about it
// next time.
pub fn append_current_dir(path: &Path, cwd: &Path) -> CargoResult<()> {
let mut f = try!(File::open_mode(path, io::Open, io::ReadWrite));
let contents = try!(f.read_to_end());
try!(f.seek(0, io::SeekSet));
try!(f.write(cwd.as_vec()));
try!(f.write(&[0]));
try!(f.write(&contents[]));
Ok(())
}
28 changes: 23 additions & 5 deletions src/cargo/ops/cargo_rustc/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -489,6 +489,7 @@ fn rustc(package: &Package, target: &Target,

let rustc_dep_info_loc = root.join(target.file_stem()).with_extension("d");
let dep_info_loc = fingerprint::dep_info_loc(cx, package, target, kind);
let cwd = cx.cwd.clone();

Ok((Work::new(move |desc_tx| {
let mut rustc = rustc;
Expand Down Expand Up @@ -525,6 +526,7 @@ fn rustc(package: &Package, target: &Target,
}));

try!(fs::rename(&rustc_dep_info_loc, &dep_info_loc));
try!(fingerprint::append_current_dir(&dep_info_loc, &cwd));

Ok(())

Expand Down Expand Up @@ -559,11 +561,10 @@ fn prepare_rustc(package: &Package, target: &Target, crate_types: Vec<&str>,
fn rustdoc(package: &Package, target: &Target,
cx: &mut Context) -> CargoResult<Work> {
let kind = Kind::Target;
let pkg_root = package.get_root();
let cx_root = cx.layout(package, kind).proxy().dest().join("doc");
let rustdoc = try!(process(CommandType::Rustdoc, package, target, cx))
.cwd(pkg_root.clone());
let mut rustdoc = rustdoc.arg(target.get_src_path())
let rustdoc = try!(process(CommandType::Rustdoc, package, target, cx));
let mut rustdoc = rustdoc.arg(root_path(cx, package, target))
.cwd(cx.cwd.clone())
.arg("-o").arg(cx_root)
.arg("--crate-name").arg(target.get_name());

Expand Down Expand Up @@ -614,15 +615,32 @@ fn rustdoc(package: &Package, target: &Target,
}))
}

// The path that we pass to rustc is actually fairly important because it will
// show up in error messages and the like. For this reason we take a few moments
// to ensure that something shows up pretty reasonably.
//
// The heuristic here is fairly simple, but the key idea is that the path is
// always "relative" to the current directory in order to be found easily. The
// path is only actually relative if the current directory is an ancestor if it.
// This means that non-path dependencies (git/registry) will likely be shown as
// absolute paths instead of relative paths.
fn root_path(cx: &Context, pkg: &Package, target: &Target) -> Path {
let absolute = pkg.get_root().join(target.get_src_path());
absolute.path_relative_from(&cx.cwd).unwrap_or(absolute)
}

fn build_base_args(cx: &Context,
mut cmd: CommandPrototype,
pkg: &Package,
target: &Target,
crate_types: &[&str]) -> CommandPrototype {
let metadata = target.get_metadata();

// Move to cwd so the root_path() passed below is actually correct
cmd = cmd.cwd(cx.cwd.clone());

// TODO: Handle errors in converting paths into args
cmd = cmd.arg(target.get_src_path());
cmd = cmd.arg(root_path(cx, pkg, target));

cmd = cmd.arg("--crate-name").arg(target.get_name());

Expand Down
4 changes: 2 additions & 2 deletions tests/test_cargo_bench.rs
Original file line number Diff line number Diff line change
Expand Up @@ -840,8 +840,8 @@ test!(bench_with_examples {
execs().with_status(0)
.with_stdout(format!("\
{compiling} testbench v6.6.6 ({url})
{running} `rustc {dir}{sep}src{sep}lib.rs --crate-name testbench --crate-type lib [..]`
{running} `rustc {dir}{sep}src{sep}lib.rs --crate-name testbench --crate-type lib [..]`
{running} `rustc src{sep}lib.rs --crate-name testbench --crate-type lib [..]`
{running} `rustc src{sep}lib.rs --crate-name testbench --crate-type lib [..]`
{running} `rustc benches{sep}testb1.rs --crate-name testb1 --crate-type bin \
[..] --test [..]`
{running} `{dir}{sep}target{sep}release{sep}testb1-[..] --bench`
Expand Down
2 changes: 1 addition & 1 deletion tests/test_cargo_build_lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ fn setup() {
fn verbose_output_for_lib(p: &ProjectBuilder) -> String {
format!("\
{compiling} {name} v{version} ({url})
{running} `rustc {dir}{sep}src{sep}lib.rs --crate-name {name} --crate-type lib -g \
{running} `rustc src{sep}lib.rs --crate-name {name} --crate-type lib -g \
-C metadata=[..] \
-C extra-filename=-[..] \
--out-dir {dir}{sep}target \
Expand Down
10 changes: 5 additions & 5 deletions tests/test_cargo_compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -765,7 +765,7 @@ test!(lto_build {
assert_that(p.cargo_process("build").arg("-v").arg("--release"),
execs().with_status(0).with_stdout(format!("\
{compiling} test v0.0.0 ({url})
{running} `rustc {dir}{sep}src{sep}main.rs --crate-name test --crate-type bin \
{running} `rustc src{sep}main.rs --crate-name test --crate-type bin \
-C opt-level=3 \
-C lto \
--cfg ndebug \
Expand Down Expand Up @@ -794,7 +794,7 @@ test!(verbose_build {
assert_that(p.cargo_process("build").arg("-v"),
execs().with_status(0).with_stdout(format!("\
{compiling} test v0.0.0 ({url})
{running} `rustc {dir}{sep}src{sep}lib.rs --crate-name test --crate-type lib -g \
{running} `rustc src{sep}lib.rs --crate-name test --crate-type lib -g \
-C metadata=[..] \
-C extra-filename=-[..] \
--out-dir {dir}{sep}target \
Expand Down Expand Up @@ -822,7 +822,7 @@ test!(verbose_release_build {
assert_that(p.cargo_process("build").arg("-v").arg("--release"),
execs().with_status(0).with_stdout(format!("\
{compiling} test v0.0.0 ({url})
{running} `rustc {dir}{sep}src{sep}lib.rs --crate-name test --crate-type lib \
{running} `rustc src{sep}lib.rs --crate-name test --crate-type lib \
-C opt-level=3 \
--cfg ndebug \
-C metadata=[..] \
Expand Down Expand Up @@ -867,7 +867,7 @@ test!(verbose_release_build_deps {
assert_that(p.cargo_process("build").arg("-v").arg("--release"),
execs().with_status(0).with_stdout(format!("\
{compiling} foo v0.0.0 ({url})
{running} `rustc {dir}{sep}foo{sep}src{sep}lib.rs --crate-name foo \
{running} `rustc foo{sep}src{sep}lib.rs --crate-name foo \
--crate-type dylib --crate-type rlib -C prefer-dynamic \
-C opt-level=3 \
--cfg ndebug \
Expand All @@ -878,7 +878,7 @@ test!(verbose_release_build_deps {
-L dependency={dir}{sep}target{sep}release{sep}deps \
-L dependency={dir}{sep}target{sep}release{sep}deps`
{compiling} test v0.0.0 ({url})
{running} `rustc {dir}{sep}src{sep}lib.rs --crate-name test --crate-type lib \
{running} `rustc src{sep}lib.rs --crate-name test --crate-type lib \
-C opt-level=3 \
--cfg ndebug \
-C metadata=[..] \
Expand Down
8 changes: 4 additions & 4 deletions tests/test_cargo_compile_custom_build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -364,9 +364,9 @@ test!(links_passes_env_vars {
execs().with_status(0)
.with_stdout(format!("\
{compiling} [..] v0.5.0 (file://[..])
{running} `rustc build.rs [..]`
{running} `rustc [..]build.rs [..]`
{compiling} [..] v0.5.0 (file://[..])
{running} `rustc build.rs [..]`
{running} `rustc [..]build.rs [..]`
{running} `[..]`
{running} `[..]`
{running} `[..]`
Expand Down Expand Up @@ -563,7 +563,7 @@ test!(propagation_of_l_flags {
execs().with_status(0)
.with_stdout(format!("\
{compiling} a v0.5.0 (file://[..])
{running} `rustc build.rs [..]`
{running} `rustc a[..]build.rs [..]`
{compiling} b v0.5.0 (file://[..])
{running} `rustc [..] --crate-name b [..]-L foo[..]`
{running} `[..]a-[..]build-script-build[..]`
Expand Down Expand Up @@ -691,7 +691,7 @@ test!(build_cmd_with_a_build_cmd {
{compiling} b v0.5.0 (file://[..])
{running} `rustc [..] --crate-name b [..]`
{compiling} a v0.5.0 (file://[..])
{running} `rustc build.rs [..] --extern b=[..]`
{running} `rustc a[..]build.rs [..] --extern b=[..]`
{running} `[..]a-[..]build-script-build[..]`
{running} `rustc [..]lib.rs --crate-name a --crate-type lib -g \
-C metadata=[..] -C extra-filename=-[..] \
Expand Down
12 changes: 6 additions & 6 deletions tests/test_cargo_cross_compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -495,7 +495,7 @@ test!(cross_with_a_build_script {
{compiling} foo v0.0.0 (file://[..])
{running} `rustc build.rs [..] --out-dir {dir}{sep}target{sep}build{sep}foo-[..]`
{running} `{dir}{sep}target{sep}build{sep}foo-[..]build-script-build`
{running} `rustc {dir}{sep}src{sep}main.rs [..] --target {target} [..]`
{running} `rustc src{sep}main.rs [..] --target {target} [..]`
", compiling = COMPILING, running = RUNNING, target = target,
dir = p.root().display(), sep = path::SEP).as_slice()));
});
Expand Down Expand Up @@ -562,21 +562,21 @@ test!(build_script_needed_for_host_and_target {
execs().with_status(0)
.with_stdout(format!("\
{compiling} d1 v0.0.0 (file://{dir})
{running} `rustc build.rs [..] --out-dir {dir}{sep}target{sep}build{sep}d1-[..]`
{running} `rustc d1{sep}build.rs [..] --out-dir {dir}{sep}target{sep}build{sep}d1-[..]`
{running} `{dir}{sep}target{sep}build{sep}d1-[..]build-script-build`
{running} `{dir}{sep}target{sep}build{sep}d1-[..]build-script-build`
{running} `rustc {dir}{sep}d1{sep}src{sep}lib.rs [..] --target {target} [..] \
{running} `rustc d1{sep}src{sep}lib.rs [..] --target {target} [..] \
-L /path/to/{target}`
{running} `rustc {dir}{sep}d1{sep}src{sep}lib.rs [..] \
{running} `rustc d1{sep}src{sep}lib.rs [..] \
-L /path/to/{host}`
{compiling} d2 v0.0.0 (file://{dir})
{running} `rustc {dir}{sep}d2{sep}src{sep}lib.rs [..] \
{running} `rustc d2{sep}src{sep}lib.rs [..] \
-L /path/to/{host}`
{compiling} foo v0.0.0 (file://{dir})
{running} `rustc build.rs [..] --out-dir {dir}{sep}target{sep}build{sep}foo-[..] \
-L /path/to/{host}`
{running} `{dir}{sep}target{sep}build{sep}foo-[..]build-script-build`
{running} `rustc {dir}{sep}src{sep}main.rs [..] --target {target} [..] \
{running} `rustc src{sep}main.rs [..] --target {target} [..] \
-L /path/to/{target}`
", compiling = COMPILING, running = RUNNING, target = target, host = host,
dir = p.root().display(), sep = path::SEP).as_slice()));
Expand Down
2 changes: 1 addition & 1 deletion tests/test_cargo_doc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ test!(doc_only_bin {
pub fn bar() {}
"#);

assert_that(p.cargo_process("doc"),
assert_that(p.cargo_process("doc").arg("-v"),
execs().with_status(0));

assert_that(&p.root().join("target/doc"), existing_dir());
Expand Down
6 changes: 3 additions & 3 deletions tests/test_cargo_profiles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ test!(profile_overrides {
assert_that(p.cargo_process("build").arg("-v"),
execs().with_status(0).with_stdout(format!("\
{compiling} test v0.0.0 ({url})
{running} `rustc {dir}{sep}src{sep}lib.rs --crate-name test --crate-type lib \
{running} `rustc src{sep}lib.rs --crate-name test --crate-type lib \
-C opt-level=1 \
--cfg ndebug \
-C metadata=[..] \
Expand Down Expand Up @@ -81,7 +81,7 @@ test!(top_level_overrides_deps {
assert_that(p.cargo_process("build").arg("-v").arg("--release"),
execs().with_status(0).with_stdout(format!("\
{compiling} foo v0.0.0 ({url})
{running} `rustc {dir}{sep}foo{sep}src{sep}lib.rs --crate-name foo \
{running} `rustc foo{sep}src{sep}lib.rs --crate-name foo \
--crate-type dylib --crate-type rlib -C prefer-dynamic \
-C opt-level=1 \
-g \
Expand All @@ -92,7 +92,7 @@ test!(top_level_overrides_deps {
-L dependency={dir}{sep}target{sep}release{sep}deps \
-L dependency={dir}{sep}target{sep}release{sep}deps`
{compiling} test v0.0.0 ({url})
{running} `rustc {dir}{sep}src{sep}lib.rs --crate-name test --crate-type lib \
{running} `rustc src{sep}lib.rs --crate-name test --crate-type lib \
-C opt-level=1 \
-g \
-C metadata=[..] \
Expand Down
8 changes: 4 additions & 4 deletions tests/test_cargo_run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ test!(example_with_release_flag {
assert_that(p.cargo_process("run").arg("-v").arg("--release").arg("--example").arg("a"),
execs().with_status(0).with_stdout(format!("\
{compiling} bar v0.0.1 ({url})
{running} `rustc src{sep}bar.rs --crate-name bar --crate-type lib \
{running} `rustc bar{sep}src{sep}bar.rs --crate-name bar --crate-type lib \
-C opt-level=3 \
--cfg ndebug \
-C metadata=[..] \
Expand All @@ -276,7 +276,7 @@ test!(example_with_release_flag {
-L dependency={dir}{sep}target{sep}release{sep}deps \
-L dependency={dir}{sep}target{sep}release{sep}deps`
{compiling} foo v0.0.1 ({url})
{running} `rustc {dir}{sep}examples{sep}a.rs --crate-name a --crate-type bin \
{running} `rustc examples{sep}a.rs --crate-name a --crate-type bin \
-C opt-level=3 \
--cfg ndebug \
--out-dir {dir}{sep}target{sep}release{sep}examples \
Expand All @@ -297,7 +297,7 @@ fast2
assert_that(p.process(cargo_dir().join("cargo")).arg("run").arg("-v").arg("--example").arg("a"),
execs().with_status(0).with_stdout(format!("\
{compiling} bar v0.0.1 ({url})
{running} `rustc src{sep}bar.rs --crate-name bar --crate-type lib \
{running} `rustc bar{sep}src{sep}bar.rs --crate-name bar --crate-type lib \
-g \
-C metadata=[..] \
-C extra-filename=[..] \
Expand All @@ -306,7 +306,7 @@ fast2
-L dependency={dir}{sep}target{sep}deps \
-L dependency={dir}{sep}target{sep}deps`
{compiling} foo v0.0.1 ({url})
{running} `rustc {dir}{sep}examples{sep}a.rs --crate-name a --crate-type bin \
{running} `rustc examples{sep}a.rs --crate-name a --crate-type bin \
-g \
--out-dir {dir}{sep}target{sep}examples \
--emit=dep-info,link \
Expand Down

0 comments on commit 25fa147

Please sign in to comment.