Skip to content

Port streaming commands in bootstrap to BootstrapCommand and remove as_command_mut #143354

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

Shourya742
Copy link
Contributor

This PR adds streaming capabilities to BootstrapCommand and migrate existing command streaming scenario's used in bootstrap.

r? @Kobzol

@rustbot rustbot added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Jul 3, 2025
@rust-log-analyzer

This comment has been minimized.

Copy link
Member

@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.

Thanks! Left a few comments.

@Shourya742 Shourya742 force-pushed the 2025-07-03-bye-bye-as_mut-command branch from 57f20c5 to 868ef64 Compare July 4, 2025 12:49
@rust-log-analyzer

This comment has been minimized.


/// Spawn the command in background, while capturing and returning stdout, and printing stderr.
#[track_caller]
pub fn stream_capture_stdout(
Copy link
Member

Choose a reason for hiding this comment

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

We only need stream_capture_stdout right now, right? So we can remove stream_capture. Reading both stdout and stderr in a streaming fashion while avoiding deadlocks is relatively complicated anyway, and we don't need it currently in bootstrap.

Copy link
Member

Choose a reason for hiding this comment

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

Looks like the stream_capture method is still there?

@Shourya742 Shourya742 marked this pull request as ready for review July 4, 2025 15:25
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 4, 2025
@rust-log-analyzer

This comment has been minimized.

@Shourya742 Shourya742 force-pushed the 2025-07-03-bye-bye-as_mut-command branch from 64ac851 to 03c7625 Compare July 4, 2025 16:03
@Kobzol Kobzol changed the title Bye bye as command mut Port streaming commands in bootstrap to BootstrapCommand and remove as_command_mut Jul 4, 2025
@Shourya742 Shourya742 force-pushed the 2025-07-03-bye-bye-as_mut-command branch 2 times, most recently from ee110a1 to 216ef55 Compare July 5, 2025 05:58
@Shourya742 Shourya742 force-pushed the 2025-07-03-bye-bye-as_mut-command branch from 216ef55 to c4fae7c Compare July 5, 2025 06:03
@rust-log-analyzer

This comment has been minimized.

@Shourya742 Shourya742 force-pushed the 2025-07-03-bye-bye-as_mut-command branch from c4fae7c to 536d15a Compare July 5, 2025 06:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants