pre-upload: fix chdir/output finishing when running hooks
The current _run_project_hooks helper will call os.chdir but then
return early before restoring its state to the previous location.
To fix this, rip the core of _run_project_hooks into a new helper
called _run_project_hooks_in_cwd, and then wrap that call in a try
block so we can put the chdir call into the finally section.
We have a similar problem with the output helper where we don't
guarantee calling output.finish() before returning. To solve that,
we have to rework the Output API slightly by delaying the setup of
the num_hooks method to the point where we can calculate it.
Bug: 124462370
Test: `repo upload` still works and summarizes across multiple repos
Change-Id: I18780bcfe91db1cd276d7bdfdfd62badfc83a0d7
diff --git a/pre-upload.py b/pre-upload.py
index ac976e7..517ee79 100755
--- a/pre-upload.py
+++ b/pre-upload.py
@@ -63,18 +63,25 @@
FAILED = COLOR.color(COLOR.RED, 'FAILED')
WARNING = COLOR.color(COLOR.YELLOW, 'WARNING')
- def __init__(self, project_name, num_hooks):
+ def __init__(self, project_name):
"""Create a new Output object for a specified project.
Args:
project_name: name of project.
- num_hooks: number of hooks to be run.
"""
self.project_name = project_name
- self.num_hooks = num_hooks
+ self.num_hooks = None
self.hook_index = 0
self.success = True
+ 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
+
def commit_start(self, commit, commit_summary):
"""Emit status for new commit.
@@ -214,14 +221,13 @@
'attempting to upload again.\n', file=sys.stderr)
-def _run_project_hooks(project_name, proj_dir=None,
- commit_list=None):
- """For each project run its project specific hook from the hooks dictionary.
+def _run_project_hooks_in_cwd(project_name, proj_dir, output, commit_list=None):
+ """Run the project-specific hooks in the cwd.
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.
+ 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.
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.
@@ -229,26 +235,6 @@
Returns:
False if any errors were found, else True.
"""
- if proj_dir is None:
- cmd = ['repo', 'forall', project_name, '-c', 'pwd']
- result = rh.utils.run_command(cmd, capture_output=True)
- proj_dirs = result.output.split()
- if len(proj_dirs) == 0:
- print('%s cannot be found.' % project_name, file=sys.stderr)
- print('Please specify a valid project.', file=sys.stderr)
- return False
- if len(proj_dirs) > 1:
- print('%s is associated with multiple directories.' % project_name,
- file=sys.stderr)
- print('Please specify a directory to help disambiguate.',
- file=sys.stderr)
- return False
- proj_dir = proj_dirs[0]
-
- pwd = os.getcwd()
- # Hooks assume they are run from the root of the project.
- os.chdir(proj_dir)
-
# If the repo has no pre-upload hooks enabled, then just return.
config = _get_project_config()
if not config:
@@ -257,6 +243,8 @@
if not hooks:
return True
+ output.set_num_hooks(len(hooks))
+
# Set up the environment like repo would with the forall command.
try:
remote = rh.git.get_upstream_remote()
@@ -273,7 +261,6 @@
'REPO_RREV': rh.git.get_remote_revision(upstream_branch, remote),
})
- output = Output(project_name, len(hooks))
project = rh.Project(name=project_name, dir=proj_dir, remote=remote)
if not commit_list:
@@ -311,11 +298,52 @@
if fixup_func_list:
_attempt_fixes(fixup_func_list, commit_list)
- output.finish()
- os.chdir(pwd)
return ret
+def _run_project_hooks(project_name, proj_dir=None, commit_list=None):
+ """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.
+ 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:
+ False if any errors were found, else True.
+ """
+ output = Output(project_name)
+
+ if proj_dir is None:
+ cmd = ['repo', 'forall', project_name, '-c', 'pwd']
+ result = rh.utils.run_command(cmd, capture_output=True)
+ proj_dirs = result.output.split()
+ if len(proj_dirs) == 0:
+ print('%s cannot be found.' % project_name, file=sys.stderr)
+ print('Please specify a valid project.', file=sys.stderr)
+ return False
+ if len(proj_dirs) > 1:
+ print('%s is associated with multiple directories.' % project_name,
+ file=sys.stderr)
+ print('Please specify a directory to help disambiguate.',
+ file=sys.stderr)
+ return False
+ proj_dir = proj_dirs[0]
+
+ pwd = os.getcwd()
+ 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,
+ commit_list=commit_list)
+ finally:
+ output.finish()
+ os.chdir(pwd)
+
+
def main(project_list, worktree_list=None, **_kwargs):
"""Main function invoked directly by repo.