avoid updating LLVM submodule during bootstrap unit tests

[onur-ozkan]

To test this, make sure you don't have src/llvm-project fetched and then set llvm.download-ci-llvm=true and run x test bootstrap.

[rustbot]

r? @albertlarsan68

rustbot has assigned @albertlarsan68.
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

[rustbot]

This PR modifies src/bootstrap/src/core/config.

If appropriate, please update CONFIG_CHANGE_HISTORY in src/bootstrap/src/utils/change_tracker.rs.

[Kobzol]

Could we use a hard-coded config.toml file (string) for unit tests, to avoid sprinkling these cfg gates throughout bootstrap's codebase?

[onur-ozkan]

Could we use a hard-coded config.toml file (string) for unit tests, to avoid sprinkling these cfg gates throughout bootstrap's codebase?

We don't use a consistent configuration for all unit tests, which is the problem. You can find some examples in the config test module.

[Kobzol]

Hmm, yeah, that might better be refactored to share some default test configuration, but that's a bigger change.

You can r=me if CI is green. But CI checks out all submodules anyway, so I don't expect that this should break bootstrap tests. I tried to deinit all submodules locally and bootstrap tests were also green.

[onur-ozkan]

Hmm, yeah, that might better be refactored to share some default test configuration, but that's a bigger change.

I guess that can't help either as we intentionally set some options to test them.

@bors r=Kobzol

[bors]

📌 Commit 2af4636 has been approved by Kobzol

It is now in the queue for this repository.

[ehuss]

This seems to not work if you don't have the submodules already checked out. There are some steps that require submodules to exist. This PR causes failures like:

---- core::builder::tests::validate_path_remap stdout ----
warning: unable to check if origin/master is old due to error: No such file or directory (os error 2)
warning: origin/master is used to determine if files have been modified
warning: if it is not updated, this may cause files to be needlessly reformatted
submodule src/doc/book does not appear to be checked out, but it is required for this step
thread 'core::builder::tests::validate_path_remap' panicked at /Users/eric/Temp/z25/rust/src/tools/build_helper/src/util.rs:22:9:
status code: 1

To reproduce:

git submodule deinit --all --force
gh pr checkout 130306
./x test bootstrap

[onur-ozkan]

This seems to not work if you don't have the submodules already checked out. There are some steps that require submodules to exist. This PR causes failures like:

---- core::builder::tests::validate_path_remap stdout ----
warning: unable to check if origin/master is old due to error: No such file or directory (os error 2)
warning: origin/master is used to determine if files have been modified
warning: if it is not updated, this may cause files to be needlessly reformatted
submodule src/doc/book does not appear to be checked out, but it is required for this step
thread 'core::builder::tests::validate_path_remap' panicked at /Users/eric/Temp/z25/rust/src/tools/build_helper/src/util.rs:22:9:
status code: 1

To reproduce:

git submodule deinit --all --force
gh pr checkout 130306
./x test bootstrap

Hmm, I guess for now we should only ignore llvm-submodule as the others are necessary even for the unit test and dry-run builds.

@bors r-

[Kobzol]

Interesting, I did the same (just deinited the submodules after checking out this PR), and it worked for me 🤔

[onur-ozkan]

@rustbot ready

[Kobzol]

@bors r+

[bors]

📌 Commit f03bfb8 has been approved by Kobzol

It is now in the queue for this repository.