Skip to content
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

Store all args in the unsupported Command implementation #123633

Merged
merged 2 commits into from
Apr 10, 2024

Conversation

bjorn3
Copy link
Member

@bjorn3 bjorn3 commented Apr 8, 2024

This allows printing them in the Debug impl as well as getting them again using the get_args() method. This allows programs that would normally spawn another process to more easily show which program they would have spawned if not for the fact that the target doesn't support spawning child processes without requiring intrusive changes to keep the args. For example rustc compiled to wasi will show the full linker invocation that would have been done.

This allows printing them in the Debug impl as well as getting them
again using the get_args() method. This allows programs that would
normally spawn another process to more easily show which program they
would have spawned if not for the fact that the target doesn't support
spawning child processes without requiring intrusive changes to keep the
args. For example rustc compiled to wasi will show the full linker
invocation that would have been done.
@rustbot
Copy link
Collaborator

rustbot commented Apr 8, 2024

r? @jhpratt

rustbot has assigned @jhpratt.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Apr 8, 2024
@jhpratt
Copy link
Member

jhpratt commented Apr 9, 2024

I was a little skeptical when I first saw this due to the significant increase in size of Command, even if it is only on certain platforms. Given your example of wasi and your background, I have no reason to believe that this PR would be a bad decision despite the additional overhead. For that reason, I have no objection to this PR landing.

@bors r+

@bors
Copy link
Contributor

bors commented Apr 9, 2024

📌 Commit bbd82ff has been approved by jhpratt

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 Apr 9, 2024
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Apr 9, 2024
…=jhpratt

Store all args in the unsupported Command implementation

This allows printing them in the Debug impl as well as getting them again using the get_args() method. This allows programs that would normally spawn another process to more easily show which program they would have spawned if not for the fact that the target doesn't support spawning child processes without requiring intrusive changes to keep the args. For example rustc compiled to wasi will show the full linker invocation that would have been done.
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 9, 2024
…llaumeGomez

Rollup of 6 pull requests

Successful merges:

 - rust-lang#123485 (macOS: Use `libc` definitions for copyfile)
 - rust-lang#123633 (Store all args in the unsupported Command implementation)
 - rust-lang#123638 (rustdoc: synthetic auto: filter out clauses from the implementor's ParamEnv)
 - rust-lang#123653 (Split `non_local_definitions` lint tests in separate test files)
 - rust-lang#123662 (Don't rely on upvars being assigned just because coroutine-closure kind is assigned)
 - rust-lang#123665 (Fix typo in `Future::poll()` docs)

r? `@ghost`
`@rustbot` modify labels: rollup
@GuillaumeGomez
Copy link
Member

I think it's the one which failed in #123667.

@bjorn3
Copy link
Member Author

bjorn3 commented Apr 9, 2024

I can't reproduce it locally, but it should be fixed now.

@bjorn3 bjorn3 added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 9, 2024
@jhpratt
Copy link
Member

jhpratt commented Apr 9, 2024

I wonder why that failed 🤔 Allowing the lint is certainly reasonable.

@bors r+

@bors
Copy link
Contributor

bors commented Apr 9, 2024

📌 Commit b4a395b has been approved by jhpratt

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 Apr 9, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 10, 2024
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#121884 (Port exit-code run-make test to use rust)
 - rust-lang#122200 (Unconditionally show update nightly hint on ICE)
 - rust-lang#123568 (Clean up tests/ui by removing `does-nothing.rs`)
 - rust-lang#123609 (Don't use bytepos offsets when computing semicolon span for removal)
 - rust-lang#123612 (Set target-abi module flag for RISC-V targets)
 - rust-lang#123633 (Store all args in the unsupported Command implementation)
 - rust-lang#123668 (async closure coroutine by move body MirPass refactoring)

Failed merges:

 - rust-lang#123701 (Only assert for child/parent projection compatibility AFTER checking that theyre coming from the same place)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit a3f10a4 into rust-lang:master Apr 10, 2024
11 checks passed
@rustbot rustbot added this to the 1.79.0 milestone Apr 10, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 10, 2024
Rollup merge of rust-lang#123633 - bjorn3:unsupported_command_data, r=jhpratt

Store all args in the unsupported Command implementation

This allows printing them in the Debug impl as well as getting them again using the get_args() method. This allows programs that would normally spawn another process to more easily show which program they would have spawned if not for the fact that the target doesn't support spawning child processes without requiring intrusive changes to keep the args. For example rustc compiled to wasi will show the full linker invocation that would have been done.
@bjorn3 bjorn3 deleted the unsupported_command_data branch April 11, 2024 11:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants