Skip to content

Implement TryFrom<std::process::Child> for tokio::process::Child #7388

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 1 commit into
base: master
Choose a base branch
from

Conversation

schneems
Copy link

@schneems schneems commented Jun 5, 2025

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.

Motivation

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.

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 when tokio::process::Command::spawn is called, after the process is spawned.

@schneems schneems marked this pull request as draft June 5, 2025 22:00
@schneems schneems force-pushed the schneems/standard_command_child branch from 4f0ff19 to 3a07dd5 Compare June 5, 2025 22:01
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 schneems force-pushed the schneems/standard_command_child branch from 3a07dd5 to e44dab5 Compare June 5, 2025 22:29
@schneems schneems marked this pull request as ready for review June 5, 2025 23:03
@Darksonn Darksonn added A-tokio Area: The main tokio crate M-process Module: tokio/process labels Jun 8, 2025
@Darksonn Darksonn requested a review from ipetkov June 8, 2025 18:55
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 }),
})
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

Whoops we can't call Command::build_child because it takes self, but effectively we should DRY these with a common helper

Ok(Child {
child: FusedChild::Child(ChildDropGuard {
inner: spawned_child.child,
kill_on_drop: false,
Copy link
Member

Choose a reason for hiding this comment

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

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)

Copy link
Contributor

Choose a reason for hiding this comment

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

We don't add things to the changelog in PRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate M-process Module: tokio/process
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants