Skip to content

Commit

Permalink
Auto merge of rust-lang#125426 - jieyouxu:rmake-support-env-reset, r=…
Browse files Browse the repository at this point in the history
…<try>

Update `compiler-builtins` test to not clear essential env vars

Noticed in rust-lang#122580 (comment), the `compiler-builtins` test failed on Windows for a `cargo` invocation because necessary env vars `TMP` and `TEMP` were cleared by `Command::env_clear`, causing temp dir eventually used by codegen to fallback to the Windows directory, which will trigger permission errors.

This PR adds a `clear_non_essential_env_vars` helper, which is a more conservative `Command::env_clear` that does not clear `TMP` or `TEMP` on Windows, and does not clear `TMPDIR` on non-Windows platforms.

cc `@ChrisDenton` do you happen to know if there are any more "essential" env vars that we should not clear (on Windows or other platforms)?

r? `@saethlin` (feel free to reroll, since you authored the test)

try-job: x86_64-msvc
try-job: test-various
  • Loading branch information
bors committed Jun 3, 2024
2 parents 9f2d0b3 + f31f59e commit 932d86e
Showing 1 changed file with 2 additions and 1 deletion.
3 changes: 2 additions & 1 deletion tests/run-make/compiler-builtins/rmake.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ fn main() {
"--target",
&target,
])
.env_clear()
// FIXME(jieyouxu): so what happens if we don't `env_clear`?
//.env_clear()
.env("PATH", path)
.env("RUSTC", rustc)
.env("RUSTFLAGS", "-Copt-level=0 -Cdebug-assertions=yes")
Expand Down

0 comments on commit 932d86e

Please sign in to comment.