Download GCC from CI on test builders #138395

Kobzol · 2025-03-12T11:03:48Z

This should reduce the duration of the x86_64-gnu-llvm-18 job, which runs on PR CI, which is currently the only one that builds GCC (outside of the x64 dist builder).

Since we handle the GCC download in the GCC step, and not eagerly in config, we can set this flag globally across all test builders, as it won't do anything unless they actually try to build GCC.

Opening as a draft to test if it works on CI, because I still need to implement logic to avoid the download if there are any local modifications to GCC (essentially the "if-unchanged" mode, although I want to try something a bit different).

r? @ghost

Kobzol · 2025-03-12T11:04:25Z

The LLVM 18 PR CI job took ~60 minutes before, let's see if this helps.

Kobzol · 2025-03-12T16:26:15Z

@bors try

Download GCC from CI on test builders This should reduce the duration of the `x86_64-gnu-llvm-18` job, which runs on PR CI, which is currently the only one that builds GCC (outside of the x64 dist builder). Since we handle the GCC download in the GCC step, and not eagerly in config, we can set this flag globally across all test builders, as it won't do anything unless they actually try to build GCC. Opening as a draft to test if it works on CI, because I still need to implement logic to avoid the download if there are any local modifications to GCC (essentially the "if-unchanged" mode, although I want to try something a bit different). r? `@ghost`

bors · 2025-03-12T16:27:27Z

⌛ Trying commit 293a246 with merge de4ba24...

bors · 2025-03-12T18:54:51Z

☀️ Try build successful - checks-actions
Build commit: de4ba24 (de4ba246353c1d095c81f3c37159eaec194f7829)

Build GCC on CI with GCC, not Clang It seems that GCC built with Clang misbehaves. Prerequisite for rust-lang#138395. r? `@ghost`

