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

Command::spawn has weird rules for finding binaries on Windows #37519

Open
Tracked by #3
ollie27 opened this issue Nov 1, 2016 · 13 comments
Open
Tracked by #3

Command::spawn has weird rules for finding binaries on Windows #37519

ollie27 opened this issue Nov 1, 2016 · 13 comments
Labels
A-process Area: std::process and std::env C-feature-request Category: A feature request, i.e: not implemented / a PR. O-windows Operating system: Windows T-libs Relevant to the library team, which will review and decide on the PR/issue.

Comments

@ollie27
Copy link
Member

ollie27 commented Nov 1, 2016

The offending code is here which is apparently to have it search the child's %Path% for the binary to fix #15149. It has a few issues though.

  1. It is only triggered when the child's environment has been customised. This means Command::new("foo.exe").spawn() and Command::new("foo.exe").env("Thing", "").spawn() uses different rules for finding the binary which seems unexpected.
  2. It will replace any extension on the program name with .exe. This means Command::new("foo.bar").env("Thing", "").spawn() attempts to launch foo.exe first. Command::new("foo.bar").spawn() correctly tries to launch foo.bar as CreateProcess will not replace the extension.
  3. If the program name is an absolute path then this is still triggered but will just try that path for every item in the child's %Path%. For example Command::new(r"C:\foo.bar").env("Thing", "").spawn() looks for C:\foo.exe several times.
  4. The use of .to_str().unwrap() means it panic!s if the program name is not valid Unicode.
  5. Prehaps the biggest issue, but maybe it's itentional, is that none of the logic for finding binaries seems to be documented (std::process::Command).

An easy way to fix this is to just remove the hack so we just rely on the behaviour of CreateProcess on Windows which is a least documented. The behaviour is already very different between Windows and Linux and we should probably just accept that in order to get reliable results cross-platform it's best to use absolute paths.

@retep998
Copy link
Member

retep998 commented Nov 1, 2016

Personally I'd much rather we didn't have this hack and just accepted that Windows and Unix are different and document their differences.

@alexcrichton
Copy link
Member

@ollie27 these all sound like bugs to me rather than weird behavior. Removing this behavior is not an option as the are plenty of programs currently relying on this behavior. Would be great to fix the issues, however!

@luser
Copy link
Contributor

luser commented Feb 1, 2018

It would definitely be nicer if Command didn't try to be too smart here, but breaking existing usage would also suck. For sccache I contributed to the which crate, which will append an exe extension on Windows if necessary (but not replace an existing extension). One thing that the which crate doesn't implement is support for using the extensions from the PATHEXT environment variable, which on my Windows 10 machine does include .COM. (The Python which package does support this.)

@jokeyrhyme
Copy link

Another issue is that .spawn() assumes that ".exe" is the only extension that Windows' executables have: https://github.com/rust-lang/rust/blob/1.24.0/src/libstd/sys/windows/process.rs#L154
This ignores ".cmd", ".com", etc

@cuviper
Copy link
Member

cuviper commented May 18, 2018

Another issue is that .spawn() assumes that ".exe" is the only extension that Windows' executables have

I just found in #50870 (comment) that it's consistent with CreateProcess to only try ".exe".

@BasixKOR
Copy link

BasixKOR commented Mar 6, 2020

I just found a documentation about how file extensions work in Windows, and it might help since CMD finds the application to open a file with %PATHEXT% using this information (I found this on Stack Overflow answer, but it may be wrong.).

@anp
Copy link
Member

anp commented Aug 23, 2020

One thing that the which crate doesn't implement is support for using the extensions from the PATHEXT environment variable, which on my Windows 10 machine does include .COM.

By the way @luser, after reading through this thread I found that in the interim the which crate has gained support for other extensions through PATHEXT.

@ChrisDenton ChrisDenton added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Jul 3, 2023
@workingjubilee workingjubilee added the A-process Area: std::process and std::env label Jul 22, 2023
@kryptan
Copy link
Contributor

kryptan commented Aug 26, 2023

Command::spawn cannot launch *.cmd file located in PATH if you don't specify extension.

E.g. launching VSCode doesn't work:

Command::new("code").spawn().unwrap(); // Error { kind: NotFound, message: "program not found" }

CMD and PowerShell launch code without extension just fine. I would expect Rust's Command behaviour to match CMD and PowerShell.

In Rust you need to specify extension Command::new("code.cmd") or launch with CMD Command::new("cmd").arg("/C").arg("code").

@blaenk
Copy link
Contributor

blaenk commented Aug 26, 2023

If some of you weren't aware, it might help to know that the which crate has logic for handling this without requiring you to append .cmd, .com, .bat, .exe, etc. This is based on the PATHEXT environment variable on Windows, which you don't have to set it yourself.

See https://github.com/harryfei/which-rs/blob/e5ec71143965a7c7751c853fb229cb973bfcaa13/src/finder.rs#L158-L162

Hope that helps others like it helped me!

@ChrisDenton
Copy link
Contributor

This thread was about buggy behaviour that has since been fixed (aside from improving the documentation).

The fact that Command is a thin wrapper around CreateProcessW is intentional. If you would like more shell-like behaviour then using a crate is definitely the preferred method.

I opened #94743 to discuss running scripts, etc, from Command but there was a distinct lack of enthusiasm for it. I'd suggest moving conversation there if there's a strong argument for it. Or open an API Change Proposal (ACP) on the library team's repo.

@tisonkun
Copy link
Contributor

Perhaps helpful - https://github.com/cli/safeexec

@sourcefrog
Copy link
Contributor

This thread was about buggy behaviour that has since been fixed (aside from improving the documentation).

The fact that Command is a thin wrapper around CreateProcessW is intentional. If you would like more shell-like behaviour then using a crate is definitely the preferred method.

I opened #94743 to discuss running scripts, etc, from Command but there was a distinct lack of enthusiasm for it. I'd suggest moving conversation there if there's a strong argument for it. Or open an API Change Proposal (ACP) on the library team's repo.

Adding more rustdocs to clarify this sounds like a good idea. Maybe someone who regularly works with this on Windows can put up a PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-process Area: std::process and std::env C-feature-request Category: A feature request, i.e: not implemented / a PR. O-windows Operating system: Windows T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests