Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Aug 10, 2022

Conversation

Nemo157
Copy link
Member

@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
Copy link
Member 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