From<std::process::Command> ignores pre-configured values for stdin/stdout/stderr

[brooswajne]

Hello! Thanks for the great library, I'm finding it very useful in my current project however I think I've recently run into a bug when spawning an async command converted from a std::process::Command. This should be a simplified reproducible test case highlighting the problem:

async fn main() {
    let mut command = std::process::Command::new("echo");
    command.arg("hello world")
        .stdout(std::process::Stdio::null());
    async_process::Command::from(command)
        .status()
        .await
        .unwrap();
}

When running the above, you should see that hello world is printed to the console, even though we've explicitly said that Stdout should be redirected to Stdio::null().

As far as I can see, this is due to:

1. the implementation of spawn() defaults the value of stdin to Stdio::inherit():

   async-process/src/lib.rs

   Lines 971 to 983 in e84c3fd

   	pub fn spawn(&mut self) -> io::Result<Child> {
   	if !self.stdin {
   	self.inner.stdin(Stdio::inherit());
   	}
   	if !self.stdout {
   	self.inner.stdout(Stdio::inherit());
   	}
   	if !self.stderr {
   	self.inner.stderr(Stdio::inherit());
   	}
   	Child::new(self)
   	}

2. the implementation of from() marks stdin as not having been explicitly set (ie. stdin: false), even though it might have been explicitly set on the inner command:

   async-process/src/lib.rs

   Lines 1040 to 1051 in e84c3fd

   	impl From<std::process::Command> for Command {
   	fn from(inner: std::process::Command) -> Self {
   	Self {
   	inner,
   	stdin: false,
   	stdout: false,
   	stderr: false,
   	reap_on_drop: true,
   	kill_on_drop: false,
   	}
   	}
   	}

The work-around is to set .stdin()/.stdout()/.stderr() after converting from a sync command to an async command, but obviously this is a bit of a gotcha and isn't immediately obvious that it's necessary.

Ideally the solution would be to set stdout: true in the implementation of From<std::process::Command> when it's already been explicitly set for inner, I'm not sure if there is a way to know whether that's the case? Alternatively, maybe this behaviour just needs to be explicitly documented?
Or perhaps: why do we even need to handle the defaulting in this library? Isn't that going to be handled by std::process::Command anyways? The documentation does state that "By default, stdin, stdout and stderr are inherited from the parent.", so perhaps there isn't any reason to do so in this library?

I'm happy to implement any changes / fixes (including just docs improvements), but would just need guidance as per the above on what you think the right solution might be. Thanks!

[notgull]

Ideally the solution would be to set stdout: true in the implementation of From<std::process::Command> when it's already been explicitly set for inner, I'm not sure if there is a way to know whether that's the case? Alternatively, maybe this behaviour just needs to be explicitly documented?

If there is a way to detect if the values were set in the builder, I'm not aware of it. I need to do some reading, but from a quick glance the Stdio values are set for a reason, so I think adding docs would be the right call.

[kanpov]

Encountered this too, makes absolutely no sense to do it