Snap for 11684396 from e106ee3301113116bdc4e11cdb9af60ea946d12b to androidx-paging-release
Change-Id: I77900455b7618de5b2484ee85f110a36cea3bbbd
diff --git a/PREUPLOAD.cfg b/PREUPLOAD.cfg
index 8ee46ac..31de3b0 100644
--- a/PREUPLOAD.cfg
+++ b/PREUPLOAD.cfg
@@ -2,6 +2,7 @@
# 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
diff --git a/pre-upload.py b/pre-upload.py
index 5c820c6..18bf11f 100755
--- a/pre-upload.py
+++ b/pre-upload.py
@@ -20,6 +20,7 @@
"""
import argparse
+import concurrent.futures
import datetime
import os
import signal
@@ -63,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)
@@ -74,22 +76,15 @@
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
-
- def set_num_hooks(self, num_hooks):
- """Keep track of how many hooks we'll be running.
-
- Args:
- num_hooks: number of hooks to be run.
- """
- self.num_hooks = num_hooks
+ # Cache number of invisible characters in our banner.
+ self._banner_esc_chars = len(self.COLOR.color(self.COLOR.YELLOW, ''))
def set_num_commits(self, num_commits: int) -> None:
"""Keep track of how many commits we'll be running.
@@ -100,10 +95,11 @@
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.
"""
@@ -113,47 +109,58 @@
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(f'{self._curr_hook_name} hook', 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)
@@ -169,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
@@ -200,7 +222,7 @@
error_ret = ''
warning_ret = ''
for result in results:
- if result:
+ if result or result.is_warning():
ret = ''
if result.files:
ret += f' FILES: {rh.shell.cmd_to_str(result.files)}\n'
@@ -242,51 +264,86 @@
return rh.config.PreUploadSettings(paths=paths, global_paths=global_paths)
-def _attempt_fixes(fixup_list, commit_list, cwd):
- """Attempts to run |fixup_list| given |commit_list|."""
- if len(fixup_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_cmd, fixup_files = fixup_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 = rh.utils.run(
- fixup_cmd + list(fixup_files),
- cwd=cwd,
- combine_stdout_stderr=True,
- capture_output=True,
- check=False,
- input='',
- )
- if result.returncode:
- print(f'Attempt to fix "{hook_name}" for commit "{commit}" failed: '
- f'{result.stdout}',
- 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
@@ -294,20 +351,20 @@
uploaded.
Returns:
- True if everything passed, else False.
+ 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:
@@ -316,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)
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,
@@ -334,50 +396,59 @@
ignore_merged_commits=config.ignore_merged_commits)
output.set_num_commits(len(commit_list))
- ret = True
- fixup_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:
- if rel_proj_dir in exclusion_scope:
- continue
- output.hook_start(name)
- 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_cmd:
- fixup_list.append(
- (name, commit, result.fixup_cmd, result.files))
-
- if fixup_list:
- _attempt_fixes(fixup_list, commit_list, proj_dir)
+ 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
@@ -385,7 +456,7 @@
uploaded.
Returns:
- True if everything passed, else False.
+ All the results for this project.
"""
output = Output(project_name)
@@ -409,9 +480,9 @@
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)
@@ -420,6 +491,7 @@
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:
@@ -428,6 +500,7 @@
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
@@ -437,20 +510,24 @@
Returns:
True if everything passed, else False.
"""
- ret = True
+ results = []
for project, worktree in zip(project_list, worktree_list):
- if not _run_project_hooks(
+ result = _run_project_hooks(
project,
proj_dir=worktree,
+ jobs=jobs,
from_git=from_git,
commit_list=commit_list,
- ):
- ret = False
+ )
+ 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)
- return ret
+
+ _attempt_fixes(results)
+ return not any(results)
def main(project_list, worktree_list=None, **_kwargs):
@@ -528,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)
@@ -553,8 +635,8 @@
parser.error(f"Couldn't identify the project of {opts.dir}")
try:
- if _run_projects_hooks([opts.project], [opts.dir], from_git=opts.git,
- commit_list=opts.commits):
+ 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)
diff --git a/rh/hooks.py b/rh/hooks.py
index c90c0a3..6cb92a0 100644
--- a/rh/hooks.py
+++ b/rh/hooks.py
@@ -346,15 +346,17 @@
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')
ret.append(rh.results.HookResult(
'bpfmt', project, commit,
error=result.stdout,
@@ -621,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:
@@ -858,13 +860,14 @@
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_cmd = [gofmt, '-w']
ret.append(rh.results.HookResult(
'gofmt', project, commit, error=result.stdout,
files=(d.file,), fixup_cmd=fixup_cmd))
@@ -943,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
@@ -1013,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')
diff --git a/rh/results.py b/rh/results.py
index a754cef..65e0052 100644
--- a/rh/results.py
+++ b/rh/results.py
@@ -16,7 +16,7 @@
import os
import sys
-from typing import List, Optional
+from typing import List, NamedTuple, Optional
_path = os.path.realpath(__file__ + '/../..')
if sys.path[0] != _path:
@@ -50,9 +50,11 @@
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
@@ -67,7 +69,37 @@
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/utils.py b/rh/utils.py
index 157e31b..4f1a063 100644
--- a/rh/utils.py
+++ b/rh/utils.py
@@ -369,18 +369,27 @@
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.
