Add a scheme for always using the default toolchain, running clippy and fmt before running any other command #2381
Conversation
There was a problem hiding this comment.
I am not entirely convinced by this. It seems like the kind of thing that I would solve by just having a local file xmiri like this:
#!/bin/sh
set -e
./rustup-toolchain
./miri fmt
./miri clippy
./miri "$@"Why do we need this upstream? Is this a common problem? @saethlin might be good to ask about this.
Also the name "check-everything" is not very apt IMO, since ./rustup-toolchain is not just a check. I personally might want the fmt and clippy parts of this but I certainly never would want the rustup part of this; I do regularly use other toolchains than the one given in that file.
We could have files .auto-fmt, .auto-clippy, .auto-toolchain to control the three aspects of this separately. 😂 and/or .auto-everything for what it says on the tin.
miri
Outdated
| clippy() { | ||
| $CARGO clippy $CARGO_EXTRA_FLAGS --manifest-path "$MIRIDIR"/Cargo.toml --all-targets "$@" | ||
| $CARGO clippy $CARGO_EXTRA_FLAGS --manifest-path "$MIRIDIR"/ui_test/Cargo.toml --all-targets "$@" | ||
| $CARGO clippy $CARGO_EXTRA_FLAGS --manifest-path "$MIRIDIR"/cargo-miri/Cargo.toml "$@" |
There was a problem hiding this comment.
You can avoid this refactoring by actually calling $0 recursively, with some environment variable to avoid endless loops.
There was a problem hiding this comment.
idk... that seems more roundabout/fragile to me, but if you prefer, I can do that.
There was a problem hiding this comment.
I don't have a strong opinion either way, but your current implementation is incorrect. So I think what I suggested is actually less fragile.
@saethlin, a mentee of mine and I myself all run into The other things (fmt and checks) are something that I do locally with |
|
Hm okay, fair. So what about the Also the part of the docs that talks about building Miri against a locally built rustc should warn to remove the |
| # Run this first, so that the toolchain doesn't change after | ||
| # other code has run. | ||
| if [ -f ".auto-everything" ] || [ -f ".auto-toolchain" ] ; then | ||
| "$MIRIDIR"/rustup-toolchain |
There was a problem hiding this comment.
That was too far up. 😂 now MIRIDIR is not defined yet.
There was a problem hiding this comment.
Huh... I tested it and it seemed to work. Maybe bash just makes unknown env vars be empty?
There was a problem hiding this comment.
Maybe bash just makes unknown env vars be empty?
Yes it definitely does that. That is "expected" behavior in shell script land. ;)
…nd fmt before running any other command
|
@bors r+ |
|
☀️ Test successful - checks-actions |
I keep forgetting to run rustup-toolchain on rebases across toolchain updates
I also keep forgetting to run rustfmt and clippy. The former isn't run by vscode if I don't explicitly save (I have autosave on).