Skip to content
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

Implement Command::raw_arg on Windows #5810

Closed
m-haug opened this issue Jun 21, 2023 · 3 comments
Closed

Implement Command::raw_arg on Windows #5810

m-haug opened this issue Jun 21, 2023 · 3 comments
Labels
A-tokio Area: The main tokio crate C-feature-request Category: A feature request. M-process Module: tokio/process

Comments

@m-haug
Copy link

m-haug commented Jun 21, 2023

Is your feature request related to a problem? Please describe.
In watchexec/watchexec#613, it was discovered that CMD does not handle quoted arguments properly. This is known behaviour and led to the inclusion of a CommandExt::raw_arg method in std.

Describe the solution you'd like
For parity with std, it would be nice to implement Command::raw_arg on Windows.

Describe alternatives you've considered
As a workaround, users can create a StdCommand and convert this to tokio's Command using the provided From implementation. However, this means two additional imports (StdCommand and CommandExt), and might potentially confuse users. It's also not as nice to use since method chaining is no longer possible.

Additional context
I've included three example programs to demonstrate the issue when using tokio, a solution using only std, and the workaround described above.

Problematic tokio code:

use std::process::Output;

use tokio::process::Command;

#[tokio::main]
async fn main() {
    let output = Command::new("cmd.exe")
        .arg("/C")
        .arg(r#""C:\Users\Markus Haug\scoop\apps\tectonic\current\tectonic.exe" -X build --open"#)
        .output()
        .await;

    if let Ok(Output { stderr, .. }) = output {
        let stderr = String::from_utf8(stderr).unwrap();
        eprintln!("{stderr}")
    }
}

Pure std solution:

use std::os::windows::process::CommandExt;
use std::process::Command;
use std::process::Output;

#[tokio::main]
async fn main() {
    let output = Command::new("cmd.exe")
        .arg("/C")
        .raw_arg(
            r#""C:\Users\Markus Haug\scoop\apps\tectonic\current\tectonic.exe" -X build --open"#,
        )
        .output();

    if let Ok(Output { stderr, .. }) = output {
        let stderr = String::from_utf8(stderr).unwrap();
        eprintln!("{stderr}")
    }
}

Workaround using conversion from StdCommand:

use std::os::windows::process::CommandExt;
use std::process::Command as StdCommand;
use std::process::Output;

use tokio::process::Command as TokioCommand;

#[tokio::main]
async fn main() {
    let mut command = StdCommand::new("cmd.exe");
    command.arg("/C").raw_arg(
        r#""C:\Users\Markus Haug\scoop\apps\tectonic\current\tectonic.exe" -X build --open"#,
    );

    let output = TokioCommand::from(command).output().await;

    if let Ok(Output { stderr, .. }) = output {
        let stderr = String::from_utf8(stderr).unwrap();
        eprintln!("{stderr}")
    }
}
@m-haug m-haug added A-tokio Area: The main tokio crate C-feature-request Category: A feature request. labels Jun 21, 2023
korrat added a commit to korrat/watchexec that referenced this issue Jun 23, 2023
CMD uses special handling for arguments to passed to /C. Unfortunately,
this causes errors with quoted program names in these arguments.

This commit implements a workaround: When CMD is requested, we build a
std::process::Command and pass the the argument to /C using the special
std::os::windows::process::CommandExt::raw_arg method. The StdCommand is
then converted to a TokioCommand and returned.

Once tokio-rs/tokio#5810 is fixed, this workaround can be removed.

Fixes watchexec#613
korrat added a commit to korrat/watchexec that referenced this issue Jun 23, 2023
CMD uses special handling for arguments to passed to /C. Unfortunately,
this causes errors with quoted program names in these arguments.

This commit implements a workaround: When CMD is requested, we build a
std::process::Command and pass the argument to /C using the special
std::os::windows::process::CommandExt::raw_arg method. The StdCommand is
then converted to a TokioCommand and returned.

Once tokio-rs/tokio#5810 is fixed, this workaround can be removed.

Fixes watchexec#613
@Darksonn Darksonn added the M-process Module: tokio/process label Jul 3, 2023
@joriskleiber
Copy link
Contributor

I already implemented that (#5704) and it’s an unstable feature since tokio v1.29.0. But I don’t know why it doesn’t show up in the docs.

@m-haug
Copy link
Author

m-haug commented Jul 12, 2023

Apologies, I did only search for raw_arg in issues, not PRs. Good to see that this is already implemented.

I think it's not listed on docs.rs because it's #[cfg(windows)] and, therefore, not built when generating documentation. Comparing the cfg_windows macro, it seems to include explicit provisions to include the item in documentation builds.

So, now it's just a matter of waiting for tokio to increase its MSRV to 1.62.

@Darksonn
Copy link
Contributor

Closing as completed by #5704. I opened #5862 for the docs issue.

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 C-feature-request Category: A feature request. M-process Module: tokio/process
Projects
None yet
Development

No branches or pull requests

3 participants