Skip to content

Add central execution context to bootstrap #141909

New issue

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

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

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
Jun 10, 2025

Conversation

Shourya742
Copy link
Contributor

This PR continues the effort toward command centralization as outlined in #126819. It introduces a centralized execution context through which all commands will be executed. Previously, centralization was limited to build methods; this PR extends it to the config module and updates the remaining methods accordingly.

Best reviewed commit by commit.

r? @Kobzol

@rustbot rustbot added A-bootstrap-stamp Area: bootstrap stamp logic 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-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Jun 2, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jun 2, 2025

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

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

This PR changes how GCC is built. Consider updating src/bootstrap/download-ci-gcc-stamp.

This PR changes how LLVM is built. Consider updating src/bootstrap/download-ci-llvm-stamp.

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.

This is awesome ❤️ I wanted to do this for a long time, and probably have like 3 unfinished branches with something like this locally 😆 Left some comments.

use crate::{BehaviorOnFailure, BootstrapCommand, CommandOutput, OutputMode, RefCell, exit};

#[derive(Clone)]
pub struct ExecutionContext {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please add some doc comment on top of this that would explain what is it for? :)

@@ -422,6 +423,8 @@ pub struct Config {

/// Cache for determining path modifications
pub path_modification_cache: Arc<Mutex<HashMap<Vec<&'static str>, PathFreshness>>>,

pub execution_context: ExecutionContext,
Copy link
Contributor

@Kobzol Kobzol Jun 2, 2025

Choose a reason for hiding this comment

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

Just noting to myself: the execution context shouldn't be stored inside Config; I would rather put it into the Builder.

For the migration, storing it inside Config is of course the easiest. I wonder how hard it would be to put it inside the builder already in this PR?

@Kobzol
Copy link
Contributor

Kobzol commented Jun 2, 2025

Btw you probably know this, but looking for usages of as_command_mut is a good source of figuring out what other old-school command invocations we'd ideally like to remove.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@Shourya742 Shourya742 force-pushed the 2025-06-01-add-execution-context branch from 8f7b375 to 8aed926 Compare June 3, 2025 18:54
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@Shourya742 Shourya742 force-pushed the 2025-06-01-add-execution-context branch from a4b29b5 to 8b9bd93 Compare June 4, 2025 06:02
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@Kobzol
Copy link
Contributor

Kobzol commented Jun 4, 2025

Looks like the command invocation from the error is swallowing errors. Not sure what/where fails.

@bors
Copy link
Collaborator

bors commented Jun 5, 2025

☔ The latest upstream changes (presumably #142070) made this pull request unmergeable. Please resolve the merge conflicts.

@Shourya742 Shourya742 force-pushed the 2025-06-01-add-execution-context branch from 576f422 to 587c691 Compare June 8, 2025 08:20
@Shourya742 Shourya742 requested a review from Kobzol June 9, 2025 12:05
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.

Left a few additional comments. I think that if Config::parse created the execuion context from the passed flags, rather than receiving it as a parameter, the diff of the PR could be reduced quite a lot, because the tests wouldn't need to change.

@Shourya742 Shourya742 force-pushed the 2025-06-01-add-execution-context branch from 587c691 to 51fbd14 Compare June 9, 2025 15:45
@Kobzol
Copy link
Contributor

Kobzol commented Jun 10, 2025

Thank you! I agree, let's land like this and we can make further refactorings in follow-up PRs.

@bors r+

@bors
Copy link
Collaborator

bors commented Jun 10, 2025

📌 Commit 51fbd14 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 10, 2025
fmease added a commit to fmease/rust that referenced this pull request Jun 10, 2025
…n-context, r=Kobzol

Add central execution context to bootstrap

This PR continues the effort toward command centralization as outlined in rust-lang#126819. It introduces a centralized execution context through which all commands will be executed. Previously, centralization was limited to build methods; this PR extends it to the `config` module and updates the remaining methods accordingly.

Best reviewed commit by commit.

r? `@Kobzol`
bors added a commit that referenced this pull request Jun 10, 2025
Rollup of 14 pull requests

Successful merges:

 - #134442 (Specify the behavior of `file!`)
 - #134841 (Look at proc-macro attributes when encountering unknown attribute)
 - #140372 (Exhaustively handle parsed attributes in CheckAttr)
 - #140766 (Stabilize keylocker)
 - #141642 (Note the version and PR of removed features when using it)
 - #141909 (Add central execution context to bootstrap)
 - #141992 (use `#[naked]` for `__rust_probestack`)
 - #142102 (docs: Small clarification on the usage of read_to_string and read_to_end trait methods)
 - #142124 (Allow transmute casts in pre-runtime-MIR)
 - #142240 (deduplicate the rest of AST walker functions)
 - #142258 (platform-support.md: Mention specific Linux kernel version or later)
 - #142262 (Mark `core::slice::memchr` as `#[doc(hidden)]`)
 - #142271 (compiler: fn ptrs should hit different lints based on ABI)
 - #142288 (const_eval: fix some outdated comments)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit that referenced this pull request Jun 10, 2025
Rollup of 16 pull requests

Successful merges:

 - #134442 (Specify the behavior of `file!`)
 - #140372 (Exhaustively handle parsed attributes in CheckAttr)
 - #140766 (Stabilize keylocker)
 - #141642 (Note the version and PR of removed features when using it)
 - #141818 (Don't create .msi installer for gnullvm hosts)
 - #141909 (Add central execution context to bootstrap)
 - #141992 (use `#[naked]` for `__rust_probestack`)
 - #142101 (core::ptr: deduplicate more method docs)
 - #142102 (docs: Small clarification on the usage of read_to_string and read_to_end trait methods)
 - #142124 (Allow transmute casts in pre-runtime-MIR)
 - #142240 (deduplicate the rest of AST walker functions)
 - #142258 (platform-support.md: Mention specific Linux kernel version or later)
 - #142262 (Mark `core::slice::memchr` as `#[doc(hidden)]`)
 - #142271 (compiler: fn ptrs should hit different lints based on ABI)
 - #142275 (rustdoc: Refractor `clean_ty_generics`)
 - #142288 (const_eval: fix some outdated comments)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit b9a578e into rust-lang:master Jun 10, 2025
10 checks passed
@rustbot rustbot added this to the 1.89.0 milestone Jun 10, 2025
rust-timer added a commit that referenced this pull request Jun 10, 2025
Rollup merge of #141909 - Shourya742:2025-06-01-add-execution-context, r=Kobzol

Add central execution context to bootstrap

This PR continues the effort toward command centralization as outlined in #126819. It introduces a centralized execution context through which all commands will be executed. Previously, centralization was limited to build methods; this PR extends it to the `config` module and updates the remaining methods accordingly.

Best reviewed commit by commit.

r? ``@Kobzol``
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-bootstrap-stamp Area: bootstrap stamp logic A-testsuite Area: The testsuite used to check the correctness of rustc 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