Running ./x test mir-opt rebuilds std every time
#131437
Assignees
Labels
A-contributor-roadblock
Area: Makes things more difficult for new contributors to rust itself
C-bug
Category: This is a bug.
T-bootstrap
Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Comments
Zalathar
added
A-contributor-roadblock
rustbot
added
the
needs-triage
|
cc @jieyouxu |
jieyouxu
added
C-bug
|
I will note that |
|
Is there some way for CI to error on spurious rebuilds? I don't know what that would look like but this has happened in a few different places for different reasons so it'd be nice if there were some way to catch it early. |
|
Usually the spurious rebuilds happen if you try to run the same command, twice. I don't think that's very tractable for CI, but am happy to be proven wrong. Maybe we can try to devise some kind of mechanism in bootstrap to make some invalidations impossible, but I don't know how such a mechanism would work in general. |
|
One mechanism for that would be to keep the rustflags logic in a single sealed module, so they can't be set externally from the individual build steps, which means they can't invalidate the build cache. Next to that approach we can add test coverage to rustflags logic. |
|
I opened a short-term immediate fix in #131442, but the long term solution to prevent resurrgence of |
|
Also this one actually builds same stage std twice for host if host == target (i.e. not cross-compile) because std was |
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this issue
Oct 10, 2024
…zkan
Match std `RUSTFLAGS` for host and target for `mir-opt` test suite to fix double std build/rebuilds
Previously the bootstrap compiletest `Step::run` flow had:
```rs
// ensure that `libproc_macro` is available on the host.
builder.ensure(compile::Std::new(compiler, compiler.host));
// ...
if suite == "mir-opt" {
builder.ensure(compile::Std::new_for_mir_opt_tests(compiler, target));
} else {
builder.ensure(compile::Std::new(compiler, target));
}
```
This can cause unnecessary std rebuilds (even on the same invocation) because if host == target then `builder.ensure(compile::Std::new_for_mir_opt_tests(compiler, target))` will have different `RUSTFLAGS` than `builder.ensure(compile::Std::new(compiler, compiler.host))`.
This PR fixes that by matching up std `RUSTFLAGS` if the test suite is `mir-opt`:
```rs
if suite == "mir-opt" {
builder.ensure(compile::Std::new_for_mir_opt_tests(compiler, compiler.host));
} else {
builder.ensure(compile::Std::new(compiler, compiler.host));
}
```
This is a short-term fix, the better fix is to enforce how `RUSTFLAGS` are handled as described in rust-lang#131437 (comment).
Fixes rust-lang#131437.
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this issue
Oct 10, 2024
…zkan
Match std `RUSTFLAGS` for host and target for `mir-opt` test suite to fix double std build/rebuilds
Previously the bootstrap compiletest `Step::run` flow had:
```rs
// ensure that `libproc_macro` is available on the host.
builder.ensure(compile::Std::new(compiler, compiler.host));
// ...
if suite == "mir-opt" {
builder.ensure(compile::Std::new_for_mir_opt_tests(compiler, target));
} else {
builder.ensure(compile::Std::new(compiler, target));
}
```
This can cause unnecessary std rebuilds (even on the same invocation) because if host == target then `builder.ensure(compile::Std::new_for_mir_opt_tests(compiler, target))` will have different `RUSTFLAGS` than `builder.ensure(compile::Std::new(compiler, compiler.host))`.
This PR fixes that by matching up std `RUSTFLAGS` if the test suite is `mir-opt`:
```rs
if suite == "mir-opt" {
builder.ensure(compile::Std::new_for_mir_opt_tests(compiler, compiler.host));
} else {
builder.ensure(compile::Std::new(compiler, compiler.host));
}
```
This is a short-term fix, the better fix is to enforce how `RUSTFLAGS` are handled as described in rust-lang#131437 (comment).
Fixes rust-lang#131437.
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this issue
Oct 10, 2024
…zkan
Match std `RUSTFLAGS` for host and target for `mir-opt` test suite to fix double std build/rebuilds
Previously the bootstrap compiletest `Step::run` flow had:
```rs
// ensure that `libproc_macro` is available on the host.
builder.ensure(compile::Std::new(compiler, compiler.host));
// ...
if suite == "mir-opt" {
builder.ensure(compile::Std::new_for_mir_opt_tests(compiler, target));
} else {
builder.ensure(compile::Std::new(compiler, target));
}
```
This can cause unnecessary std rebuilds (even on the same invocation) because if host == target then `builder.ensure(compile::Std::new_for_mir_opt_tests(compiler, target))` will have different `RUSTFLAGS` than `builder.ensure(compile::Std::new(compiler, compiler.host))`.
This PR fixes that by matching up std `RUSTFLAGS` if the test suite is `mir-opt`:
```rs
if suite == "mir-opt" {
builder.ensure(compile::Std::new_for_mir_opt_tests(compiler, compiler.host));
} else {
builder.ensure(compile::Std::new(compiler, compiler.host));
}
```
This is a short-term fix, the better fix is to enforce how `RUSTFLAGS` are handled as described in rust-lang#131437 (comment).
Fixes rust-lang#131437.
workingjubilee
added a commit
to workingjubilee/rustc
that referenced
this issue
Oct 10, 2024
…zkan
Match std `RUSTFLAGS` for host and target for `mir-opt` test suite to fix double std build/rebuilds
Previously the bootstrap compiletest `Step::run` flow had:
```rs
// ensure that `libproc_macro` is available on the host.
builder.ensure(compile::Std::new(compiler, compiler.host));
// ...
if suite == "mir-opt" {
builder.ensure(compile::Std::new_for_mir_opt_tests(compiler, target));
} else {
builder.ensure(compile::Std::new(compiler, target));
}
```
This can cause unnecessary std rebuilds (even on the same invocation) because if host == target then `builder.ensure(compile::Std::new_for_mir_opt_tests(compiler, target))` will have different `RUSTFLAGS` than `builder.ensure(compile::Std::new(compiler, compiler.host))`.
This PR fixes that by matching up std `RUSTFLAGS` if the test suite is `mir-opt`:
```rs
if suite == "mir-opt" {
builder.ensure(compile::Std::new_for_mir_opt_tests(compiler, compiler.host));
} else {
builder.ensure(compile::Std::new(compiler, compiler.host));
}
```
This is a short-term fix, the better fix is to enforce how `RUSTFLAGS` are handled as described in rust-lang#131437 (comment).
Fixes rust-lang#131437.
rust-timer
added a commit
to rust-lang-ci/rust
that referenced
this issue
Oct 10, 2024
Rollup merge of rust-lang#131442 - jieyouxu:mir-opt-rebuild, r=onur-ozkan Match std `RUSTFLAGS` for host and target for `mir-opt` test suite to fix double std build/rebuilds Previously the bootstrap compiletest `Step::run` flow had: ```rs // ensure that `libproc_macro` is available on the host. builder.ensure(compile::Std::new(compiler, compiler.host)); // ... if suite == "mir-opt" { builder.ensure(compile::Std::new_for_mir_opt_tests(compiler, target)); } else { builder.ensure(compile::Std::new(compiler, target)); } ``` This can cause unnecessary std rebuilds (even on the same invocation) because if host == target then `builder.ensure(compile::Std::new_for_mir_opt_tests(compiler, target))` will have different `RUSTFLAGS` than `builder.ensure(compile::Std::new(compiler, compiler.host))`. This PR fixes that by matching up std `RUSTFLAGS` if the test suite is `mir-opt`: ```rs if suite == "mir-opt" { builder.ensure(compile::Std::new_for_mir_opt_tests(compiler, compiler.host)); } else { builder.ensure(compile::Std::new(compiler, compiler.host)); } ``` This is a short-term fix, the better fix is to enforce how `RUSTFLAGS` are handled as described in rust-lang#131437 (comment). Fixes rust-lang#131437.
5 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Using
profile = compiler, running./x test mir-optcauses bootstrap to rebuild all of std before running the tests, even when the build cache ought to be warm from a previous./x test mir-opt.Needless to say, this is not very fun when actually trying to run one or more mir-opt tests.
The text was updated successfully, but these errors were encountered: