Snap for 6198741 from 36d2ce62750acc984116f6af9b67b19b23a1afc5 to sdk-release
Change-Id: I5aaf35af3ab2574e6054f8746d90b0838ef9738f
diff --git a/pre-upload.py b/pre-upload.py
index 0862dea..febbfa5 100755
--- a/pre-upload.py
+++ b/pre-upload.py
@@ -23,6 +23,7 @@
from __future__ import print_function
import argparse
+import datetime
import os
import sys
@@ -81,6 +82,7 @@
self.num_hooks = None
self.hook_index = 0
self.success = True
+ self.start_time = datetime.datetime.now()
def set_num_hooks(self, num_hooks):
"""Keep track of how many hooks we'll be running.
@@ -146,10 +148,11 @@
def finish(self):
"""Print summary for all the hooks."""
- status_line = '[%s] repohooks for %s %s' % (
+ status_line = '[%s] repohooks for %s %s in %s' % (
self.PASSED if self.success else self.FAILED,
self.project_name,
- 'passed' if self.success else 'failed')
+ 'passed' if self.success else 'failed',
+ rh.utils.timedelta_str(datetime.datetime.now() - self.start_time))
rh.terminal.print_status_line(status_line, print_newline=True)
@@ -263,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,))
@@ -335,8 +338,8 @@
if proj_dir is None:
cmd = ['repo', 'forall', project_name, '-c', 'pwd']
- result = rh.utils.run_command(cmd, capture_output=True)
- proj_dirs = result.output.split()
+ result = rh.utils.run(cmd, capture_output=True)
+ 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)
@@ -404,8 +407,8 @@
a blank string upon failure.
"""
cmd = ['repo', 'forall', '.', '-c', 'echo ${REPO_PROJECT}']
- return rh.utils.run_command(cmd, capture_output=True, redirect_stderr=True,
- cwd=path).output.strip()
+ return rh.utils.run(cmd, capture_output=True, redirect_stderr=True,
+ cwd=path).stdout.strip()
def direct_main(argv):
@@ -437,8 +440,8 @@
# project from CWD.
if opts.dir is None:
cmd = ['git', 'rev-parse', '--git-dir']
- git_dir = rh.utils.run_command(cmd, capture_output=True,
- redirect_stderr=True).output.strip()
+ git_dir = rh.utils.run(cmd, capture_output=True,
+ 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 cbe8c42..baf669c 100644
--- a/rh/git.py
+++ b/rh/git.py
@@ -34,13 +34,13 @@
"""Returns the current upstream remote name."""
# First get the current branch name.
cmd = ['git', 'rev-parse', '--abbrev-ref', 'HEAD']
- result = rh.utils.run_command(cmd, capture_output=True)
- branch = result.output.strip()
+ result = rh.utils.run(cmd, capture_output=True)
+ branch = result.stdout.strip()
# Then get the remote associated with this branch.
cmd = ['git', 'config', 'branch.%s.remote' % branch]
- result = rh.utils.run_command(cmd, capture_output=True)
- return result.output.strip()
+ result = rh.utils.run(cmd, capture_output=True)
+ return result.stdout.strip()
def get_upstream_branch():
@@ -50,21 +50,21 @@
Error if there is no tracking branch
"""
cmd = ['git', 'symbolic-ref', 'HEAD']
- result = rh.utils.run_command(cmd, capture_output=True)
- current_branch = result.output.strip().replace('refs/heads/', '')
+ result = rh.utils.run(cmd, capture_output=True)
+ 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_command(cmd, capture_output=True)
- full_upstream = result.output.strip()
+ result = rh.utils.run(cmd, capture_output=True)
+ 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_command(cmd, capture_output=True)
- remote = result.output.strip()
+ result = rh.utils.run(cmd, capture_output=True)
+ remote = result.stdout.strip()
if not remote or not full_upstream:
raise ValueError('Need to be on a tracking branch')
@@ -74,8 +74,8 @@
def get_commit_for_ref(ref):
"""Returns the latest commit for this ref."""
cmd = ['git', 'rev-parse', ref]
- result = rh.utils.run_command(cmd, capture_output=True)
- return result.output.strip()
+ result = rh.utils.run(cmd, capture_output=True)
+ 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_command(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_command(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_command(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_command(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_command(cmd, capture_output=True).output
+ return rh.utils.run(cmd, capture_output=True).stdout
def find_repo_root(path=None):
@@ -202,5 +202,5 @@
def is_git_repository(path):
"""Returns True if the path is a valid git repository."""
cmd = ['git', 'rev-parse', '--resolve-git-dir', os.path.join(path, '.git')]
- result = rh.utils.run_command(cmd, quiet=True, error_code_ok=True)
+ result = rh.utils.run(cmd, capture_output=True, check=False)
return result.returncode == 0
diff --git a/rh/hooks.py b/rh/hooks.py
index 3ea4b50..fd86dc0 100644
--- a/rh/hooks.py
+++ b/rh/hooks.py
@@ -186,13 +186,13 @@
return self.expand_vars([tool_path])[0]
-def _run_command(cmd, **kwargs):
+def _run(cmd, **kwargs):
"""Helper command for checks that tend to gather output."""
kwargs.setdefault('redirect_stderr', True)
kwargs.setdefault('combine_stdout_stderr', True)
kwargs.setdefault('capture_output', True)
- kwargs.setdefault('error_code_ok', True)
- return rh.utils.run_command(cmd, **kwargs)
+ kwargs.setdefault('check', False)
+ return rh.utils.run(cmd, **kwargs)
def _match_regex_list(subject, expressions):
@@ -259,9 +259,9 @@
parameter in HookCommandResult.
"""
def wrapper():
- result = _run_command(cmd, **kwargs)
+ result = _run(cmd, **kwargs)
if result.returncode not in (None, 0):
- return result.output
+ return result.stdout
return None
return wrapper
@@ -269,7 +269,7 @@
def _check_cmd(hook_name, project, commit, cmd, fixup_func=None, **kwargs):
"""Runs |cmd| and returns its result as a HookCommandResult."""
return [rh.results.HookCommandResult(hook_name, project, commit,
- _run_command(cmd, **kwargs),
+ _run(cmd, **kwargs),
fixup_func=fixup_func)]
@@ -298,10 +298,10 @@
ret = []
for d in filtered:
data = rh.git.get_file_content(commit, d.file)
- result = _run_command(cmd, input=data)
- if result.output:
+ result = _run(cmd, input=data)
+ 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
@@ -528,10 +528,10 @@
ret = []
for d in filtered:
data = rh.git.get_file_content(commit, d.file)
- result = _run_command(cmd, input=data)
- if result.output:
+ result = _run(cmd, input=data)
+ 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/hooks_unittest.py b/rh/hooks_unittest.py
index 8f74d17..4124f10 100755
--- a/rh/hooks_unittest.py
+++ b/rh/hooks_unittest.py
@@ -213,10 +213,10 @@
"""Verify misc utility functions."""
def testRunCommand(self):
- """Check _run_command behavior."""
+ """Check _run behavior."""
# Most testing is done against the utils.RunCommand already.
# pylint: disable=protected-access
- ret = rh.hooks._run_command(['true'])
+ ret = rh.hooks._run(['true'])
self.assertEqual(ret.returncode, 0)
def testBuildOs(self):
@@ -236,7 +236,7 @@
-@mock.patch.object(rh.utils, 'run_command')
+@mock.patch.object(rh.utils, 'run')
@mock.patch.object(rh.hooks, '_check_cmd', return_value=['check_cmd'])
class BuiltinHooksTests(unittest.TestCase):
"""Verify the builtin hooks."""
@@ -339,6 +339,8 @@
'subj',
'subj\n\nBUG=1234\n',
'subj\n\nBUG: 1234\n',
+ 'subj\n\nBug: N/A\n',
+ 'subj\n\nBug:\n',
))
def test_commit_msg_changeid_field(self, _mock_check, _mock_run):
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 a1aca6e..c6c77e0 100644
--- a/rh/utils.py
+++ b/rh/utils.py
@@ -37,14 +37,46 @@
from rh.sixish import string_types
-class CommandResult(object):
- """An object to store various attributes of a child process."""
+def timedelta_str(delta):
+ """A less noisy timedelta.__str__.
- def __init__(self, cmd=None, error=None, output=None, returncode=None):
- self.cmd = cmd
- self.error = error
- self.output = output
- self.returncode = returncode
+ The default timedelta stringification contains a lot of leading zeros and
+ uses microsecond resolution. This makes for noisy output.
+ """
+ total = delta.total_seconds()
+ hours, rem = divmod(total, 3600)
+ mins, secs = divmod(rem, 60)
+ ret = '%i.%03is' % (secs, delta.microseconds // 1000)
+ if mins:
+ ret = '%im%s' % (mins, ret)
+ if hours:
+ ret = '%ih%s' % (hours, ret)
+ return ret
+
+
+class CompletedProcess(getattr(subprocess, 'CompletedProcess', object)):
+ """An object to store various attributes of a child process.
+
+ 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):
@@ -52,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)
@@ -89,15 +143,8 @@
def __str__(self):
return self.stringify()
- def __eq__(self, other):
- return (isinstance(other, type(self)) and
- self.args == other.args)
- def __ne__(self, other):
- return not self.__eq__(other)
-
-
-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
@@ -105,7 +152,7 @@
"""
-def sudo_run_command(cmd, user='root', **kwargs):
+def sudo_run(cmd, user='root', **kwargs):
"""Run a command via sudo.
Client code must use this rather than coming up with their own RunCommand
@@ -124,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.
@@ -136,7 +183,7 @@
sudo_cmd = ['sudo']
if user == 'root' and os.geteuid() == 0:
- return run_command(cmd, **kwargs)
+ return run(cmd, **kwargs)
if user != 'root':
sudo_cmd += ['-u', user]
@@ -153,7 +200,7 @@
sudo_cmd.extend(cmd)
- return run_command(sudo_cmd, **kwargs)
+ return run(sudo_cmd, **kwargs)
def _kill_child_process(proc, int_timeout, kill_timeout, cmd, original_handler,
@@ -196,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):
@@ -230,10 +276,10 @@
if e.errno == errno.EPERM:
# Kill returns either 0 (signal delivered), or 1 (signal wasn't
# delivered). This isn't particularly informative, but we still
- # need that info to decide what to do, thus error_code_ok=True.
- ret = sudo_run_command(['kill', '-%i' % signum, str(self.pid)],
- redirect_stdout=True,
- redirect_stderr=True, error_code_ok=True)
+ # need that info to decide what to do, thus check=False.
+ ret = sudo_run(['kill', '-%i' % signum, str(self.pid)],
+ redirect_stdout=True,
+ redirect_stderr=True, check=False)
if ret.returncode == 1:
# The kill binary doesn't distinguish between permission
# denied and the pid is missing. Denied can only occur
@@ -253,20 +299,16 @@
# We use the keyword arg |input| which trips up pylint checks.
# pylint: disable=redefined-builtin,input-builtin
-def run_command(cmd, error_message=None, redirect_stdout=False,
- redirect_stderr=False, cwd=None, input=None,
- shell=False, env=None, extra_env=None, ignore_sigint=False,
- combine_stdout_stderr=False, log_stdout_to_file=None,
- error_code_ok=False, int_timeout=1, kill_timeout=1,
- stdout_to_pipe=False, capture_output=False,
- quiet=False, close_fds=True):
+def run(cmd, redirect_stdout=False, redirect_stderr=False, cwd=None, input=None,
+ shell=False, env=None, extra_env=None, combine_stdout_stderr=False,
+ check=True, int_timeout=1, kill_timeout=1, capture_output=False,
+ close_fds=True):
"""Runs a command.
Args:
cmd: cmd to run. Should be input to subprocess.Popen. If a string, shell
must be true. Otherwise the command must be an array of arguments,
and shell must be false.
- error_message: Prints out this message when an error occurs.
redirect_stdout: Returns the stdout.
redirect_stderr: Holds stderr output until input is communicated.
cwd: The working directory to run this cmd.
@@ -277,43 +319,31 @@
env: If non-None, this is the environment for the new process.
extra_env: If set, this is added to the environment for the new process.
This dictionary is not used to clear any entries though.
- ignore_sigint: If True, we'll ignore signal.SIGINT before calling the
- child. This is the desired behavior if we know our child will handle
- Ctrl-C. If we don't do this, I think we and the child will both get
- Ctrl-C at the same time, which means we'll forcefully kill the child.
combine_stdout_stderr: Combines stdout and stderr streams into stdout.
- log_stdout_to_file: If set, redirects stdout to file specified by this
- path. If |combine_stdout_stderr| is set to True, then stderr will
- also be logged to the specified file.
- error_code_ok: Does not raise an exception when command returns a non-zero
- exit code. Instead, returns the CommandResult object containing the
- exit code.
+ check: Whether to raise an exception when command returns a non-zero exit
+ 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.
kill_timeout: If we're interrupted, how long (in seconds) should we give
the invoked process to shutdown from a SIGTERM before we SIGKILL it.
- stdout_to_pipe: Redirect stdout to pipe.
capture_output: Set |redirect_stdout| and |redirect_stderr| to True.
- quiet: Set |stdout_to_pipe| and |combine_stdout_stderr| to True.
close_fds: Whether to close all fds before running |cmd|.
Returns:
- A CommandResult object.
+ A CompletedProcess object.
Raises:
- RunCommandError: Raises exception on error with optional error_message.
+ CalledProcessError: Raises exception on error.
"""
if capture_output:
redirect_stdout, redirect_stderr = True, True
- if quiet:
- stdout_to_pipe, combine_stdout_stderr = True, True
-
# Set default for variables.
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.
@@ -343,11 +373,7 @@
# view of the file.
# The Popen API accepts either an int or a file handle for stdout/stderr.
# pylint: disable=redefined-variable-type
- if log_stdout_to_file:
- stdout = open(log_stdout_to_file, 'w+')
- elif stdout_to_pipe:
- stdout = subprocess.PIPE
- elif redirect_stdout:
+ if redirect_stdout:
stdout = _get_tempfile()
if combine_stdout_stderr:
@@ -384,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,12 +419,8 @@
close_fds=close_fds)
old_sigint = signal.getsignal(signal.SIGINT)
- if ignore_sigint:
- handler = signal.SIG_IGN
- else:
- handler = functools.partial(
- _kill_child_process, proc, int_timeout, kill_timeout, cmd,
- old_sigint)
+ handler = functools.partial(_kill_child_process, proc, int_timeout,
+ kill_timeout, cmd, old_sigint)
signal.signal(signal.SIGINT, handler)
old_sigterm = signal.getsignal(signal.SIGTERM)
@@ -407,42 +429,44 @@
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)
- if stdout and not log_stdout_to_file and not stdout_to_pipe:
+ if stdout:
# 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 not error_code_ok and proc.returncode:
+ if check and proc.returncode:
msg = 'cwd=%s' % cwd
if extra_env:
msg += ', extra env=%s' % extra_env
- if error_message:
- msg += '\n%s' % error_message
- 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 error_code_ok:
- cmd_result = CommandResult(cmd=cmd, error=estr, returncode=255)
+ if not check:
+ 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.
@@ -452,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 881d031..8f50998 100755
--- a/rh/utils_unittest.py
+++ b/rh/utils_unittest.py
@@ -18,6 +18,7 @@
from __future__ import print_function
+import datetime
import os
import sys
import unittest
@@ -35,132 +36,132 @@
from rh.sixish import mock
-class CommandResultTests(unittest.TestCase):
- """Verify behavior of CommandResult object."""
+class TimeDeltaStrTests(unittest.TestCase):
+ """Verify behavior of timedelta_str object."""
+
+ def test_same(self):
+ """Check timedelta of 0 seconds."""
+ delta = datetime.timedelta(0)
+ self.assertEqual('0.000s', rh.utils.timedelta_str(delta))
+
+ def test_millisecondss(self):
+ """Check timedelta of milliseconds."""
+ delta = datetime.timedelta(seconds=0.123456)
+ self.assertEqual('0.123s', rh.utils.timedelta_str(delta))
+
+ def test_seconds(self):
+ """Check timedelta of seconds."""
+ delta = datetime.timedelta(seconds=12.3)
+ self.assertEqual('12.300s', rh.utils.timedelta_str(delta))
+
+ def test_minutes(self):
+ """Check timedelta of minutes."""
+ delta = datetime.timedelta(seconds=72.3)
+ self.assertEqual('1m12.300s', rh.utils.timedelta_str(delta))
+
+ def test_hours(self):
+ """Check timedelta of hours."""
+ delta = datetime.timedelta(seconds=4000.3)
+ self.assertEqual('1h6m40.300s', rh.utils.timedelta_str(delta))
+
+
+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))
- def test_eq(self):
- """Check object equality."""
- # Note: We explicitly do not use assertEqual here.
- # We also explicitly compare to ourselves to make sure our __eq__ works.
- # We also disable bad-option-value because this is a pylint3 warning.
- # pylint: disable=bad-option-value,comparison-with-itself
- err1 = rh.utils.RunCommandError('msg', self.result)
- self.assertTrue(err1 == err1)
- err2 = rh.utils.RunCommandError('msg', self.result)
- self.assertTrue(err1 == err2)
- err3 = rh.utils.RunCommandError('foo', self.result)
- self.assertFalse(err1 == err3)
-
- def test_ne(self):
- """Check object inequality."""
- # Note: We explicitly do not use assertNotEqual here.
- # We also explicitly compare to ourselves to make sure our __eq__ works.
- # We also disable bad-option-value because this is a pylint3 warning.
- # pylint: disable=bad-option-value,comparison-with-itself
- err1 = rh.utils.RunCommandError('msg', self.result)
- self.assertFalse(err1 != err1)
- err2 = rh.utils.RunCommandError('msg', self.result)
- self.assertFalse(err1 != err2)
- err3 = rh.utils.RunCommandError('foo', self.result)
- self.assertTrue(err1 != err3)
-
# We shouldn't require sudo to run unittests :).
-@mock.patch.object(rh.utils, 'run_command')
+@mock.patch.object(rh.utils, 'run')
@mock.patch.object(os, 'geteuid', return_value=1000)
class SudoRunCommandTests(unittest.TestCase):
- """Verify behavior of sudo_run_command helper."""
+ """Verify behavior of sudo_run helper."""
def test_run_as_root_as_root(self, mock_geteuid, mock_run):
"""Check behavior when we're already root."""
mock_geteuid.return_value = 0
- ret = rh.utils.sudo_run_command(['ls'], user='root')
+ ret = rh.utils.sudo_run(['ls'], user='root')
self.assertIsNotNone(ret)
args, _kwargs = mock_run.call_args
self.assertEqual((['ls'],), args)
def test_run_as_root_as_nonroot(self, _mock_geteuid, mock_run):
"""Check behavior when we're not already root."""
- ret = rh.utils.sudo_run_command(['ls'], user='root')
+ ret = rh.utils.sudo_run(['ls'], user='root')
self.assertIsNotNone(ret)
args, _kwargs = mock_run.call_args
self.assertEqual((['sudo', '--', 'ls'],), args)
def test_run_as_nonroot_as_nonroot(self, _mock_geteuid, mock_run):
"""Check behavior when we're not already root."""
- ret = rh.utils.sudo_run_command(['ls'], user='nobody')
+ ret = rh.utils.sudo_run(['ls'], user='nobody')
self.assertIsNotNone(ret)
args, _kwargs = mock_run.call_args
self.assertEqual((['sudo', '-u', 'nobody', '--', 'ls'],), args)
def test_env(self, _mock_geteuid, mock_run):
"""Check passing through env vars."""
- ret = rh.utils.sudo_run_command(['ls'], extra_env={'FOO': 'bar'})
+ ret = rh.utils.sudo_run(['ls'], extra_env={'FOO': 'bar'})
self.assertIsNotNone(ret)
args, _kwargs = mock_run.call_args
self.assertEqual((['sudo', 'FOO=bar', '--', 'ls'],), args)
@@ -168,46 +169,44 @@
def test_shell(self, _mock_geteuid, _mock_run):
"""Check attempts to use shell code are rejected."""
with self.assertRaises(AssertionError):
- rh.utils.sudo_run_command('foo')
+ rh.utils.sudo_run('foo')
with self.assertRaises(AssertionError):
- rh.utils.sudo_run_command(['ls'], shell=True)
+ rh.utils.sudo_run(['ls'], shell=True)
class RunCommandTests(unittest.TestCase):
- """Verify behavior of run_command helper."""
+ """Verify behavior of run helper."""
def test_basic(self):
"""Simple basic test."""
- ret = rh.utils.run_command(['true'])
+ 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_command(['echo', 'hi'], redirect_stdout=True)
- self.assertEqual('hi\n', ret.output)
- self.assertIsNone(ret.error)
+ ret = rh.utils.run(['echo', 'hi'], redirect_stdout=True)
+ self.assertEqual('hi\n', ret.stdout)
+ self.assertIsNone(ret.stderr)
def test_stderr_capture(self):
"""Verify stderr capturing works."""
- ret = rh.utils.run_command(['sh', '-c', 'echo hi >&2'],
- redirect_stderr=True)
- self.assertIsNone(ret.output)
- self.assertEqual('hi\n', ret.error)
+ ret = rh.utils.run(['sh', '-c', 'echo hi >&2'], redirect_stderr=True)
+ 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_command(['printf', r'\xc3\x9f'],
- redirect_stdout=True)
- self.assertEqual(u'ß', ret.output)
- self.assertIsNone(ret.error)
+ ret = rh.utils.run(['printf', r'\xc3\x9f'], redirect_stdout=True)
+ self.assertEqual(u'ß', ret.stdout)
+ self.assertIsNone(ret.stderr)
def test_stdin_utf8(self):
"""Verify writing UTF-8 data works."""
- ret = rh.utils.run_command(['cat'], redirect_stdout=True, input=u'ß')
- self.assertEqual(u'ß', ret.output)
- self.assertIsNone(ret.error)
+ ret = rh.utils.run(['cat'], redirect_stdout=True, input=u'ß')
+ 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 1b1c9de..7fa5b10 100755
--- a/tools/clang-format.py
+++ b/tools/clang-format.py
@@ -83,13 +83,13 @@
# Fail gracefully if clang-format itself aborts/fails.
try:
- result = rh.utils.run_command(cmd, capture_output=True)
- except rh.utils.RunCommandError as e:
+ result = rh.utils.run(cmd, capture_output=True)
+ 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.
@@ -102,8 +102,7 @@
if diff_filenames:
if opts.fix:
- result = rh.utils.run_command(['git', 'apply'], input=stdout,
- error_code_ok=True)
+ result = rh.utils.run(['git', 'apply'], input=stdout, check=False)
if result.returncode:
print('Error: Unable to automatically fix things.\n'
' Make sure your checkout is clean first.\n'
diff --git a/tools/google-java-format.py b/tools/google-java-format.py
index a9ac4d1..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_command(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:
@@ -91,10 +91,8 @@
if not opts.sort_imports:
cmd.extend(['--skip-sorting-imports'])
- stdout = rh.utils.run_command(cmd,
- input=diff,
- capture_output=True,
- extra_env=extra_env).output
+ stdout = rh.utils.run(cmd, input=diff, capture_output=True,
+ 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' %
