Skip to content

Ensure rustc-echo-wrapper works with an overridden build.target-dir#10962

Merged
bors merged 1 commit intorust-lang:masterfrom
Nemo157:override-target-dir
Aug 10, 2022
Merged

Ensure rustc-echo-wrapper works with an overridden build.target-dir#10962
bors merged 1 commit intorust-lang:masterfrom
Nemo157:override-target-dir

Conversation

@Nemo157
Copy link
Contributor

@Nemo157 Nemo157 commented Aug 9, 2022

Without this when running with an overridden target-dir there are about a dozen test failures like

> mkdir .cargo
> echo '[build]\ntarget-dir = "not-target"' > .cargo/config
> cargo test -- fix::does_not_crash
---- fix::does_not_crash_with_rustc_wrapper stdout ----
running `/home/nemo157/sources/cargo/not-target/debug/cargo build`
running `/home/nemo157/sources/cargo/not-target/debug/cargo fix --allow-no-vcs`
thread 'fix::does_not_crash_with_rustc_wrapper' panicked at '
test failed running `/home/nemo157/sources/cargo/not-target/debug/cargo fix --allow-no-vcs`
error: process exited with code 101 (expected 0)
--- stdout

--- stderr
error: failed to run `rustc` to learn about target-specific information

Caused by:
  could not execute process `/home/nemo157/sources/cargo/not-target/tmp/cit/rustc-echo-wrapper/target/debug/rustc-echo-wrapper rustc - --crate-name ___ --print=file-names --crate-type bin --crate-type rlib --crate-type dylib --crate-type cdylib --crate-type staticlib --crate-type proc-macro --print=sysroot --print=cfg` (never executed)

Caused by:
  No such file or directory (os error 2)
', tests/testsuite/fix.rs:1228:10
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Because the rustc-echo-wrapper is compiled to not-target/debug/rustc-echo-wrapper.

This is resolved by forcing the target-dir to be a manifest-relative one for this specific build.

@rust-highfive
Copy link

r? @ehuss

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 9, 2022
@ehuss
Copy link
Contributor

ehuss commented Aug 10, 2022

Thanks! I think the issue here is that the sandbox limits aren't getting set correctly. I think something like this would be a more correct fix:

diff --git a/crates/cargo-test-support/src/lib.rs b/crates/cargo-test-support/src/lib.rs
index 1ed9bd54e..ed9e50f33 100644
--- a/crates/cargo-test-support/src/lib.rs
+++ b/crates/cargo-test-support/src/lib.rs
@@ -1209,7 +1209,7 @@ pub trait TestEnv: Sized {
             .current_dir(&paths::root())
             .env("HOME", paths::home())
             .env("CARGO_HOME", paths::home().join(".cargo"))
-            .env("__CARGO_TEST_ROOT", paths::root())
+            .env("__CARGO_TEST_ROOT", paths::global_root())
             // Force Cargo to think it's on the stable channel for all tests, this
             // should hopefully not surprise us as we add cargo features over time and
             // cargo rides the trains.

That will ensure that things like echo-wrapper don't read any configuration settings from above the cit directory.

@Nemo157 Nemo157 force-pushed the override-target-dir branch from b7a9ce8 to 61c4b11 Compare August 10, 2022 15:38
@Nemo157
Copy link
Contributor Author

Nemo157 commented Aug 10, 2022

Yep, that seems to work and makes a lot more sense.

@ehuss
Copy link
Contributor

ehuss commented Aug 10, 2022

Thanks!

@bors r+

@bors
Copy link
Contributor

bors commented Aug 10, 2022

📌 Commit 61c4b11 has been approved by ehuss

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 10, 2022
@bors
Copy link
Contributor

bors commented Aug 10, 2022

⌛ Testing commit 61c4b11 with merge a120cfe...

@bors
Copy link
Contributor

bors commented Aug 10, 2022

☀️ Test successful - checks-actions
Approved by: ehuss
Pushing a120cfe to master...

@bors bors merged commit a120cfe into rust-lang:master Aug 10, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 12, 2022
Update cargo

8 commits in ce40690a5e4e315d3dab0aae1eae69d0252c52ac..efd4ca3dc0b89929dc8c5f5c023d25978d76cb61
2022-08-09 22:32:17 +0000 to 2022-08-12 01:28:28 +0000
- Use `std::thread::scope` to replace crossbeam (rust-lang/cargo#10977)
- [docs] Remove extra "in" from `cargo-test.md` (rust-lang/cargo#10978)
- Enable two windows tests (rust-lang/cargo#10930)
- Improve error msg for get target runner (rust-lang/cargo#10968)
- Ensure rustc-echo-wrapper works with an overridden build.target-dir (rust-lang/cargo#10962)
- Switch back to `available_parallelism` (rust-lang/cargo#10969)
- Only override published resolver when the workspace is different (rust-lang/cargo#10961)
- Add `CARGO_LOG` to "Environment variables Cargo reads" (rust-lang/cargo#10967)
@ehuss ehuss added this to the 1.65.0 milestone Aug 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants