homepage

Repro:
```python
from pathlib import Path
import subprocess
subprocess.run([Path('/bin/ls')]) # Works Fine
subprocess.run(Path('/bin/ls')) # Fails
```
The problem seems to originate from here:
https://github.com/python/cpython/blob/master/Lib/subprocess.py#L1237-L1240
This file auspiciously avoids importing pathlib, which I'm guessing is somewhat intentional? Would the correct fix be to check for the existence of a `__fspath__` attribute as per https://www.python.org/dev/peps/pep-0519/ ?

Ignore my comment re: pathlib, it looks like PathLike is defined in `os` and not `pathlib`.

I was able to make a test that reproduces your code, and expectedly fails. Also implemented a fix for it. See a temporary diff here: https://pastebin.com/C9JWkg0i
However, there is also a specific MS Windows version of _execute_child() (a phrase only computer-folks can say out loud without feeling very...wrong...). It uses a different method to extract and parse arguments, and it should be corrected and tested under windows, before I submit a PR for this.
Also, my test messes up the formatting. Instead of saying "....ok", they both say "... total 0\nok". I have no idea why.

While researching this, I discovered that on MS Windows
>>> subprocess.run([pathlike_object, additional_arguments])
did not run like it did on Posix. My PR includes this problem and it's fix.

New changeset dd42cb71f2cb02f3a32f016137b12a146bc0d0e2 by Gregory P. Smith (Anders Lorentsen) in branch 'master':
bpo-31961: subprocess now accepts path-like args (GH-4329)
https://github.com/python/cpython/commit/dd42cb71f2cb02f3a32f016137b12a146bc0d0e2

Thanks for the PR! I accepted it as this makes sense as a feature.
<meme> pathlib all the things! </meme>
I think the code be polished up a bit to not rely on catching TypeError but this gets the feature in before the 3.7 feature freeze (just). If further changes are needed or bugs are found, they can be addressed during the 3.7 betas. :)

This made tests failing on Windows. See issue32764.

Actually this feature looks wrong to me. The args argument is either a sequence containing a program name and arguments, or a command line string. In the first case supporting path-like objects makes sense, and this was supported. But the command line is not a path-like object. It is a concatenation of quoted program name and arguments. Neither operation for path-like objects makes sense for a command line.
I think this change should be reverted.

Don't revert something just because you found a bug, we can fix it. fwiw, the PR passed appveyor's Windows run: https://ci.appveyor.com/project/python/cpython/build/3.7build11551
So if there's a bug, we're missing some kind of test coverage.

The bug itself can be easily fixed. But I think this PR shouldn't be merged at first place. Not all functions should accept path-like objects for arbitrary arguments. Only if the argument semantically is a path, a path-like object should be accepted. Several similar propositions already were rejected for this reason.

got any pointers to those? I want to familiarize myself with existing arguments for/against such a feature in subprocess to decide.

I don't remember all details, here is what I have found.
Issue28230 and issue28231 -- the support of path-like object was added only for external names (paths of archives, paths of added files, paths for extraction), but not for internal names.
Issue29447 -- the initial idea was rejected, and it seems to me that the tempfile module doesn't need any changes. Path-like objects already are accepted for the dir argument, but they shouldn't be accepted for arguments like pre or suf which are not paths.
Issue29448 is only tangentially related. It's rejection is based on issue29447.

I think I agree with Serhiy here... the whole difference between run([a]) and run(a) is that in the first case 'a' is treated as a path, and in the second it isn't. This distinction long predates pathlib.
Say I have an oddly named binary with an embedded space in its name:
bin = Path("/bin/ls -l")
# Check that this weird file actually exists:
assert bin.exists()
# runs this weird file
subprocess.run([bin])
# Currently an error; if this is implemented, would run
# /bin/ls, and pass it the -l argument. Refers to something
# completely different than our .exists() call above.
subprocess.run(bin)

I agree with both of you at this point.
TODO: Revert the PR and backport that to the 3.7 branch (i'll take care of it)

> # runs this weird file
> subprocess.run([bin])
> # Currently an error; if this is implemented, would run
> # /bin/ls, and pass it the -l argument. Refers to something
> # completely different than our .exists() call above.
I do not understand where it is stated that this is the behavior. My understanding was that if run() is passed a single argument, it will interpret the string "/bin/ls -l" to mean "run the `/bin/ls` program, and pass it one argument `-l`, whereas if instead run() is given a list, it will literally treat the first element as the program to run, regardless of how many spaces or whatever else it contains ... And as such, if `bin` is a Path-like object, this issue tries to resolve that
run([bin]) works as excepted, but run(bin) fails horribly.
Sure, I can understand that for strings it may not feel natural how these two things are interpreted differently, but if `bin` is a Path-like object, it at least feels very natural to me that then it should be run as the program name itself (as "paths" does not have arguments).

Anders, the problem is that running subprocess.run(executable) instead of subprocess.run([executable]) (where executable is a path to a program) is a bug. For example it doesn't work if the path contains spaces. And PR 4329 encourages making this bug.

What do you mean "is a bug", and "the PR would encourage this"? Can't it be fixed?
Are you saying that just because it is a bug now, we should be discouraged from making it work in the way you'd expect it to work?
If `exe` is a path, these two lines of code should do exactly the same, right? It seems unintuitive to me if they produce different results.
run(exe)
run([exe])

Nathaniel's specific description of a problem is wrong. A Path with embedded spaces is treated no different than one without by default.
Where things change, and what needs fixing, is when shell=True is passed. In that case a PathLike object should not be allowed as it will be turned into a flat string passed to the shell for parsing.
# I've created an executable bash script called "x/mybin -h" for this that just does: echo "hello from mybin $*"
>>> run("x/mybin -h", shell=True)
/bin/sh: 1: x/mybin: not found
CompletedProcess(args='x/mybin -h', returncode=127)
>>> run("x/mybin -h")
hello from mybin 
CompletedProcess(args='x/mybin -h', returncode=0)
>>> run(Path("x/mybin -h"), shell=True)
/bin/sh: 1: x/mybin: not found
CompletedProcess(args=PosixPath('x/mybin -h'), returncode=127)
>>> run([Path("x/mybin -h")], shell=True)
/bin/sh: 1: x/mybin: not found
CompletedProcess(args=[PosixPath('x/mybin -h')], returncode=127)
>>> run(Path("x/mybin -h"))
hello from mybin 
CompletedProcess(args=PosixPath('x/mybin -h'), returncode=0)
>>> run([Path("x/mybin -h")])
hello from mybin

Oh, my mistake. I thought the case when args is a string and shell=False is deprecated.

"I thought the case when args is a string and shell=False is deprecated."
IMO it ought to be :) See issue 7839 for background.

