compiletest: call cargo-miri directly rather than via 'cargo run' #3318
Conversation
| // Set the `cargo-miri` binary, which we expect to be in the same folder as the `miri` binary. | ||
| // (It's a separate crate, so we don't get an env var from cargo.) | ||
| let mut prog = miri_path(); | ||
| prog.set_file_name("cargo-miri"); |
There was a problem hiding this comment.
This needs the .exe extension on Windows, right?
There was a problem hiding this comment.
Does it?
I guess CI will tell...
There was a problem hiding this comment.
".exe" is in the PATHEXT env var so it should just execute (that's how my miri.bat file works). Though since this is executed in bash on msys2 I'm not sure if that applies or not as I do not use that. But CI seems to be doing something.
There was a problem hiding this comment.
I thought PATHEXT only applied to cmd.exe and ShellExecute and not CreateProcess. std::process::Command uses CreateProcessW. The documentation of CreateProcessW doesn't mention whether or not it does apply
There was a problem hiding this comment.
Windows CI seems to be working so it does seem to add the .exe. 🤷
There was a problem hiding this comment.
Command, for security reasons, uses its own logic for finding the exe and for compatibility reasons it always assumes .exe was intended if no extension is present.
There was a problem hiding this comment.
@ChrisDenton do you magically show up when someone says the word "Windows" in a Rust repo? ;)
Anyway, thanks for the explanation! :)
There was a problem hiding this comment.
Yes, but only if the proper rituals are observed
|
Will that work on a clean build running |
|
It should. miri-script already builds cargo-miri before running the test suite. |
|
Yes, |
|
@bors r+ |
|
☀️ Test successful - checks-actions |
Fixes #3297. Thanks to @bjorn3 for figuring out the cause of this.
r? @oli-obk