[issue10197] subprocess.getoutput fails on win32

[Gregory P. Smith]

Gregory P. Smith added the comment:

http://bugs.python.org/issue23508 to track the fall out of that.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue10197] subprocess.getoutput fails on win32

[Gregory P. Smith]

Gregory P. Smith added the comment:

A side effect of the changes made within are that getstatusoutput() on POSIX 
systems now returns a different value for status.

The old implementation present in Python 2 and Python 3.3 before this patch 
returned the raw waitpid() status result as the status value.  ie: 
getstatusoutput("exit 1")[0] == 256.  the lower 8 bits were reserved for the 
signal number the process died with, if any.

Now it returns the sanitized subprocess style returncode: positive numbers are 
the process exit code (so the above example returns 1) and negative numbers are 
the negative signal number the process died with.

I prefer the new behavior, but this API change is not documented anywhere that 
I can find.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue10197] subprocess.getoutput fails on win32

[Roundup Robot]

Roundup Robot added the comment:

New changeset 34df43c9c74a by R David Murray in branch '3.3':
#10197: Update get[status]output versionchanged with actual version.
http://hg.python.org/cpython/rev/34df43c9c74a

New changeset ee277b383d33 by R David Murray in branch 'default':
#10197: Update get[status]output versionchanged with actual version.
http://hg.python.org/cpython/rev/ee277b383d33

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue10197] subprocess.getoutput fails on win32

[Tim Golden]

Tim Golden added the comment:

Thanks: final outdated comments removed

--
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue10197] subprocess.getoutput fails on win32

[Arfrever Frehtes Taifersar Arahesis]

Arfrever Frehtes Taifersar Arahesis added the comment:

Lib/subprocess.py still has outdated comment:

# NB This only works (and is only relevant) for POSIX.

--
nosy: +Arfrever

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue10197] subprocess.getoutput fails on win32

[Terry J. Reedy]

Terry J. Reedy added the comment:

Is this ready to be reclosed?

--
nosy: +terry.reedy

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue10197] subprocess.getoutput fails on win32

[Roundup Robot]

Roundup Robot added the comment:

New changeset 0aa2aedc6a21 by Tim Golden in branch 'default':
Issue #10197 Tweak docs for subprocess.getstatusoutput and align the 
documentation, the module docstring, and the function docstring.
http://hg.python.org/cpython/rev/0aa2aedc6a21

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue10197] subprocess.getoutput fails on win32

[Tim Golden]

Tim Golden added the comment:

Good point. I've added the versionchanged tag.

The issue with bytes-string encoding goes all the way back to 
Popen.communicate if universal newlines mode is used so I've simply put 
in a reference to the existing notes on the subject higher up in the docs.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue10197] subprocess.getoutput fails on win32

[Roundup Robot]

Roundup Robot added the comment:

New changeset 2924a63aab73 by Tim Golden in branch '3.3':
Issue #10197: Indicate availability of subprocess.get[status]output on Windows 
and add a note about the effects of universal newlines
http://hg.python.org/cpython/rev/2924a63aab73

New changeset effad2bda4cb by Tim Golden in branch 'default':
Issue #10197: Indicate availability of subprocess.get[status]output on Windows 
and add a note about the effects of universal newlines
http://hg.python.org/cpython/rev/effad2bda4cb

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue10197] subprocess.getoutput fails on win32

[Gregory P. Smith]

Gregory P. Smith added the comment:

The documentation needs updating to state that these are available on Windows 
(currently it says UNIX) with a versionchanged annotation.

http://docs.python.org/3.3/library/subprocess.html#legacy-shell-invocation-functions

--
nosy: +gregory.p.smith
status: closed -> open

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue10197] subprocess.getoutput fails on win32

[Tim Golden]

Tim Golden added the comment:

Code & tests now work on Windows. Applied to 3.3 & 3.4.

--
resolution:  -> fixed
stage: test needed -> committed/rejected
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue10197] subprocess.getoutput fails on win32

[Roundup Robot]

Roundup Robot added the comment:

New changeset c34e163c0086 by Tim Golden in branch '3.3':
Issue #10197 Rework subprocess.get[status]output to use subprocess 
functionality and thus to work on Windows. Patch by Nick Coghlan.
http://hg.python.org/cpython/rev/c34e163c0086

New changeset 05ce1bd1a4c2 by Tim Golden in branch 'default':
Issue #10197 Rework subprocess.get[status]output to use subprocess 
functionality and thus to work on Windows. Patch by Nick Coghlan.
http://hg.python.org/cpython/rev/05ce1bd1a4c2

