fix ./x.py test miri #2407

oli-obk · 2022-07-21T11:46:13Z

Ok, this one works now. I had to manually remove .cargo/bin from my path to reproduce the rustc failure, and then figure out a few shenanigans around how cargo forwards arguments.

CONTRIBUTING.md

RalfJung · 2022-07-21T12:11:07Z

cargo-miri/bin.rs

@@ -668,7 +668,10 @@ fn phase_cargo_miri(mut args: env::Args) {
            "WARNING: Ignoring `RUSTC` environment variable; set `MIRI` if you want to control the binary used as the driver."
        );
    }
-    cmd.env_remove("RUSTC");
+
+    if env::var_os("RUSTC_STAGE").is_none() {


Please add a comment. Note the comment above:

Having both RUSTC_WRAPPER and RUSTC set does some odd things, so let's avoid that.

RalfJung · 2022-07-21T12:12:33Z

cargo-miri/bin.rs

@@ -1181,6 +1190,13 @@ fn main() {
            let binary = Path::new(arg);
            if binary.exists() {
                assert!(!arg.starts_with("--")); // not a flag
+                if let Some(rustc) = std::env::var_os("RUSTC") {
+                    if rustc == arg {
+                        // In bootstrap we can't rely on rustc just being named rustc


Uh yeah I think this is exactly why we did env_remove("RUSTC") above. I'm a bit worried we might get some weird mixup here...

Without RUSTC cargo would call rustc which doesn't exist on rust's CI. It needs to use the rustc set in RUSTC by rust's build system, not the system rustc.

the problem is that we need to be able to specify a specific rustc with the RUSTC env var, as cargo just invokes the env var or rustc when doing -vV. Unfortunately we can only set it to a binary, not a script or similar.

Without RUSTC cargo would call rustc which doesn't exist on rust's CI. It needs to use the rustc set in RUSTC by rust's build system, not the system rustc.

cargo-miri should never invoke rustc. It should always only invoke miri itself, including when it needs rustc. That's what all that MIRI_BE_RUSTC stuff is for.

We used to invoke both miri and rustc but that leads to strange errors very quickly when those two are not exactly in sync. It's much better to ensure that everything goes through the same binary, miri, which can do both compilation and interpretation.

cargo just invokes the env var or rustc when doing -vV

Oh wait it ignores RUSTC_WRAPPER for that? That sounds like a cargo bug to me.

You might need something like this

miri/cargo-miri/bin.rs

Lines 480 to 486 in 88ad9ca

let cargo_miri_path = std::env::current_exe().expect("current executable path invalid");

if env::var_os("RUSTC_STAGE").is_some() {

command.env("RUSTC_REAL", &cargo_miri_path);

} else {

command.env("RUSTC", &cargo_miri_path);

}

command.env("MIRI_CALLED_FROM_XARGO", "1");

not sure how that helps. we're not invoking the bootstrap rustc thingy that forwards to RUSTC_REAL, if RUSTC is empty, cargo literally invokes rustc -vV

Oh wait it ignores RUSTC_WRAPPER for that? That sounds like a cargo bug to me.

yea, I found no way to influence that binary other than via the RUSTC env var

Yeah IMO that's definitely a cargo bug. Reported as rust-lang/cargo#10885.

I guess you could set RUSTC to the underlying Miri driver? For -vV that'll be fine and thanks to the RUSTC_WRAPPER all the other calls should end up in the right spot. Let's use an absolute canonical path for this so it should not be possible to have confusion in the phase selection logic in main.

cargo-miri/bin.rs

oli-obk · 2022-07-21T12:25:57Z

uuh... what happened to our CI, something seems to be out of disk space

cargo-miri/bin.rs

CONTRIBUTING.md

RalfJung · 2022-07-21T12:45:11Z

uuh... what happened to our CI, something seems to be out of disk space

Seems to work again? At least the linux builder.

RalfJung · 2022-07-21T13:23:02Z

cargo-miri/bin.rs

-        Some("rustc") => phase_rustc(args, RustcPhase::Build),
+        // Work around a cargo bug where it ignores `RUSTC_WRAPPER`, so we get
+        // called ourselves and handle the special case for `rustc -vV`.
+        Some("-vV") => {


When I tried this, cargo miri test in our test-cargo-miri failed because the libc build script invokes RUSTC --version...

huh... it works in the ui test suite, I didn't check test-cargo-miri

Yeah it fails that way on CI now

error: failed to run custom build command for libc v0.2.126

bors · 2022-07-21T15:01:12Z

☔ The latest upstream changes (presumably #2409) made this pull request unmergeable. Please resolve the merge conflicts.

oli-obk added 2 commits July 21, 2022 11:44

Fix rustc bootstrap

959ac75

Avoid rustformatting on autosave

0d289d1

RalfJung reviewed Jul 21, 2022

View reviewed changes

CONTRIBUTING.md Outdated Show resolved Hide resolved

RalfJung reviewed Jul 21, 2022

View reviewed changes

cargo-miri/bin.rs Outdated Show resolved Hide resolved

RalfJung reviewed Jul 21, 2022

View reviewed changes

cargo-miri/bin.rs Outdated Show resolved Hide resolved

oli-obk added 2 commits July 21, 2022 12:23

Stop removing the RUSTC env var and handle it at the recipient site

822f1bb

Use a MIRI namespaced env var name for auto ops

fe7295b

RalfJung reviewed Jul 21, 2022

View reviewed changes

cargo-miri/bin.rs Show resolved Hide resolved

RalfJung reviewed Jul 21, 2022

View reviewed changes

CONTRIBUTING.md Show resolved Hide resolved

oli-obk added 3 commits July 21, 2022 13:16

Generalize the argument processing over any iterator

4adc93f

Reintroduce RUSTC_WRAPPER and RUSTC message

93115bb

Work around a cargo bug by invoking the wrapper directly

c06f6df

RalfJung changed the title ~~Rustup~~ fix ./x.py test miri Jul 21, 2022

RalfJung reviewed Jul 21, 2022

View reviewed changes

oli-obk closed this Jul 21, 2022

RalfJung deleted the rustup branch July 24, 2022 12:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix ./x.py test miri #2407

fix ./x.py test miri #2407

oli-obk commented Jul 21, 2022

RalfJung Jul 21, 2022

RalfJung Jul 21, 2022

bjorn3 Jul 21, 2022

oli-obk Jul 21, 2022

RalfJung Jul 21, 2022 •

edited

Loading

RalfJung Jul 21, 2022 •

edited

Loading

RalfJung Jul 21, 2022

RalfJung Jul 21, 2022

oli-obk Jul 21, 2022

oli-obk Jul 21, 2022

RalfJung Jul 21, 2022 •

edited

Loading

oli-obk commented Jul 21, 2022

RalfJung commented Jul 21, 2022

RalfJung Jul 21, 2022

oli-obk Jul 21, 2022

RalfJung Jul 21, 2022

bors commented Jul 21, 2022

	let cargo_miri_path = std::env::current_exe().expect("current executable path invalid");
	if env::var_os("RUSTC_STAGE").is_some() {
	command.env("RUSTC_REAL", &cargo_miri_path);
	} else {
	command.env("RUSTC", &cargo_miri_path);
	}
	command.env("MIRI_CALLED_FROM_XARGO", "1");

fix ./x.py test miri #2407

fix ./x.py test miri #2407

Conversation

oli-obk commented Jul 21, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RalfJung Jul 21, 2022 • edited Loading

Choose a reason for hiding this comment

RalfJung Jul 21, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RalfJung Jul 21, 2022 • edited Loading

Choose a reason for hiding this comment

oli-obk commented Jul 21, 2022

RalfJung commented Jul 21, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bors commented Jul 21, 2022

RalfJung Jul 21, 2022 •

edited

Loading

RalfJung Jul 21, 2022 •

edited

Loading

RalfJung Jul 21, 2022 •

edited

Loading