utils: rename RunCommandError to CalledProcessError
The Python 3.6 subprocess uses the |CalledProcessError| name.
Also adjust all of its attributes to match.
Bug: None
Test: unittests pass
Change-Id: I2474e7e74b259b4082824fe573337329a94bc751
diff --git a/pre-upload.py b/pre-upload.py
index fa44243..febbfa5 100755
--- a/pre-upload.py
+++ b/pre-upload.py
@@ -266,7 +266,7 @@
try:
remote = rh.git.get_upstream_remote()
upstream_branch = rh.git.get_upstream_branch()
- except rh.utils.RunCommandError as e:
+ except rh.utils.CalledProcessError as e:
output.error('Upstream remote/tracking branch lookup',
'%s\nDid you run repo start? Is your HEAD detached?' %
(e,))
@@ -339,7 +339,7 @@
if proj_dir is None:
cmd = ['repo', 'forall', project_name, '-c', 'pwd']
result = rh.utils.run(cmd, capture_output=True)
- proj_dirs = result.output.split()
+ proj_dirs = result.stdout.split()
if not proj_dirs:
print('%s cannot be found.' % project_name, file=sys.stderr)
print('Please specify a valid project.', file=sys.stderr)
@@ -408,7 +408,7 @@
"""
cmd = ['repo', 'forall', '.', '-c', 'echo ${REPO_PROJECT}']
return rh.utils.run(cmd, capture_output=True, redirect_stderr=True,
- cwd=path).output.strip()
+ cwd=path).stdout.strip()
def direct_main(argv):
@@ -441,7 +441,7 @@
if opts.dir is None:
cmd = ['git', 'rev-parse', '--git-dir']
git_dir = rh.utils.run(cmd, capture_output=True,
- redirect_stderr=True).output.strip()
+ redirect_stderr=True).stdout.strip()
if not git_dir:
parser.error('The current directory is not part of a git project.')
opts.dir = os.path.dirname(os.path.abspath(git_dir))
diff --git a/rh/git.py b/rh/git.py
index c052d14..baf669c 100644
--- a/rh/git.py
+++ b/rh/git.py
@@ -35,12 +35,12 @@
# First get the current branch name.
cmd = ['git', 'rev-parse', '--abbrev-ref', 'HEAD']
result = rh.utils.run(cmd, capture_output=True)
- branch = result.output.strip()
+ branch = result.stdout.strip()
# Then get the remote associated with this branch.
cmd = ['git', 'config', 'branch.%s.remote' % branch]
result = rh.utils.run(cmd, capture_output=True)
- return result.output.strip()
+ return result.stdout.strip()
def get_upstream_branch():
@@ -51,20 +51,20 @@
"""
cmd = ['git', 'symbolic-ref', 'HEAD']
result = rh.utils.run(cmd, capture_output=True)
- current_branch = result.output.strip().replace('refs/heads/', '')
+ current_branch = result.stdout.strip().replace('refs/heads/', '')
if not current_branch:
raise ValueError('Need to be on a tracking branch')
cfg_option = 'branch.' + current_branch + '.%s'
cmd = ['git', 'config', cfg_option % 'merge']
result = rh.utils.run(cmd, capture_output=True)
- full_upstream = result.output.strip()
+ full_upstream = result.stdout.strip()
# If remote is not fully qualified, add an implicit namespace.
if '/' not in full_upstream:
full_upstream = 'refs/heads/%s' % full_upstream
cmd = ['git', 'config', cfg_option % 'remote']
result = rh.utils.run(cmd, capture_output=True)
- remote = result.output.strip()
+ remote = result.stdout.strip()
if not remote or not full_upstream:
raise ValueError('Need to be on a tracking branch')
@@ -75,7 +75,7 @@
"""Returns the latest commit for this ref."""
cmd = ['git', 'rev-parse', ref]
result = rh.utils.run(cmd, capture_output=True)
- return result.output.strip()
+ return result.stdout.strip()
def get_remote_revision(ref, remote):
@@ -89,7 +89,7 @@
def get_patch(commit):
"""Returns the patch for this commit."""
cmd = ['git', 'format-patch', '--stdout', '-1', commit]
- return rh.utils.run(cmd, capture_output=True).output
+ return rh.utils.run(cmd, capture_output=True).stdout
def get_file_content(commit, path):
@@ -103,7 +103,7 @@
content will not have any newlines.
"""
cmd = ['git', 'show', '%s:%s' % (commit, path)]
- return rh.utils.run(cmd, capture_output=True).output
+ return rh.utils.run(cmd, capture_output=True).stdout
class RawDiffEntry(object):
@@ -145,7 +145,7 @@
entries = []
cmd = ['git', 'diff', '--no-ext-diff', '-M', '--raw', target]
- diff = rh.utils.run(cmd, cwd=path, capture_output=True).output
+ diff = rh.utils.run(cmd, cwd=path, capture_output=True).stdout
diff_lines = diff.strip().splitlines()
for line in diff_lines:
match = DIFF_RE.match(line)
@@ -175,13 +175,13 @@
cmd = ['git', 'log', '%s..' % get_upstream_branch(), '--format=%H']
if ignore_merged_commits:
cmd.append('--first-parent')
- return rh.utils.run(cmd, capture_output=True).output.split()
+ return rh.utils.run(cmd, capture_output=True).stdout.split()
def get_commit_desc(commit):
"""Returns the full commit message of a commit."""
cmd = ['git', 'log', '--format=%B', commit + '^!']
- return rh.utils.run(cmd, capture_output=True).output
+ return rh.utils.run(cmd, capture_output=True).stdout
def find_repo_root(path=None):
diff --git a/rh/hooks.py b/rh/hooks.py
index 3e0c30b..fd86dc0 100644
--- a/rh/hooks.py
+++ b/rh/hooks.py
@@ -261,7 +261,7 @@
def wrapper():
result = _run(cmd, **kwargs)
if result.returncode not in (None, 0):
- return result.output
+ return result.stdout
return None
return wrapper
@@ -299,9 +299,9 @@
for d in filtered:
data = rh.git.get_file_content(commit, d.file)
result = _run(cmd, input=data)
- if result.output:
+ if result.stdout:
ret.append(rh.results.HookResult(
- 'bpfmt', project, commit, error=result.output,
+ 'bpfmt', project, commit, error=result.stdout,
files=(d.file,)))
return ret
@@ -529,9 +529,9 @@
for d in filtered:
data = rh.git.get_file_content(commit, d.file)
result = _run(cmd, input=data)
- if result.output:
+ if result.stdout:
ret.append(rh.results.HookResult(
- 'gofmt', project, commit, error=result.output,
+ 'gofmt', project, commit, error=result.stdout,
files=(d.file,)))
return ret
diff --git a/rh/results.py b/rh/results.py
index 062aaa3..bdac83c 100644
--- a/rh/results.py
+++ b/rh/results.py
@@ -64,12 +64,12 @@
class HookCommandResult(HookResult):
- """A single hook result based on a CommandResult."""
+ """A single hook result based on a CompletedProcess."""
def __init__(self, hook, project, commit, result, files=(),
fixup_func=None):
HookResult.__init__(self, hook, project, commit,
- result.error if result.error else result.output,
+ result.stderr if result.stderr else result.stdout,
files=files, fixup_func=fixup_func)
self.result = result
diff --git a/rh/utils.py b/rh/utils.py
index e6f8dc1..c6c77e0 100644
--- a/rh/utils.py
+++ b/rh/utils.py
@@ -54,14 +54,29 @@
return ret
-class CommandResult(object):
- """An object to store various attributes of a child process."""
+class CompletedProcess(getattr(subprocess, 'CompletedProcess', object)):
+ """An object to store various attributes of a child process.
- def __init__(self, cmd=None, error=None, output=None, returncode=None):
- self.cmd = cmd
- self.error = error
- self.output = output
- self.returncode = returncode
+ This is akin to subprocess.CompletedProcess.
+ """
+
+ # The linter is confused by the getattr usage above.
+ # TODO(vapier): Drop this once we're Python 3-only and we drop getattr.
+ # pylint: disable=bad-option-value,super-on-old-class
+ def __init__(self, args=None, returncode=None, stdout=None, stderr=None):
+ if sys.version_info.major < 3:
+ self.args = args
+ self.stdout = stdout
+ self.stderr = stderr
+ self.returncode = returncode
+ else:
+ super(CompletedProcess, self).__init__(
+ args=args, returncode=returncode, stdout=stdout, stderr=stderr)
+
+ @property
+ def cmd(self):
+ """Alias to self.args to better match other subprocess APIs."""
+ return self.args
@property
def cmdstr(self):
@@ -69,36 +84,58 @@
return rh.shell.cmd_to_str(self.cmd)
-class RunCommandError(Exception):
- """Error caught in RunCommand() method."""
+class CalledProcessError(subprocess.CalledProcessError):
+ """Error caught in run() function.
- def __init__(self, msg, result, exception=None):
- self.msg, self.result, self.exception = msg, result, exception
+ This is akin to subprocess.CalledProcessError. We do not support |output|,
+ only |stdout|.
+
+ Attributes:
+ returncode: The exit code of the process.
+ cmd: The command that triggered this exception.
+ msg: Short explanation of the error.
+ exception: The underlying Exception if available.
+ """
+
+ def __init__(self, returncode, cmd, stdout=None, stderr=None, msg=None,
+ exception=None):
if exception is not None and not isinstance(exception, Exception):
- raise ValueError('exception must be an exception instance; got %r'
- % (exception,))
- Exception.__init__(self, msg)
- self.args = (msg, result, exception)
+ raise TypeError('exception must be an exception instance; got %r'
+ % (exception,))
- def stringify(self, error=True, output=True):
+ super(CalledProcessError, self).__init__(returncode, cmd, stdout)
+ # The parent class will set |output|, so delete it.
+ del self.output
+ # TODO(vapier): When we're Python 3-only, delete this assignment as the
+ # parent handles it for us.
+ self.stdout = stdout
+ # TODO(vapier): When we're Python 3-only, move stderr to the init above.
+ self.stderr = stderr
+ self.msg = msg
+ self.exception = exception
+
+ @property
+ def cmdstr(self):
+ """Return self.cmd as a well shell-quoted string for debugging."""
+ return '' if self.cmd is None else rh.shell.cmd_to_str(self.cmd)
+
+ def stringify(self, stdout=True, stderr=True):
"""Custom method for controlling what is included in stringifying this.
- Each individual argument is the literal name of an attribute
- on the result object; if False, that value is ignored for adding
- to this string content. If true, it'll be incorporated.
-
Args:
- error: See comment about individual arguments above.
- output: See comment about individual arguments above.
+ stdout: Whether to include captured stdout in the return value.
+ stderr: Whether to include captured stderr in the return value.
+
+ Returns:
+ A summary string for this result.
"""
items = [
- 'return code: %s; command: %s' % (
- self.result.returncode, self.result.cmdstr),
+ 'return code: %s; command: %s' % (self.returncode, self.cmdstr),
]
- if error and self.result.error:
- items.append(self.result.error)
- if output and self.result.output:
- items.append(self.result.output)
+ if stderr and self.stderr:
+ items.append(self.stderr)
+ if stdout and self.stdout:
+ items.append(self.stdout)
if self.msg:
items.append(self.msg)
return '\n'.join(items)
@@ -107,7 +144,7 @@
return self.stringify()
-class TerminateRunCommandError(RunCommandError):
+class TerminateCalledProcessError(CalledProcessError):
"""We were signaled to shutdown while running a command.
Client code shouldn't generally know, nor care about this class. It's
@@ -134,7 +171,7 @@
See RunCommand documentation.
Raises:
- This function may immediately raise RunCommandError if we're operating
+ This function may immediately raise CalledProcessError if we're operating
in a strict sudo context and the API is being misused.
Barring that, see RunCommand's documentation- it can raise the same things
RunCommand does.
@@ -206,9 +243,8 @@
if not rh.signals.relay_signal(original_handler, signum, frame):
# Mock up our own, matching exit code for signaling.
- cmd_result = CommandResult(cmd=cmd, returncode=signum << 8)
- raise TerminateRunCommandError('Received signal %i' % signum,
- cmd_result)
+ raise TerminateCalledProcessError(
+ signum << 8, cmd, msg='Received signal %i' % signum)
class _Popen(subprocess.Popen):
@@ -285,7 +321,7 @@
This dictionary is not used to clear any entries though.
combine_stdout_stderr: Combines stdout and stderr streams into stdout.
check: Whether to raise an exception when command returns a non-zero exit
- code, or return the CommandResult object containing the exit code.
+ code, or return the CompletedProcess object containing the exit code.
Note: will still raise an exception if the cmd file does not exist.
int_timeout: If we're interrupted, how long (in seconds) should we give
the invoked process to clean up before we send a SIGTERM.
@@ -295,10 +331,10 @@
close_fds: Whether to close all fds before running |cmd|.
Returns:
- A CommandResult object.
+ A CompletedProcess object.
Raises:
- RunCommandError: Raises exception on error.
+ CalledProcessError: Raises exception on error.
"""
if capture_output:
redirect_stdout, redirect_stderr = True, True
@@ -307,7 +343,7 @@
stdout = None
stderr = None
stdin = None
- cmd_result = CommandResult()
+ result = CompletedProcess()
# Force the timeout to float; in the process, if it's not convertible,
# a self-explanatory exception will be thrown.
@@ -374,7 +410,7 @@
env = env.copy() if env is not None else os.environ.copy()
env.update(extra_env if extra_env else {})
- cmd_result.cmd = cmd
+ result.args = cmd
proc = None
try:
@@ -393,7 +429,7 @@
signal.signal(signal.SIGTERM, handler)
try:
- (cmd_result.output, cmd_result.error) = proc.communicate(input)
+ (result.stdout, result.stderr) = proc.communicate(input)
finally:
signal.signal(signal.SIGINT, old_sigint)
signal.signal(signal.SIGTERM, old_sigterm)
@@ -402,31 +438,35 @@
# The linter is confused by how stdout is a file & an int.
# pylint: disable=maybe-no-member,no-member
stdout.seek(0)
- cmd_result.output = stdout.read()
+ result.stdout = stdout.read()
stdout.close()
if stderr and stderr != subprocess.STDOUT:
# The linter is confused by how stderr is a file & an int.
# pylint: disable=maybe-no-member,no-member
stderr.seek(0)
- cmd_result.error = stderr.read()
+ result.stderr = stderr.read()
stderr.close()
- cmd_result.returncode = proc.returncode
+ result.returncode = proc.returncode
if check and proc.returncode:
msg = 'cwd=%s' % cwd
if extra_env:
msg += ', extra env=%s' % extra_env
- raise RunCommandError(msg, cmd_result)
+ raise CalledProcessError(
+ result.returncode, result.cmd, stdout=result.stdout,
+ stderr=result.stderr, msg=msg)
except OSError as e:
estr = str(e)
if e.errno == errno.EACCES:
estr += '; does the program need `chmod a+x`?'
if not check:
- cmd_result = CommandResult(cmd=cmd, error=estr, returncode=255)
+ result = CompletedProcess(args=cmd, stderr=estr, returncode=255)
else:
- raise RunCommandError(estr, CommandResult(cmd=cmd), exception=e)
+ raise CalledProcessError(
+ result.returncode, result.cmd, stdout=result.stdout,
+ stderr=result.stderr, msg=estr, exception=e)
finally:
if proc is not None:
# Ensure the process is dead.
@@ -436,10 +476,10 @@
None, None)
# Make sure output is returned as a string rather than bytes.
- if cmd_result.output is not None:
- cmd_result.output = cmd_result.output.decode('utf-8', 'replace')
- if cmd_result.error is not None:
- cmd_result.error = cmd_result.error.decode('utf-8', 'replace')
+ if result.stdout is not None:
+ result.stdout = result.stdout.decode('utf-8', 'replace')
+ if result.stderr is not None:
+ result.stderr = result.stderr.decode('utf-8', 'replace')
- return cmd_result
+ return result
# pylint: enable=redefined-builtin,input-builtin
diff --git a/rh/utils_unittest.py b/rh/utils_unittest.py
index 9cacb84..8f50998 100755
--- a/rh/utils_unittest.py
+++ b/rh/utils_unittest.py
@@ -65,72 +65,69 @@
self.assertEqual('1h6m40.300s', rh.utils.timedelta_str(delta))
-class CommandResultTests(unittest.TestCase):
- """Verify behavior of CommandResult object."""
+class CompletedProcessTests(unittest.TestCase):
+ """Verify behavior of CompletedProcess object."""
def test_empty_cmdstr(self):
"""Check cmdstr with an empty command."""
- result = rh.utils.CommandResult(cmd=[])
+ result = rh.utils.CompletedProcess(args=[])
self.assertEqual('', result.cmdstr)
def test_basic_cmdstr(self):
"""Check cmdstr with a basic command command."""
- result = rh.utils.CommandResult(cmd=['ls', 'a b'])
+ result = rh.utils.CompletedProcess(args=['ls', 'a b'])
self.assertEqual("ls 'a b'", result.cmdstr)
def test_str(self):
"""Check str() handling."""
# We don't enforce much, just that it doesn't crash.
- result = rh.utils.CommandResult()
+ result = rh.utils.CompletedProcess()
self.assertNotEqual('', str(result))
- result = rh.utils.CommandResult(cmd=[])
+ result = rh.utils.CompletedProcess(args=[])
self.assertNotEqual('', str(result))
def test_repr(self):
"""Check repr() handling."""
# We don't enforce much, just that it doesn't crash.
- result = rh.utils.CommandResult()
+ result = rh.utils.CompletedProcess()
self.assertNotEqual('', repr(result))
- result = rh.utils.CommandResult(cmd=[])
+ result = rh.utils.CompletedProcess(args=[])
self.assertNotEqual('', repr(result))
-class RunCommandErrorTests(unittest.TestCase):
- """Verify behavior of RunCommandError object."""
-
- def setUp(self):
- self.result = rh.utils.CommandResult(cmd=['mycmd'])
+class CalledProcessErrorTests(unittest.TestCase):
+ """Verify behavior of CalledProcessError object."""
def test_basic(self):
"""Basic test we can create a normal instance."""
- rh.utils.RunCommandError('msg', self.result)
- rh.utils.RunCommandError('msg', self.result, exception=Exception('bad'))
+ rh.utils.CalledProcessError(0, ['mycmd'])
+ rh.utils.CalledProcessError(1, ['mycmd'], exception=Exception('bad'))
def test_stringify(self):
"""Check stringify() handling."""
# We don't assert much so we leave flexibility in changing format.
- err = rh.utils.RunCommandError('msg', self.result)
+ err = rh.utils.CalledProcessError(0, ['mycmd'])
self.assertIn('mycmd', err.stringify())
- err = rh.utils.RunCommandError('msg', self.result,
- exception=Exception('bad'))
+ err = rh.utils.CalledProcessError(
+ 0, ['mycmd'], exception=Exception('bad'))
self.assertIn('mycmd', err.stringify())
def test_str(self):
"""Check str() handling."""
# We don't assert much so we leave flexibility in changing format.
- err = rh.utils.RunCommandError('msg', self.result)
+ err = rh.utils.CalledProcessError(0, ['mycmd'])
self.assertIn('mycmd', str(err))
- err = rh.utils.RunCommandError('msg', self.result,
- exception=Exception('bad'))
+ err = rh.utils.CalledProcessError(
+ 0, ['mycmd'], exception=Exception('bad'))
self.assertIn('mycmd', str(err))
def test_repr(self):
"""Check repr() handling."""
# We don't assert much so we leave flexibility in changing format.
- err = rh.utils.RunCommandError('msg', self.result)
+ err = rh.utils.CalledProcessError(0, ['mycmd'])
self.assertNotEqual('', repr(err))
- err = rh.utils.RunCommandError('msg', self.result,
- exception=Exception('bad'))
+ err = rh.utils.CalledProcessError(
+ 0, ['mycmd'], exception=Exception('bad'))
self.assertNotEqual('', repr(err))
@@ -184,32 +181,32 @@
"""Simple basic test."""
ret = rh.utils.run(['true'])
self.assertEqual('true', ret.cmdstr)
- self.assertIsNone(ret.output)
- self.assertIsNone(ret.error)
+ self.assertIsNone(ret.stdout)
+ self.assertIsNone(ret.stderr)
def test_stdout_capture(self):
"""Verify output capturing works."""
ret = rh.utils.run(['echo', 'hi'], redirect_stdout=True)
- self.assertEqual('hi\n', ret.output)
- self.assertIsNone(ret.error)
+ self.assertEqual('hi\n', ret.stdout)
+ self.assertIsNone(ret.stderr)
def test_stderr_capture(self):
"""Verify stderr capturing works."""
ret = rh.utils.run(['sh', '-c', 'echo hi >&2'], redirect_stderr=True)
- self.assertIsNone(ret.output)
- self.assertEqual('hi\n', ret.error)
+ self.assertIsNone(ret.stdout)
+ self.assertEqual('hi\n', ret.stderr)
def test_stdout_utf8(self):
"""Verify reading UTF-8 data works."""
ret = rh.utils.run(['printf', r'\xc3\x9f'], redirect_stdout=True)
- self.assertEqual(u'ß', ret.output)
- self.assertIsNone(ret.error)
+ self.assertEqual(u'ß', ret.stdout)
+ self.assertIsNone(ret.stderr)
def test_stdin_utf8(self):
"""Verify writing UTF-8 data works."""
ret = rh.utils.run(['cat'], redirect_stdout=True, input=u'ß')
- self.assertEqual(u'ß', ret.output)
- self.assertIsNone(ret.error)
+ self.assertEqual(u'ß', ret.stdout)
+ self.assertIsNone(ret.stderr)
if __name__ == '__main__':
diff --git a/tools/clang-format.py b/tools/clang-format.py
index ce3b301..7fa5b10 100755
--- a/tools/clang-format.py
+++ b/tools/clang-format.py
@@ -84,12 +84,12 @@
# Fail gracefully if clang-format itself aborts/fails.
try:
result = rh.utils.run(cmd, capture_output=True)
- except rh.utils.RunCommandError as e:
+ except rh.utils.CalledProcessError as e:
print('clang-format failed:\n%s' % (e,), file=sys.stderr)
print('\nPlease report this to the clang team.', file=sys.stderr)
return 1
- stdout = result.output
+ stdout = result.stdout
if stdout.rstrip('\n') == 'no modified files to format':
# This is always printed when only files that clang-format does not
# understand were modified.
diff --git a/tools/google-java-format.py b/tools/google-java-format.py
index e80e50a..ed5ce28 100755
--- a/tools/google-java-format.py
+++ b/tools/google-java-format.py
@@ -83,7 +83,7 @@
# https://github.com/google/google-java-format/issues/107
diff_cmd = ['git', 'diff', '--no-ext-diff', '-U0', '%s^!' % opts.commit]
diff_cmd.extend(['--'] + opts.files)
- diff = rh.utils.run(diff_cmd, capture_output=True).output
+ diff = rh.utils.run(diff_cmd, capture_output=True).stdout
cmd = [opts.google_java_format_diff, '-p1', '--aosp']
if opts.fix:
@@ -92,7 +92,7 @@
cmd.extend(['--skip-sorting-imports'])
stdout = rh.utils.run(cmd, input=diff, capture_output=True,
- extra_env=extra_env).output
+ extra_env=extra_env).stdout
if stdout:
print('One or more files in your commit have Java formatting errors.')
print('You can run `%s --fix %s` to fix this' %