If a PathLike args value is supported in Windows, then it must be processed through list2cmdline, in case it needs to be quoted. Also, the preferred usage is to pass args as a list when shell is false. This common case shouldn't be penalized as a TypeError. Also, passing `executable` as PathLike should be supported, as is already the case in the POSIX implementation. For example.
_execute_child:
 if executable is not None and not isinstance(executable, str):
 executable = os.fsdecode(executable)
 if not isinstance(args, str):
 try:
 args = list2cmdline(args)
 except TypeError:
 if isinstance(args, bytes):
 args = os.fsdecode(args)
 elif isinstance(args, os.PathLike):
 args = list2cmdline([args])
 else:
 raise
 
list2cmdline should support PathLike arguments via os.fsdecode. This removes an existing inconsistency since the POSIX implementation converts args via PyUnicode_FSConverter in Modules/_posixsubprocess.c. For example:
list2cmdline:
 for arg in seq:
 if not isinstance(arg, str):
 arg = os.fsdecode(arg)
Finally, passing args as a string when shell is false can never be deprecated on Windows. list2cmdline works in most cases, but Windows CreateProcess takes a command line, and applications are free to parse the command line however they want. IMHO, the case that should be deprecated is passing args as a list/sequence when shell is true.

This is a complex issue for doing it right. Related issues -- supporting bytes paths on Windows, supporting path-like executable, deprecating some corner cases. I suggest to revert this change now and try to do it right in 3.8.

What's the status of this issue? We need to make a decision soon about what to do for 3.7.0.

I'm fine with undoing this for 3.7 in light of the many things we don't do "right" all over the place with path like objects.
subprocess still presents as a mix of high and low level APIs so if we accept a path like object in a subset of places it seems like that'll add complexity to the APIs rather than being consistent.

Ned: when removing a feature added in beta1, should we remove the original NEWS entry created for it? Or add a new NEWS entry that states that the previous feature has been reverted?

We should either remove the entry in Misc/NEWS/3.7.0b1.rst or, perhaps better, add a line to it noting that it was removed in 3.7.0b2.

There are two alternate PRs. PR 5912 removes this feature. PR 5914 fixes it.

New changeset be50a7b627d0aa37e08fa8e2d5568891f19903ce by Ned Deily (Serhiy Storchaka) in branch 'master':
Revert "bpo-31961: subprocess now accepts path-like args (GH-4329)" (#5912)
https://github.com/python/cpython/commit/be50a7b627d0aa37e08fa8e2d5568891f19903ce

New changeset b7dcae358e9d5a3ac9dafec9f6b64e9a66843ad8 by Ned Deily (Miss Islington (bot)) in branch '3.7':
Revert "bpo-31961: subprocess now accepts path-like args (GH-4329)" (GH-5912) (GH-5931)
https://github.com/python/cpython/commit/b7dcae358e9d5a3ac9dafec9f6b64e9a66843ad8

Thanks for everyone's input and thanks for the PRs! Since there are still outstanding review comments, I decided to revert this from both master and 3.7 for 3.7.0b2. I would suggest getting a polished version stabilized in master for 3.8.0. We could then discuss the possibility of a 3.7 backport.

Gregory, could you please make a look at PR 5914?
Differences from PR 4329:
* Any item of args can be an iterable of bytes and path-like objects on Windows (not just a first item as in PR 4329).
* Accepts bytes and path-like objects as executable on Windows.
* Accepts bytes as cwd on Windows.
* Raises a TypeError when shell is true and args is a path-like object (or bytes on Windows).
* Does not use raising and catching a TypeError when args is an iterable on Windows or a path-like object on POSIX (this makes the code clearer and a tiny bit more efficient).

Ping.

gregory.p.smith, I believe Serhiy was looking for a review on PR5914. Maybe it's not too late for this to get into 3.8? Thanks!

New changeset 9e3c4526394856d6376eed4968d27d53e1d69b7d by Serhiy Storchaka in branch 'master':
bpo-31961: Fix support of path-like executables in subprocess. (GH-5914)
https://github.com/python/cpython/commit/9e3c4526394856d6376eed4968d27d53e1d69b7d