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

Running ./x.py test miri --stage 0 twice rebuilds miri, cargo-miri, and rustdoc #123177

Closed
RalfJung opened this issue Mar 28, 2024 · 7 comments · Fixed by #123192
Closed

Running ./x.py test miri --stage 0 twice rebuilds miri, cargo-miri, and rustdoc #123177

RalfJung opened this issue Mar 28, 2024 · 7 comments · Fixed by #123192
Labels
T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Comments

@RalfJung
Copy link
Member

This is fairly easy to reproduce:

./x.py test miri --stage 0 --test-args hello
./x.py test miri --stage 0 --test-args hello

(The --test-args just serves to make this take less long.)

The second command shouldn't have to build anything, just run the tests. But somehow it actually rebuilds miri, cargo-miri, and rustdoc.

This is a recent regression introduced by #123028. Somehow building rustdoc seems to destroy the build cache for miri and cargo-miri, and vice versa.
Cc @onur-ozkan

@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Mar 28, 2024
@onur-ozkan onur-ozkan added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Mar 29, 2024
@onur-ozkan
Copy link
Member

Building rustdoc using the stage 1 compiler causes this, which is weird. I suspect that previously invoked miri related steps may have broken the cache somehow. Or we may need to improve/enhance Builder::ensure.

@RalfJung
Copy link
Member Author

I've just found this in the rustdoc sources:

// Similar to `compile::Assemble`, build with the previous stage's compiler. Otherwise
// we'd have stageN/bin/rustc and stageN/bin/rustdoc be effectively different stage
// compilers, which isn't what we want. Rustdoc should be linked in the same way as the
// rustc compiler it's paired with, so it must be built with the previous stage compiler.
let build_compiler = builder.compiler(target_compiler.stage - 1, builder.config.build);

Maybe Miri should do something similar. Then ./x.py test miri should also Do The Right Thing without adding --stage 0 (since actually stage 1 would be the right stage to use). And maybe then the rebuilding issue also vanishes. Or maybe that's unrelated, not sure.

@onur-ozkan
Copy link
Member

That would work actually, it seems to be the right path.

@RalfJung
Copy link
Member Author

Unfortunately it doesn't help. We probably still should do it.

But somehow building rustdoc seems to delete the build cache for Miri and cargo-miri, and vice versa.

@RalfJung
Copy link
Member Author

Cargo has this to say about why things are rebuilt:

       Dirty libc v0.2.153: the rustflags changed

And indeed, rustdoc sets

cargo.rustflag("--cfg=parallel_compiler");

That's a bit annoying to have this set in the tool, it means each tool needs to repeat the same logic.

@onur-ozkan
Copy link
Member

onur-ozkan commented Mar 30, 2024

iirc rustdoc sets that to not dirt the compiler builds, we should probably handle that more globally.

@RalfJung
Copy link
Member Author

iirc rustdoc sets that to not dirt the compiler builds, we should probably handle that more globally.

Yup, that's what the last commit in #123192 does now.

bors added a commit to rust-lang-ci/rust that referenced this issue Mar 31, 2024
…-ozkan

Refactor the way bootstrap invokes `cargo miri`

Instead of basically doing `cargo run --manifest-path=<cargo-miri's manifest> -- miri`, let's invoke the `cargo-miri` binary directly. That means less indirections, and also makes it easier to e.g. run the libcore test suite in Miri. (But there are still other issues with that.)

Also also adjusted Miri's stage numbering so that it is consistent with rustc/rustdoc.

This also makes `./x.py test miri` honor `--no-doc`.

And this fixes rust-lang#123177 by moving where we handle parallel_compiler.
bors added a commit to rust-lang-ci/rust that referenced this issue Mar 31, 2024
…-ozkan

Refactor the way bootstrap invokes `cargo miri`

Instead of basically doing `cargo run --manifest-path=<cargo-miri's manifest> -- miri`, let's invoke the `cargo-miri` binary directly. That means less indirections, and also makes it easier to e.g. run the libcore test suite in Miri. (But there are still other issues with that.)

Also also adjusted Miri's stage numbering so that it is consistent with rustc/rustdoc.

This also makes `./x.py test miri` honor `--no-doc`.

And this fixes rust-lang#123177 by moving where we handle parallel_compiler.
@bors bors closed this as completed in 871df0d Apr 1, 2024
github-actions bot pushed a commit to rust-lang/miri that referenced this issue Apr 3, 2024
Refactor the way bootstrap invokes `cargo miri`

Instead of basically doing `cargo run --manifest-path=<cargo-miri's manifest> -- miri`, let's invoke the `cargo-miri` binary directly. That means less indirections, and also makes it easier to e.g. run the libcore test suite in Miri. (But there are still other issues with that.)

Also also adjusted Miri's stage numbering so that it is consistent with rustc/rustdoc.

This also makes `./x.py test miri` honor `--no-doc`.

And this fixes rust-lang/rust#123177 by moving where we handle parallel_compiler.
lnicola pushed a commit to lnicola/rust-analyzer that referenced this issue Apr 7, 2024
Refactor the way bootstrap invokes `cargo miri`

Instead of basically doing `cargo run --manifest-path=<cargo-miri's manifest> -- miri`, let's invoke the `cargo-miri` binary directly. That means less indirections, and also makes it easier to e.g. run the libcore test suite in Miri. (But there are still other issues with that.)

Also also adjusted Miri's stage numbering so that it is consistent with rustc/rustdoc.

This also makes `./x.py test miri` honor `--no-doc`.

And this fixes rust-lang/rust#123177 by moving where we handle parallel_compiler.
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this issue Apr 27, 2024
Refactor the way bootstrap invokes `cargo miri`

Instead of basically doing `cargo run --manifest-path=<cargo-miri's manifest> -- miri`, let's invoke the `cargo-miri` binary directly. That means less indirections, and also makes it easier to e.g. run the libcore test suite in Miri. (But there are still other issues with that.)

Also also adjusted Miri's stage numbering so that it is consistent with rustc/rustdoc.

This also makes `./x.py test miri` honor `--no-doc`.

And this fixes rust-lang/rust#123177 by moving where we handle parallel_compiler.
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.

3 participants