Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Include miri in config.example.toml[build] # tools= #129171

Closed
dilyanpalauzov opened this issue Aug 16, 2024 · 6 comments · Fixed by #129243
Closed

Include miri in config.example.toml[build] # tools= #129171

dilyanpalauzov opened this issue Aug 16, 2024 · 6 comments · Fixed by #129243
Labels
T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Comments

@dilyanpalauzov
Copy link

In the past, the default tools=[...] in config.toml included Miri. The default of building Miri was removed at #100134 and #102028. The latter is called “Do not build Miri by default”, and as it changes 1211 files obviously I do not follow what exactly it does.

In any case, as in rustc-1.80.1-src.tar.xz:config.example.toml[build] # tools = [ "cargo", "clippy", "rustdoc", "rustfmt", "rust-analyzer", "rust-analyzer-proc-macro-srv", "analysis", "src", "rust-demangler", # if profiler = true]

does not include miri, my understanding is that

$ ../configure --enable-optimize-llvm --enable-extended --llvm-root=/usr/local --enable-profiler --enable-llvm-link-shared --enable-sanitizers --enable-local-rust --disable-docs --target=x86_64-unknown-linux-gnu

and

$ ../configure --enable-optimize-llvm --enable-extended --llvm-root=/usr/local --enable-profiler --enable-llvm-link-shared --enable-sanitizers --enable-local-rust --disable-docs --target=x86_64-unknown-linux-gnu --tools=cargo,clippy,rustdoc,rustfmt,rust-analyzer,analysis,src,rust-demangler

should produce identical result (except for rust-analyzer-proc-macro-srv). But the first ./configure does lead to installing Miri, while the second skips installing Miri.

As my observation is that now Miri is built by default (is part of the implicit tools=[…]), in config.example.toml tools= should indclude miri.

To be precise the first ./configure I ran against rustc-1.79.0-src.tar.xz and the second against rustc-1.80.1-src.tar.xz.

@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Aug 16, 2024
@saethlin
Copy link
Member

The default of building Miri was removed at #100134 and #102028

That PR shipped with Rust 1.66 so I'm not sure why you are linking it when you seem to be intending to report a change from Rust 1.79 to Rust 1.80.

@dilyanpalauzov
Copy link
Author

I am not reporting a changbe between 1.79 and 1.80.

I am saying that old Rust versions built Miri by default (miri was in the implicit tools=[]), then it was removed from tools=[], and now miri is again in the implicit tools=[], but not in config.example.toml:tools=[...]

@jieyouxu jieyouxu added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Aug 17, 2024
@saethlin saethlin removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Aug 17, 2024
@RalfJung
Copy link
Member

Miri was removed at #100134 and #102028. The latter is called “Do not build Miri by default”

The latter is called "Make miri a subtree instead of a submodule", which should give you an idea of why it changes so many files. ;)

I am saying that old Rust versions built Miri by default (miri was in the implicit tools=[]), then it was removed from tools=[], and now miri is again in the implicit tools=[], but not in config.example.toml:tools=[...]

Cc @rust-lang/bootstrap ; I don't know if this was a deliberate change or not or how the implicit "tools" array is even computed. Miri is a nightly-only tool so we probably do not want it to be built implicitly on stable.

@onur-ozkan
Copy link
Member

Cc @rust-lang/bootstrap ; I don't know if this was a deliberate change or not or how the implicit "tools" array is even computed. Miri is a nightly-only tool so we probably do not want it to be built implicitly on stable.

For dev/nightly channels, we simply build every tool by default:

fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
let builder = run.builder;
run.path($path).default_condition(
builder.config.extended
&& builder.config.tools.as_ref().map_or(
// By default, on nightly/dev enable all tools, else only
// build stable tools.
$stable || builder.build.unstable_features(),
// If `tools` is set, search list for this tool.
|tools| {
tools.iter().any(|tool| match tool.as_ref() {
"clippy" => $tool_name == "clippy-driver",
x => $tool_name == x,
})

If channel isn't dev/nightly, we only build tools are are marked as stable=true here and (although cargo-miri is) Miri isn't one of them.

@RalfJung
Copy link
Member

RalfJung commented Aug 18, 2024 via email

@onur-ozkan
Copy link
Member

Okay so if the sample config.toml is supposed to show the default it should show both of these, presumably?

Done that in #129243 (a629a0b)

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Aug 23, 2024
do not build `cargo-miri` by default on stable channel

Skips `cargo-miri` build on `stable` channel just like `miri`.

Closes rust-lang#129171

cc `@RalfJung`
@bors bors closed this as completed in e5cd26a Aug 23, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Aug 23, 2024
Rollup merge of rust-lang#129243 - onur-ozkan:stuff, r=Kobzol

do not build `cargo-miri` by default on stable channel

Skips `cargo-miri` build on `stable` channel just like `miri`.

Closes rust-lang#129171

cc ``@RalfJung``
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants