From 5af71ce907848a546199f0d64a5dfba1db8adca0 Mon Sep 17 00:00:00 2001
From: Ram Peri <ramperi@google.com>
Date: Tue, 21 Apr 2026 00:02:47 +0000
Subject: [PATCH] Add timing keyword argument for hooks

Measure the duration of the sync operation in the Execute method of the
Sync command and pass it to post-sync hooks as a standard keyword
argument (`sync_duration_seconds`).

Updates based on code review:
- Update _API_ARGS in hooks.py to allow sync_duration_seconds for post-sync hooks.
- Do not cast sync_duration_seconds to int for better granularity.
- Update docs/repo-hooks.md to document sync_duration_seconds.
- Add unit test for argument validation in test_hooks.py.

Test: Ran run_tests using venv python, all 554 tests passed.
Bug: TBD
Change-Id: Ie29e002a5d283460d993ad96c224dbf4b6d7985c
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/575021
Tested-by: Arif Kasim <arifkasim@google.com>
Commit-Queue: Ram Peri <ramperi@google.com>
Reviewed-by: Arif Kasim <arifkasim@google.com>
Reviewed-by: Gavin Mak <gavinmak@google.com>
---
 docs/repo-hooks.md  |  3 ++-
 hooks.py            |  2 +-
 subcmds/sync.py     | 11 ++++++++---
 tests/test_hooks.py | 47 +++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 58 insertions(+), 5 deletions(-)

diff --git a/docs/repo-hooks.md b/docs/repo-hooks.md
index a56f261c7..696ad5a9f 100644
--- a/docs/repo-hooks.md
+++ b/docs/repo-hooks.md
@@ -163,13 +163,14 @@ Example:
 The `post-sync.py` file should be defined like:
 
 ```py
-def main(repo_topdir=None, **kwargs):
+def main(repo_topdir=None, sync_duration_seconds=None, **kwargs):
     """Main function invoked directly by repo.
 
     We must use the name "main" as that is what repo requires.
 
     Args:
       repo_topdir: The absolute path to the top-level directory of the repo workspace.
+      sync_duration_seconds: The duration of the sync operation in seconds.
       kwargs: Leave this here for forward-compatibility.
     """
 ```
diff --git a/hooks.py b/hooks.py
index d6e1e3bbc..0637dcec8 100644
--- a/hooks.py
+++ b/hooks.py
@@ -25,7 +25,7 @@ from git_refs import HEAD
 # The API we've documented to hook authors.  Keep in sync with repo-hooks.md.
 _API_ARGS = {
     "pre-upload": {"project_list", "worktree_list"},
-    "post-sync": {"repo_topdir"},
+    "post-sync": {"repo_topdir", "sync_duration_seconds"},
 }
 
 
diff --git a/subcmds/sync.py b/subcmds/sync.py
index bfbe1937d..7e7a2d3c0 100644
--- a/subcmds/sync.py
+++ b/subcmds/sync.py
@@ -1986,17 +1986,19 @@ later is required to fix a server side protocol bug.
 
     def Execute(self, opt, args):
         errors = []
+        start_time = time.time()
         try:
             self._ExecuteHelper(opt, args, errors)
+            sync_duration_seconds = time.time() - start_time
         except (RepoExitError, RepoChangedException):
             raise
         except (KeyboardInterrupt, Exception) as e:
             raise RepoUnhandledExceptionError(e, aggregate_errors=errors)
 
         # Run post-sync hook only after successful sync
-        self._RunPostSyncHook(opt)
+        self._RunPostSyncHook(opt, sync_duration_seconds=sync_duration_seconds)
 
-    def _RunPostSyncHook(self, opt):
+    def _RunPostSyncHook(self, opt, sync_duration_seconds=None):
         """Run post-sync hook if configured in manifest <repo-hooks>."""
         hook = RepoHook.FromSubcmd(
             hook_type="post-sync",
@@ -2004,7 +2006,10 @@ later is required to fix a server side protocol bug.
             opt=opt,
             abort_if_user_denies=False,
         )
-        success = hook.Run(repo_topdir=self.client.topdir)
+        success = hook.Run(
+            repo_topdir=self.client.topdir,
+            sync_duration_seconds=sync_duration_seconds,
+        )
         if not success:
             print("Warning: post-sync hook reported failure.")
 
diff --git a/tests/test_hooks.py b/tests/test_hooks.py
index 9d52d1849..14c226c75 100644
--- a/tests/test_hooks.py
+++ b/tests/test_hooks.py
@@ -14,6 +14,9 @@
 
 """Unittests for the hooks.py module."""
 
+from io import StringIO
+import sys
+
 import pytest
 
 import hooks
@@ -58,3 +61,47 @@ def test_direct_interp(shebang: str, interp: str) -> None:
 def test_env_interp(shebang: str, interp: str) -> None:
     """Lines whose shebang launches through `env`."""
     assert hooks.RepoHook._ExtractInterpFromShebang(shebang) == interp
+
+
+def test_post_sync_argument_validation() -> None:
+    """Test that post-sync hook requires exact API arguments."""
+
+    class FakeProject:
+
+        def __init__(self):
+            self.worktree = "/some/path"
+            self.enabled_repo_hooks = ["post-sync"]
+
+    hook = hooks.RepoHook(
+        hook_type="post-sync",
+        hooks_project=FakeProject(),
+        repo_topdir="/topdir",
+        manifest_url="https://gerrit",
+        allow_all_hooks=True,
+    )
+
+    old_stderr = sys.stderr
+    sys.stderr = StringIO()
+
+    try:
+        # Call with missing arg `sync_duration_seconds`
+        res = hook.Run(repo_topdir="/topdir")
+        assert res is False
+        assert "hook 'post-sync' called incorrectly" in sys.stderr.getvalue()
+
+        # Mock _CheckHook and _ExecuteHook to test success path
+        hook._CheckHook = lambda: None
+
+        executed_kwargs = {}
+
+        def fake_execute(**kw):
+            executed_kwargs.update(kw)
+
+        hook._ExecuteHook = fake_execute
+
+        res = hook.Run(repo_topdir="/topdir", sync_duration_seconds=12.345)
+        assert res is True
+        assert executed_kwargs.get("sync_duration_seconds") == 12.345
+
+    finally:
+        sys.stderr = old_stderr