Skip to content

Commit

Permalink
Tweak how source paths are passed to the compiler
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
alexcrichton committed Jan 14, 2015
1 parent 5532111 commit 4a91d01
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 @@ -751,7 +751,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 @@ -780,7 +780,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 @@ -808,7 +808,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 @@ -853,7 +853,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 @@ -864,7 +864,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 4a91d01

Please sign in to comment.