New changeset b6efaa97ee0e by Tim Golden in branch '3.3':
Issue #10197: merge heads
http://hg.python.org/cpython/rev/b6efaa97ee0e

New changeset 28a0ae3dcb16 by Tim Golden in branch 'default':
Issue #10197: merge heads
http://hg.python.org/cpython/rev/28a0ae3dcb16

New changeset fe828884a077 by Tim Golden in branch 'default':
Issue #10197: merge 3.3
http://hg.python.org/cpython/rev/fe828884a077

--
nosy: +python-dev

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue10197] subprocess.getoutput fails on win32

[Tim Golden]

Tim Golden added the comment:

Patched according to Nick Coghlan's suggestion in 
http://bugs.python.org/issue9922#msg150093. Ad hoc tests look ok on Windows. 
I'll add tests & look at *nix later.

--
Added file: http://bugs.python.org/file32450/issue10197.diff

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue10197] subprocess.getoutput fails on win32

[Tim Golden]

Changes by Tim Golden :


--
assignee:  -> tim.golden
versions: +Python 3.4 -Python 3.2

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue10197] subprocess.getoutput fails on win32

[Éric Araujo]

Éric Araujo  added the comment:

I think that adding safer wrappers and deprecating things are valuable but 
different bugs.  In the short term, we could apply the proposed small patch to 
just fix the issue at hand.  Can one of the Windows experts weigh in?

The patch does this:

if mswindows:
pipe = os.popen('( ' + cmd + ' ) 2>&1', 'r')
else:
pipe = os.popen('{ ' + cmd + '; } 2>&1', 'r')

It was tested manually; a test should be simple to write.

--
keywords: +needs review
nosy: +tim.golden
stage:  -> test needed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue10197] subprocess.getoutput fails on win32

[Éric Araujo]

Éric Araujo  added the comment:

> IMO, both should be deprecated:
> - they're not cross-platform
Isn’t the purpose of this report to fix that? :)

> - they invoke the shell implicitly, which subprocess promises never to do
One could argue that it’s not implicit if it’s documented.  Nonetheless, I 
agree that they don’t fit well with the subprocess promises.

So, +1 on deprecating and +1 on new, safer helpers.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue10197] subprocess.getoutput fails on win32

[Nick Coghlan]

Nick Coghlan  added the comment:

Without knowing this issue existed, I recently started working on adding some 
convenience APIs for shell invocation to shutil: 
http://bugs.python.org/issue13238

I think the getstatus and getstatusoutput APIs were copied from the commands 
module in 3.0 without sufficient thought being given to whether or not they fit 
with the design principles of the subprocess module. IMO, both should be 
deprecated:
   - they're not cross-platform
   - they invoke the shell implicitly, which subprocess promises never to do

--
nosy: +ncoghlan

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue10197] subprocess.getoutput fails on win32

[bpoaugust]

bpoaugust  added the comment:

I got the () syntax from:

http://technet.microsoft.com/en-us/library/cc737438%28WS.10%29.aspx

which refers to grouping, not subshell.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue10197] subprocess.getoutput fails on win32

[Éric Araujo]

Éric Araujo  added the comment:

If Windows shell syntax is similar to POSIX one, then () will run in a 
sub-shell, which would be a different behavior than using {} (which merely 
group statements and their streams).

--
versions:  -Python 3.1

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue10197] subprocess.getoutput fails on win32

[Florent Xicluna]

Changes by Florent Xicluna :


--
nosy: +flox

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue10197] subprocess.getoutput fails on win32

[STINNER Victor]

Changes by STINNER Victor :


--
versions: +Python 3.3

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue10197] subprocess.getoutput fails on win32

[bpoaugust]

Changes by bpoaugust :


--
versions:  -Python 3.3

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue10197] subprocess.getoutput fails on win32

[bpoaugust]

bpoaugust  added the comment:

A better fix, which supports multiple windows commands:

if mswindows:
pipe = os.popen('( ' + cmd + ' ) 2>&1', 'r') # Windows uses () rather 
than { }
else:
pipe = os.popen('{ ' + cmd + '; } 2>&1', 'r')

This works with the command

subprocess.getoutput("echo before & python -V & echo after")

Note that python -V writes to stderr, so without the enclosing ( ) the version 
information is not captured.

--
Added file: http://bugs.python.org/file23433/subprocess.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue10197] subprocess.getoutput fails on win32

[bpoaugust]

bpoaugust  added the comment:

subprocess.getoutput does not currently work at all on Windows.
So it's not necessary to maintain backwards compatibility.

The following fix works for me on WinXP/Python 3.2.2.

Replace

pipe = os.popen('{ ' + cmd + '; } 2>&1', 'r') # line 613 of subprocess.py

