bootstrap: Switch download-rustc back on by default for profile = library

[jyn514]

See #142505 for context. There are actually three changes here, I left detailed commit messages.

[rustbot]

r? @clubby789

rustbot has assigned @clubby789.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: bootstrap
• bootstrap expanded to 6 candidates
• Random selection from Mark-Simulacrum, clubby789, jieyouxu

[Kobzol]

I'm not sure if we should propagate download-rustc in its current state, tbh. It has been hacky even before the stage0 redesign, and since then I have zero confidence that it works correctly, I expect that there are many issues with it, and I don't think we have the bandwidth at the moment to deal with them. Of course, some people are using download-rustc today and if it works for them, then that's fine. But enabling it by default when we know it's broken does not seem right.

I think that long-term the best solution is to rip out download-rustc completely and reimplement it in a maintainable way.

[jyn514]

I wish you had brought this up a couple years ago when I proposed the stage 0 redesign :/ what would you consider to be a "more maintainable way"?

[jyn514]

If it helps, I'm happy to use my work time to maintain download-rustc so that the burden isn't falling on you and the rest of the bootstrap team.

[Kobzol]

I wish you had brought this up a couple years ago when I proposed the stage 0 redesign :/ what would you consider to be a "more maintainable way"?

I only really learned how download-rustc is implemented only after the stage 0 redesign has landed 😅 To clarify, I think that the stage 0 redesign was a great thing, and I don't think that we should have blocked it on the redesign of download-rustc, I'm quite happy that we went with it. I also think that after the redesign, download-rustc is.. actually not needed that much? There are far less rebuilds happening today in bootstrap thanks to the stage 0 redesign. And the plan to use download-rustc to save CI time IMO doesn't really work, as we saw in practice, because there are just too many PRs that change something that could affect rustc, and we had several nasty cases where the cache-busting logic was producing more harm than utility.

But of course I also understand that for some users download-rustc offers benefit, so we should try to bring it to a reasonable state.

Now about the implementation. It is essentially implemented in a way where there are tens (I think last time I counted around 30?) of if conditions scattered across bootstrap, some at quite load-bearing places, that perform some custom logic to make download-rustc work. This was always very fragile, because that logic was sometimes assuming things like "at this point, we had to be called from step B that was called from step C, so we will do this special little thing that will make step D, which surely has to be called after this, work with download-rustc". As we do not have any tests for this (since it's very difficult to test), any changes around this logic lead to bugs that are quite hard to track down. Another problem that the implementation has is that it downloads a stage 2 rustc, but locally that download rustc is primarily used as a stage 1 rustc. That means that the ad-hoc logic has to do all sorts of hacks to convince bootstrap to treat the downloaded rustc as stage 1, which is... as robust as it sounds.

All this existed before the stage 0 redesign, but in a sense it was a bit better, both because of the previous staging logic, but mainly because we did not yet go through a major change of bootstrap for which the implementation was not prepared 😆 Onur did a heroic effort to make it work with the redesign, but it was essentially impossible to keep it working after we were making further changes to bootstrap. When I was systematically going over all steps last year to clean things up after the redesign, there were a few situations where things were only "working" because of two bugs that were cancelling each other out. I think that kind of sums the state it is in 😆 We were also seeing various bugs around download-rustc that were hard to fix without breaking something else, it was the classic bootstrap bug "whack-a-mole". That's where I thought that it would be better to just disable it by default until it is reimplemented.

The way I imagine it would be implemented is ideally by being centralized in a single place in bootstrap, so that we don't need a special case for download-rustc in every third bootstrap step just to make it work. To do that, we would ideally have to make download-rustc less "special". My plan for that was to get rid of all the stage renumbering hacks, which are IMO one of the biggest points here. I would like to do that by turning download-rustc into a stage 1 rustc, so that you can download an archive from CI that you unpack on disk, and it would behave in the same way as if you built rustc locally. And then, if someone actually needs stage 2 download-rustc for some use-cases, we would also add that to CI, either in the same component or in a separate component (there are clear trade-offs there around complexity and archive download size).

I started working on this redesign last year, but then I realized that there is another issue with download-rustc 😆 Which is that it is actually quite heavily intertwined with the implementation of download-ci-llvm, which is also scattered across the codebase, rather than being at a single place. So rewriting download-ci-llvm came into the critical path. I wanted to first refactor that to make it more like download-ci-gcc, which I tried to implement so that it is self-contained and doesn't bleed into other parts of bootstrap. This LLVM refactoring is still ongoing, I guess, I have some WIP branch of is somewhere, but I didn't have time to work on bootstrap properly for over half a year, so... :)

I also wrote about this last year on Zulip.

[rust-bors]

☔ The latest upstream changes (presumably #154958) made this pull request unmergeable. Please resolve the merge conflicts.