Skip to content

Remove check run bootstrap #142499

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged

Conversation

Shourya742
Copy link
Contributor

This PR migrates all usage of check_run to new execution context api's.

r? @Kobzol

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Jun 14, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jun 14, 2025

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

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

@rust-log-analyzer

This comment has been minimized.

@Shourya742 Shourya742 force-pushed the 2025-06-14-remove-check-run-bootstrap branch from a750def to c9eeeb4 Compare June 14, 2025 13:02
Copy link
Contributor

@Kobzol Kobzol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice ❤️ Maybe we don't need run_always() everywhere, but I'd rather reproduce the old behavior faithfully, we can always optimize dry run later.

@@ -1407,26 +1408,23 @@ impl Config {
git.arg(relative_path);
git
};
if !self.check_run(&mut update(true)) {
self.check_run(&mut update(false));
if !update(true).allow_failure().run(self) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

run_always

"HEAD",
]));
let has_local_modifications =
!submodule_git().allow_failure().args(["diff-index", "--quiet", "HEAD"]).run(self);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

if has_local_modifications {
self.check_run(submodule_git().args(["stash", "push"]));
submodule_git().allow_failure().args(["stash", "push"]).run(self);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

}

self.check_run(submodule_git().args(["reset", "-q", "--hard"]));
self.check_run(submodule_git().args(["clean", "-qdfx"]));
submodule_git().allow_failure().args(["reset", "-q", "--hard"]).run(self);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same


if has_local_modifications {
self.check_run(submodule_git().args(["stash", "pop"]));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

@@ -255,7 +244,7 @@ impl Config {
curl.arg("--retry-all-errors");
}
curl.arg(url);
if !self.check_run(&mut curl) {
if !curl.run(self) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

@Shourya742
Copy link
Contributor Author

@Kobzol , if we look at the implementation of the check_run method, we can see that it skips method execution when a dry run is set and run_always is not specified. By explicitly setting run_always for the commands in this PR, won’t it cause these commands to run even in dry run mode?

@Kobzol
Copy link
Contributor

Kobzol commented Jun 15, 2025

Sorry, I only saw the check_run function, and missed that there is also a method with the same name that does actually check dry_run. You're absolutely right!

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Jun 15, 2025

📌 Commit c9eeeb4 has been approved by Kobzol

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 15, 2025
bors added a commit that referenced this pull request Jun 16, 2025
Rollup of 10 pull requests

Successful merges:

 - #133952 (Remove wasm legacy abi)
 - #134661 (Reduce precedence of expressions that have an outer attr)
 - #141769 (Move metadata object generation for dylibs to the linker code )
 - #141937 (Report never type lints in dependencies)
 - #142347 (Async drop - fix for StorageLive/StorageDead codegen for pinned future)
 - #142389 (Apply ABI attributes on return types in `rustc_codegen_cranelift`)
 - #142470 (Add some missing mailmap entries)
 - #142481 (Add `f16` inline asm support for LoongArch)
 - #142499 (Remove check run bootstrap)
 - #142543 (Suggest adding semicolon in user code rather than macro impl details)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit b83fb80 into rust-lang:master Jun 16, 2025
10 checks passed
@rustbot rustbot added this to the 1.89.0 milestone Jun 16, 2025
rust-timer added a commit that referenced this pull request Jun 16, 2025
Rollup merge of #142499 - Shourya742:2025-06-14-remove-check-run-bootstrap, r=Kobzol

Remove check run bootstrap

This PR migrates all usage of check_run to new execution context api's.

r? `@Kobzol`
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants