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

rustbuild: check for final stage properly and so on #38752

Closed
wants to merge 1 commit into from

Conversation

xen0n
Copy link
Contributor

@xen0n xen0n commented Jan 1, 2017

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 in lib.rs but failed to account for the broken contract in dist.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 got r+'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 to nightly to observe dist-analysis) and IMO it's working as desired:

bootstrap plan of dist:

bootstrap top targets:
        Step { name: "dist-analysis", stage: 1, host: "x86_64-unknown-linux-gnu", target: "x86_64-unknown-linux-gnu" }
        Step { name: "dist-rustc", stage: 1, host: "x86_64-unknown-linux-gnu", target: "x86_64-unknown-linux-gnu" }
        Step { name: "dist-docs", stage: 1, host: "x86_64-unknown-linux-gnu", target: "x86_64-unknown-linux-gnu" }
        Step { name: "dist-mingw", stage: 1, host: "x86_64-unknown-linux-gnu", target: "x86_64-unknown-linux-gnu" }
        Step { name: "dist-src", stage: 1, host: "x86_64-unknown-linux-gnu", target: "x86_64-unknown-linux-gnu" }
        Step { name: "dist-std", stage: 1, host: "x86_64-unknown-linux-gnu", target: "x86_64-unknown-linux-gnu" }
bootstrap build plan:
        Step { name: "create-sysroot", stage: 1, host: "x86_64-unknown-linux-gnu", target: "x86_64-unknown-linux-gnu" }
        Step { name: "startup-objects", stage: 1, host: "x86_64-unknown-linux-gnu", target: "x86_64-unknown-linux-gnu" }
        Step { name: "create-sysroot", stage: 0, host: "x86_64-unknown-linux-gnu", target: "x86_64-unknown-linux-gnu" }
        Step { name: "rustc", stage: 0, host: "x86_64-unknown-linux-gnu", target: "x86_64-unknown-linux-gnu" }
        Step { name: "startup-objects", stage: 0, host: "x86_64-unknown-linux-gnu", target: "x86_64-unknown-linux-gnu" }
        Step { name: "build-crate-std_shim", stage: 0, host: "x86_64-unknown-linux-gnu", target: "x86_64-unknown-linux-gnu" }
        Step { name: "libstd-link", stage: 0, host: "x86_64-unknown-linux-gnu", target: "x86_64-unknown-linux-gnu" }
        Step { name: "libstd", stage: 0, host: "x86_64-unknown-linux-gnu", target: "x86_64-unknown-linux-gnu" }
        Step { name: "build-crate-test_shim", stage: 0, host: "x86_64-unknown-linux-gnu", target: "x86_64-unknown-linux-gnu" }
        Step { name: "libtest-link", stage: 0, host: "x86_64-unknown-linux-gnu", target: "x86_64-unknown-linux-gnu" }
        Step { name: "libtest", stage: 0, host: "x86_64-unknown-linux-gnu", target: "x86_64-unknown-linux-gnu" }
        Step { name: "llvm", stage: 0, host: "x86_64-unknown-linux-gnu", target: "x86_64-unknown-linux-gnu" }
        Step { name: "build-crate-rustc-main", stage: 0, host: "x86_64-unknown-linux-gnu", target: "x86_64-unknown-linux-gnu" }
        Step { name: "librustc-link", stage: 0, host: "x86_64-unknown-linux-gnu", target: "x86_64-unknown-linux-gnu" }
        Step { name: "librustc", stage: 0, host: "x86_64-unknown-linux-gnu", target: "x86_64-unknown-linux-gnu" }
        Step { name: "rustc", stage: 1, host: "x86_64-unknown-linux-gnu", target: "x86_64-unknown-linux-gnu" }
        Step { name: "build-crate-std_shim", stage: 1, host: "x86_64-unknown-linux-gnu", target: "x86_64-unknown-linux-gnu" }
        Step { name: "libstd-link", stage: 1, host: "x86_64-unknown-linux-gnu", target: "x86_64-unknown-linux-gnu" }
        Step { name: "build-crate-test_shim", stage: 1, host: "x86_64-unknown-linux-gnu", target: "x86_64-unknown-linux-gnu" }
        Step { name: "libtest-link", stage: 1, host: "x86_64-unknown-linux-gnu", target: "x86_64-unknown-linux-gnu" }
        Step { name: "build-crate-rustc-main", stage: 1, host: "x86_64-unknown-linux-gnu", target: "x86_64-unknown-linux-gnu" }
        Step { name: "librustc-link", stage: 1, host: "x86_64-unknown-linux-gnu", target: "x86_64-unknown-linux-gnu" }
        Step { name: "dist-std", stage: 1, host: "x86_64-unknown-linux-gnu", target: "x86_64-unknown-linux-gnu" }
        Step { name: "dist-analysis", stage: 1, host: "x86_64-unknown-linux-gnu", target: "x86_64-unknown-linux-gnu" }
        Step { name: "dist-rustc", stage: 1, host: "x86_64-unknown-linux-gnu", target: "x86_64-unknown-linux-gnu" }
        Step { name: "tool-rustbook", stage: 0, host: "x86_64-unknown-linux-gnu", target: "x86_64-unknown-linux-gnu" }
        Step { name: "doc-book", stage: 1, host: "x86_64-unknown-linux-gnu", target: "x86_64-unknown-linux-gnu" }
        Step { name: "doc-nomicon", stage: 1, host: "x86_64-unknown-linux-gnu", target: "x86_64-unknown-linux-gnu" }
        Step { name: "doc-crate-std_shim", stage: 1, host: "x86_64-unknown-linux-gnu", target: "x86_64-unknown-linux-gnu" }
        Step { name: "tool-error-index", stage: 0, host: "x86_64-unknown-linux-gnu", target: "x86_64-unknown-linux-gnu" }
        Step { name: "doc-error-index", stage: 1, host: "x86_64-unknown-linux-gnu", target: "x86_64-unknown-linux-gnu" }
        Step { name: "doc-standalone", stage: 1, host: "x86_64-unknown-linux-gnu", target: "x86_64-unknown-linux-gnu" }
        Step { name: "dist-docs", stage: 1, host: "x86_64-unknown-linux-gnu", target: "x86_64-unknown-linux-gnu" }
        Step { name: "dist-mingw", stage: 1, host: "x86_64-unknown-linux-gnu", target: "x86_64-unknown-linux-gnu" }
        Step { name: "dist-src", stage: 1, host: "x86_64-unknown-linux-gnu", target: "x86_64-unknown-linux-gnu" }

ls build/dist:

doc
rust-analysis-nightly-x86_64-unknown-linux-gnu.tar.gz
rustc-nightly-src.tar.gz
rustc-nightly-x86_64-unknown-linux-gnu.tar.gz
rust-docs-nightly-x86_64-unknown-linux-gnu.tar.gz
rust-src-nightly.tar.gz
rust-std-nightly-x86_64-unknown-linux-gnu.tar.gz

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 a 2017-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.)

And don't forget anything that's supposed to be done in the final stage,
either.
@xen0n
Copy link
Contributor Author

xen0n commented Jan 1, 2017

I believe after this PR the force_use_stage1 function should be useless already, as

  • it returns false for full bootstraps, and
  • stage is never greater than 2 for non-full bootstraps.

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.

@xen0n xen0n changed the title [WIP] rustbuild: check for final stage properly and so on rustbuild: check for final stage properly and so on Jan 1, 2017
@alexcrichton
Copy link
Member

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. build/$target/stage1/bin/rustc is actually not what we want to use. That's the one which cannot work with plugins, for example. As a result I think "final stage" here is a bit misleading because stage 2 really is still the final stage. The build system just has an "optimization" where assembling the target libraries for stage 2 is free, we just copy them from before.

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 cargo doc which if run in stage 2 would rebuild a bunch of business. You can see that mapping here.

@xen0n
Copy link
Contributor Author

xen0n commented Jan 1, 2017

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 dist-analysis rule defaults to stage2. I can take everything out and retain this sole change needed to fix dist. What's your opinion?

@alexcrichton
Copy link
Member

Ah ok I think I see what's going on here now, thanks for the comments! I think we should:

  • Let's back out the addition of is_final_stage for now, as it may be confusing in the future.
  • Let's just set RUSTC_SAVE_ANALYSIS always on nightly
  • Let's remove the stage condition in dist::analysis

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?

@xen0n
Copy link
Contributor Author

xen0n commented Jan 1, 2017

I've done that and minimized the changes. I'm closing this PR and opening a new one.

@xen0n xen0n closed this Jan 1, 2017
bors added a commit that referenced this pull request Jan 1, 2017
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
@xen0n xen0n deleted the i-dont-like-red-bots branch January 18, 2017 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants