x.py: do not build Miri by default on stable/beta #73232
Conversation
rust-highfive
added
the
S-waiting-on-review
src/bootstrap/tool.rs
Outdated
| Cargofmt, rustfmt, "src/tools/rustfmt", "cargo-fmt", default=true, {}; | ||
| CargoClippy, clippy, "src/tools/clippy", "cargo-clippy", default=true, {}; | ||
| Clippy, clippy, "src/tools/clippy", "clippy-driver", default=true, {}; | ||
| Miri, miri, "src/tools/miri", "miri", default=false, {}; |
There was a problem hiding this comment.
This will disable miri on nightly and dev builds.
Something like that should enable miri for nightly and dev builds:
Miri, miri, "src/tools/miri", "miri", default=builder.build.unstable_features(), {};Alternatively enable for nightly only:
Miri, miri, "src/tools/miri", "miri", default=builder.build.config.channel == "nightly", {};There was a problem hiding this comment.
I can't use run-time values though if I put this into the DEFAULT constant (whatever that does). Also I think hygiene will prevent me from referring to builder, but I could use a closure to get around that.
There was a problem hiding this comment.
Right, maybe Mark comes up with an idea.
There was a problem hiding this comment.
For now I changed the macro to say whether a tool is "stable", and then just enable all tools per default on nightly/dev builds.
But I still have no idea what to do with that undocumented DEFAULT constant.
RalfJung
force-pushed
the
miri-no-default
branch
2 times, most recently
from
141d895 to
8135c5a
Compare
This comment has been minimized.
This comment has been minimized.
RalfJung
force-pushed
the
miri-no-default
branch
from
8135c5a to
4d93516
Compare
src/bootstrap/tool.rs
Outdated
| @@ -606,17 +607,22 @@ macro_rules! tool_extended { | |||
|
|
|||
| impl Step for $name { | |||
| type Output = Option<PathBuf>; | |||
| const DEFAULT: bool = true; | |||
| const DEFAULT: bool = $stable; // FIXME What does this do? | |||
There was a problem hiding this comment.
DEFAULT-enabled tools means that this step will be run by default (e.g., on x.py build). In this case it's also subject to the default_condition in should_run, though.
There was a problem hiding this comment.
@Mark-Simulacrum I updated the PR. Does it make sense now?
RalfJung
force-pushed
the
miri-no-default
branch
from
4d93516 to
416b010
Compare
Mark-Simulacrum
changed the title
|
Thanks for adding a comment to DEFAULT while we're at it :) This does indeed look correct to me, though (as all defaults) it should be noted that people can override this by explicitly requesting a miri build (e.g. @bors r+ |
|
📌 Commit 416b010 has been approved by |
bors
added
S-waiting-on-bors
Yeah, or by adding it to |
|
@bors p=1 |
|
☀️ Test successful - checks-azure |
Fixes #73117
Do I need to do anything to make sure Miri is still built by the tools CI builder? Are there other tools that should be off-by-default?
Also, unfortunately the
DEFAULTassociated const has no doc comment, so I have no idea what it does, or why there are semmingly two places where the default build of tools is controlled.