Don't clone LLVM submodule when download-ci-llvm is set

[jyn514]

Previously, downloading_llvm would check self.build while it was
still an empty string, and think it was always false. This fixes the
check.

This addresses the worst part of #76653. There are still some large submodules being downloaded (in particular, rustc-by-example is 146 MB, and all the submodules combined are 311 MB), but this is a lot better than the whopping 1.4 GB before.

[rust-highfive]

r? @Mark-Simulacrum

(rust-highfive has picked a reviewer for you, use r? to override)

[Mark-Simulacrum]

Hm, do we need to be careful around LLVM submodule being out of date locally perhaps as well? I am worried it'll be a pain point for contributors that already have it checked out and now x.py will not update it for them. But maybe we handle that case?

[jyn514]

I am worried it'll be a pain point for contributors that already have it checked out and now x.py will not update it for them.

When would they need to use src/llvm-project when downloading CI llvm? If you're changing anything there, I'd expect you to be building LLVM from source.

[jyn514]

Also, if they really do need the old behavior, they can always use git submodule update src/llvm-project.

[Mark-Simulacrum]

I'm saying they don't need it but already have it checked out. If they're used to x.py keeping submodules up to date, then figuring out how to avoid the change to the submodule is going to be difficult for most people. Can we make sure that we keep it updated if it exists, even if download-llvm is set?

[jyn514]

Ok, I see, because git will start marking it as out of date. Sure, seems reasonable. You actually added that when you wrote the feature originally, I don't think it needs further changes:

rust/src/bootstrap/bootstrap.py

Lines 935 to 939 in db115f1

	# not being built. Also, if the submodule has been initialized
	# already, sync it anyways so that it doesn't mess up contributor
	# pull requests.
	if external_llvm_provided or not llvm_needed:
	if self.get_toml('lld') != 'true' and not llvm_checked_out:

[nagisa]

When would they need to use src/llvm-project when downloading CI llvm?

Reading the source code is a quite relevant use-case here. Maybe introduce another option and/or value for this behaviour so that we don't need to guess?

[jyn514]

@nagisa right, but you can do that with git submodule update anyway. My question is why would you want it to be automatic (and add another button to toggle) when you can run the command once and have it updated going forward?

[Mark-Simulacrum]

Heh, ok, sounds like past me did the right thing already :)

@bors r+

[bors]

📌 Commit db115f1 has been approved by Mark-Simulacrum