-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
base: master
Are you sure you want to change the base?
Conversation
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.
3a07dd5
to
e44dab5
Compare
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 }), | ||
}) |
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.
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
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.
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, |
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 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)
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.
We don't add things to the changelog in PRs.
Currently, you can turn a
std::process::Command
into atokio::process::Command
, however, if you have astd::process::Child
there 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
tokio
feature might look like when I hit a bit of a snag. I can't find a reliable way to convert astd::process:Child
into 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
Command
and&mut Command
. If I want to add a feature flaggedasync fn spawn
fortokio::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::Child
and 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::spawn
is called, after the process is spawned.