with

if mswindows:
pipe = os.popen(cmd + ' 2>&1', 'r') # Windows does not support { }
else:
pipe = os.popen('{ ' + cmd + '; } 2>&1', 'r')

--
nosy: +bpoaugust
versions: +Python 3.3

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue10197] subprocess.getoutput fails on win32

[STINNER Victor]

STINNER Victor  added the comment:

> I tried to add a shell argument (to be able to disable the shell) and
> to accept any Popen keyword, but I don't know how to implement
> shell=False if the input is a list of arguments. list2cmdline() is
> unsafe on UNIX (see #8972).

Example of function to escape a list of arguments on UNIX:

def escapeargs(*args):
   return ' '.join(pipes.quote(arg) for arg in args)

R. David Murray disagree with me to allow getoutput(list) (shell=True) because 
Popen(list, shell=True) behaves differently.

subprocess.Popen(['echo Hello'], shell=True) writes 'Hello', whereas 
subprocess.Popen(['echo', 'Hello'], shell=True) writes nothing (because echo 
has no argument.

I would like to do something like that: getoutput(['echo', 'Hello']) calls 
Popen('echo Hello', shell=True) using escapeargs() function defined above. So 
getoutput(list) calls shell -c "arg1 arg2", whereas Popen(list, shell=True) 
calls shell -c "arg1" arg2 arg3 ...

See also issue #7839 for Popen(str, shell=False) and Popen(list, shell=True) 
cases.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue10197] subprocess.getoutput fails on win32

[STINNER Victor]

STINNER Victor  added the comment:

subprocess_getoutput.patch: patch subprocess.getstatusoutput() to use directly 
Popen, instead of os.popen, with stderr=subprocess.STDOUT instead of "2>&1" 
shell redirection. It strips also all trailing spaces and newlines, not just 
the last one. And finally, it removes "Availability: UNIX." from the 
documentation.

I tried to add a shell argument (to be able to disable the shell) and to accept 
any Popen keyword, but I don't know how to implement shell=False if the input 
is a list of arguments. list2cmdline() is unsafe on UNIX (see #8972). And if 
getstatusoutput() doesn't accept argument list, it becomes useless with 
shell=False (it doesn't support to call a program with arguments).

Note: the status is still shifted on UNIX to be compatible with the wait() 
format.

--
keywords: +patch
Added file: http://bugs.python.org/file20985/subprocess_getoutput.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue10197] subprocess.getoutput fails on win32

[Éric Araujo]

Éric Araujo  added the comment:

> The question that remains is, does removing the {} change the output
> obtained from a command sequence in any way?
{} are used to group output from the commands into one stream.  I believe the 
stdout and stderr arguments to Popen allow us to get compatible behavior.  
Tests need to prove that, of course.

> Which makes me wonder if getoutput/getstatusoutput should just be
> documentationally deprecated instead.  (I never use them myself, FWIW)
They were relocated from the commands module, resulting in slightly overlapping 
functions that don’t share a naming patter: call, check_call, getoutput, 
getstatusoutput.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue10197] subprocess.getoutput fails on win32

[Ned Deily]

Ned Deily  added the comment:

See also Issue9922

--
nosy: +ned.deily

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue10197] subprocess.getoutput fails on win32

[R. David Murray]

R. David Murray  added the comment:

Ah, I did not realize that getstatusoutput was implemented using os.popen.  I 
thought it already used Popen.  Now, in python3, os.popen is in turn 
implemented using subprocess.Popen, so removing that level of indirection seems 
sensible.

The question that remains is, does removing the {} change the output obtained 
from a command sequence in any way?

Note that for backward compatibility you will need to re-munge the status code 
into C format.  Which makes me wonder if getoutput/getstatusoutput should just 
be documentationally deprecated instead.  (I never use them myself, FWIW)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue10197] subprocess.getoutput fails on win32

[Éric Araujo]

Éric Araujo  added the comment:

> The function already exists in Python 3.1, but it is not used in Python 
> source code

We don’t know what code out there uses.  This would be an incompatible change.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue10197] subprocess.getoutput fails on win32

[STINNER Victor]

STINNER Victor  added the comment:

-def getstatusoutput(cmd):
+def getstatusoutput(cmd, shell=True):

shell=True is dangerous, it can lead to shell command injection. I would prefer 
to set its default value to False. The function already exists in Python 3.1, 
but it is not used in Python source code. Is it too late to fix its API to 
avoid security vulnerabilities?

--
nosy: +haypo

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue10197] subprocess.getoutput fails on win32

[Éric Araujo]

Éric Araujo  added the comment:

