rust-lang / miri

An interpreter for Rust's mid-level intermediate representation
Apache License 2.0
4.16k stars 318 forks source link

Cannot add new targets after installation of toolchain #3584

Closed RossSmyth closed 1 month ago

RossSmyth commented 1 month ago

Basically the annoyance is:

  1. Run .\miri toolchain
  2. Later, need more targets for the toolchain (Linux, OSX, ...)
  3. Run .\miri toolchain -t x86_64-unknown-linux-gnu aarch64-apple-darwin
  4. Instantly returns without doing anything

I believe this is a combination of factors. One being this portion of miri-script ignoring components & targets:

https://github.com/rust-lang/miri/blob/cf2df2d7d84c8ac06d3ab8200f290e9ec8128951/miri-script/src/commands.rs#L184-L203

Which doesn't pass the command to rustup-toolchain-install-master at all, the also the fact that rustup-toolchain-install-master doesn't add new targets to installed toolchains anyways.

The solution is to delete the toolchain with rustup then rerun .\miri toolchain -t whatever right now.

kennytm/rustup-toolchain-install-master#19 semi-related

Since the upstream tool seems to be mostly inactive I'm not going to work on this unless there is any indication of activity upstream.

RalfJung commented 1 month ago

I agree that you can't add more toolchains to the existing ./miri toolchain, but as you note that's a bug in RTIM so I don't see why it would be tracked in Miri.

However, I don't think it is ever necessary to install other toolchains. Miri runs on arbitrary foreign targets without installing their toolchain. What is your reason for installing extra toolchains?

RossSmyth commented 1 month ago

Ok, I figured out what I was doing wrong. I was reading this

https://github.com/rust-lang/miri/blob/cf2df2d7d84c8ac06d3ab8200f290e9ec8128951/CONTRIBUTING.md?plain=1#L64

And instead running miri test --target

The output ```powershell > .\miri toolchain ... blah blah blah downloading everything ... > .\miri test --target i686-unknown-linux-gnu ... blah blah blah compile everything ... error[E0463]: can't find crate for `core` | = note: the `i686-unknown-linux-gnu` target may not be installed = help: consider downloading the target with `rustup target add i686-unknown-linux-gnu` = help: consider building the standard library from source with `cargo build -Zbuild-std` For more information about this error, try `rustc --explain E0463`. error: could not compile `cfg-if` (lib) due to 1 previous error warning: build failed, waiting for other jobs to finish... error[E0463]: can't find crate for `std` | = note: the `i686-unknown-linux-gnu` target may not be installed = help: consider downloading the target with `rustup target add i686-unknown-linux-gnu` = help: consider building the standard library from source with `cargo build -Zbuild-std` error: could not compile `once_cell` (lib) due to 1 previous error error: could not compile `lazy_static` (lib) due to 1 previous error error: could not compile `bitflags` (lib) due to 1 previous error error: could not compile `memchr` (lib) due to 1 previous error Error: command exited with non-zero code `cargo +miri test --manifest-path C:\Users\Ross\Documents\miri\Cargo.toml --target i686-unknown-linux-gnu > rustup toolchain uninstall miri > .\miri toolchain -t i686-unknown-linux-gnu ... blah blah blah downloading everything ... > .\miri test --target i686-unknown-linux-gnu ... blah blah blah compiling everything ... ```

Instead of doing $Env:MIRI_TEST_TARGET="i686-unknown-linux-gnu" then .\miri test

Is there a reason why miri can't intercept the --target flag passed to test? I guess technically someone could be cross compiling miri tests I guess but that is a very niche case. Mainly because env var's on Windows cannot be scoped to a single command like on Linux so I try to avoid them.

Also for working on miri I had the same issue before but that was for the cross-clippy work that didn't pan out.

saethlin commented 1 month ago

Mainly because env var's on Windows cannot be scoped to a single command like on Linux so I try to avoid them.

Ouch. This is good to keep in mind. I think in a lot of cases we slip into using environment variables because adding a CLI flag requires a lot more code. For an env var, you just look it up where you want to use it (yay global state), instead of adding parsing logic to the CLI then plumbing the value down to where you need it.

RalfJung commented 1 month ago

Yeah, Windows users unfortunately have to deal with very sub-par shells. :/ Env vars are a standard part of controlling program behavior on Unix platforms, and as I'm not paid by Microsoft I am not very inclined to spend a lot of extra work to work around issues they created for themselves and their users. I already spent quite a lot of time adding support for Windows shims -- it's less than on Linux, but getting everything in std::env and std::path to work was still a bunch of work (I didn't do all of that but a significant chunk). At some point I think it's reasonable to say that contributions are welcome but there's only so far I will go myself.

https://github.com/rust-lang/miri/issues/2051 is probably our worst offender in that direction, since it affects all Miri users on Windows, not "just" devs.

Is there a reason why miri can't intercept the --target flag passed to test?

No specific reason, it just was easier to use the env var that we have to use on CI anyway. PRs welcome. :)

Everything after ./miri test (except for a leading --bless) is just passed to cargo. We could also do special processing for leading --target. Then ./miri test --target foo filter would work, but ./miri test filter --target foo would still pass --target to cargo. I guess we could warn about that or so -- I don't think I'd want to add further logic that parses all flags, that quickly gets out of hand and makes it a lot harder to figure out what a command even does.

Or... we could entirely remove the "forward everything to cargo". Just parse --target, and then pass the rest to the test harness (i.e., after a -- in the cargo test invocation). That seems like the best option honestly. I don't know if anyone ever uses any of these flags anyway, all I do is set the filter -- and be annoyed when I have to use -- to set more than one filter.)