Skip to content

Commit 044e52e

Browse files
vapierLUCI
authored and
LUCI
committed
hooks: add internal check for external hook API
Add an internal check to make sure we always follow the API we've documented for external authors. Since the internal call is a bit ad-hoc, it can be easy to miss a call site. Change-Id: Ie8cd298d1fc34f10f3c5eb353512a3e881f42252 Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/481721 Reviewed-by: Nasser Grainawi <[email protected]> Reviewed-by: Gavin Mak <[email protected]> Tested-by: Mike Frysinger <[email protected]> Commit-Queue: Mike Frysinger <[email protected]>
1 parent 0cb88a8 commit 044e52e

File tree

1 file changed

+24
-0
lines changed

1 file changed

+24
-0
lines changed

hooks.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,12 @@
2222
from git_refs import HEAD
2323

2424

25+
# The API we've documented to hook authors. Keep in sync with repo-hooks.md.
26+
_API_ARGS = {
27+
"pre-upload": {"project_list", "worktree_list"},
28+
}
29+
30+
2531
class RepoHook:
2632
"""A RepoHook contains information about a script to run as a hook.
2733
@@ -56,6 +62,7 @@ def __init__(
5662
hooks_project,
5763
repo_topdir,
5864
manifest_url,
65+
bug_url=None,
5966
bypass_hooks=False,
6067
allow_all_hooks=False,
6168
ignore_hooks=False,
@@ -75,6 +82,7 @@ def __init__(
7582
run with CWD as this directory.
7683
If you have a manifest, this is manifest.topdir.
7784
manifest_url: The URL to the manifest git repo.
85+
bug_url: The URL to report issues.
7886
bypass_hooks: If True, then 'Do not run the hook'.
7987
allow_all_hooks: If True, then 'Run the hook without prompting'.
8088
ignore_hooks: If True, then 'Do not abort action if hooks fail'.
@@ -85,6 +93,7 @@ def __init__(
8593
self._hooks_project = hooks_project
8694
self._repo_topdir = repo_topdir
8795
self._manifest_url = manifest_url
96+
self._bug_url = bug_url
8897
self._bypass_hooks = bypass_hooks
8998
self._allow_all_hooks = allow_all_hooks
9099
self._ignore_hooks = ignore_hooks
@@ -414,6 +423,20 @@ def Run(self, **kwargs):
414423
ignore the result through the option combinations as listed in
415424
AddHookOptionGroup().
416425
"""
426+
# Make sure our own callers use the documented API.
427+
exp_kwargs = _API_ARGS.get(self._hook_type, set())
428+
got_kwargs = set(kwargs.keys())
429+
if exp_kwargs != got_kwargs:
430+
print(
431+
"repo internal error: "
432+
f"hook '{self._hook_type}' called incorrectly\n"
433+
f" got: {sorted(got_kwargs)}\n"
434+
f" expected: {sorted(exp_kwargs)}\n"
435+
f"Please file a bug: {self._bug_url}",
436+
file=sys.stderr,
437+
)
438+
return False
439+
417440
# Do not do anything in case bypass_hooks is set, or
418441
# no-op if there is no hooks project or if hook is disabled.
419442
if (
@@ -472,6 +495,7 @@ def FromSubcmd(cls, manifest, opt, *args, **kwargs):
472495
"manifest_url": manifest.manifestProject.GetRemote(
473496
"origin"
474497
).url,
498+
"bug_url": manifest.contactinfo.bugurl,
475499
}
476500
)
477501
return cls(*args, **kwargs)

0 commit comments

Comments
 (0)