use download-ci-llvm=true in the default compiler config by Urgau · Pull Request #129473 · rust-lang/rust (original) (raw)

Not checking out LLVM by default is the correct behaviour, and should not have been changed by #129231

Just to be clear, #129231 did not change anything on that behaviour. It just made bootstrap to checkout llvm earlier during config parse so we get the correct result on "did the llvm-project changed?" logic. Just tested it on 37d56da, it fetches the submodule when it's not synced. So the "avoid cloning if not changed" thing did not really work all the time.

Using true to restore that behaviour (and making it the default) would be a huge mistake

I don't really think ending up with a huge mistake is possible with this PR. We have two options, and neither of them would lead to a a huge mistake.

With the other option (using if-unchanged to check out only if the submodule is initialized), there will initially be no llvm submodule to modify. To modify LLVM, developers would need to either temporarily set download-ci-llvm to false to fetch the submodule and then switch back to if-unchanged, or run a git command. Given this perspective, setting download-ci-llvm=true as the default seems to be the best option here. Since needing to modify LLVM is a rare case (as you mentioned in the Zulip thread), people can generally use download-ci-llvm=if-unchanged for their own build configurations. In fact, we may consider creating a specific default profile for working on the LLVM submodule similar to the codegen profile we had before.