-
Notifications
You must be signed in to change notification settings - Fork 13.3k
rustbuild: check for final stage properly and so on #38752
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
Conversation
And don't forget anything that's supposed to be done in the final stage, either.
I believe after this PR the
However seeing there are a lot of rustbuild PRs floating around, I've decided to defer removing it to a later PR, hopefully minimizing rebase effort if any merge conflict arises. |
No worries for the breakage, and thanks for the PRs! I'm always more than willing to review build system changes :) I think, though, that this may not quite be the right approach. The stage1 compiler (e.g. I haven't looked too much into the dist failures, but I'll test out locally to see what needs to happen. Note, though, that is indeed correct that we still generate documentation and such in stage 1 because we're literally running |
Oh I see, the stage2, while doing very little work, still needs to happen as a separate stage technically. As for the dist failures, I believe the save-analysis from stage1 (which is correctly generated with #38736) should be copied to stage2 too, as the |
Ah ok I think I see what's going on here now, thanks for the comments! I think we should:
I'd like to rework all the various conditions in the dist pieces as I think they're causing problems, but those steps should suffice for now I think? |
I've done that and minimized the changes. I'm closing this PR and opening a new one. |
rustbuild: fix dist-analysis with full bootstrap disabled Really fixes #38734, per discussion in #38752 which was solving the underlying problem the wrong way. This just mirrors the [similar logic] in documentation building as suggested, that just takes the stage1 compiler artifacts instead in case of non-full-bootstrap builds. Actually copying the artifacts around seems to be unnecessary. r? @alexcrichton [similar logic]: https://github.com/rust-lang/rust/blob/7b659cfdbce094a790dbb246da2681a47565782a/src/bootstrap/doc.rs#L140-L144
The current non-full-bootstrap build is not stopped at stage1, but rather it's faking a stage2 and making adjustments for that here and there with the
force_use_stage1
usage. This is causing major headache and for example, contributed to the partial fix of #38736 which fixed the envvar setup inlib.rs
but failed to account for the broken contract indist.rs
. I was in a hurry so I didn't actually run./x.py dist
but instead was only looking for the environment variable, so that oversight also went unnoticed. The fact that the bug even survived code review and gotr+
'd is also highly indicative of the situation right now.Let's clean this up by properly stopping at stage1 for non-full-bootstrap builds. Stage dependency is properly indicated to the
dist
rules, and default stage for top-level steps are configured from build config instead of being hardcoded to 2.With that I did a
./x.py dist
(with channel set tonightly
to observedist-analysis
) and IMO it's working as desired:bootstrap plan of
dist
:ls build/dist
:And with that the
force_use_stage1
workaround could be dropped altogether, but I'm yet to do that. Hopefully we can at least get a2017-01-02
nightly for everyone's rustup pleasure...r? @alexcrichton (Sorry for so many rustbuild PRs and bugs introduced by my forgetfulness. I'm always trying to understand everything up there but sometimes I do forget details.)