Skip to content

Commit

Permalink
Auto merge of #8819 - EmbarkStudios:target-root-path, r=ehuss
Browse files Browse the repository at this point in the history
Make host_root return host.root(), not host.dest()

Also create host_dest function to let other callsites retain their old functionality. Fixes #8817, verified it works on the original problem reported in the `rust-gpu` repo.

I did two things here:

1) Rename `host_root` (which returns `self.host.dest()`) to be `host_dest`. This has three callsites. I did this to make it more clear that it returns dest, not root.
2) For the callsite that's relevant in #8817, I created a "new" `host_root` function (that returns `self.host.root()`). This means that the callsite that this PR is actually fixing doesn't show up in this diff :/ - but I thought it was more clear this way.

(Also copied the example path docs over from `layout.rs` to hopefully avoid this mistake again in the future)

I tried to look into if the other two callsites should actually be calling `host.root()` instead of `dest`, because I imagine the same mistake could have been made again, but it quickly grew out of my understanding (this is my first time in the cargo codebase). Feel free to let me know if they should also call `host.root()` too, and I can update them.

Thanks! (oh gosh I have no idea what I'm doing, I hope this is right)

r? `@alexcrichton`
  • Loading branch information
bors committed Oct 29, 2020
2 parents becb4c2 + 628f701 commit 7197c66
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 4 deletions.
9 changes: 7 additions & 2 deletions src/cargo/core/compiler/context/compilation_files.rs
Original file line number Diff line number Diff line change
Expand Up @@ -207,11 +207,16 @@ impl<'a, 'cfg: 'a> CompilationFiles<'a, 'cfg> {
}
}

/// Returns the root of the build output tree for the host
pub fn host_root(&self) -> &Path {
/// Returns the final artifact path for the host (`/…/target/debug`)
pub fn host_dest(&self) -> &Path {
self.host.dest()
}

/// Returns the root of the build output tree for the host (`/…/target`)
pub fn host_root(&self) -> &Path {
self.host.root()
}

/// Returns the host `deps` directory path.
pub fn host_deps(&self) -> &Path {
self.host.deps()
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/core/compiler/custom_build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ fn build_work(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<Job> {
let output_file = script_run_dir.join("output");
let err_file = script_run_dir.join("stderr");
let root_output_file = script_run_dir.join("root-output");
let host_target_root = cx.files().host_root().to_path_buf();
let host_target_root = cx.files().host_dest().to_path_buf();
let all = (
id,
pkg_name.clone(),
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/core/compiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ fn rustc(cx: &mut Context<'_, '_>, unit: &Unit, exec: &Arc<dyn Executor>) -> Car
exec.init(cx, unit);
let exec = exec.clone();

let root_output = cx.files().host_root().to_path_buf();
let root_output = cx.files().host_dest().to_path_buf();
let target_dir = cx.bcx.ws.target_dir().into_path_unlocked();
let pkg_root = unit.pkg.root().to_path_buf();
let cwd = rustc
Expand Down
36 changes: 36 additions & 0 deletions tests/testsuite/dep_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,42 @@ fn build_dep_info_dylib() {
assert!(p.example_lib("ex", "dylib").with_extension("d").is_file());
}

#[cargo_test]
fn dep_path_inside_target_has_correct_path() {
let p = project()
.file("Cargo.toml", &basic_bin_manifest("a"))
.file("target/debug/blah", "")
.file(
"src/main.rs",
r#"
fn main() {
let x = include_bytes!(concat!(env!("CARGO_MANIFEST_DIR"), "/target/debug/blah"));
}
"#,
)
.build();

p.cargo("build").run();

let depinfo_path = &p.bin("a").with_extension("d");

assert!(depinfo_path.is_file(), "{:?}", depinfo_path);

let depinfo = p.read_file(depinfo_path.to_str().unwrap());

let bin_path = p.bin("a");
let target_debug_blah = Path::new("target").join("debug").join("blah");
if !depinfo.lines().any(|line| {
line.starts_with(&format!("{}:", bin_path.display()))
&& line.contains(target_debug_blah.to_str().unwrap())
}) {
panic!(
"Could not find {:?}: {:?} in {:?}",
bin_path, target_debug_blah, depinfo_path
);
}
}

#[cargo_test]
fn no_rewrite_if_no_change() {
let p = project().file("src/lib.rs", "").build();
Expand Down

0 comments on commit 7197c66

Please sign in to comment.