Implement TryFrom<std::process::Child> for tokio::process::Child
#7388
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
schneems
force-pushed
the
schneems/standard_command_child
branch
from
4f0ff19 to
3a07dd5
Compare
Currently, you can turn a `std::process::Command` into a `tokio::process::Command`, however, if you have a `std::process::Child` there is no way to turn it into a `tokio::process::Child`. This provides a mechanism. ## Context The use case I'm after is this: I have a small crate for running commands in Rust. I was wondering what a `tokio` feature might look like when I hit a bit of a snag. I can't find a reliable way to convert a `std::process:Child` into something `await`-able. The primary interface is this trait https://docs.rs/fun_run/0.6.0/fun_run/trait.CommandWithName.html. It is implemented for `Command` and `&mut Command`. If I want to add a feature flagged `async fn spawn` for `tokio::process::Command`, then I also need a way to reproduce the same interface in my other already-existing implementations. My thought was that for the default implementation, I can produce a regular `std::process::Child` and then, if I can turn it into a `tokio::process::Child`. Then I can standardize on that interface.
schneems
force-pushed
the
schneems/standard_command_child
branch
from
3a07dd5 to
e44dab5
Compare
Darksonn
added
A-tokio
ipetkov
requested changes
Jun 8, 2025
Comment on lines
+1219
to
+1229
| let spawned_child = imp::build_child(value)?; | ||
|
|
||
| Ok(Child { | ||
| child: FusedChild::Child(ChildDropGuard { | ||
| inner: spawned_child.child, | ||
| kill_on_drop: false, | ||
| }), | ||
| stdin: spawned_child.stdin.map(|inner| ChildStdin { inner }), | ||
| stdout: spawned_child.stdout.map(|inner| ChildStdout { inner }), | ||
| stderr: spawned_child.stderr.map(|inner| ChildStderr { inner }), | ||
| }) |
Member
There was a problem hiding this comment.
Rather than duplicate all this I'd prefer if we turned this into a call to Command::build_child so that these implementations never drift
Member
There was a problem hiding this comment.
Whoops we can't call Command::build_child because it takes self, but effectively we should DRY these with a common helper
ipetkov
reviewed
Jun 8, 2025
| Ok(Child { | ||
| child: FusedChild::Child(ChildDropGuard { | ||
| inner: spawned_child.child, | ||
| kill_on_drop: false, |
Member
There was a problem hiding this comment.
This might end up being a foot gun, there's no way to specify kill_on_drop: true. Maybe instead of using TryFrom we should add a conversion method? That way we can take an extra parameter (or more easily deprecate it in the future if necessary)
Darksonn
reviewed
Jun 9, 2025
Contributor
There was a problem hiding this comment.
We don't add things to the changelog in PRs.
Member
|
Hi @schneems , are you still working on this PR? |
ADD-SP
added
the
S-waiting-on-author
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently, you can turn a
std::process::Commandinto atokio::process::Command, however, if you have astd::process::Childthere is no way to turn it into atokio::process::Child. This provides a mechanism.Motivation
I have a small crate for running commands in Rust. I was wondering what a
tokiofeature might look like when I hit a bit of a snag. I can't find a reliable way to convert astd::process:Childinto somethingawait-able.The primary interface is this trait https://docs.rs/fun_run/0.6.0/fun_run/trait.CommandWithName.html. It is implemented for
Commandand&mut Command. If I want to add a feature flaggedasync fn spawnfortokio::process::Command, then I also need a way to reproduce the same interface in my other already-existing implementations. My thought was that for the default implementation, I can produce a regularstd::process::Childand then, if I can turn it into atokio::process::Child. Then I can standardize on that interface.Solution
Implementation: The code is taken nearly wholesale from
tokio::process::build_child(&self, child: StdChild). It's pretty much the exact thing that happens whentokio::process::Command::spawnis called, after the process is spawned.