cargo-miri swaps the values of enviroment variable set by build script #1661
Comments
ghost
mentioned this issue
ghost
changed the title
|
Ugh... wtf.^^ Maybe the way we are hooking into cargo wasn't the best choice after all. It's not clear what else we should do though. Does this need |
RalfJung
added
A-cargo
I really want cargo to get native support for jit execution, aka cargo would pass a flag that causes rustc (or in this case miri) to directly execute the program instead of expecting it to produce an executable. That would also be useful for cg_clif (https://github.com/bjorn3/rustc_codegen_cranelift/issues/1066). |
Yes. This does not reproduce at least on my side if I remove |
Yeah that would be pretty nice. |
|
Is
But the build script is only ever executed the same way here, is it? Where do you see two build script invocations? |
|
Don't these crates have cyclic dependencies? cargo-miri-bugs has and a-cool-macro has Are you sure that such cyclic dependencies are supported by cargo? Cc @ehuss |
|
Looks like the build script is run twice in parallel: Miri doesn't do anything funny with the env vars that exist during compilation either... so I cannot see how this could be a cargo-miri problem, TBH. It might be a race condition in cargo that only surfaces with cargo-miri? |
It's not standard. It's set in the build script: fn main() {
println!("cargo:rustc-env=CRATE_TARGET={}", std::env::var("TARGET").unwrap())
}
I agree. This seems a Cargo bug, not a cargo-miri bug. cargo-miri just exposes it. Sorry for misinterpreting.
I should explain that clearer before. I was cross compiling for
Normal
IIRC I tried with |
|
Hm, yeah, I can also still reproduce this with |
|
If I comment out this line Line 662 in 6fdda8a the error seems to go away. What that changes is that when an env var is set both at build time and at run time, it will prefer the build time value. So looks like eprintln!("[cargo-miri runner] CRATE_TARGET = {:?}", env::var("CRATE_TARGET"));before that loop, and indeed I can see when the test passes and when it fails. So, cargo is calling the runner with different environment variables. There might still also be a cargo-miri issue here, but that definitely seems fishy -- Cc @ehuss. |
|
I tried to understand why the build script env vars are passed to the runner in the first place. Looks like this happens via the Looks like the way this is done is prone to ordering issues, and it's not always the "right" build script invocation whose values get forwarded to the runner. |
|
We now fixed this on the Miri side, but @ehuss I'd still be interested in learning about the intended behavior of cargo wrt. whether |
|
Env vars are not set when the program is run with They are set when building with It looks like as you mentioned above something is fishy with the build script running twice. I'm not familiar with how |
|
@ehuss "normal cargo invocation": cargo new --lib cargo-bug --vcs none
cd cargo-bug/
echo 'proc-macro-crate = { path = "proc-macro-crate" }
[lib]
doctest = false
[workspace]' >> Cargo.toml
cargo new --lib proc-macro-crate --vcs none
cd proc-macro-crate/
echo '[lib]
proc-macro = true
[dev-dependencies]
cargo-bug = { path = ".." }' >> Cargo.toml
cd ..
echo 'fn main() { println!("cargo:rustc-env=CRATE_TARGET={}", std::env::var("TARGET").unwrap()) }' > build.rs
echo '#[test]
fn target() {
assert_eq!(env!("CRATE_TARGET"), "i686-unknown-linux-musl");
assert_eq!(std::env::var_os("CRATE_TARGET"), None);
}' > src/lib.rs
cargo test --workspace --target i686-unknown-linux-musl -vvThe test sometimes fails with And sometimes fails with |
They definitely are though -- when I dump the environment in the @hyd-dev ah, so you have a cargo-only reproducer? Might be worth opening a cargo issue for this. |
Not so "cargo-only". I added assert_eq!(std::env::var_os("CRATE_TARGET"), None);to the test, to check the environment variable when the built program runs.
I'm going to open one. |
Yeah but there's no |
ghost
mentioned this issue
This requires complex dependency tree to reproduce and happens with today's nightly:
(Reminder: the top
set -ewill make your shell exit if any command fails, which is used to make sure errors from commands likecargo cleanare not missed, so this can't be a cache issue or caused by other spurious reasons. Remember toset +eif you want to continue to use that shell like normal.)This doesn't reproduce it reliably, so there's a
whileloop at the end.The assertion sometimes fails (I'm on
x86_64-unknown-linux-gnu):Complete output
Complete output with
CARGO_LOG=traceIt seems that cargo-miri swaped the values of
CRATE_TARGETset by two build script invocations (according to the "Complete output").I also tried
CARGO_BUILD_JOBS=1, the issue didn't go away, so this is not caused by a race condition.This can't be reproduced if both
testanddoctestare disabled fora-cool-macro(but can be reproduced if only one of them is disabled).I think this is a cargo-miri bug, becauseif I replace the target withwasm32-wasi, replacecargo miri testwithcargo test, settarget.wasm32-wasi.runnerproperly and setRUSTC_WRAPPERto asleepscript, the assertion never fails (at least I'm not able to reproduce, this can't be reproduced reliably).However,it'salsopossible that the core issue is in Cargo, cargo-miri just exposed it.The text was updated successfully, but these errors were encountered: