From fc5840d17534559b5155a70667b70a7b1431cf1e Mon Sep 17 00:00:00 2001 From: Jade Date: Tue, 27 Apr 2021 04:25:46 -0700 Subject: [PATCH] Fix dep-info files emitting paths relative to deps' roots Sample `shoo.d` file prior to this change is below, note the `build.rs` at the end, which was not from my package. From booping the debugger, I found this was coming from `compiler_builtins`. This is not really their bug though: if a build.rs asks for rerun-if-changed on some crate relative path, this will happen in general. So I've fixed it in Cargo and added a test to prevent it regressing. ``` target/riscv64imac-mu-shoo-elf/release/shoo: /home/jade/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/../../stdarch/crates/core_arch/src/core_arch_docs.md /home/jade/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/../../stdarch/crates/core_arch/src/macros.rs /home/jade/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/../../stdarch/crates/core_arch/src/mod.rs /home/jade/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/../../stdarch/crates/core_arch/src/simd.rs /home/jade/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/../../stdarch/crates/core_arch/src/simd_llvm.rs crates/build_bits/src/lib.rs shoo/src/main.rs shoo/src/task.rs shoo/src/vectors.s build.rs ``` This change fixes it so it's like: ``` target/riscv64imac-mu-shoo-elf/release/shoo: /home/jade/.cargo/registry/src/github.com-1ecc6299db9ec823/compiler_builtins-0.1.39/build.rs /home/jade/.cargo/registry/src/github.com-1ecc6299db9ec823/log-0.4.14/build.rs /home/jade/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/../../stdarch/crates/core_arch/src/core_arch_docs.md /home/jade/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/../../stdarch/crates/core_arch/src/macros.rs /home/jade/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/../../stdarch/crates/core_arch/src/mod.rs /home/jade/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/../../stdarch/crates/core_arch/src/simd.rs /home/jade/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/../../stdarch/crates/core_arch/src/simd_llvm.rs crates/build_bits/src/lib.rs shoo/src/main.rs shoo/src/task.rs shoo/src/vectors.s ``` --- src/cargo/core/compiler/output_depinfo.rs | 3 + tests/testsuite/build_script.rs | 95 ++++++++++++++++++++++- 2 files changed, 97 insertions(+), 1 deletion(-) diff --git a/src/cargo/core/compiler/output_depinfo.rs b/src/cargo/core/compiler/output_depinfo.rs index 8804c7ba1..b55dd8fa5 100644 --- a/src/cargo/core/compiler/output_depinfo.rs +++ b/src/cargo/core/compiler/output_depinfo.rs @@ -81,6 +81,9 @@ fn add_deps_for_unit( if let Some(metadata) = cx.find_build_script_metadata(unit) { if let Some(output) = cx.build_script_outputs.lock().unwrap().get(metadata) { for path in &output.rerun_if_changed { + // The paths we have saved from the unit are of arbitrary relativeness and may be + // relative to the crate root of the dependency. + let path = unit.pkg.root().join(path); deps.insert(path.into()); } } diff --git a/tests/testsuite/build_script.rs b/tests/testsuite/build_script.rs index a43856548..29082d961 100644 --- a/tests/testsuite/build_script.rs +++ b/tests/testsuite/build_script.rs @@ -1,8 +1,9 @@ //! Tests for build.rs scripts. -use cargo_test_support::paths::CargoPathExt; +use cargo_test_support::project_in_home; use cargo_test_support::registry::Package; use cargo_test_support::{basic_manifest, cross_compile, is_coarse_mtime, project}; +use cargo_test_support::{lines_match, paths::CargoPathExt}; use cargo_test_support::{rustc_host, sleep_ms, slow_cpu_multiplier, symlink_supported}; use cargo_util::paths::remove_dir_all; use std::env; @@ -2602,6 +2603,98 @@ fn fresh_builds_possible_with_multiple_metadata_overrides() { .run(); } +#[cargo_test] +fn generate_good_d_files() { + // this is here to stop regression on an issue where build.rs rerun-if-changed paths aren't + // made absolute properly, which in turn interacts poorly with the dep-info-basedir setting, + // and the dep-info files have other-crate-relative paths spat out in them + let dep = project_in_home("awoo") + .file( + "Cargo.toml", + r#" + [project] + name = "awoo" + version = "0.5.0" + build = "build.rs" + "#, + ) + .file("src/lib.rs", "") + .file( + "build.rs", + r#" + fn main() { + println!("cargo:rerun-if-changed=build.rs"); + println!("cargo:rerun-if-changed=barkbarkbark"); + } + "#, + ) + .build(); + + let p = project_in_home("meow") + .file( + "Cargo.toml", + &format!( + r#" + [project] + name = "meow" + version = "0.5.0" + [dependencies] + awoo = {{ path = "{path}" }} + "#, + path = dep.root().to_str().unwrap(), + ), + ) + .file("src/main.rs", "fn main() {}") + .build(); + + p.cargo("build -v").run(); + + let dot_d_path = p.bin("meow").with_extension("d"); + println!("*meow at* {:?}", dot_d_path); + let dot_d = fs::read_to_string(&dot_d_path).unwrap(); + + println!("*.d file content*: {}", &dot_d); + + assert!( + lines_match( + "[..]/target/debug/meow: [..]/awoo/barkbarkbark [..]/awoo/build.rs[..]", + &dot_d + ) || lines_match( + "[..]/target/debug/meow: [..]/awoo/build.rs [..]/awoo/barkbarkbark[..]", + &dot_d + ) + ); + + // paths relative to dependency roots should not be allowed + assert!(!dot_d.split_whitespace().any(|v| v == "build.rs")); + + p.change_file( + ".cargo/config.toml", + r#" + [build] + dep-info-basedir="." + "#, + ); + p.cargo("build -v").run(); + + let dot_d = fs::read_to_string(&dot_d_path).unwrap(); + + println!("*.d file content with dep-info-basedir*: {}", &dot_d); + + assert!( + lines_match( + "target/debug/meow: [..]/awoo/barkbarkbark [..]/awoo/build.rs[..]", + &dot_d + ) || lines_match( + "target/debug/meow: [..]/awoo/build.rs [..]/awoo/barkbarkbark[..]", + &dot_d + ) + ); + + // paths relative to dependency roots should not be allowed + assert!(!dot_d.split_whitespace().any(|v| v == "build.rs")); +} + #[cargo_test] fn rebuild_only_on_explicit_paths() { let p = project()