unset RUSTC_WRAPPER for xargo invocation #1426
Conversation
| // Make sure there are no other wrappers or flags getting in our way | ||
| // (Cc https://github.com/rust-lang/miri/issues/1421). | ||
| command.env_remove("RUSTC_WRAPPER"); | ||
| command.env_remove("RUSTFLAGS"); |
There was a problem hiding this comment.
Isn't it too invasive? Many flags wouldn't interfere with miri and could be required in certain circumstances.
There was a problem hiding this comment.
This is preexisting. Also note that this just affects the libstd build, not the build of the local crate. When you do RUSTFLAGS=foo cargo build, your libstd also is not affected by the flags you set. So I would say this is consistent with non-Miri operation.
| @@ -347,6 +346,10 @@ path = "lib.rs" | |||
| command.env("RUSTC", find_miri()); | |||
| } | |||
| command.env("MIRI_BE_RUSTC", "1"); | |||
| // Make sure there are no other wrappers or flags getting in our way | |||
| // (Cc https://github.com/rust-lang/miri/issues/1421). | |||
| command.env_remove("RUSTC_WRAPPER"); | |||
There was a problem hiding this comment.
Maybe it shouldn't be done in invisible to a user, manner? I.e. throw an error if there are certain variables that conflict with miri?
There was a problem hiding this comment.
Hm, good point. Throwing an error might be too extreme though. What about printing a warning, similar to what we do on MIRI_SYSROOT=something cargo miri setup?
There was a problem hiding this comment.
well, before I faced some errors in miri setup with the caching I didn't even know it removes certain variables. It would be at least nice to add the warnings that "x and y are being overriden".
There was a problem hiding this comment.
Yes we have such a warning for RUSTC_WRAPPER now.
There was a problem hiding this comment.
Yeah, I've noticed it right after this comment. Thanks a bunch for this!
Yes, in case of rustc both |
|
All right, I added a warning for when @bors r+ |
|
📌 Commit cf7d88f has been approved by |
|
☀️ Test successful - checks-travis, status-appveyor |
Fixes #1421
@bjorn3 @oli-obk do you think that is a reasonable way to solve this issue?