bpo-31961: Fix support of path-like executables in subprocess.

[serhiy-storchaka]

https://bugs.python.org/issue31961

[eryksun]

This needs to be addressed in list2cmdline to be consistent with how POSIX converts every element of args via PyUnicode_FSConverter. For example:

if isinstance(args, bytes):
    args = os.fsdecode(args)
elif isinstance(args, os.PathLike):
    if shell:
        raise ValueError('PathLike args is not allowed when '
                         'shell is true.')
    else:
        args = [args]

if not isinstance(args, str):
    args = list2cmdline(args)

if executable is not None:
    executable = os.fsdecode(executable)

Then in list2cmdline, decode the items via os.fsdecode :

for arg in seq:
    if not isinstance(arg, str):
        arg = os.fsdecode(arg)

[gpshead]

This does look like it would be nicer, allowing more mixing and matching of types.

[serhiy-storchaka]

@eryksun 's comment is related to the old version of the PR. I addressed it, except that that I decided to reject bytes args on Windows with shell=True as it is ambiguous and is not actually needed. It is enough to have a difference in interpreting str args.

[eryksun]

I still think bytes and pathlike should be supported for every item of args. The POSIX implementation doesn't limit this to args[0] (though not documented), and I think that's proper. A script shouldn't have to special-case calling os.fsdecode for a pathlike argument. That's inefficient for POSIX, for which os.fsencode would be better. While os.fspath would be efficient, it's insufficient because it might return bytes on Windows.

[serhiy-storchaka]

The POSIX implementation accepts also bytes as args and as any item of args. If allow path-like arguments, we should consider also adding support of bytes arguments and program name.

[serhiy-storchaka]

You convinced me. I made bytes and pathlike be supported for every item of args. But bytes as a single argument is not supported because this case is ambiguous. It is not clear how to interpret it: as a full command line (as single str value) or as a program file name (as single path-like value).

[serhiy-storchaka]

@gpshead, could you please take a look?

[gpshead]

The elif chain could be simplified to have less repeated code:

elif isinstance(args, (bytes, os.PathLike)):
    if shell:
        raise TypeError(f'{type(args)} args not allowed when shell=True on Windows')`
    args = list2cmdline([args])

[serhiy-storchaka]

Error messages should be different. Path-like args is not allowed on all platforms, bytes args is not allowed only on Windows.

[gpshead]

for consistency of error messages with my suggestion above I think TypeError(f'{type(args)} args is not allowed when ' ...). It is good for that to render as <class os.PathLike> as that is less confusing to the error message reader than "path-like" which they might be more likely to mis-interpret in English as suggesting anything that looks like a path within a string.

[gpshead]

I like to make the first line descriptive comment a docstring. it shows up nicely in verbose testing mode.

[gpshead]

FYI - these 3.6 and 3.7 versionchanged updates should be made to the docs today to clarify the existing behavior on their own (and backported to at least the 3.7 docs)

[gpshead]

This does look like it would be nicer, allowing more mixing and matching of types.

[asvetlov]

LGTM.
@gpshead merging it into Python 3.8 would be the really nice improvement of subprocess support on Windows

[asvetlov]

#13628 depends on this PR for Windows tests green (we can skip Windows, sure. But I honestly prefer to fix upstream :)

[serhiy-storchaka]

I think that I addressed all review comments, but if I missed something it can be fixed in the other PR (even after beta1).

[asvetlov]

Thanks, @serhiy-storchaka !