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 run-make tests performs unnecessary rebuilds #126464

Closed
Tracked by #131726
Kobzol opened this issue Jun 14, 2024 · 7 comments · Fixed by #126479
Closed
Tracked by #131726

Running run-make tests performs unnecessary rebuilds #126464

Kobzol opened this issue Jun 14, 2024 · 7 comments · Fixed by #126479
Labels
C-bug Category: This is a bug. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Comments

@Kobzol
Copy link
Contributor

Kobzol commented Jun 14, 2024

With the latest master, when I run a run-make test twice in a row, bootstrap rebuilds lld-wrapper and rustdoc:

./x test tests/run-make/CURRENT_RUSTC_VERSION/
./x test tests/run-make/CURRENT_RUSTC_VERSION/
<lld-wrapper + rustdoc rebuilt>

This has to be a recent regression, it didn't happen until ~yesterday. Confirmed with @jieyouxu that it also happens for them.

@Kobzol Kobzol added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) C-bug Category: This is a bug. labels Jun 14, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jun 14, 2024
@Kobzol
Copy link
Contributor Author

Kobzol commented Jun 14, 2024

Sounds suspiciously similar to #122491.

@jieyouxu jieyouxu removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jun 14, 2024
@Kobzol
Copy link
Contributor Author

Kobzol commented Jun 14, 2024

Yeah this is again caused by llvm-bitcode-linker being enabled 😆 @kjetilkjeka Do you think that there's some more general solution to the rebuild problem?

@kjetilkjeka
Copy link
Contributor

Thanks for notifying me about this. I am travelling this weekend, meaning i might not have a chance to thoroughly investigate before Monday. (But I will at the latest be able to look on Monday).

The last time it was due to dependency on the same crates with different rustflags. One from llbc-linker and one from some other tool (rustdoc?). The two different invocations maps to the same cache entry but is invalid between each other and thus triggers a rebuild.

The solution last time was to mimic the rustflags being used other places in llbc-linker. That might be a good short term solution now as well.

I don't know about the long term solution but it seems like either caching multiple versions of a crate or unifying the rustflag somewhere might be alternatives?

@Kobzol
Copy link
Contributor Author

Kobzol commented Jun 14, 2024

Thanks for notifying me about this. I am travelling this weekend, meaning i might not have a chance to thoroughly investigate before Monday.

Of course, there's no rush, I just wanted to let you know, given that you already have some experience in debugging this 😆

This seems rather brittle though, as you mention. I wonder what's so unique about this tool that it causes these rebuilds so often.

@onur-ozkan You mentioned here that we cannot share flags between all Step invocations. But maybe it would make sense to better share flags between at least this tool and other steps, rather than hardcoding selected flags? (as was done last time with the parallel compiler).

@Kobzol
Copy link
Contributor Author

Kobzol commented Jun 14, 2024

Actually, I wonder if we really need to enable this linker by default in all four bootstrap profiles.

@onur-ozkan
Copy link
Member

@onur-ozkan You mentioned #122491 (comment) that we cannot share flags between all Step invocations. But maybe it would make sense to better share flags between at least this tool and other steps, rather than hardcoding selected flags? (as was done last time with the parallel compiler).

Sure, seems fine to me. I didn't expect it to be a problem this frequently.

@Kobzol
Copy link
Contributor Author

Kobzol commented Jun 14, 2024

#126479 posted instead to disable this flag by default, I don't think it should be enabled for most contributors.

@bors bors closed this as completed in bc3618f Jun 16, 2024
github-actions bot pushed a commit to rust-lang/miri that referenced this issue Jun 17, 2024
…-ozkan

Disable `llvm-bitcode-linker` in the default bootstrap profiles

I don't think that we really need to enable `llvm-bitcode-linker` in the default bootstrap profiles, since it seems that it is only useful for running `nvptx` tests. It should be enabled on CI, which it is, and that should be enough. People can enable it easily locally, if they want.

The linker causes occasionally some rebuild issues (rust-lang/rust#122491, rust-lang/rust#126464), but more importantly it is just needless work to build it locally.

I kept it enabled for `dist`, because it is distributed as a `rustup` component (for some reason it's not included in `extended`? not sure).

Fixes: rust-lang/rust#126464
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. 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.

5 participants