…meGomez Build GCC on CI with GCC, not Clang It seems that GCC built with Clang misbehaves. I have tested that cg_gcc tests [pass](https://github.com/rust-lang/rust/actions/runs/13842365913/job/38732750617?pr=138451) on CI with a downloaded GCC that was built in this way. Prerequisite for rust-lang#138395. r? `@ghost`

…meGomez Build GCC on CI with GCC, not Clang It seems that GCC built with Clang misbehaves. I have tested that cg_gcc tests [pass](https://github.com/rust-lang/rust/actions/runs/13842365913/job/38732750617?pr=138451) on CI with a downloaded GCC that was built in this way. Prerequisite for rust-lang#138395. r? ``@ghost``

…meGomez Build GCC on CI with GCC, not Clang It seems that GCC built with Clang misbehaves. I have tested that cg_gcc tests [pass](https://github.com/rust-lang/rust/actions/runs/13842365913/job/38732750617?pr=138451) on CI with a downloaded GCC that was built in this way. Prerequisite for rust-lang#138395. r? ```@ghost```

Rollup merge of rust-lang#138451 - Kobzol:gcc-ci-build-gcc, r=GuillaumeGomez Build GCC on CI with GCC, not Clang It seems that GCC built with Clang misbehaves. I have tested that cg_gcc tests [pass](https://github.com/rust-lang/rust/actions/runs/13842365913/job/38732750617?pr=138451) on CI with a downloaded GCC that was built in this way. Prerequisite for rust-lang#138395. r? ```@ghost```

Kobzol · 2025-03-15T15:09:32Z

Good, the x86_64-gnu-llvm-18 is now 5 minutes quicker, because it downloads GCC rather than building it. Now I just need to implement the behavior of "if-unchanged".

Refactor git change detection in bootstrap While working on rust-lang#138395, I finally found the courage to delve into the insides of git path change detection in bootstrap, which is used (amongst other things) to detect if we should rebuilt od download `[llvm|rustc|gcc]`. I found it a bit hard to understand, and given that this code was historically quite fragile, I thought that it would be better to rebuild it from scratch. The previous approach had a bunch of limitations: - It separated the computation of "are there local changes?" and "what upstream SHA should we use?" even though these two things are intertwined. - It used hacks to work around what happens on CI. - It had special cases for CI scattered throughout the codebase, rather than centralized in one place. - It wasn't documented enough and didn't have tests for the git behavior. The current approach should hopefully resolve all of that. I implemented a single entrypoint called `check_path_modifications` (naming bikeshed pending, half of the time I spend on this PR was thinking about names, as it's quite tricky here..) that explicitly receives a mode of operation (in CI or outside CI), and accordingly figures out that upstream SHA that we should use for downloading artifacts and it also figures out if there are any local changes. Users of this function can then use this unified output to implement `download-ci-X` and other functionality. Notably, this change detection no longer uses `git merge-base`, which makes it easier to use and doesn't require setting up remotes. I also added a bunch of integration tests that literally spawn a git repository on disk and then check that the function can deal with various situations (PR CI, auto/try CI, local builds). After I built this inner layer, I used it for downloading GCC, LLVM and rustc. The latter two (and especially rustc) were using the `last_modified_commit` function before, but in all cases but one this function was actually only used to check if there are any local changes, which was IMO confusing. The LLVM handling would deserve a bit of refactoring, but that's a larger change that can be done as a follow-up. I hope that the implementation is now clear and easy to understand, so that in combination with the tests we can have more confidence that it does what we want. I tried to include a lot of documentation in the code, so I won't be repeating the actual implementation details here, if there are any questions, I'll add the answers to the documentation too :) The new approach explicitly supports three scenarios: - Running on PR CI, where we have one upstream bors parent commit and one PR merge commit made by GitHub. - Running on try/auto CI, where we have one upstream bors parent commit and one PR merge commit made by bors. - Running locally, where we assume that we have at least one upstream bors parent commit in our git history. I removed the handling of upstreams on CI, as I think that it shouldn't be needed and I considered it to be a hack. However, it's possible that there are other use-cases that I haven't considered, so I want to ask around if people have other situations than the three use-cases described above. If there are other such use-cases, I would like to include them in the new centralized implementation and add them to the git test suite, rather than going back to the old ways :) In particular, the code before relied on `git merge-base`, but I don't see why we can't just lookup the most recent bors commit and assume that is a merge commit that is also upstream? I might be running into Chesterton's Fence here :) CC `@pietroalbini` To make sure that this won't break downstream users of Rust's CI. Best reviewed commit by commit. Companion PRs: - For testing beta: rust-lang#138597 r? `@onur-ozkan` Fixes: rust-lang#101907 try-job: x86_64-gnu-aux try-job: aarch64-gnu try-job: dist-x86_64-apple

rustbot · 2025-04-23T10:50:30Z

This PR modifies bootstrap.example.toml.

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

Kobzol · 2025-04-23T10:50:46Z

Great, the download seems to be working fine now!

@rustbot ready

r? @Mark-Simulacrum

Kobzol · 2025-04-23T10:51:30Z

src/ci/run.sh

@@ -183,6 +183,9 @@ else
    RUST_CONFIGURE_ARGS="$RUST_CONFIGURE_ARGS --set llvm.static-libstdcpp"
  fi

+  # Download GCC from CI on test builders
+  RUST_CONFIGURE_ARGS="$RUST_CONFIGURE_ARGS --set gcc.download-ci-gcc=true"


Note that for GCC, the download logic is simplified, as I'm not sure if we need if-unchanged. true means: "if there are changes, build, if there are no changes, download".

Yeah, that seems okay. IIRC for llvm we added true to avoid jumping into a build unexpectedly with e.g. accidental submodule updates. But seems okay for here.

Refactor git change detection in bootstrap While working on rust-lang/rust#138395, I finally found the courage to delve into the insides of git path change detection in bootstrap, which is used (amongst other things) to detect if we should rebuilt od download `[llvm|rustc|gcc]`. I found it a bit hard to understand, and given that this code was historically quite fragile, I thought that it would be better to rebuild it from scratch. The previous approach had a bunch of limitations: - It separated the computation of "are there local changes?" and "what upstream SHA should we use?" even though these two things are intertwined. - It used hacks to work around what happens on CI. - It had special cases for CI scattered throughout the codebase, rather than centralized in one place. - It wasn't documented enough and didn't have tests for the git behavior. The current approach should hopefully resolve all of that. I implemented a single entrypoint called `check_path_modifications` (naming bikeshed pending, half of the time I spend on this PR was thinking about names, as it's quite tricky here..) that explicitly receives a mode of operation (in CI or outside CI), and accordingly figures out that upstream SHA that we should use for downloading artifacts and it also figures out if there are any local changes. Users of this function can then use this unified output to implement `download-ci-X` and other functionality. Notably, this change detection no longer uses `git merge-base`, which makes it easier to use and doesn't require setting up remotes. I also added a bunch of integration tests that literally spawn a git repository on disk and then check that the function can deal with various situations (PR CI, auto/try CI, local builds). After I built this inner layer, I used it for downloading GCC, LLVM and rustc. The latter two (and especially rustc) were using the `last_modified_commit` function before, but in all cases but one this function was actually only used to check if there are any local changes, which was IMO confusing. The LLVM handling would deserve a bit of refactoring, but that's a larger change that can be done as a follow-up. I hope that the implementation is now clear and easy to understand, so that in combination with the tests we can have more confidence that it does what we want. I tried to include a lot of documentation in the code, so I won't be repeating the actual implementation details here, if there are any questions, I'll add the answers to the documentation too :) The new approach explicitly supports three scenarios: - Running on PR CI, where we have one upstream bors parent commit and one PR merge commit made by GitHub. - Running on try/auto CI, where we have one upstream bors parent commit and one PR merge commit made by bors. - Running locally, where we assume that we have at least one upstream bors parent commit in our git history. I removed the handling of upstreams on CI, as I think that it shouldn't be needed and I considered it to be a hack. However, it's possible that there are other use-cases that I haven't considered, so I want to ask around if people have other situations than the three use-cases described above. If there are other such use-cases, I would like to include them in the new centralized implementation and add them to the git test suite, rather than going back to the old ways :) In particular, the code before relied on `git merge-base`, but I don't see why we can't just lookup the most recent bors commit and assume that is a merge commit that is also upstream? I might be running into Chesterton's Fence here :) CC `@pietroalbini` To make sure that this won't break downstream users of Rust's CI. Best reviewed commit by commit. Companion PRs: - For testing beta: rust-lang/rust#138597 r? `@onur-ozkan` Fixes: rust-lang/rust#101907 try-job: x86_64-gnu-aux try-job: aarch64-gnu try-job: dist-x86_64-apple

Mark-Simulacrum

r=me

Mark-Simulacrum · 2025-04-27T15:05:42Z

src/ci/run.sh

@@ -183,6 +183,9 @@ else
    RUST_CONFIGURE_ARGS="$RUST_CONFIGURE_ARGS --set llvm.static-libstdcpp"
  fi

+  # Download GCC from CI on test builders
+  RUST_CONFIGURE_ARGS="$RUST_CONFIGURE_ARGS --set gcc.download-ci-gcc=true"


Yeah, that seems okay. IIRC for llvm we added true to avoid jumping into a build unexpectedly with e.g. accidental submodule updates. But seems okay for here.

Mark-Simulacrum · 2025-04-27T19:22:01Z

@bors r+

bors · 2025-04-27T19:22:04Z

📌 Commit d4011ae has been approved by Mark-Simulacrum

It is now in the queue for this repository.

…ulacrum Download GCC from CI on test builders This should reduce the duration of the `x86_64-gnu-llvm-18` job, which runs on PR CI, which is currently the only one that builds GCC (outside of the x64 dist builder). Since we handle the GCC download in the GCC step, and not eagerly in config, we can set this flag globally across all test builders, as it won't do anything unless they actually try to build GCC. Opening as a draft to test if it works on CI, because I still need to implement logic to avoid the download if there are any local modifications to GCC (essentially the "if-unchanged" mode, although I want to try something a bit different). r? `@ghost`

…enton Rollup of 10 pull requests Successful merges: - rust-lang#136160 (Remove backticks from `ShouldPanic::YesWithMessage`'s `TrFailedMsg`) - rust-lang#138395 (Download GCC from CI on test builders) - rust-lang#138737 (uefi: Update r-efi) - rust-lang#138939 (Add `Arc::is_unique`) - rust-lang#139224 (fix(test): Expose '--no-capture' in favor of `--nocapture`) - rust-lang#139546 (std(docs): clarify how std::fs::set_permisions works with symlinks) - rust-lang#139883 (crashes: more tests) - rust-lang#140345 (Avoid re-interning in `LateContext::get_def_path`) - rust-lang#140351 (docs: fix incorrect stability markers on `std::{todo, matches}`) - rust-lang#140359 (specify explicit safety guidance for from_utf8_unchecked) r? `@ghost` `@rustbot` modify labels: rollup

…ulacrum Download GCC from CI on test builders This should reduce the duration of the `x86_64-gnu-llvm-18` job, which runs on PR CI, which is currently the only one that builds GCC (outside of the x64 dist builder). Since we handle the GCC download in the GCC step, and not eagerly in config, we can set this flag globally across all test builders, as it won't do anything unless they actually try to build GCC. Opening as a draft to test if it works on CI, because I still need to implement logic to avoid the download if there are any local modifications to GCC (essentially the "if-unchanged" mode, although I want to try something a bit different). r? ``@ghost``

…enton Rollup of 9 pull requests Successful merges: - rust-lang#138395 (Download GCC from CI on test builders) - rust-lang#138737 (uefi: Update r-efi) - rust-lang#138939 (Add `Arc::is_unique`) - rust-lang#139224 (fix(test): Expose '--no-capture' in favor of `--nocapture`) - rust-lang#139546 (std(docs): clarify how std::fs::set_permisions works with symlinks) - rust-lang#139883 (crashes: more tests) - rust-lang#140345 (Avoid re-interning in `LateContext::get_def_path`) - rust-lang#140351 (docs: fix incorrect stability markers on `std::{todo, matches}`) - rust-lang#140359 (specify explicit safety guidance for from_utf8_unchecked) r? `@ghost` `@rustbot` modify labels: rollup

…enton Rollup of 8 pull requests Successful merges: - rust-lang#138395 (Download GCC from CI on test builders) - rust-lang#138737 (uefi: Update r-efi) - rust-lang#138939 (Add `Arc::is_unique`) - rust-lang#139224 (fix(test): Expose '--no-capture' in favor of `--nocapture`) - rust-lang#139546 (std(docs): clarify how std::fs::set_permisions works with symlinks) - rust-lang#140345 (Avoid re-interning in `LateContext::get_def_path`) - rust-lang#140351 (docs: fix incorrect stability markers on `std::{todo, matches}`) - rust-lang#140359 (specify explicit safety guidance for from_utf8_unchecked) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#138395 - Kobzol:ci-download-gcc, r=Mark-Simulacrum Download GCC from CI on test builders This should reduce the duration of the `x86_64-gnu-llvm-18` job, which runs on PR CI, which is currently the only one that builds GCC (outside of the x64 dist builder). Since we handle the GCC download in the GCC step, and not eagerly in config, we can set this flag globally across all test builders, as it won't do anything unless they actually try to build GCC. Opening as a draft to test if it works on CI, because I still need to implement logic to avoid the download if there are any local modifications to GCC (essentially the "if-unchanged" mode, although I want to try something a bit different). r? ```@ghost```

rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Mar 12, 2025

This comment was marked as outdated.

Sign in to view

Kobzol force-pushed the ci-download-gcc branch from 07d7fde to 70a372a Compare March 12, 2025 11:15