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

rustc build broken after #95906 #96188

Closed
klensy opened this issue Apr 18, 2022 · 3 comments
Closed

rustc build broken after #95906 #96188

klensy opened this issue Apr 18, 2022 · 3 comments
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

@klensy
Copy link
Contributor

klensy commented Apr 18, 2022

After #95906 (0516711), building rustc (stage0 stdlib) with python x.py --stage=0 b fails with

$ python x.py build --stage=0
Updating only changed submodules
  Submodules updated in 0.02 seconds
Building rustbuild
    Finished dev [unoptimized] target(s) in 0.21s
thread 'main' panicked at '`should_run.paths` should correspond to real on-disk paths - use `alias` if there is no relevant path: src/llvm-project/llvm', src\bootstrap\builder.rs:391:21
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Build completed unsuccessfully in 0:00:01

The same true for perf runner: https://perf.rust-lang.org/status.html (https://perf.rust-lang.org/compare.html?start=6fd7e9010db6be7605241c39eab7c5078ee2d5bd&end=0516711ab057d9731f55511f00f9d426bc9db724)
cc @jyn514
@rustbot label +A-rustbuild

@rustbot rustbot added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Apr 18, 2022
@jyn514
Copy link
Member

jyn514 commented Apr 19, 2022

Thanks for catching this. The issue is that src/llvm-project/llvm doesn't exist if the llvm-project submodule isn't checked out locally. As a workaround, you can run git submodule update --init src/llvm-project.

I am not quite sure the right fix - I guess I could check in Llvm::should_run whether the submodule is checked out? But it seems confusing to have the possible paths be different based on your git state ... alternatively, I could revert the hard requirement, but then this will regress in the future.

I suppose a third alternative is to add a ShouldRun::submodule_path function that calls either path or alias depending on whether the submodule is checked out. But that seems a little overcomplicated.

cc @Mark-Simulacrum - happy to revert this assertion for now to unbreak things

@rustbot label +C-bug

@rustbot rustbot added the C-bug Category: This is a bug. label Apr 19, 2022
@klensy
Copy link
Contributor Author

klensy commented Apr 19, 2022

As consequence of that, every next perf run missing bootstrap timings.

P.S. Bootstrap timings in perf run missing, but no indication of error on the page (for example https://perf.rust-lang.org/compare.html?start=8305398d7ae6128811ec2b3223939bcd067530c2&end=311e2683e1bad87715b1558f7900e294d24ce491), except for first time (https://perf.rust-lang.org/compare.html?start=6fd7e9010db6be7605241c39eab7c5078ee2d5bd&end=0516711ab057d9731f55511f00f9d426bc9db724, but there bootstrap time show only partially)

jyn514 added a commit to jyn514/rust that referenced this issue Apr 19, 2022
This breaks on submodules (see rust-lang#96188). Disable the assertion for now until I can think of a proper
fix.

This doesn't revert any of the changes in `Step`s themselves, only what
`ShouldRun::paths` does.
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Apr 20, 2022
Remove assertion that all paths in `ShouldRun` exist

This breaks on submodules (see rust-lang#96188). Disable the assertion for now until I can think of a proper
fix.

This doesn't revert any of the changes in `Step`s themselves, only what
`ShouldRun::paths` does.
@jyn514
Copy link
Member

jyn514 commented Feb 3, 2023

Fixed in #96196

@jyn514 jyn514 closed this as completed Feb 3, 2023
onur-ozkan added a commit to onur-ozkan/rust that referenced this issue Apr 4, 2023
This commit resolves an internal FIXME note within the bootstrap by implementing submodule detection.
This is accomplished through an iterative process over the `.gitmodules` file.

Signed-off-by: ozkanonur <work@onurozkan.dev>
onur-ozkan added a commit to onur-ozkan/rust that referenced this issue Apr 5, 2023
This commit resolves an internal FIXME note within the bootstrap by implementing submodule detection.
This is accomplished through an iterative process over the `.gitmodules` file.

Signed-off-by: ozkanonur <work@onurozkan.dev>
JohnTitor added a commit to JohnTitor/rust that referenced this issue Apr 5, 2023
submodule detection for proper fix on rust-lang#96188

This commit resolves an internal FIXME note within the bootstrap by implementing submodule detection. This is accomplished through an iterative process over the `.gitmodules` file.

r? `@albertlarsan68`
bors added a commit to rust-lang-ci/rust that referenced this issue Apr 5, 2023
Rollup of 6 pull requests

Successful merges:

 - rust-lang#107236 (Add T-bootstrap label to tools)
 - rust-lang#109847 (Only create graphviz nodes for reachable MIR bb's)
 - rust-lang#109848 (submodule detection for proper fix on rust-lang#96188)
 - rust-lang#109932 (Source code scrollbar)
 - rust-lang#109952 (Move comment about python2 closer to the place it's used)
 - rust-lang#109956 (Tweak debug outputs to make debugging new solver easier)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
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

No branches or pull requests

3 participants