(U) The examples for the function still show the return code in the form os.popen would produce (a program exiting with status 1 would return 256 as the status), but the new code from #10197 makes the status 1, not 256.

(U) This is a breaking change for code relying on what was already a legacy interface. Either the docs should call out the change, or the code needs to restore the previous behavior.

(U) Ultra simple repro:

>>> subprocess.getstatusoutput('python -c "exit(1)"')
Expected:
(256, '')
Actual:
(1, '')

It probably comes from this change:
---
changeset:   86879:c34e163c0086
branch:      3.3
parent:      86870:dbff708e393f
user:        Tim Golden <mail@timgolden.me.uk>
date:        Sun Nov 03 12:53:17 2013 +0000
files:       Lib/subprocess.py Lib/test/test_subprocess.py Misc/NEWS
description:
Issue #10197 Rework subprocess.get[status]output to use subprocess functionality and thus to work on Windows. Patch by Nick Coghlan.
---

> (U) This is a breaking change for code relying on what was already a legacy interface.

CommandTests.test_getoutput() only checks that an error returns a non-zero status :-/ It doens't check for the exact status. We should use a small Python program using a specific exit code (ex: sys.exit(5)) to check the status.

Oh, I now understand why I feel guilty, I proposed a patch rewriting getstatusoutput() in the issue #10197. My patch ends with:

+    if os.name != 'nt':
+        # convert status to be interpreted according to the wait() rules
+        sts = sts << 8

This fix is that simple, but it means that depending on the Python version, you will get a different status...

Python 3.3 doesn't accept bugfixes anymore and so cannot be changed.

Ah blech. Can someone with privileges edit my original message to remove the junk at the beginning of each paragraph? Habit from an old job. Wish I could just edit the message.

> Can someone with privileges edit my original message to remove the junk at the beginning of each paragraph?

It's not possible to edit a message, only to remove it. I don't like removing the initial message of an issue.

Don't worry, (U) looks a bullet, it can read it ;-)

You are right, it did change in 3.3.4 (see issue 10197).  That change should not have been applied to 3.3, and obviously there was a missing test concerning the return code format.

At this point I think we are stuck with changing the documentation.  The new return code is both more convenient and more consistent with the rest of the subprocess module.  It is very unfortunate that it did not follow our normal backward compatibility rules, but we are stuck with the mistake now.

> You are right, it did change in 3.3.4 (see issue 10197).  That change should not have been applied to 3.3, ...

The purpose of the issue #10197 was to fix a bug on Windows: getoutput() didn't work.

But it was also a feature addition: getoutput had not been *intended* to work on Windows.  I understand why the mistake was made (the argument that it was a bug has weight), but the fact that a versionchanged was needed mentioning 3.3.4 indicates it wasn't really a bug fix.

I correct the doc concerning subprocess.getstatusoutput(cmd) function.
More specifically the part on examples and I add a quick explanation about how are manage the return code.

I improve the patch a little bit, following the recommendations of the Stinner's review. 
I replace "status" mention by "exit code".

According to the Victor's review, I remove "Pay attention" words and change exit code by returncode.

I disagree with Victor.  The name of the function is "getstatusoutput".  I think the docs should continue to use (status, output) as the names for the return values.  The clarification is that 'status' is now the raw return code, not the shifted return code that it formerly returned.

Also, the patch should include a new test that checks the actual return code value.

issue22635-3.diff: LGTM.


R. David Murray: "I disagree with Victor.  The name of the function is "getstatusoutput".  I think the docs should continue to use (status, output) as the names for the return values."

In Python, we use "exitcode" or "returncode" names for the exit code, but "status" for the thing that should be parsed with os.WEXITSTATUS(status).

Just to contradict me, the manual page of the exit() function uses the "status" term: "void _exit(int status);", not "code".

"The clarification is that 'status' is now the raw return code, not the shifted return code that it formerly returned."

Sorry, I'm confused by this sentence :-) getstatusoutput() returns an exit code, the parameter of exit(), no more the annoying "status" thing that should be passed to os.WEXITSTATUS(status) to get a regular exit code.

