utils: rename run_command to run
Align our APIs to Python 3.6+ subprocess.run by renaming our
run_command() to run().
Bug: None
Test: unittests pass
Change-Id: Ib65a0e7bb098a8ab17b190e12656450f1b314d6c
diff --git a/pre-upload.py b/pre-upload.py
index 0fb4015..fa44243 100755
--- a/pre-upload.py
+++ b/pre-upload.py
@@ -338,7 +338,7 @@
if proj_dir is None:
cmd = ['repo', 'forall', project_name, '-c', 'pwd']
- result = rh.utils.run_command(cmd, capture_output=True)
+ result = rh.utils.run(cmd, capture_output=True)
proj_dirs = result.output.split()
if not proj_dirs:
print('%s cannot be found.' % project_name, file=sys.stderr)
@@ -407,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).output.strip()
def direct_main(argv):
@@ -440,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).output.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..70ad091 100644
--- a/rh/git.py
+++ b/rh/git.py
@@ -34,12 +34,12 @@
"""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)
+ result = rh.utils.run(cmd, capture_output=True)
branch = result.output.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)
+ result = rh.utils.run(cmd, capture_output=True)
return result.output.strip()
@@ -50,20 +50,20 @@
Error if there is no tracking branch
"""
cmd = ['git', 'symbolic-ref', 'HEAD']
- result = rh.utils.run_command(cmd, capture_output=True)
+ result = rh.utils.run(cmd, capture_output=True)
current_branch = result.output.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)
+ result = rh.utils.run(cmd, capture_output=True)
full_upstream = result.output.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)
+ result = rh.utils.run(cmd, capture_output=True)
remote = result.output.strip()
if not remote or not full_upstream:
raise ValueError('Need to be on a tracking branch')
@@ -74,7 +74,7 @@
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)
+ result = rh.utils.run(cmd, capture_output=True)
return result.output.strip()
@@ -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).output
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).output
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).output
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).output.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).output
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, quiet=True, error_code_ok=True)
return result.returncode == 0
diff --git a/rh/hooks.py b/rh/hooks.py
index 3ea4b50..e1b530d 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)
+ return rh.utils.run(cmd, **kwargs)
def _match_regex_list(subject, expressions):
@@ -259,7 +259,7 @@
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 None
@@ -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,7 +298,7 @@
ret = []
for d in filtered:
data = rh.git.get_file_content(commit, d.file)
- result = _run_command(cmd, input=data)
+ result = _run(cmd, input=data)
if result.output:
ret.append(rh.results.HookResult(
'bpfmt', project, commit, error=result.output,
@@ -528,7 +528,7 @@
ret = []
for d in filtered:
data = rh.git.get_file_content(commit, d.file)
- result = _run_command(cmd, input=data)
+ result = _run(cmd, input=data)
if result.output:
ret.append(rh.results.HookResult(
'gofmt', project, commit, error=result.output,
diff --git a/rh/hooks_unittest.py b/rh/hooks_unittest.py
index 4510c6e..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."""
diff --git a/rh/utils.py b/rh/utils.py
index a4b7b5f..1114e07 100644
--- a/rh/utils.py
+++ b/rh/utils.py
@@ -122,7 +122,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
@@ -153,7 +153,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]
@@ -170,7 +170,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,
@@ -248,9 +248,9 @@
# 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)
+ ret = sudo_run(['kill', '-%i' % signum, str(self.pid)],
+ redirect_stdout=True,
+ redirect_stderr=True, error_code_ok=True)
if ret.returncode == 1:
# The kill binary doesn't distinguish between permission
# denied and the pid is missing. Denied can only occur
@@ -270,13 +270,13 @@
# 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, 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):
"""Runs a command.
Args:
diff --git a/rh/utils_unittest.py b/rh/utils_unittest.py
index f60e22a..aed007c 100755
--- a/rh/utils_unittest.py
+++ b/rh/utils_unittest.py
@@ -161,36 +161,36 @@
# 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)
@@ -198,44 +198,42 @@
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)
def test_stdout_capture(self):
"""Verify output capturing works."""
- ret = rh.utils.run_command(['echo', 'hi'], redirect_stdout=True)
+ ret = rh.utils.run(['echo', 'hi'], redirect_stdout=True)
self.assertEqual('hi\n', ret.output)
self.assertIsNone(ret.error)
def test_stderr_capture(self):
"""Verify stderr capturing works."""
- ret = rh.utils.run_command(['sh', '-c', 'echo hi >&2'],
- redirect_stderr=True)
+ ret = rh.utils.run(['sh', '-c', 'echo hi >&2'], redirect_stderr=True)
self.assertIsNone(ret.output)
self.assertEqual('hi\n', ret.error)
def test_stdout_utf8(self):
"""Verify reading UTF-8 data works."""
- ret = rh.utils.run_command(['printf', r'\xc3\x9f'],
- redirect_stdout=True)
+ ret = rh.utils.run(['printf', r'\xc3\x9f'], redirect_stdout=True)
self.assertEqual(u'ß', ret.output)
self.assertIsNone(ret.error)
def test_stdin_utf8(self):
"""Verify writing UTF-8 data works."""
- ret = rh.utils.run_command(['cat'], redirect_stdout=True, input=u'ß')
+ ret = rh.utils.run(['cat'], redirect_stdout=True, input=u'ß')
self.assertEqual(u'ß', ret.output)
self.assertIsNone(ret.error)
diff --git a/tools/clang-format.py b/tools/clang-format.py
index 1b1c9de..af28b9e 100755
--- a/tools/clang-format.py
+++ b/tools/clang-format.py
@@ -83,7 +83,7 @@
# Fail gracefully if clang-format itself aborts/fails.
try:
- result = rh.utils.run_command(cmd, capture_output=True)
+ result = rh.utils.run(cmd, capture_output=True)
except rh.utils.RunCommandError as e:
print('clang-format failed:\n%s' % (e,), file=sys.stderr)
print('\nPlease report this to the clang team.', file=sys.stderr)
@@ -102,8 +102,8 @@
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,
+ error_code_ok=True)
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..e80e50a 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).output
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).output
if stdout:
print('One or more files in your commit have Java formatting errors.')
print('You can run `%s --fix %s` to fix this' %
