diff --git a/completion.zsh b/completion.zsh index 704e584f8..ac82ce78d 100644 --- a/completion.zsh +++ b/completion.zsh @@ -400,6 +400,7 @@ _repo() { '--no-verify[Do not verify]' \ '--verify[Verify]' \ '--ignore-hooks[Ignore hooks]' \ + '--fix[Automatically fix]' \ '*: :->project' ;; version) diff --git a/docs/repo-hooks.md b/docs/repo-hooks.md index c3adee8ad..7f69217da 100644 --- a/docs/repo-hooks.md +++ b/docs/repo-hooks.md @@ -88,7 +88,14 @@ be useful when deploying automatic fixes. If the repo command that triggered the hook supports a "yes" option (e.g., `repo upload --yes`), this option is propagated to the hook's `main` function as `yes` parameter (defaulting to `False`). Hooks can use this to bypass -interactive confirmation prompts when they can automatically fix issues. +interactive confirmation prompts for safe non-modifying operations. + +### Automated Fixes + +If the repo command that triggered the hook supports a "fix" option (e.g., +`repo upload --fix`), this option is propagated to the hook's `main` function +as `fix` parameter (defaulting to `False`). Hooks can use this to automatically +apply fixes without prompting the user. ### Shebang Handling @@ -126,7 +133,7 @@ This hook runs when people run `repo upload`. The `pre-upload.py` file should be defined like: ```py -def main(project_list, worktree_list=None, yes=False, **kwargs): +def main(project_list, worktree_list=None, fix=False, yes=False, **kwargs): """Main function invoked directly by repo. We must use the name "main" as that is what repo requires. @@ -137,6 +144,7 @@ def main(project_list, worktree_list=None, yes=False, **kwargs): project_list, so that each entry in project_list matches with a directory in worktree_list. If None, we will attempt to calculate the directories automatically. + fix: Whether to automatically apply fixes without prompting. yes: Whether to answer yes to all safe prompts (see [Safe Prompts](#safe-prompts)). kwargs: Leave this here for forward-compatibility. diff --git a/hooks.py b/hooks.py index 5c763db5e..af3134d5e 100644 --- a/hooks.py +++ b/hooks.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import optparse import os import re import sys @@ -69,6 +70,7 @@ class RepoHook: ignore_hooks=False, abort_if_user_denies=False, yes=False, + fix=False, ): """RepoHook constructor. @@ -91,6 +93,7 @@ class RepoHook: abort_if_user_denies: If True, we'll abort running the hook if the user doesn't allow us to run the hook. yes: If True, then 'Yes' is assumed for any prompts. + fix: If True, then 'Fix' is assumed for any fixup prompts. """ self._hook_type = hook_type self._hooks_project = hooks_project @@ -102,6 +105,7 @@ class RepoHook: self._ignore_hooks = ignore_hooks self._abort_if_user_denies = abort_if_user_denies self._yes = yes + self._fix = fix # Store the full path to the script for convenience. self._script_fullpath = None @@ -380,6 +384,7 @@ class RepoHook: kwargs = { **kwargs, "hook_should_take_kwargs": True, + "fix": self._fix, "yes": self._yes, } @@ -504,12 +509,17 @@ class RepoHook: ).url, "bug_url": manifest.contactinfo.bugurl, "yes": getattr(opt, "yes", False), + "fix": getattr(opt, "fix", False), } ) return cls(*args, **kwargs) @staticmethod - def AddOptionGroup(parser, name): + def AddOptionGroup( + parser: optparse.OptionParser, + name: str, + allow_fix: bool = False, + ) -> None: """Help options relating to the various hooks.""" # Note that verify and no-verify are NOT opposites of each other, which @@ -533,3 +543,10 @@ class RepoHook: action="store_true", help="Do not abort if %s hooks fail." % name, ) + if allow_fix: + group.add_option( + "--fix", + action="store_true", + default=False, + help="Automatically apply %s fixes without prompting." % name, + ) diff --git a/man/repo-upload.1 b/man/repo-upload.1 index 02de7b533..ad17b6987 100644 --- a/man/repo-upload.1 +++ b/man/repo-upload.1 @@ -1,5 +1,5 @@ .\" DO NOT MODIFY THIS FILE! It was generated by help2man. -.TH REPO "1" "June 2026" "repo upload" "Repo Manual" +.TH REPO "1" "August 2026" "repo upload" "Repo Manual" .SH NAME repo \- repo upload - manual page for repo upload .SH SYNOPSIS @@ -112,6 +112,9 @@ Run the pre\-upload hook without prompting. .TP \fB\-\-ignore\-hooks\fR Do not abort if pre\-upload hooks fail. +.TP +\fB\-\-fix\fR +Automatically apply pre\-upload fixes without prompting. .PP Run `repo help upload` to view the detailed manual. .SH DETAILS diff --git a/subcmds/upload.py b/subcmds/upload.py index 56a2066e7..31cdde191 100644 --- a/subcmds/upload.py +++ b/subcmds/upload.py @@ -379,7 +379,7 @@ Gerrit Code Review: https://www.gerritcodereview.com/ default=True, help="disable verifying ssl certs (unsafe)", ) - RepoHook.AddOptionGroup(p, "pre-upload") + RepoHook.AddOptionGroup(p, "pre-upload", allow_fix=True) def _SingleBranch(self, opt, branch, people): project = branch.project diff --git a/tests/test_hooks.py b/tests/test_hooks.py index 33ccba7b6..3c05d4fe4 100644 --- a/tests/test_hooks.py +++ b/tests/test_hooks.py @@ -15,6 +15,7 @@ """Unittests for the hooks.py module.""" from io import StringIO +from pathlib import Path import sys import pytest @@ -108,11 +109,11 @@ def test_post_sync_argument_validation() -> None: @pytest.mark.parametrize("yes_val", (True, False)) -def test_repo_upload_yes_arg(tmp_path, yes_val: bool) -> None: +def test_repo_upload_yes_arg(tmp_path: Path, yes_val: bool) -> None: """Test that yes is passed in kwargs during hook execution.""" class FakeProject: - def __init__(self, worktree): + def __init__(self, worktree: str) -> None: self.worktree = worktree self.enabled_repo_hooks = ["pre-upload"] self.config = None @@ -139,3 +140,64 @@ def main(project_list, **kwargs): assert res is True assert project_list == [yes_val] + + +@pytest.mark.parametrize("fix_val", (True, False)) +def test_repo_upload_fix_arg(tmp_path: Path, fix_val: bool) -> None: + """Test that fix is passed in kwargs during hook execution.""" + + class FakeProject: + def __init__(self, worktree: str) -> None: + self.worktree = worktree + self.enabled_repo_hooks = ["pre-upload"] + self.config = None + + hook_file = tmp_path / "pre-upload.py" + + hook_content = """ +def main(project_list, **kwargs): + project_list.append(kwargs.get("fix")) +""" + hook_file.write_text(hook_content) + + hook = hooks.RepoHook( + hook_type="pre-upload", + hooks_project=FakeProject(str(tmp_path)), + repo_topdir=str(tmp_path), + manifest_url="https://gerrit", + allow_all_hooks=True, + fix=fix_val, + ) + + project_list = [] + res = hook.Run(project_list=project_list, worktree_list=[]) + + assert res is True + assert project_list == [fix_val] + + +def test_from_subcmd_without_fix_option() -> None: + """Test that FromSubcmd works when opt does not have fix attribute.""" + + class Remote: + url = "https://gerrit" + + class FakeManifest: + repo_hooks_project = None + topdir = "/fake/topdir" + + class manifestProject: + @staticmethod + def GetRemote(name: str) -> "Remote": + return Remote() + + class contactinfo: + bugurl = "https://bugs" + + class FakeOpt: + bypass_hooks = False + allow_all_hooks = False + ignore_hooks = False + + hook = hooks.RepoHook.FromSubcmd(FakeManifest(), FakeOpt(), "post-sync") + assert hook._fix is False