-
Notifications
You must be signed in to change notification settings - Fork 13.4k
Add central execution context to bootstrap #141909
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
Add central execution context to bootstrap #141909
Conversation
This PR modifies If appropriate, please update 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. |
There was a problem hiding this 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 { |
There was a problem hiding this comment.
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, |
There was a problem hiding this comment.
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?
Btw you probably know this, but looking for usages of |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
8f7b375
to
8aed926
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
a4b29b5
to
8b9bd93
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Looks like the command invocation from the error is swallowing errors. Not sure what/where fails. |
☔ The latest upstream changes (presumably #142070) made this pull request unmergeable. Please resolve the merge conflicts. |
576f422
to
587c691
Compare
There was a problem hiding this 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.
…e methods via config
…ocation according to suggestions
…from the execution context, add getters and setters in the config, and update the tests and other relevant areas accordingly.
587c691
to
51fbd14
Compare
Thank you! I agree, let's land like this and we can make further refactorings in follow-up PRs. @bors r+ |
…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`
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
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
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``
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