"Also, the patch should include a new test that checks the actual return code value."

FYI acassaigne is a newcomer currently in a sprint and this issue is tagged as Documentation. I suggest to first push a doc change and then add an unit test.

Ah, I see.  So the function's name is wrong now, too :(.

OK, I guess using exitcode is more accurate then.  Surprising that the original docs did not link to os.WEXITSTATUS.  Maybe we could make a crosslink in the versionchanged entry to os.WEXITSTATUS in the way of explanation of what the difference between 'status' and 'exitcode' is?  Otherwise the versionchanged sentence doesn't seem to tell the reader anything.

I add a crosslink to WEXITSTATUS function. According David Murray advices.

Bug mentionned on a Twitter discussion:
https://twitter.com/zhenech/status/894444040992829441

I've added Greg to the nosy list and filed an issue with subprocess32 about this: https://github.com/google/python-subprocess32/issues/30

I did that mainly to make sure they don't inadvertently backport this regression, but also to ask if subprocess32 might potentially provide a way to get the old intended-but-not-actually-tested behaviour back in 3.x.

I intend subprocess32 to Python 2 only.  Python 3 users should use the standard library subprocess module.

subprocess32 does not include getstatusoutput() or getoutput() functions as those are available in the Python 2 stdlib commands module.

If you want something API compatible that can be used in a Python 2 and 3 application that wants a getstatusoutput() API I think an appropriate place to put it would be to add a commands modules to https://pypi.python.org/pypi/past/ with a consistent getstatusoutput() API.  Either there or something in https://pypi.python.org/pypi/six/.

The inadvertent API difference between 2.7 commands.getstatusoutput() and 3.3.4 subprocess.getstatusoutput() is unfortunate but - I agree - too late to change.  Fixing the documentation to mention the discrepancy is the least worst option.

IMHO it's too late to change *again* subprocess.getstatusoutput() behaviour. Otherwise, it would mean that a complex test like "if not((3, 4) <= sys.version_info < (3, 7))" would be needed to workaround the bug... Whereas right now, basically we only have to check if we are running on Python 3 or not. (Python 3.0-3.3 is almost not used in the wild.)

Instead we should just *document* the behaviour change using ".. versionchanged:: 3.4" in Doc/library/subprocess.rst.

Any volunteer to do that?

I'll get to it.  acassaigne's patches are already a good attempt at that, i'll probably find wording tweaks to make as I apply them.

Issue filed with six about this: https://github.com/benjaminp/six/issues/207

It turns out to be somewhat timely on that side as well, as it turns out that six has a not-yet-released change adding "six.moves.commands", which doesn't currently account for this inadvertent incompatibility in the way exit codes are reported.

New changeset 738b7d9766e1a794aaaabfba0d515a467ba833ca by Gregory P. Smith in branch 'master':
bpo-22635: subprocess.getstatusoutput doc update. (#3398)
https://github.com/python/cpython/commit/738b7d9766e1a794aaaabfba0d515a467ba833ca

New changeset dee54f6010ee2df60322c350a5f1dc8bfcf367d6 by Christian Heimes (Miss Islington (bot)) in branch '3.6':
[3.6] bpo-22635: subprocess.getstatusoutput doc update. (GH-3398) (#3411)
https://github.com/python/cpython/commit/dee54f6010ee2df60322c350a5f1dc8bfcf367d6

marking "closed/resolved" but also "won't fix" as the behavior change remains, but we've done the safest thing at this point: documented the fact.

New changeset 2eb0cb4787d02d995a9bb6dc075983792c12835c by Gregory P. Smith in branch 'master':
bpo-22635: Update the getstatusoutput docstring. (#3435)
https://github.com/python/cpython/commit/2eb0cb4787d02d995a9bb6dc075983792c12835c

New changeset fb4c28c032e26b3cdbe67eae3769d45207ac3507 by Gregory P. Smith (Miss Islington (bot)) in branch '3.6':
[3.6] bpo-22635: Update the getstatusoutput docstring. (GH-3435) (#3439)
https://github.com/python/cpython/commit/fb4c28c032e26b3cdbe67eae3769d45207ac3507