Make check_output of subprocess2 compatible with Python's subprocess.

Make check_output of subprocess2 compatible with Python's subprocess.

According to Python's doc (
https://docs.python.org/2/library/subprocess.html#subprocess.check_output
):
if check_output raises exception CalledProcessError, the exception object
should contain stdout data as `output` attribute. Before this commit,
subprocess2.CalledProcessError had `output` always None, and used `stdout`
instead. This commit fixes this problem storing the same data in both `stdout`
and `output`.

BUG=NONE

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=292036

[tandrii(chromium), 2014-09-18 20:54:12]

tandrii@chromium.org changed reviewers:
+ maruel@google.com

[tandrii(chromium), 2014-09-18 20:54:12]

I found this incompatibility while writing this CL:
https://codereview.chromium.org/578423002/ (see line 412-416).

WDYT?

[iannucci, 2014-09-18 20:59:03]

iannucci@chromium.org changed reviewers:
+ iannucci@chromium.org

[iannucci, 2014-09-18 20:59:04]

tbh, I'd rather there be one way to do it... if `output` is the right way, then
we should remove stdout. With this change, we could see:

except CPE as e:
  if 'bob' in e.output:
    print >>sys.stderr, "Bob showed up!"
    print >>sys.stderr, "stdout " + "="*40
    print >>sys.stderr, e.stdout
    print >>sys.stderr, "stderr " + "+"*40
    print >>sys.stderr, e.stderr

Which would be confusing. They should either both be output or both be stdout.
IMO it's more confusing to have 'output'. 'stdout' is pretty unambiguous.

https://codereview.chromium.org/582933002/diff/1/subprocess2.py
File subprocess2.py (right):

https://codereview.chromium.org/582933002/diff/1/subprocess2.py#newcode38
subprocess2.py:38: super(CalledProcessError, self).__init__(returncode, cmd,
output=output)
why not just output=stdout

https://codereview.chromium.org/582933002/diff/1/subprocess2.py#newcode39
subprocess2.py:39: self.stdout = stdout
and then say `self.stdout = self.output` for compatibility

[tandrii(chromium), 2014-09-18 22:23:05]

I think the rational for using output in Python standard is because one can
combine stderr and stdout (using check_call(..., stderr=STDOUT).

Having only one is the Zen of Python. However, I don't think removing stdout
attribute is feasible given a lot of dependent code living outside of this repo.

https://codereview.chromium.org/582933002/diff/1/subprocess2.py
File subprocess2.py (right):

https://codereview.chromium.org/582933002/diff/1/subprocess2.py#newcode38
subprocess2.py:38: super(CalledProcessError, self).__init__(returncode, cmd,
output=output)
On 2014/09/18 20:59:03, iannucci wrote:
> why not just output=stdout

Done.

https://codereview.chromium.org/582933002/diff/1/subprocess2.py#newcode39
subprocess2.py:39: self.stdout = stdout
On 2014/09/18 20:59:03, iannucci wrote:
> and then say `self.stdout = self.output` for compatibility

Done.

[M-A Ruel, 2014-09-18 22:42:45]

maruel@chromium.org changed reviewers:
+ maruel@chromium.org

[M-A Ruel, 2014-09-18 22:42:46]

FTR, I initially wrote this script when we were stuck on python 2.4~2.6. It's
not a problem anymore as we don't support using <2.7 so the majority of this
file should be deleted.

I wrote a separate script
https://code.google.com/p/swarming/source/browse/utils/subprocess42.py?repo=c...
that is 2.7+ but adds Popen.yield_any() and timeouts which are super useful in
practice.

https://codereview.chromium.org/582933002/diff/40001/subprocess2.py
File subprocess2.py (right):

https://codereview.chromium.org/582933002/diff/40001/subprocess2.py#newcode38
subprocess2.py:38: self.stdout = self.output  # for backward compatability.
compatibility

[M-A Ruel, 2014-09-18 22:42:55]

On 2014/09/18 22:42:46, M-A Ruel wrote:
> FTR, I initially wrote this script when we were stuck on python 2.4~2.6. It's
> not a problem anymore as we don't support using <2.7 so the majority of this
> file should be deleted.
> 
> I wrote a separate script
>
https://code.google.com/p/swarming/source/browse/utils/subprocess42.py?repo=c...
> that is 2.7+ but adds Popen.yield_any() and timeouts which are super useful in
> practice.
> 
> https://codereview.chromium.org/582933002/diff/40001/subprocess2.py
> File subprocess2.py (right):
> 
> https://codereview.chromium.org/582933002/diff/40001/subprocess2.py#newcode38
> subprocess2.py:38: self.stdout = self.output  # for backward compatability.
> compatibility

and lgtm

[tandrii(chromium), 2014-09-19 11:48:34]

The CQ bit was checked by tandrii@chromium.org

[commit-bot: I haz the power, 2014-09-19 11:49:39]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/582933002/60001

[commit-bot: I haz the power, 2014-09-19 11:51:45]

Committed patchset #4 (id:60001) as 292036

[pgervais, 2014-09-22 04:28:48]

On 2014/09/19 11:51:45, I haz the power (commit-bot) wrote:
> Committed patchset #4 (id:60001) as 292036

Note that this modification is not compatible with Python 2.6.