[do not merge] beta test for git change detection (#138591) #138597
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
(cherry picked from commit a6ee2f4)
[beta] Prepare Rust 1.86.0 This includes one backport: - fix musl's CVE-2025-26519 rust-lang#137127 r? cuviper
As an i586 target, it should not have SSE. This caused the following warning to be emitted: ``` warning: target feature `sse2` must be enabled to ensure that the ABI of the current target can be implemented correctly | = note: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! = note: for more information, see issue rust-lang#116344 <rust-lang#116344> warning: 1 warning emitted ``` (cherry picked from commit 1c66d5b)
[beta] stage0 update and a backport - bump stage0 to 1.85.0 - Remove SSE ABI from i586-pc-windows-msvc rust-lang#137149 r? cuviper
…nglo [beta-1.86] Update cargo 1 commits in ce948f4616e3d4277e30c75c8bb01e094910df39..fcb465caf719e68f05671947db75a66ca7fadb2a 2025-02-14 20:32:07 +0000 to 2025-02-23 08:30:05 -0800 - [beta-1.86] depend on openssl-sys to correctly pin its version (rust-lang/cargo#15226)
(cherry picked from commit 36c314c)
(cherry picked from commit 08b4f6d)
(cherry picked from commit 33e7f9b)
(cherry picked from commit 92f31b9)
(cherry picked from commit 35febd7)
Signed-off-by: onur-ozkan <work@onurozkan.dev> (cherry picked from commit d2203ad)
…ssions (cherry picked from commit bab03bf)
(cherry picked from commit 2797936)
(cherry picked from commit bad8e98)
Signed-off-by: onur-ozkan <work@onurozkan.dev> (cherry picked from commit e4ca11f)
(cherry picked from commit 25617c7)
[beta] backports - Pass vendored sources from bootstrap to generate-copyright rust-lang#137020 - Fix `-win7-windows-msvc` target since 26eeac1* rust-lang#137270 - skip submodule updating logics on tarballs rust-lang#137338 - Improve behavior of `IF_LET_RESCOPE` around temporaries and place expressions rust-lang#137444 - downgrade bootstrap `cc` rust-lang#137460 - Remove latest Windows SDK from 32-bit CI rust-lang#137753 - [beta-1.86] Ensure we can package directories ending with '.rs' (rust-lang/cargo#15248) rust-lang#137842 r? cuviper
(cherry picked from commit 28d3fef)
(cherry picked from commit 8089fce)
(cherry picked from commit fbe0075)
(cherry picked from commit 5f575bc)
(cherry picked from commit e3117e6)
[beta] backports - Don't infer attributes of virtual calls based on the function body rust-lang#137669 - Revert "store ScalarPair via memset when one side is undef and the other side can be memset" rust-lang#137894 - Do not install rustup on Rust for Linux job rust-lang#137947 r? cuviper
(cherry picked from commit 0dfe2ae)
This comment has been minimized.
This comment has been minimized.
|
Can you make sure it doesn't break when CI rustc is in use? You can add bootstrap to allowed paths for if-unchanged logic. |
|
I'm not fully sure what are you asking, tbh 😅 It's not trivial to make this PR green, because I selectively cherry picked only a few commits, we are in the middle of the beta cycle, so getting a beta PR green is kind of difficult. I only added a log of the detected SHAs, and that's it :) The logic should work the same way for GCC, LLVM and rustc, the only difference is the set of modified paths, otherwise the logic is now unified. |
|
Ah, you want me to test a builder that should trigger |
Yes, that's right :) |
|
Hmm, I realized that there are way more changes in this PR than just bootstrap though, but anyway, I'll allowlist all. |
This comment has been minimized.
This comment has been minimized.
|
@bors try |
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Mar 18, 2025
[do not merge] beta test for git change detection (rust-lang#138591) Opening to test CI/bootstrap changes from rust-lang#138591. r? `@ghost` try-job: x86_64-gnu-aux
This comment has been minimized.
This comment has been minimized.
|
💔 Test failed - checks-actions |
|
@bors try |
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Mar 18, 2025
[do not merge] beta test for git change detection (rust-lang#138591) Opening to test CI/bootstrap changes from rust-lang#138591. r? `@ghost` try-job: x86_64-gnu-aux
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
💔 Test failed - checks-actions |
|
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Mar 18, 2025
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. 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). The tests are super fast and run in parallel, as they are currently in `build_helper` and not in `bootstrap`. 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. In the future we could cache the results of `check_path_modifications` to reduce the number of git invocations, but I don't think that it should be excessive even now. 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` try-job: x86_64-gnu-aux
promote-release
bot
force-pushed
the
beta
branch
from
7824ede to
2848101
Compare
|
☔ The latest upstream changes (presumably #138784) made this pull request unmergeable. Please resolve the merge conflicts. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Opening to test CI/bootstrap changes from #138591.
r? @ghost
try-job: x86_64-gnu-aux