Author eryksun
Recipients Yoni Rozenshein, eryksun, giampaolo.rodola, paul.moore, steve.dower, tim.golden, vstinner, zach.ware
Date 2018-06-07.00:51:47
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1528332710.39.0.592728768989.issue33780@psf.upfronthosting.co.za>
In-reply-to
Content
> To get the correct output, cmd has a "/u" switch (this switch has 
> probably existed forever - at least since Windows NT 4.0, by my 
> internet search). The output can then be decoded using 
> encoding='utf-16-le', like any native Windows string.

However, the /u switch doesn't affect how CMD reads from stdin when it's a disk file or pipe. For example, `set /p` will stop at the first NUL byte. In general this is mismatched with subprocess, which provides a single `encoding` value for all 3 standard I/O streams. For example:

    >>> r = subprocess.run('cmd /d /v /u /c "set /p spam= & echo !spam!"',
    ...     capture_output=True, input='spam', encoding='oem')
    >>> r.stdout
    's\x00p\x00a\x00m\x00\n\x00\n\x00'

With UTF-16 input, CMD only reads up to "s" instead of reading the entire "s\x00p\x00a\x00m\x00" string that was written to stdin:

    >>> r = subprocess.run('cmd /d /v /u /c "set /p spam= & echo !spam!"',
    ...     capture_output=True, input='spam', encoding='utf-16le')
    >>> r.stdout
    's\n'

> 1. A new argument to Popen

This may lead to confusion and false bug reports by people who expect the setting to also affect external programs run via the shell (e.g. tasklist.exe). It's also not consistent with how CMD reads from stdin, as shown above. 

I can see the use of adding a cross-platform get_shell_path() function that returns the fully-qualified path to the shell that's used by shell=True. This way programs don't have to figure it out on their own if they need custom shell options. 

Common CMD shell options in Windows include /d (skip AutoRun commands), /v (enable delayed expansion of environment variables via "!"), /e (enable command extensions), /k (remain running after the command), and /u. I'd prefer subprocess to use /d by default. It's strange that the CRT's system() command doesn't use it.

Currently the shell path can be "/bin/sh" or "/system/bin/sh" in POSIX and os.environ.get("COMSPEC", "cmd.exe") in Windows. I'd prefer that Windows instead used:

    shell_path = os.path.abspath(os.environ.get('ComSpec',
                    os.path.join(_winapi.GetSystemDirectory(), 'cmd.exe')))

i.e. never use an unqualified, relative path such as "cmd.exe". 

Instead of the single-use GetSystemDirectory function, it could instead use _winapi.SHGetKnownFolderPath(_winapi.FOLDERID_System), or _winapi.SHGetKnownFolderPath('{1AC14E77-02E7-4E5D-B744-2EB1AE5198B7}') if the GUID constants aren't added.
History
Date User Action Args
2018-06-07 00:51:50eryksunsetrecipients: + eryksun, paul.moore, vstinner, giampaolo.rodola, tim.golden, zach.ware, steve.dower, Yoni Rozenshein
2018-06-07 00:51:50eryksunsetmessageid: <1528332710.39.0.592728768989.issue33780@psf.upfronthosting.co.za>
2018-06-07 00:51:50eryksunlinkissue33780 messages
2018-06-07 00:51:47eryksuncreate