Snap for 11462495 from e106ee3301113116bdc4e11cdb9af60ea946d12b to mainline-tzdata2-release
Change-Id: I76f502970ba27f8995e9dfd620b3e58b942933c2
diff --git a/PREUPLOAD.cfg b/PREUPLOAD.cfg
index 631915d..31de3b0 100644
--- a/PREUPLOAD.cfg
+++ b/PREUPLOAD.cfg
@@ -2,7 +2,9 @@
# Only list fast unittests here.
config_unittest = ./rh/config_unittest.py
hooks_unittest = ./rh/hooks_unittest.py
+results_unittest = ./rh/results_unittest.py
shell_unittest = ./rh/shell_unittest.py
+terminal_unittest = ./rh/terminal_unittest.py
utils_unittest = ./rh/utils_unittest.py
android_test_mapping_format_unittest = ./tools/android_test_mapping_format_unittest.py
clang-format unittest = ./tools/clang-format_unittest.py
diff --git a/pre-upload.py b/pre-upload.py
index 0109133..18bf11f 100755
--- a/pre-upload.py
+++ b/pre-upload.py
@@ -20,9 +20,12 @@
"""
import argparse
+import concurrent.futures
import datetime
import os
+import signal
import sys
+from typing import List, Optional
# Assert some minimum Python versions as we don't test or support any others.
@@ -61,6 +64,7 @@
PASSED = COLOR.color(COLOR.GREEN, 'PASSED')
FAILED = COLOR.color(COLOR.RED, 'FAILED')
WARNING = COLOR.color(COLOR.YELLOW, 'WARNING')
+ FIXUP = COLOR.color(COLOR.MAGENTA, 'FIXUP')
# How long a hook is allowed to run before we warn that it is "too slow".
_SLOW_HOOK_DURATION = datetime.timedelta(seconds=30)
@@ -72,70 +76,91 @@
project_name: name of project.
"""
self.project_name = project_name
+ self.hooks = None
self.num_hooks = None
- self.hook_index = 0
+ self.num_commits = None
+ self.commit_index = 0
self.success = True
self.start_time = datetime.datetime.now()
self.hook_start_time = None
- self._curr_hook_name = None
+ # Cache number of invisible characters in our banner.
+ self._banner_esc_chars = len(self.COLOR.color(self.COLOR.YELLOW, ''))
- def set_num_hooks(self, num_hooks):
- """Keep track of how many hooks we'll be running.
+ def set_num_commits(self, num_commits: int) -> None:
+ """Keep track of how many commits we'll be running.
Args:
- num_hooks: number of hooks to be run.
+ num_commits: Number of commits to be run.
"""
- self.num_hooks = num_hooks
+ self.num_commits = num_commits
+ self.commit_index = 1
- def commit_start(self, commit, commit_summary):
+ def commit_start(self, hooks, commit, commit_summary):
"""Emit status for new commit.
Args:
+ hooks: All the hooks to be run for this commit.
commit: commit hash.
commit_summary: commit summary.
"""
- status_line = f'[{self.COMMIT} {commit[0:12]}] {commit_summary}'
+ status_line = (
+ f'[{self.COMMIT} '
+ f'{self.commit_index}/{self.num_commits} '
+ f'{commit[0:12]}] {commit_summary}'
+ )
rh.terminal.print_status_line(status_line, print_newline=True)
- self.hook_index = 1
+ self.commit_index += 1
- def hook_start(self, hook_name):
- """Emit status before the start of a hook.
+ # Initialize the pending hooks line too.
+ self.hooks = set(hooks)
+ self.num_hooks = len(hooks)
+ self.hook_banner()
- Args:
- hook_name: name of the hook.
- """
- self._curr_hook_name = hook_name
- self.hook_start_time = datetime.datetime.now()
- status_line = (f'[{self.RUNNING} {self.hook_index}/{self.num_hooks}] '
- f'{hook_name}')
- self.hook_index += 1
+ def hook_banner(self):
+ """Display the banner for current set of hooks."""
+ pending = ', '.join(x.name for x in self.hooks)
+ status_line = (
+ f'[{self.RUNNING} '
+ f'{self.num_hooks - len(self.hooks)}/{self.num_hooks}] '
+ f'{pending}'
+ )
+ if self._banner_esc_chars and sys.stderr.isatty():
+ cols = os.get_terminal_size(sys.stderr.fileno()).columns
+ status_line = status_line[0:cols + self._banner_esc_chars]
rh.terminal.print_status_line(status_line)
- def hook_finish(self):
+ def hook_finish(self, hook, duration):
"""Finish processing any per-hook state."""
- duration = datetime.datetime.now() - self.hook_start_time
+ self.hooks.remove(hook)
if duration >= self._SLOW_HOOK_DURATION:
d = rh.utils.timedelta_str(duration)
self.hook_warning(
+ hook,
f'This hook took {d} to finish which is fairly slow for '
'developers.\nPlease consider moving the check to the '
'server/CI system instead.')
- def hook_error(self, error):
+ # Show any hooks still pending.
+ if self.hooks:
+ self.hook_banner()
+
+ def hook_error(self, hook, error):
"""Print an error for a single hook.
Args:
+ hook: The hook that generated the output.
error: error string.
"""
- self.error(self._curr_hook_name, error)
+ self.error(f'{hook.name} hook', error)
- def hook_warning(self, warning):
+ def hook_warning(self, hook, warning):
"""Print a warning for a single hook.
Args:
+ hook: The hook that generated the output.
warning: warning string.
"""
- status_line = f'[{self.WARNING}] {self._curr_hook_name}'
+ status_line = f'[{self.WARNING}] {hook.name}'
rh.terminal.print_status_line(status_line, print_newline=True)
print(warning, file=sys.stderr)
@@ -151,6 +176,21 @@
print(error, file=sys.stderr)
self.success = False
+ def hook_fixups(
+ self,
+ project_results: rh.results.ProjectResults,
+ hook_results: List[rh.results.HookResult],
+ ) -> None:
+ """Display summary of possible fixups for a single hook."""
+ for result in (x for x in hook_results if x.fixup_cmd):
+ cmd = result.fixup_cmd + list(result.files)
+ for line in (
+ f'[{self.FIXUP}] {result.hook} has automated fixups available',
+ f' cd {rh.shell.quote(project_results.workdir)} && \\',
+ f' {rh.shell.cmd_to_str(cmd)}',
+ ):
+ rh.terminal.print_status_line(line, print_newline=True)
+
def finish(self):
"""Print summary for all the hooks."""
header = self.PASSED if self.success else self.FAILED
@@ -182,10 +222,10 @@
error_ret = ''
warning_ret = ''
for result in results:
- if result:
+ if result or result.is_warning():
ret = ''
if result.files:
- ret += f' FILES: {result.files}'
+ ret += f' FILES: {rh.shell.cmd_to_str(result.files)}\n'
lines = result.error.splitlines()
ret += '\n'.join(f' {x}' for x in lines)
if result.is_warning():
@@ -224,44 +264,86 @@
return rh.config.PreUploadSettings(paths=paths, global_paths=global_paths)
-def _attempt_fixes(fixup_func_list, commit_list):
- """Attempts to run |fixup_func_list| given |commit_list|."""
- if len(fixup_func_list) != 1:
- # Only single fixes will be attempted, since various fixes might
- # interact with each other.
+def _attempt_fixes(projects_results: List[rh.results.ProjectResults]) -> None:
+ """Attempts to fix fixable results."""
+ # Filter out any result that has a fixup.
+ fixups = []
+ for project_results in projects_results:
+ fixups.extend((project_results.workdir, x)
+ for x in project_results.fixups)
+ if not fixups:
return
- hook_name, commit, fixup_func = fixup_func_list[0]
-
- if commit != commit_list[0]:
- # If the commit is not at the top of the stack, git operations might be
- # needed and might leave the working directory in a tricky state if the
- # fix is attempted to run automatically (e.g. it might require manual
- # merge conflict resolution). Refuse to run the fix in those cases.
- return
-
- prompt = (f'An automatic fix can be attempted for the "{hook_name}" hook. '
- 'Do you want to run it?')
- if not rh.terminal.boolean_prompt(prompt):
- return
-
- result = fixup_func()
- if result:
- print(f'Attempt to fix "{hook_name}" for commit "{commit}" failed: '
- f'{result}',
- file=sys.stderr)
+ if len(fixups) > 1:
+ banner = f'Multiple fixups ({len(fixups)}) are available.'
else:
- print('Fix successfully applied. Amend the current commit before '
- 'attempting to upload again.\n', file=sys.stderr)
+ banner = 'Automated fixups are available.'
+ print(Output.COLOR.color(Output.COLOR.MAGENTA, banner), file=sys.stderr)
+ # If there's more than one fixup available, ask if they want to blindly run
+ # them all, or prompt for them one-by-one.
+ mode = 'some'
+ if len(fixups) > 1:
+ while True:
+ response = rh.terminal.str_prompt(
+ 'What would you like to do',
+ ('Run (A)ll', 'Run (S)ome', '(D)ry-run', '(N)othing [default]'))
+ if not response:
+ print('', file=sys.stderr)
+ return
+ if response.startswith('a') or response.startswith('y'):
+ mode = 'all'
+ break
+ elif response.startswith('s'):
+ mode = 'some'
+ break
+ elif response.startswith('d'):
+ mode = 'dry-run'
+ break
+ elif response.startswith('n'):
+ print('', file=sys.stderr)
+ return
-def _run_project_hooks_in_cwd(project_name, proj_dir, output, from_git=False, commit_list=None):
+ # Walk all the fixups and run them one-by-one.
+ for workdir, result in fixups:
+ if mode == 'some':
+ if not rh.terminal.boolean_prompt(
+ f'Run {result.hook} fixup for {result.commit}'
+ ):
+ continue
+
+ cmd = tuple(result.fixup_cmd) + tuple(result.files)
+ print(
+ f'\n[{Output.RUNNING}] cd {rh.shell.quote(workdir)} && '
+ f'{rh.shell.cmd_to_str(cmd)}', file=sys.stderr)
+ if mode == 'dry-run':
+ continue
+
+ cmd_result = rh.utils.run(cmd, cwd=workdir, check=False)
+ if cmd_result.returncode:
+ print(f'[{Output.WARNING}] command exited {cmd_result.returncode}',
+ file=sys.stderr)
+ else:
+ print(f'[{Output.PASSED}] great success', file=sys.stderr)
+
+ print(f'\n[{Output.FIXUP}] Please amend & rebase your tree before '
+ 'attempting to upload again.\n', file=sys.stderr)
+
+def _run_project_hooks_in_cwd(
+ project_name: str,
+ proj_dir: str,
+ output: Output,
+ jobs: Optional[int] = None,
+ from_git: bool = False,
+ commit_list: Optional[List[str]] = None,
+) -> rh.results.ProjectResults:
"""Run the project-specific hooks in the cwd.
Args:
project_name: The name of this project.
proj_dir: The directory for this project (for passing on in metadata).
output: Helper for summarizing output/errors to the user.
+ jobs: How many hooks to run in parallel.
from_git: If true, we are called from git directly and repo should not be
used.
commit_list: A list of commits to run hooks against. If None or empty
@@ -269,20 +351,20 @@
uploaded.
Returns:
- False if any errors were found, else True.
+ All the results for this project.
"""
+ ret = rh.results.ProjectResults(project_name, proj_dir)
+
try:
config = _get_project_config(from_git)
except rh.config.ValidationError as e:
output.error('Loading config files', str(e))
- return False
+ return ret._replace(internal_failure=True)
# If the repo has no pre-upload hooks enabled, then just return.
hooks = list(config.callable_hooks())
if not hooks:
- return True
-
- output.set_num_hooks(len(hooks))
+ return ret
# Set up the environment like repo would with the forall command.
try:
@@ -291,11 +373,16 @@
except rh.utils.CalledProcessError as e:
output.error('Upstream remote/tracking branch lookup',
f'{e}\nDid you run repo start? Is your HEAD detached?')
- return False
+ return ret._replace(internal_failure=True)
- project = rh.Project(name=project_name, dir=proj_dir, remote=remote)
+ project = rh.Project(name=project_name, dir=proj_dir)
rel_proj_dir = os.path.relpath(proj_dir, rh.git.find_repo_root())
+ # Filter out the hooks to process.
+ hooks = [x for x in hooks if rel_proj_dir not in x.scope]
+ if not hooks:
+ return ret
+
os.environ.update({
'REPO_LREV': rh.git.get_commit_for_ref(upstream_branch),
'REPO_PATH': rel_proj_dir,
@@ -307,51 +394,61 @@
if not commit_list:
commit_list = rh.git.get_commits(
ignore_merged_commits=config.ignore_merged_commits)
+ output.set_num_commits(len(commit_list))
- ret = True
- fixup_func_list = []
+ def _run_hook(hook, project, commit, desc, diff):
+ """Run a hook, gather stats, and process its results."""
+ start = datetime.datetime.now()
+ results = hook.hook(project, commit, desc, diff)
+ (error, warning) = _process_hook_results(results)
+ duration = datetime.datetime.now() - start
+ return (hook, results, error, warning, duration)
- for commit in commit_list:
- # Mix in some settings for our hooks.
- os.environ['PREUPLOAD_COMMIT'] = commit
- diff = rh.git.get_affected_files(commit)
- desc = rh.git.get_commit_desc(commit)
- os.environ['PREUPLOAD_COMMIT_MESSAGE'] = desc
+ with concurrent.futures.ThreadPoolExecutor(max_workers=jobs) as executor:
+ for commit in commit_list:
+ # Mix in some settings for our hooks.
+ os.environ['PREUPLOAD_COMMIT'] = commit
+ diff = rh.git.get_affected_files(commit)
+ desc = rh.git.get_commit_desc(commit)
+ os.environ['PREUPLOAD_COMMIT_MESSAGE'] = desc
- commit_summary = desc.split('\n', 1)[0]
- output.commit_start(commit=commit, commit_summary=commit_summary)
+ commit_summary = desc.split('\n', 1)[0]
+ output.commit_start(hooks, commit, commit_summary)
- for name, hook, exclusion_scope in hooks:
- output.hook_start(name)
- if rel_proj_dir in exclusion_scope:
- break
- hook_results = hook(project, commit, desc, diff)
- output.hook_finish()
- (error, warning) = _process_hook_results(hook_results)
- if error is not None or warning is not None:
- if warning is not None:
- output.hook_warning(warning)
- if error is not None:
- ret = False
- output.hook_error(error)
- for result in hook_results:
- if result.fixup_func:
- fixup_func_list.append((name, commit,
- result.fixup_func))
-
- if fixup_func_list:
- _attempt_fixes(fixup_func_list, commit_list)
+ futures = (
+ executor.submit(_run_hook, hook, project, commit, desc, diff)
+ for hook in hooks
+ )
+ future_results = (
+ x.result() for x in concurrent.futures.as_completed(futures)
+ )
+ for hook, hook_results, error, warning, duration in future_results:
+ ret.add_results(hook_results)
+ if error is not None or warning is not None:
+ if warning is not None:
+ output.hook_warning(hook, warning)
+ if error is not None:
+ output.hook_error(hook, error)
+ output.hook_fixups(ret, hook_results)
+ output.hook_finish(hook, duration)
return ret
-def _run_project_hooks(project_name, proj_dir=None, from_git=False, commit_list=None):
+def _run_project_hooks(
+ project_name: str,
+ proj_dir: Optional[str] = None,
+ jobs: Optional[int] = None,
+ from_git: bool = False,
+ commit_list: Optional[List[str]] = None,
+) -> rh.results.ProjectResults:
"""Run the project-specific hooks in |proj_dir|.
Args:
project_name: The name of project to run hooks for.
proj_dir: If non-None, this is the directory the project is in. If None,
we'll ask repo.
+ jobs: How many hooks to run in parallel.
from_git: If true, we are called from git directly and repo should not be
used.
commit_list: A list of commits to run hooks against. If None or empty
@@ -359,7 +456,7 @@
uploaded.
Returns:
- False if any errors were found, else True.
+ All the results for this project.
"""
output = Output(project_name)
@@ -383,14 +480,56 @@
try:
# Hooks assume they are run from the root of the project.
os.chdir(proj_dir)
- return _run_project_hooks_in_cwd(project_name, proj_dir, output,
- from_git=from_git,
- commit_list=commit_list)
+ return _run_project_hooks_in_cwd(
+ project_name, proj_dir, output, jobs=jobs, from_git=from_git,
+ commit_list=commit_list)
finally:
output.finish()
os.chdir(pwd)
+def _run_projects_hooks(
+ project_list: List[str],
+ worktree_list: List[Optional[str]],
+ jobs: Optional[int] = None,
+ from_git: bool = False,
+ commit_list: Optional[List[str]] = None,
+) -> bool:
+ """Run all the hooks
+
+ Args:
+ project_list: List of project names.
+ worktree_list: List of project checkouts.
+ jobs: How many hooks to run in parallel.
+ from_git: If true, we are called from git directly and repo should not be
+ used.
+ commit_list: A list of commits to run hooks against. If None or empty
+ list then we'll automatically get the list of commits that would be
+ uploaded.
+
+ Returns:
+ True if everything passed, else False.
+ """
+ results = []
+ for project, worktree in zip(project_list, worktree_list):
+ result = _run_project_hooks(
+ project,
+ proj_dir=worktree,
+ jobs=jobs,
+ from_git=from_git,
+ commit_list=commit_list,
+ )
+ results.append(result)
+ if result:
+ # If a repo had failures, add a blank line to help break up the
+ # output. If there were no failures, then the output should be
+ # very minimal, so we don't add it then.
+ print('', file=sys.stderr)
+
+ _attempt_fixes(results)
+ return not any(results)
+
+
def main(project_list, worktree_list=None, **_kwargs):
"""Main function invoked directly by repo.
@@ -407,22 +546,13 @@
the directories automatically.
kwargs: Leave this here for forward-compatibility.
"""
- found_error = False
if not worktree_list:
worktree_list = [None] * len(project_list)
- for project, worktree in zip(project_list, worktree_list):
- if not _run_project_hooks(project, proj_dir=worktree):
- found_error = True
- # If a repo had failures, add a blank line to help break up the
- # output. If there were no failures, then the output should be
- # very minimal, so we don't add it then.
- print('', file=sys.stderr)
-
- if found_error:
+ if not _run_projects_hooks(project_list, worktree_list):
color = rh.terminal.Color()
print(color.color(color.RED, 'FATAL') +
': Preupload failed due to above error(s).\n'
- f'For more info, please see:\n{REPOHOOKS_URL}',
+ f'For more info, see: {REPOHOOKS_URL}',
file=sys.stderr)
sys.exit(1)
@@ -438,10 +568,11 @@
cmd = ['git', 'rev-parse', '--show-toplevel']
project_path = rh.utils.run(cmd, capture_output=True).stdout.strip()
cmd = ['git', 'rev-parse', '--show-superproject-working-tree']
- superproject_path = rh.utils.run(cmd, capture_output=True).stdout.strip()
+ superproject_path = rh.utils.run(
+ cmd, capture_output=True).stdout.strip()
module_path = project_path[len(superproject_path) + 1:]
cmd = ['git', 'config', '-f', '.gitmodules',
- '--name-only', '--get-regexp', '^submodule\..*\.path$',
+ '--name-only', '--get-regexp', r'^submodule\..*\.path$',
f"^{module_path}$"]
module_name = rh.utils.run(cmd, cwd=superproject_path,
capture_output=True).stdout.strip()
@@ -474,6 +605,11 @@
'hooks get run, since some hooks are project-specific.'
'If not specified, `repo` will be used to figure this '
'out based on the dir.')
+ parser.add_argument('-j', '--jobs', type=int,
+ help='Run up to this many hooks in parallel. Setting '
+ 'to 1 forces serial execution, and the default '
+ 'automatically chooses an appropriate number for the '
+ 'current system.')
parser.add_argument('commits', nargs='*',
help='Check specific commits')
opts = parser.parse_args(argv)
@@ -498,9 +634,13 @@
if not opts.project:
parser.error(f"Couldn't identify the project of {opts.dir}")
- if _run_project_hooks(opts.project, proj_dir=opts.dir, from_git=opts.git,
- commit_list=opts.commits):
- return 0
+ try:
+ if _run_projects_hooks([opts.project], [opts.dir], jobs=opts.jobs,
+ from_git=opts.git, commit_list=opts.commits):
+ return 0
+ except KeyboardInterrupt:
+ print('Aborting execution early due to user interrupt', file=sys.stderr)
+ return 128 + signal.SIGINT
return 1
diff --git a/rh/__init__.py b/rh/__init__.py
index 9050fb6..2b1676e 100644
--- a/rh/__init__.py
+++ b/rh/__init__.py
@@ -14,8 +14,14 @@
"""Common repohook objects/constants."""
-import collections
+from typing import NamedTuple
-# An object representing the git project that we're testing currently.
-Project = collections.namedtuple('Project', ['name', 'dir', 'remote'])
+class Project(NamedTuple):
+ """The git project that we're testing currently."""
+
+ # The name of the project.
+ name: str
+
+ # Absolute path to the project checkout.
+ dir: str
diff --git a/rh/hooks.py b/rh/hooks.py
index 47c3adb..6cb92a0 100644
--- a/rh/hooks.py
+++ b/rh/hooks.py
@@ -14,13 +14,13 @@
"""Functions that implement the actual checks."""
-import collections
import fnmatch
import json
import os
import platform
import re
import sys
+from typing import Callable, NamedTuple
_path = os.path.realpath(__file__ + '/../..')
if sys.path[0] != _path:
@@ -243,8 +243,11 @@
return self.expand_vars([tool_path])[0]
-# A callable hook.
-CallableHook = collections.namedtuple('CallableHook', ('name', 'hook', 'scope'))
+class CallableHook(NamedTuple):
+ """A callable hook."""
+ name: str
+ hook: Callable
+ scope: ExclusionScope
def _run(cmd, **kwargs):
@@ -314,26 +317,11 @@
return 'linux-x86'
-def _fixup_func_caller(cmd, **kwargs):
- """Wraps |cmd| around a callable automated fixup.
-
- For hooks that support automatically fixing errors after running (e.g. code
- formatters), this function provides a way to run |cmd| as the |fixup_func|
- parameter in HookCommandResult.
- """
- def wrapper():
- result = _run(cmd, **kwargs)
- if result.returncode not in (None, 0):
- return result.stdout
- return None
- return wrapper
-
-
-def _check_cmd(hook_name, project, commit, cmd, fixup_func=None, **kwargs):
+def _check_cmd(hook_name, project, commit, cmd, fixup_cmd=None, **kwargs):
"""Runs |cmd| and returns its result as a HookCommandResult."""
return [rh.results.HookCommandResult(hook_name, project, commit,
_run(cmd, **kwargs),
- fixup_func=fixup_func)]
+ fixup_cmd=fixup_cmd)]
# Where helper programs exist.
@@ -358,21 +346,22 @@
bpfmt = options.tool_path('bpfmt')
bpfmt_options = options.args((), filtered)
- cmd = [bpfmt, '-l'] + bpfmt_options
+ cmd = [bpfmt, '-d'] + bpfmt_options
+ fixup_cmd = [bpfmt, '-w']
+ if '-s' in bpfmt_options:
+ fixup_cmd.append('-s')
+ fixup_cmd.append('--')
+
ret = []
for d in filtered:
data = rh.git.get_file_content(commit, d.file)
result = _run(cmd, input=data)
if result.stdout:
- fixup_cmd = [bpfmt, '-w']
- if '-s' in bpfmt_options:
- fixup_cmd.append('-s')
- fixup_cmd.append(os.path.join(project.dir, d.file))
ret.append(rh.results.HookResult(
'bpfmt', project, commit,
error=result.stdout,
files=(d.file,),
- fixup_func=_fixup_func_caller(fixup_cmd)))
+ fixup_cmd=fixup_cmd))
return ret
@@ -394,9 +383,9 @@
git_clang_format] +
options.args(('--style', 'file', '--commit', commit), diff))
cmd = [tool] + tool_args
- fixup_func = _fixup_func_caller([tool, '--fix'] + tool_args)
+ fixup_cmd = [tool, '--fix'] + tool_args
return _check_cmd('clang-format', project, commit, cmd,
- fixup_func=fixup_func)
+ fixup_cmd=fixup_cmd)
def check_google_java_format(project, commit, _desc, _diff, options=None):
@@ -409,9 +398,9 @@
'--google-java-format-diff', google_java_format_diff,
'--commit', commit] + options.args()
cmd = [tool] + tool_args
- fixup_func = _fixup_func_caller([tool, '--fix'] + tool_args)
+ fixup_cmd = [tool, '--fix'] + tool_args
return _check_cmd('google-java-format', project, commit, cmd,
- fixup_func=fixup_func)
+ fixup_cmd=fixup_cmd)
def check_ktfmt(project, commit, _desc, diff, options=None):
@@ -438,16 +427,10 @@
('${PREUPLOAD_FILES}',), filtered)
result = _run(cmd)
if result.stdout:
- paths = [os.path.join(project.dir, x.file) for x in filtered]
- fixup_cmd = [ktfmt] + args + paths
- error = (
- '\nKotlin files need formatting.\n' +
- 'To reformat the kotlin files in this commit:\n' +
- rh.shell.cmd_to_str(fixup_cmd)
- )
- fixup_func = _fixup_func_caller(fixup_cmd)
- return [rh.results.HookResult('ktfmt', project, commit, error=error,
- files=paths, fixup_func=fixup_func)]
+ fixup_cmd = [ktfmt] + args
+ return [rh.results.HookResult(
+ 'ktfmt', project, commit, error='Formatting errors detected',
+ files=[x.file for x in filtered], fixup_cmd=fixup_cmd)]
return None
@@ -640,7 +623,7 @@
Multi-line Relnote example:
Relnote: "Added a new API `Class#getSize` to get the size of the class.
-This is useful if you need to know the size of the class."
+ This is useful if you need to know the size of the class."
Single-line Relnote example:
@@ -877,16 +860,17 @@
return None
gofmt = options.tool_path('gofmt')
- cmd = [gofmt, '-l'] + options.args((), filtered)
+ cmd = [gofmt, '-l'] + options.args()
+ fixup_cmd = [gofmt, '-w'] + options.args()
+
ret = []
for d in filtered:
data = rh.git.get_file_content(commit, d.file)
result = _run(cmd, input=data)
if result.stdout:
- fixup_func = _fixup_func_caller([gofmt, '-w', d.file])
ret.append(rh.results.HookResult(
'gofmt', project, commit, error=result.stdout,
- files=(d.file,), fixup_func=fixup_func))
+ files=(d.file,), fixup_cmd=fixup_cmd))
return ret
@@ -962,11 +946,9 @@
# TODO(b/164111102): rustfmt stable does not support --check on stdin.
# If no error is reported, compare stdin with stdout.
if data != result.stdout:
- msg = ('To fix, please run: ' +
- rh.shell.cmd_to_str(cmd + [d.file]))
ret.append(rh.results.HookResult(
- 'rustfmt', project, commit, error=msg,
- files=(d.file,)))
+ 'rustfmt', project, commit, error='Files not formatted',
+ files=(d.file,), fixup_cmd=cmd))
return ret
@@ -1032,7 +1014,7 @@
def check_aidl_format(project, commit, _desc, diff, options=None):
"""Checks that AIDL files are formatted with aidl-format."""
# All *.aidl files except for those under aidl_api directory.
- filtered = _filter_diff(diff, [r'\.aidl$'], [r'/aidl_api/'])
+ filtered = _filter_diff(diff, [r'\.aidl$'], [r'(^|/)aidl_api/'])
if not filtered:
return None
aidl_format = options.tool_path('aidl-format')
@@ -1044,12 +1026,10 @@
data = rh.git.get_file_content(commit, d.file)
result = _run(diff_cmd, input=data)
if result.stdout:
- fix_cmd = [aidl_format, '-w', '--clang-format-path', clang_format,
- d.file]
- fixup_func = _fixup_func_caller(fix_cmd)
+ fixup_cmd = [aidl_format, '-w', '--clang-format-path', clang_format]
ret.append(rh.results.HookResult(
'aidl-format', project, commit, error=result.stdout,
- files=(d.file,), fixup_func=fixup_func))
+ files=(d.file,), fixup_cmd=fixup_cmd))
return ret
diff --git a/rh/hooks_unittest.py b/rh/hooks_unittest.py
index c366e1f..003057e 100755
--- a/rh/hooks_unittest.py
+++ b/rh/hooks_unittest.py
@@ -179,17 +179,24 @@
@mock.patch.object(rh.git, 'find_repo_root')
def testREPO_OUTER_ROOT(self, m):
"""Verify handling of REPO_OUTER_ROOT."""
- m.side_effect=mock_find_repo_root
+ m.side_effect = mock_find_repo_root
self.assertEqual(self.replacer.get('REPO_OUTER_ROOT'),
mock_find_repo_root(path=None, outer=True))
@mock.patch.object(rh.git, 'find_repo_root')
def testREPO_ROOT(self, m):
"""Verify handling of REPO_ROOT."""
- m.side_effect=mock_find_repo_root
+ m.side_effect = mock_find_repo_root
self.assertEqual(self.replacer.get('REPO_ROOT'),
mock_find_repo_root(path=None, outer=False))
+ def testREPO_PATH(self):
+ """Verify handling of REPO_PATH."""
+ os.environ['REPO_PATH'] = ''
+ self.assertEqual(self.replacer.get('REPO_PATH'), '')
+ os.environ['REPO_PATH'] = 'foo/bar'
+ self.assertEqual(self.replacer.get('REPO_PATH'), 'foo/bar')
+
@mock.patch.object(rh.hooks, '_get_build_os_name', return_value='vapier os')
def testBUILD_OS(self, m):
"""Verify handling of BUILD_OS."""
@@ -307,8 +314,7 @@
"""Verify the builtin hooks."""
def setUp(self):
- self.project = rh.Project(name='project-name', dir='/.../repo/dir',
- remote='remote')
+ self.project = rh.Project(name='project-name', dir='/.../repo/dir')
self.options = rh.hooks.HookOptions('hook name', [], {})
def _test_commit_messages(self, func, accept, msgs, files=None):
@@ -371,7 +377,7 @@
self.project, 'commit', 'desc', diff, options=self.options)
self.assertIsNotNone(ret)
for result in ret:
- self.assertIsNotNone(result.fixup_func)
+ self.assertIsNotNone(result.fixup_cmd)
def test_checkpatch(self, mock_check, _mock_run):
"""Verify the checkpatch builtin hook."""
@@ -823,16 +829,14 @@
rh.git.RawDiffEntry(file='baz/blah.kt')]
ret = rh.hooks.check_ktfmt(
self.project, 'commit', 'desc', diff, options=self.options)
- self.assertListEqual(ret[0].files, ['/.../repo/dir/foo.kt',
- '/.../repo/dir/baz/blah.kt'])
+ self.assertListEqual(ret[0].files, ['foo.kt', 'baz/blah.kt'])
diff = [rh.git.RawDiffEntry(file='foo/f1.kt'),
rh.git.RawDiffEntry(file='bar/f2.kt'),
rh.git.RawDiffEntry(file='baz/f2.kt')]
ret = rh.hooks.check_ktfmt(self.project, 'commit', 'desc', diff,
options=rh.hooks.HookOptions('hook name', [
'--include-dirs=foo,baz'], {}))
- self.assertListEqual(ret[0].files, ['/.../repo/dir/foo/f1.kt',
- '/.../repo/dir/baz/f2.kt'])
+ self.assertListEqual(ret[0].files, ['foo/f1.kt', 'baz/f2.kt'])
def test_pylint(self, mock_check, _mock_run):
"""Verify the pylint builtin hook."""
diff --git a/rh/results.py b/rh/results.py
index a7a4b49..65e0052 100644
--- a/rh/results.py
+++ b/rh/results.py
@@ -16,6 +16,7 @@
import os
import sys
+from typing import List, NamedTuple, Optional
_path = os.path.realpath(__file__ + '/../..')
if sys.path[0] != _path:
@@ -26,7 +27,8 @@
class HookResult(object):
"""A single hook result."""
- def __init__(self, hook, project, commit, error, files=(), fixup_func=None):
+ def __init__(self, hook, project, commit, error, files=(),
+ fixup_cmd: Optional[List[str]] = None):
"""Initialize.
Args:
@@ -36,22 +38,23 @@
error: A string representation of the hook's result. Empty on
success.
files: The list of files that were involved in the hook execution.
- fixup_func: A callable that will attempt to automatically fix errors
- found in the hook's execution. Returns an non-empty string if
- this, too, fails. Can be None if the hook does not support
- automatically fixing errors.
+ fixup_cmd: A command that can automatically fix errors found in the
+ hook's execution. Can be None if the hook does not support
+ automatic fixups.
"""
self.hook = hook
self.project = project
self.commit = commit
self.error = error
self.files = files
- self.fixup_func = fixup_func
+ self.fixup_cmd = fixup_cmd
def __bool__(self):
+ """Whether this result is an error."""
return bool(self.error)
def is_warning(self):
+ """Whether this result is a non-fatal warning."""
return False
@@ -59,14 +62,44 @@
"""A single hook result based on a CompletedProcess."""
def __init__(self, hook, project, commit, result, files=(),
- fixup_func=None):
+ fixup_cmd=None):
HookResult.__init__(self, hook, project, commit,
result.stderr if result.stderr else result.stdout,
- files=files, fixup_func=fixup_func)
+ files=files, fixup_cmd=fixup_cmd)
self.result = result
def __bool__(self):
- return self.result.returncode not in (None, 0)
+ """Whether this result is an error."""
+ return self.result.returncode not in (None, 0, 77)
def is_warning(self):
+ """Whether this result is a non-fatal warning."""
return self.result.returncode == 77
+
+
+class ProjectResults(NamedTuple):
+ """All results for a single project."""
+
+ project: str
+ workdir: str
+
+ # All the results from running all the hooks.
+ results: List[HookResult] = []
+
+ # Whether there were any non-hook related errors. For example, trying to
+ # parse the project configuration.
+ internal_failure: bool = False
+
+ def add_results(self, results: Optional[List[HookResult]]) -> None:
+ """Add |results| to our results."""
+ if results:
+ self.results.extend(results)
+
+ @property
+ def fixups(self):
+ """Yield results that have a fixup available."""
+ yield from (x for x in self.results if x and x.fixup_cmd)
+
+ def __bool__(self):
+ """Whether there are any errors in this set of results."""
+ return self.internal_failure or any(self.results)
diff --git a/rh/results_unittest.py b/rh/results_unittest.py
new file mode 100755
index 0000000..93d909e
--- /dev/null
+++ b/rh/results_unittest.py
@@ -0,0 +1,105 @@
+#!/usr/bin/env python3
+# Copyright 2023 The Android Open Source Project
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""Unittests for the results module."""
+
+import os
+import sys
+import unittest
+
+_path = os.path.realpath(__file__ + '/../..')
+if sys.path[0] != _path:
+ sys.path.insert(0, _path)
+del _path
+
+# We have to import our local modules after the sys.path tweak. We can't use
+# relative imports because this is an executable program, not a module.
+# pylint: disable=wrong-import-position
+import rh
+import rh.results
+import rh.utils
+
+
+COMPLETED_PROCESS_PASS = rh.utils.CompletedProcess(returncode=0)
+COMPLETED_PROCESS_FAIL = rh.utils.CompletedProcess(returncode=1)
+COMPLETED_PROCESS_WARN = rh.utils.CompletedProcess(returncode=77)
+
+
+class HookResultTests(unittest.TestCase):
+ """Verify behavior of HookResult object."""
+
+ def test_error_warning(self):
+ """Check error & warning handling."""
+ # No errors.
+ result = rh.results.HookResult('hook', 'project', 'HEAD', False)
+ self.assertFalse(result)
+ self.assertFalse(result.is_warning())
+
+ # An error.
+ result = rh.results.HookResult('hook', 'project', 'HEAD', True)
+ self.assertTrue(result)
+ self.assertFalse(result.is_warning())
+
+
+class HookCommandResultTests(unittest.TestCase):
+ """Verify behavior of HookCommandResult object."""
+
+ def test_error_warning(self):
+ """Check error & warning handling."""
+ # No errors.
+ result = rh.results.HookCommandResult(
+ 'hook', 'project', 'HEAD', COMPLETED_PROCESS_PASS)
+ self.assertFalse(result)
+ self.assertFalse(result.is_warning())
+
+ # An error.
+ result = rh.results.HookCommandResult(
+ 'hook', 'project', 'HEAD', COMPLETED_PROCESS_FAIL)
+ self.assertTrue(result)
+ self.assertFalse(result.is_warning())
+
+ # A warning.
+ result = rh.results.HookCommandResult(
+ 'hook', 'project', 'HEAD', COMPLETED_PROCESS_WARN)
+ self.assertFalse(result)
+ self.assertTrue(result.is_warning())
+
+
+class ProjectResultsTests(unittest.TestCase):
+ """Verify behavior of ProjectResults object."""
+
+ def test_error_warning(self):
+ """Check error & warning handling."""
+ # No errors.
+ result = rh.results.ProjectResults('project', 'workdir')
+ self.assertFalse(result)
+
+ # Warnings are not errors.
+ result.add_results([
+ rh.results.HookResult('hook', 'project', 'HEAD', False),
+ rh.results.HookCommandResult(
+ 'hook', 'project', 'HEAD', COMPLETED_PROCESS_WARN),
+ ])
+ self.assertFalse(result)
+
+ # Errors are errors.
+ result.add_results([
+ rh.results.HookResult('hook', 'project', 'HEAD', True),
+ ])
+ self.assertTrue(result)
+
+
+if __name__ == '__main__':
+ unittest.main()
diff --git a/rh/shell.py b/rh/shell.py
index bece0b2..bc66f37 100644
--- a/rh/shell.py
+++ b/rh/shell.py
@@ -39,7 +39,7 @@
_SHELL_ESCAPE_CHARS = r'\"`$'
-def shell_quote(s):
+def quote(s):
"""Quote |s| in a way that is safe for use in a shell.
We aim to be safe, but also to produce "nice" output. That means we don't
@@ -93,7 +93,7 @@
return f'"{s}"'
-def shell_unquote(s):
+def unquote(s):
"""Do the opposite of ShellQuote.
This function assumes that the input is a valid escaped string.
@@ -148,7 +148,7 @@
String representing full command.
"""
# Use str before repr to translate unicode strings to regular strings.
- return ' '.join(shell_quote(arg) for arg in cmd)
+ return ' '.join(quote(arg) for arg in cmd)
def boolean_shell_value(sval, default):
diff --git a/rh/shell_unittest.py b/rh/shell_unittest.py
index f7d2bba..fec8710 100755
--- a/rh/shell_unittest.py
+++ b/rh/shell_unittest.py
@@ -59,7 +59,7 @@
class ShellQuoteTest(DiffTestCase):
- """Test the shell_quote & shell_unquote functions."""
+ """Test the quote & unquote functions."""
def testShellQuote(self):
"""Basic ShellQuote tests."""
@@ -86,10 +86,10 @@
}
def aux(s):
- return rh.shell.shell_unquote(rh.shell.shell_quote(s))
+ return rh.shell.unquote(rh.shell.quote(s))
- self._testData(rh.shell.shell_quote, tests_quote)
- self._testData(rh.shell.shell_unquote, tests_unquote)
+ self._testData(rh.shell.quote, tests_quote)
+ self._testData(rh.shell.unquote, tests_unquote)
# Test that the operations are reversible.
self._testData(aux, {k: k for k in tests_quote.values()}, False)
@@ -97,7 +97,7 @@
def testPathlib(self):
"""Verify pathlib is handled."""
- self.assertEqual(rh.shell.shell_quote(Path('/')), '/')
+ self.assertEqual(rh.shell.quote(Path('/')), '/')
def testBadInputs(self):
"""Verify bad inputs do not crash."""
@@ -105,7 +105,7 @@
(1234, '1234'),
(Exception('hi'), "Exception('hi')"),
):
- self.assertEqual(rh.shell.shell_quote(arg), exp)
+ self.assertEqual(rh.shell.quote(arg), exp)
class CmdToStrTest(DiffTestCase):
diff --git a/rh/terminal.py b/rh/terminal.py
index f69914c..a6f31d9 100644
--- a/rh/terminal.py
+++ b/rh/terminal.py
@@ -19,6 +19,7 @@
import os
import sys
+from typing import List, Optional
_path = os.path.realpath(__file__ + '/../..')
if sys.path[0] != _path:
@@ -29,6 +30,12 @@
import rh.shell
+# This will erase all content in the current line after the cursor. This is
+# useful for partial updates & progress messages as the terminal can display
+# it better.
+CSI_ERASE_LINE_AFTER = '\x1b[K'
+
+
class Color(object):
"""Conditionally wraps text in ANSI color escape sequences."""
@@ -36,7 +43,7 @@
BOLD = -1
COLOR_START = '\033[1;%dm'
BOLD_START = '\033[1m'
- RESET = '\033[0m'
+ RESET = '\033[m'
def __init__(self, enabled=None):
"""Create a new Color object, optionally disabling color output.
@@ -51,7 +58,7 @@
"""Returns a start color code.
Args:
- color: Color to use, .e.g BLACK, RED, etc.
+ color: Color to use, e.g. BLACK, RED, etc...
Returns:
If color is enabled, returns an ANSI sequence to start the given
@@ -99,25 +106,10 @@
self._enabled = not rh.shell.boolean_shell_value(
os.environ['NOCOLOR'], False)
else:
- self._enabled = is_tty(sys.stderr)
+ self._enabled = sys.stderr.isatty()
return self._enabled
-def is_tty(fh):
- """Returns whether the specified file handle is a TTY.
-
- Args:
- fh: File handle to check.
-
- Returns:
- True if |fh| is a TTY
- """
- try:
- return os.isatty(fh.fileno())
- except IOError:
- return False
-
-
def print_status_line(line, print_newline=False):
"""Clears the current terminal line, and prints |line|.
@@ -125,8 +117,8 @@
line: String to print.
print_newline: Print a newline at the end, if sys.stderr is a TTY.
"""
- if is_tty(sys.stderr):
- output = '\r' + line + '\x1B[K'
+ if sys.stderr.isatty():
+ output = '\r' + line + CSI_ERASE_LINE_AFTER
if print_newline:
output += '\n'
else:
@@ -136,6 +128,34 @@
sys.stderr.flush()
+def str_prompt(
+ prompt: str,
+ choices: List[str],
+ lower: bool = True,
+) -> Optional[str]:
+ """Helper function for processing user input.
+
+ Args:
+ prompt: The question to present to the user.
+ lower: Whether to lowercase the response.
+
+ Returns:
+ The string the user entered, or None if EOF (e.g. Ctrl+D).
+ """
+ prompt = f'{prompt} ({"/".join(choices)})? '
+ try:
+ result = input(prompt)
+ return result.lower() if lower else result
+ except EOFError:
+ # If the user hits Ctrl+D, or stdin is disabled, use the default.
+ print()
+ return None
+ except KeyboardInterrupt:
+ # If the user hits Ctrl+C, just exit the process.
+ print()
+ raise
+
+
def boolean_prompt(prompt='Do you want to continue?', default=True,
true_value='yes', false_value='no', prolog=None):
"""Helper function for processing boolean choice prompts.
@@ -161,23 +181,12 @@
else:
false_text = false_text[0].upper() + false_text[1:]
- prompt = f'\n{prompt} ({true_text}/{false_text})? '
-
if prolog:
prompt = f'\n{prolog}\n{prompt}'
+ prompt = '\n' + prompt
while True:
- try:
- response = input(prompt).lower() # pylint: disable=bad-builtin
- except EOFError:
- # If the user hits CTRL+D, or stdin is disabled, use the default.
- print()
- response = None
- except KeyboardInterrupt:
- # If the user hits CTRL+C, just exit the process.
- print()
- raise
-
+ response = str_prompt(prompt, choices=(true_text, false_text))
if not response:
return default
if true_value.startswith(response):
diff --git a/rh/terminal_unittest.py b/rh/terminal_unittest.py
new file mode 100755
index 0000000..b76b907
--- /dev/null
+++ b/rh/terminal_unittest.py
@@ -0,0 +1,199 @@
+#!/usr/bin/env python3
+# Copyright 2023 The Android Open Source Project
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""Unittests for the terminal module."""
+
+import contextlib
+import io
+import os
+import sys
+import unittest
+
+_path = os.path.realpath(__file__ + '/../..')
+if sys.path[0] != _path:
+ sys.path.insert(0, _path)
+del _path
+
+# We have to import our local modules after the sys.path tweak. We can't use
+# relative imports because this is an executable program, not a module.
+# pylint: disable=wrong-import-position
+import rh.terminal
+
+
+class ColorTests(unittest.TestCase):
+ """Verify behavior of Color class."""
+
+ def setUp(self):
+ os.environ.pop('NOCOLOR', None)
+
+ def test_enabled_auto_tty(self):
+ """Test automatic enable behavior based on tty."""
+ stderr = io.StringIO()
+ with contextlib.redirect_stderr(stderr):
+ c = rh.terminal.Color()
+ self.assertFalse(c.enabled)
+
+ stderr.isatty = lambda: True
+ c = rh.terminal.Color()
+ self.assertTrue(c.enabled)
+
+ def test_enabled_auto_env(self):
+ """Test automatic enable behavior based on $NOCOLOR."""
+ stderr = io.StringIO()
+ with contextlib.redirect_stderr(stderr):
+ os.environ['NOCOLOR'] = 'yes'
+ c = rh.terminal.Color()
+ self.assertFalse(c.enabled)
+
+ os.environ['NOCOLOR'] = 'no'
+ c = rh.terminal.Color()
+ self.assertTrue(c.enabled)
+
+ def test_enabled_override(self):
+ """Test explicit enable behavior."""
+ stderr = io.StringIO()
+ with contextlib.redirect_stderr(stderr):
+ stderr.isatty = lambda: True
+ os.environ['NOCOLOR'] = 'no'
+ c = rh.terminal.Color()
+ self.assertTrue(c.enabled)
+ c = rh.terminal.Color(False)
+ self.assertFalse(c.enabled)
+
+ stderr.isatty = lambda: False
+ os.environ['NOCOLOR'] = 'yes'
+ c = rh.terminal.Color()
+ self.assertFalse(c.enabled)
+ c = rh.terminal.Color(True)
+ self.assertTrue(c.enabled)
+
+ def test_output_disabled(self):
+ """Test output when coloring is disabled."""
+ c = rh.terminal.Color(False)
+ self.assertEqual(c.start(rh.terminal.Color.BLACK), '')
+ self.assertEqual(c.color(rh.terminal.Color.BLACK, 'foo'), 'foo')
+ self.assertEqual(c.stop(), '')
+
+ def test_output_enabled(self):
+ """Test output when coloring is enabled."""
+ c = rh.terminal.Color(True)
+ self.assertEqual(c.start(rh.terminal.Color.BLACK), '\x1b[1;30m')
+ self.assertEqual(c.color(rh.terminal.Color.BLACK, 'foo'),
+ '\x1b[1;30mfoo\x1b[m')
+ self.assertEqual(c.stop(), '\x1b[m')
+
+
+class PrintStatusLine(unittest.TestCase):
+ """Verify behavior of print_status_line."""
+
+ def test_terminal(self):
+ """Check tty behavior."""
+ stderr = io.StringIO()
+ stderr.isatty = lambda: True
+ with contextlib.redirect_stderr(stderr):
+ rh.terminal.print_status_line('foo')
+ rh.terminal.print_status_line('bar', print_newline=True)
+ csi = rh.terminal.CSI_ERASE_LINE_AFTER
+ self.assertEqual(stderr.getvalue(), f'\rfoo{csi}\rbar{csi}\n')
+
+ def test_no_terminal(self):
+ """Check tty-less behavior."""
+ stderr = io.StringIO()
+ with contextlib.redirect_stderr(stderr):
+ rh.terminal.print_status_line('foo')
+ rh.terminal.print_status_line('bar', print_newline=True)
+ self.assertEqual(stderr.getvalue(), 'foo\nbar\n')
+
+
+@contextlib.contextmanager
+def redirect_stdin(new_target):
+ """Temporarily switch sys.stdin to |new_target|."""
+ old = sys.stdin
+ try:
+ sys.stdin = new_target
+ yield
+ finally:
+ sys.stdin = old
+
+
+class StringPromptTests(unittest.TestCase):
+ """Verify behavior of str_prompt."""
+
+ def setUp(self):
+ self.stdin = io.StringIO()
+
+ def set_stdin(self, value: str) -> None:
+ """Set stdin wrapper to a string."""
+ self.stdin.seek(0)
+ self.stdin.write(value)
+ self.stdin.truncate()
+ self.stdin.seek(0)
+
+ def test_defaults(self):
+ """Test default behavior."""
+ stdout = io.StringIO()
+ with redirect_stdin(self.stdin), contextlib.redirect_stdout(stdout):
+ # Test EOF behavior.
+ self.assertIsNone(rh.terminal.str_prompt('foo', ('a', 'b')))
+
+ # Test enter behavior.
+ self.set_stdin('\n')
+ self.assertEqual(rh.terminal.str_prompt('foo', ('a', 'b')), '')
+
+ # Lowercase inputs.
+ self.set_stdin('Ok')
+ self.assertEqual(rh.terminal.str_prompt('foo', ('a', 'b')), 'ok')
+
+ # Don't lowercase inputs.
+ self.set_stdin('Ok')
+ self.assertEqual(
+ rh.terminal.str_prompt('foo', ('a', 'b'), lower=False), 'Ok')
+
+
+class BooleanPromptTests(unittest.TestCase):
+ """Verify behavior of boolean_prompt."""
+
+ def setUp(self):
+ self.stdin = io.StringIO()
+
+ def set_stdin(self, value: str) -> None:
+ """Set stdin wrapper to a string."""
+ self.stdin.seek(0)
+ self.stdin.write(value)
+ self.stdin.truncate()
+ self.stdin.seek(0)
+
+ def test_defaults(self):
+ """Test default behavior."""
+ stdout = io.StringIO()
+ with redirect_stdin(self.stdin), contextlib.redirect_stdout(stdout):
+ # Default values. Will loop to EOF when it doesn't match anything.
+ for v in ('', '\n', 'oops'):
+ self.set_stdin(v)
+ self.assertTrue(rh.terminal.boolean_prompt())
+
+ # False values.
+ for v in ('n', 'N', 'no', 'NO'):
+ self.set_stdin(v)
+ self.assertFalse(rh.terminal.boolean_prompt())
+
+ # True values.
+ for v in ('y', 'Y', 'ye', 'yes', 'YES'):
+ self.set_stdin(v)
+ self.assertTrue(rh.terminal.boolean_prompt())
+
+
+if __name__ == '__main__':
+ unittest.main()
diff --git a/rh/utils.py b/rh/utils.py
index 14553a8..4f1a063 100644
--- a/rh/utils.py
+++ b/rh/utils.py
@@ -81,25 +81,27 @@
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 TypeError(
- f'exception must be an exception instance; got {exception!r}')
+ def __init__(self, returncode, cmd, stdout=None, stderr=None, msg=None):
+ super().__init__(returncode, cmd, stdout, stderr=stderr)
- super().__init__(returncode, cmd, stdout)
- # The parent class will set |output|, so delete it.
+ # The parent class will set |output|, so delete it. If Python ever drops
+ # this output/stdout compat logic, we can drop this to match.
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._stdout = stdout
+
self.msg = msg
- self.exception = exception
+
+ @property
+ def stdout(self):
+ """Override parent's usage of .output"""
+ return self._stdout
+
+ @stdout.setter
+ def stdout(self, value):
+ """Override parent's usage of .output"""
+ self._stdout = value
@property
def cmdstr(self):
@@ -248,8 +250,7 @@
# pylint: disable=redefined-builtin
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):
+ check=True, int_timeout=1, kill_timeout=1, capture_output=False):
"""Runs a command.
Args:
@@ -275,7 +276,6 @@
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.
capture_output: Set |redirect_stdout| and |redirect_stderr| to True.
- close_fds: Whether to close all fds before running |cmd|.
Returns:
A CompletedProcess object.
@@ -364,23 +364,32 @@
try:
proc = _Popen(cmd, cwd=cwd, stdin=stdin, stdout=popen_stdout,
stderr=popen_stderr, shell=False, env=env,
- close_fds=close_fds)
+ close_fds=True)
old_sigint = signal.getsignal(signal.SIGINT)
handler = functools.partial(_kill_child_process, proc, int_timeout,
kill_timeout, cmd, old_sigint)
- signal.signal(signal.SIGINT, handler)
+ # We have to ignore ValueError in case we're run from a thread.
+ try:
+ signal.signal(signal.SIGINT, handler)
+ except ValueError:
+ old_sigint = None
old_sigterm = signal.getsignal(signal.SIGTERM)
handler = functools.partial(_kill_child_process, proc, int_timeout,
kill_timeout, cmd, old_sigterm)
- signal.signal(signal.SIGTERM, handler)
+ try:
+ signal.signal(signal.SIGTERM, handler)
+ except ValueError:
+ old_sigterm = None
try:
(result.stdout, result.stderr) = proc.communicate(input)
finally:
- signal.signal(signal.SIGINT, old_sigint)
- signal.signal(signal.SIGTERM, old_sigterm)
+ if old_sigint is not None:
+ signal.signal(signal.SIGINT, old_sigint)
+ if old_sigterm is not None:
+ signal.signal(signal.SIGTERM, old_sigterm)
if popen_stdout:
# The linter is confused by how stdout is a file & an int.
@@ -420,7 +429,7 @@
result = CompletedProcess(args=cmd, stderr=estr, returncode=255)
else:
raise CalledProcessError(
- result.returncode, result.cmd, msg=estr, exception=e,
+ result.returncode, result.cmd, msg=estr,
stdout=ensure_text(result.stdout),
stderr=ensure_text(result.stderr)) from e
finally:
diff --git a/rh/utils_unittest.py b/rh/utils_unittest.py
index 7928dd5..bf720a7 100755
--- a/rh/utils_unittest.py
+++ b/rh/utils_unittest.py
@@ -98,34 +98,40 @@
def test_basic(self):
"""Basic test we can create a normal instance."""
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.CalledProcessError(0, ['mycmd'])
self.assertIn('mycmd', err.stringify())
- 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.CalledProcessError(0, ['mycmd'])
self.assertIn('mycmd', str(err))
- 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.CalledProcessError(0, ['mycmd'])
self.assertNotEqual('', repr(err))
- err = rh.utils.CalledProcessError(
- 0, ['mycmd'], exception=Exception('bad'))
- self.assertNotEqual('', repr(err))
+
+ def test_output(self):
+ """Make sure .output is removed and .stdout works."""
+ e = rh.utils.CalledProcessError(
+ 0, ['true'], stdout='STDOUT', stderr='STDERR')
+ with self.assertRaises(AttributeError):
+ assert e.output is None
+ assert e.stdout == 'STDOUT'
+ assert e.stderr == 'STDERR'
+
+ e.stdout = 'STDout'
+ e.stderr = 'STDerr'
+ with self.assertRaises(AttributeError):
+ assert e.output is None
+ assert e.stdout == 'STDout'
+ assert e.stderr == 'STDerr'
class RunCommandTests(unittest.TestCase):
diff --git a/tools/google-java-format.py b/tools/google-java-format.py
index 8a4f739..fcb5521 100755
--- a/tools/google-java-format.py
+++ b/tools/google-java-format.py
@@ -17,8 +17,8 @@
import argparse
import os
+import shutil
import sys
-from shutil import which
_path = os.path.realpath(__file__ + '/../..')
if sys.path[0] != _path:
@@ -60,7 +60,7 @@
parser = get_parser()
opts = parser.parse_args(argv)
- format_path = which(opts.google_java_format)
+ format_path = shutil.which(opts.google_java_format)
if not format_path:
print(
f'Unable to find google-java-format at: {opts.google_java_format}',
diff --git a/tools/pylintrc b/tools/pylintrc
index 68c74ef..3abe640 100644
--- a/tools/pylintrc
+++ b/tools/pylintrc
@@ -73,68 +73,6 @@
# either give multiple identifier separated by comma (,) or put this option
# multiple time. See also the "--disable" option for examples.
enable=
- apply-builtin,
- backtick,
- bad-python3-import,
- basestring-builtin,
- buffer-builtin,
- cmp-builtin,
- cmp-method,
- coerce-builtin,
- coerce-method,
- delslice-method,
- deprecated-itertools-function,
- deprecated-str-translate-call,
- deprecated-string-function,
- deprecated-types-field,
- dict-items-not-iterating,
- dict-iter-method,
- dict-keys-not-iterating,
- dict-values-not-iterating,
- dict-view-method,
- div-method,
- exception-message-attribute,
- execfile-builtin,
- file-builtin,
- filter-builtin-not-iterating,
- getslice-method,
- hex-method,
- idiv-method,
- import-star-module-level,
- indexing-exception,
- input-builtin,
- intern-builtin,
- invalid-str-codec,
- long-builtin,
- long-suffix,
- map-builtin-not-iterating,
- metaclass-assignment,
- next-method-called,
- next-method-defined,
- nonzero-method,
- oct-method,
- old-division,
- old-ne-operator,
- old-octal-literal,
- old-raise-syntax,
- parameter-unpacking,
- print-statement,
- raising-string,
- range-builtin-not-iterating,
- raw_input-builtin,
- rdiv-method,
- reduce-builtin,
- reload-builtin,
- round-builtin,
- setslice-method,
- standarderror-builtin,
- sys-max-int,
- unichr-builtin,
- unicode-builtin,
- unpacking-in-except,
- using-cmp-argument,
- xrange-builtin,
- zip-builtin-not-iterating,
# Disable the message, report, category or checker with the given id(s). You
@@ -153,10 +91,11 @@
file-ignored,
invalid-name,
locally-disabled,
- locally-enabled,
missing-docstring,
- no-self-use,
- star-args,
+ no-else-break,
+ no-else-continue,
+ no-else-raise,
+ no-else-return,
too-few-public-methods,
too-many-arguments,
too-many-branches,
@@ -320,7 +259,7 @@
[BASIC]
# List of builtins function names that should not be used, separated by a comma
-bad-functions=map,filter,input
+bad-functions=map,filter
# Good variable names which should always be accepted, separated by a comma
good-names=i,j,k,ex,x,_