tokio icon indicating copy to clipboard operation
tokio copied to clipboard

Add `tokio::process::Command::as_std_mut()` method.

Open Jisu-Woniu opened this issue 9 months ago • 5 comments

Is your feature request related to a problem? Please describe. Sometimes I want to access a mutable reference of std::process::Command out of the tokio counterpart, so I can access some unstable functions in std to update Command, but tokio only provides an as_std method returning a shared reference.

Describe the solution you'd like Add a tokio::process::Command::as_std_mut() method, maybe like:

pub fn as_std(&mut self) -> &mut StdCommand {
    &mut self.std
}

Describe alternatives you've considered Implement From<tokio::process::Command> for std::process::Command? But we may lose information such as kill_on_drop.

Implement tokio::process::Command::groups() method. Seems like a better solution, but I may want to use other functions as well in the future.

Additional context screenshot

Jisu-Woniu avatar Apr 30 '24 13:04 Jisu-Woniu

@ipetkov Thoughts on this?

Darksonn avatar Apr 30 '24 14:04 Darksonn

Hi @Jisu-Woniu we already have a From impl to convert std::process::Command to tokio::process::Command which should do what you are asking for!

For example:

let mut cmd = std::process::Command::new("echo");
cmd.args(["hello", "world"]).groups(groups);

tokio::process::Command::from(cmd)
  .kill_on_drop(true)
  .spawn()
  .expect("failed to start")
  .await
  .expect("failed to run");

ipetkov avatar May 01 '24 01:05 ipetkov

But I have a function taking a tokio &mut Command, and I'm trying to run .groups(xxx) on that.

So I was wondering: can I convert between tokio &mut Command to std &mut Command easily?

Jisu-Woniu avatar May 01 '24 02:05 Jisu-Woniu

Biggest concern is that we prevent ourselves from making other changes in the future due to backwards compatibility, but since we already have the From impl and as_std, I don't think that's very likely. @ipetkov Thoughts?

Darksonn avatar May 01 '24 09:05 Darksonn

My initial thought was that exposing as_std_mut() could allow the caller to make changes to the underlying Command without our version knowing. I thought we used to have logic whenever each method was called (particularly around stdio handling) but that doesn't seem to be the case (anymore?) since all the fd handling is done when we call spawn

Even if that wasn't the case, you could do pretty much anything you wanted to a std::process::Command and then use the From impl to convert it so we'd still be none-the-wiser. I guess exposing it could lead to less flexibility for us in the future, though these APIs have been stable for a few years without any major issues so it might not be a big deal...

(Another upside would be that callers can use the latest methods on std::process::Command without having to wait for tokio's MSRV to catch up or use --cfg tokio_unstable)

ipetkov avatar May 01 '24 23:05 ipetkov

I think it's okay to add this.

Darksonn avatar May 05 '24 14:05 Darksonn