My idea is simply using Popen with appropriate args for stdout and stderr 
instead of using a shell command with redirections:

--- Lib/subprocess.py   (révision 86943)
+++ Lib/subprocess.py   (copie de travail)
@@ -560,11 +560,7 @@
 return ''.join(result)


-# Various tools for executing commands and looking at their output and status.
-#
-# NB This only works (and is only relevant) for UNIX.
-
-def getstatusoutput(cmd):
+def getstatusoutput(cmd, shell=True):
 """Return (status, output) of executing cmd in a shell.

 Execute the string 'cmd' in a shell with os.popen() and return a 2-tuple
@@ -581,14 +577,21 @@
 >>> subprocess.getstatusoutput('/bin/junk')
 (256, 'sh: /bin/junk: not found')
 """
-pipe = os.popen('{ ' + cmd + '; } 2>&1', 'r')
-text = pipe.read()
-sts = pipe.close()
-if sts is None: sts = 0
-if text[-1:] == '\n': text = text[:-1]
+# Wish I could use with...
+popen = Popen(cmd, shell=shell, stdout=PIPE, stderr=STDOUT)
+sts = popen.communicate() #or wait() or whatever is the right thing
+try:
+text = process.stdout.read()
+finally:
+process.stdout.close()
+if sts is None:
+sts = 0
+if text.endswith('\n'):
+text = text[:-1]
 return sts, text

(The new “shell” argument is icing on the cake, allowing us to later support a 
list as cmd argument like Popen does.)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue10197] subprocess.getoutput fails on win32

[R. David Murray]

R. David Murray  added the comment:

Do you have in implementation in mind?  I'm not clear how this would work.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue10197] subprocess.getoutput fails on win32

[Éric Araujo]

Éric Araujo  added the comment:

I think we should implement getstatusoutput and getoutput with Popen objects to 
gain portability and avoid spawning subshells.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue10197] subprocess.getoutput fails on win32

[Éric Araujo]

Éric Araujo  added the comment:

() is used to launch a command in a sub-shell and {} is used to group commands, 
for example to set up a stream redirection for all commands in brackets.

--
nosy: +eric.araujo

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue10197] subprocess.getoutput fails on win32

[Brian Curtin]

Changes by Brian Curtin :


--
components: +Windows
nosy: +brian.curtin

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue10197] subprocess.getoutput fails on win32

[R. David Murray]

R. David Murray  added the comment:

Oddly, the test suite skips getoutput and getstatusoutput on windows with the 
comment that the source says it is relevant only for posix, but the 
documentation does not have 'availability: unix' tags.  (It is also odd that 
getoutput isn't documented, but that's a different issue.)

Your workaround can't be used as a fix, since the semantics of {}s in the shell 
are different from those of ()s.

It's not clear to me what the point of the {}s is, but I have a fear that 
eliminating them would introduce subtle changes in the behavior of getoutput 
calls.  Perhaps not, though.

It looks like this issue amounts to an RFE for support of 
getoutput/getstatusoutput on Windows, though the fact that it is not documented 
as unix-only may make it a bug instead :)

The appropriate fix is probably to conditionalize the code based on platform.  
A complete patch will require unit test changes and documentation changes 
(since the docs currently mention the braces).

All of that said, it also appears that the new check_output should be preferred 
to either getoutput or getstatusoutput.  Perhaps those functions could be 
re-implemented in terms of check_output.

--
nosy: +r.david.murray

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue10197] subprocess.getoutput fails on win32

[jldm]

New submission from jldm :

Hi, first of all sorry for my English.

On windows XP SP3, the following code:

import subprocess
subprocess.getoutput("dir")

returns
'"{" is not recognized as an internal or external command,\noperable 
program or batch file.'

I made a workaround by changing in the file Lib/subprocess.py the line 574 (I 
thin in 3.2a3 is 584) (in the getstatusoutput(cmd) function definition) from:
pipe = os.popen('{ ' + cmd + '; } 2>&1', 'r')

to:
pipe = os.popen('( ' + cmd + '; ) 2>&1', 'r')

I have tested it with:

ActivePython 3.1.2.3 (ActiveState Software Inc.) based on
Python 3.1.2 (r312:79147, Mar 22 2010, 12:20:29) [MSC v.1500 32 bit (Intel)] on 
win32

and

Python 3.2a3 (r32a3:85355, Oct 10 2010, 17:11:45) [MSC v.1500 32 bit (Intel)] 
on win32


Regards from Spain.
José Luis Domenech

--
messages: 119605
nosy: jldm
priority: normal
severity: normal
status: open
title: subprocess.getoutput fails on win32
type: behavior
versions: Python 3.1, Python 3.2

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com