Snap for 11610999 from e106ee3301113116bdc4e11cdb9af60ea946d12b to androidx-fragment-releaseandroidx-fragment-release

Change-Id: I3103a2064d97b7f927e5684103c71fe1d20e924f

15 files changed, 769 insertions, 336 deletions

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 @@ when developing.
 """
 
 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 @@ class Output(object):
     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 @@ class Output(object):
           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
-
-    def hook_start(self, hook_name):
-        """Emit status before the start of a hook.
-
-        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
+        self.commit_index += 1
+
+        # Initialize the pending hooks line too.
+        self.hooks = set(hooks)
+        self.num_hooks = len(hooks)
+        self.hook_banner()
+
+    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 @@ class Output(object):
         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 @@ def _process_hook_results(results):
     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 @@ def _get_project_config(from_git=False):
     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.
-        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.
+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
 
-    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)
-
-
-def _run_project_hooks_in_cwd(project_name, proj_dir, output, from_git=False, commit_list=None):
+        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
+
+    # 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 @@ def _run_project_hooks_in_cwd(project_name, proj_dir, output, from_git=False, co
           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 @@ def _run_project_hooks_in_cwd(project_name, proj_dir, output, from_git=False, co
     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 @@ def _run_project_hooks_in_cwd(project_name, proj_dir, output, from_git=False, co
     if not commit_list:
         commit_list = rh.git.get_commits(
             ignore_merged_commits=config.ignore_merged_commits)
-
-    ret = True
-    fixup_func_list = []
-
-    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)
-
-        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)
+    output.set_num_commits(len(commit_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)
+
+    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(hooks, commit, commit_summary)
+
+            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 @@ def _run_project_hooks(project_name, proj_dir=None, from_git=False, commit_list=
           uploaded.
 
     Returns:
-      False if any errors were found, else True.
+      All the results for this project.
     """
     output = Output(project_name)
 
@@ -383,14 +480,56 @@ def _run_project_hooks(project_name, proj_dir=None, from_git=False, commit_list=
     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 @@ def main(project_list, worktree_list=None, **_kwargs):
           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 @@ def _identify_project(path, from_git=False):
         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 @@ def direct_main(argv):
                         '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 @@ def direct_main(argv):
         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 acf72af..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 @@ class HookOptions(object):
         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 @@ def _get_build_os_name():
     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 @@ def check_bpfmt(project, commit, _desc, diff, options=None):
 
     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 @@ def check_clang_format(project, commit, _desc, diff, options=None):
                   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 @@ def check_google_java_format(project, commit, _desc, _diff, options=None):
                  '--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,15 +427,10 @@ def check_ktfmt(project, commit, _desc, diff, options=None):
         ('${PREUPLOAD_FILES}',), filtered)
     result = _run(cmd)
     if result.stdout:
-        paths = [os.path.join(project.dir, x.file) for x in filtered]
-        error = (
-            f'\nKotlin files need formatting.\n'
-            'To reformat the kotlin files in this commit:\n'
-            f'{ktfmt} {" ".join(paths)}'
-        )
-        fixup_func = _fixup_func_caller([ktfmt] + paths)
-        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
 
 
@@ -639,7 +623,7 @@ release notes, you need to include a starting and closing quote.
 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:
 
@@ -876,16 +860,17 @@ def check_gofmt(project, commit, _desc, diff, options=None):
         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
 
 
@@ -961,11 +946,9 @@ def check_rustfmt(project, commit, _desc, diff, options=None):
         # 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
 
 
@@ -1031,7 +1014,7 @@ def check_android_test_mapping(project, commit, _desc, diff, options=None):
 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')
@@ -1043,12 +1026,10 @@ def check_aidl_format(project, commit, _desc, diff, options=None):
         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 @@ class PlaceholderTests(unittest.TestCase):
     @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 @@ class BuiltinHooksTests(unittest.TestCase):
     """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 @@ class BuiltinHooksTests(unittest.TestCase):
             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 @@ class BuiltinHooksTests(unittest.TestCase):
                 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 @@ del _path
 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 @@ class HookResult(object):
           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 @@ class HookCommandResult(HookResult):
     """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_QUOTABLE_CHARS = frozenset('[|&;()<> \t!{}[]=*?~$"\'\\#^')
 _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 @@ def shell_quote(s):
     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 @@ def cmd_to_str(cmd):
       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 DiffTestCase(unittest.TestCase):
 
 
 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 @@ class ShellQuoteTest(DiffTestCase):
         }
 
         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 @@ class ShellQuoteTest(DiffTestCase):
 
     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 @@ class ShellQuoteTest(DiffTestCase):
             (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 @@ This module handles terminal interaction including ANSI color codes.
 
 import os
 import sys
+from typing import List, Optional
 
 _path = os.path.realpath(__file__ + '/../..')
 if sys.path[0] != _path:
@@ -29,6 +30,12 @@ del _path
 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 @@ class Color(object):
     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 @@ class Color(object):
         """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 @@ class Color(object):
                 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 @@ def print_status_line(line, print_newline=False):
       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 @@ def print_status_line(line, print_newline=False):
     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 @@ def boolean_prompt(prompt='Do you want to continue?', default=True,
     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 @@ class CalledProcessError(subprocess.CalledProcessError):
       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 @@ class _Popen(subprocess.Popen):
 # 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 @@ def run(cmd, redirect_stdout=False, redirect_stderr=False, cwd=None, input=None,
       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 @@ def run(cmd, redirect_stdout=False, redirect_stderr=False, cwd=None, input=None,
     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 @@ def run(cmd, redirect_stdout=False, redirect_stderr=False, cwd=None, input=None,
             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 @@ class CalledProcessErrorTests(unittest.TestCase):
     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 @@ def main(argv):
     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 @@ confidence=
 # 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 @@ disable=
     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 @@ notes=FIXME,XXX,TODO
 [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,_