[PATCH b4 13/27] ty: check reachability in the repository the commit landed in

[Christian Brauner <brauner@kernel.org>]

The publish check for a queued thank-you asks the remote what it
advertises and then works out locally whether the commit is an ancestor
of one of those tips. The local half, cat-file and rev-list, runs
against the process cwd.

That was fine while the check only ever ran from inside the repository
we are thanking for. "b4 review cron" broke the assumption. It resolves
a topdir per project and passes it to everything else but never chdirs,
so a sweep over several projects, or a timer whose working directory is
wherever the scheduler put it, computes ancestry against some other
repository or against none at all. The commit looks unreachable and the
thanks waits forever.

Take the repository as an argument and pass the topdir the queue sweep
already has. Pass it to ls-remote too, so a repository-local http.* or
url.*.insteadOf applies the same way it would if the check were run by
hand from that tree.

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 src/b4/ty.py         | 32 +++++++++++++++++++++++---------
 src/tests/test_ty.py | 28 +++++++++++++++++++++-------
 2 files changed, 44 insertions(+), 16 deletions(-)

diff --git a/src/b4/ty.py b/src/b4/ty.py
index 9fe5f40..5e225a7 100644
--- a/src/b4/ty.py
+++ b/src/b4/ty.py
@@ -1050,7 +1050,7 @@ def get_check_repo_for_branch(
 
 
 def commit_reachable_on_remote(
-    commit: str, repo_url: str, branch: str = ''
+    commit: str, repo_url: str, branch: str = '', gitdir: Optional[str] = None
 ) -> Optional[bool]:
     """Check if a commit is reachable from a branch advertised by repo_url.
 
@@ -1066,8 +1066,14 @@ def commit_reachable_on_remote(
     deleted), any advertised branch is accepted as before.
 
     Ancestry is computed locally against the advertised tips, so tips we
-    do not have objects for are ignored. Returns True/False, or None if
-    the state could not be determined (e.g. the remote is unreachable).
+    do not have objects for are ignored.  That happens in *gitdir* — the
+    repository the commit was applied in.  It defaults to the process
+    cwd, which is only right for callers that operate on it; a queue
+    sweep covering several projects must name the tree each message
+    belongs to.
+
+    Returns True/False, or None if the state could not be determined
+    (e.g. the remote is unreachable).
     """
     gitargs = [
         '-c',
@@ -1078,7 +1084,7 @@ def commit_reachable_on_remote(
         '--heads',
         repo_url,
     ]
-    ecode, out = b4.git_run_command(None, gitargs)
+    ecode, out = b4.git_run_command(gitdir, gitargs)
     if ecode > 0:
         logger.debug('ls-remote failed for %s (exit code %s)', repo_url, ecode)
         return None
@@ -1104,7 +1110,7 @@ def commit_reachable_on_remote(
     # for them, so treat them as not containing the commit
     stdin = ('\n'.join(sorted(tips)) + '\n').encode()
     _ecode, out = b4.git_run_command(
-        None, ['cat-file', '--batch-check=%(objectname) %(objecttype)'], stdin=stdin
+        gitdir, ['cat-file', '--batch-check=%(objectname) %(objecttype)'], stdin=stdin
     )
     known: List[str] = []
     for line in out.splitlines():
@@ -1116,7 +1122,7 @@ def commit_reachable_on_remote(
         return False
     # Empty output means every commit reachable from ours is also
     # reachable from one of the known tips, i.e. ours is published
-    ecode, out = b4.git_run_command(None, ['rev-list', '-1', commit, '--not', *known])
+    ecode, out = b4.git_run_command(gitdir, ['rev-list', '-1', commit, '--not', *known])
     if ecode > 0:
         logger.debug('rev-list failed for %s (exit code %s)', commit, ecode)
         return None
@@ -1124,7 +1130,11 @@ def commit_reachable_on_remote(
 
 
 def _check_published(
-    checkurl: str, checkcommit: str, checkrepo: str, checkbranch: str = ''
+    checkurl: str,
+    checkcommit: str,
+    checkrepo: str,
+    checkbranch: str = '',
+    gitdir: Optional[str] = None,
 ) -> Optional[bool]:
     """Tri-state publish check for a queued thanks message.
 
@@ -1140,7 +1150,9 @@ def _check_published(
     if checkurl and not checkrepo:
         checkrepo = _get_check_repo(checkurl) or ''
     if checkcommit and checkrepo:
-        return commit_reachable_on_remote(checkcommit, checkrepo, branch=checkbranch)
+        return commit_reachable_on_remote(
+            checkcommit, checkrepo, branch=checkbranch, gitdir=gitdir
+        )
     if not checkurl:
         return True
     try:
@@ -1453,7 +1465,9 @@ def _process_queue_locked(
 
         # Check if the commit is publicly visible
         if not dryrun and (checkurl or (checkcommit and checkrepo)):
-            published = _check_published(checkurl, checkcommit, checkrepo, checkbranch)
+            published = _check_published(
+                checkurl, checkcommit, checkrepo, checkbranch, gitdir=topdir
+            )
             if published is None:
                 still_pending += 1
                 if progress_cb:
diff --git a/src/tests/test_ty.py b/src/tests/test_ty.py
index 54fa6d4..610e5af 100644
--- a/src/tests/test_ty.py
+++ b/src/tests/test_ty.py
@@ -503,7 +503,9 @@ def test_process_queue_passes_branch(
 
     calls: List[Tuple[str, str, str]] = []
 
-    def fake_reachable(commit: str, repo_url: str, branch: str = '') -> Optional[bool]:
+    def fake_reachable(
+        commit: str, repo_url: str, branch: str = '', gitdir: Optional[str] = None
+    ) -> Optional[bool]:
         calls.append((commit, repo_url, branch))
         return False
 
@@ -530,7 +532,9 @@ def test_process_queue_holds_unpublished(
 
     calls: List[Tuple[str, str]] = []
 
-    def fake_reachable(commit: str, repo_url: str, branch: str = '') -> Optional[bool]:
+    def fake_reachable(
+        commit: str, repo_url: str, branch: str = '', gitdir: Optional[str] = None
+    ) -> Optional[bool]:
         calls.append((commit, repo_url))
         return False
 
@@ -580,7 +584,9 @@ def test_process_queue_lock_held(
     monkeypatch.chdir(repo)
     _queue_test_message()
     monkeypatch.setattr(
-        b4.ty, 'commit_reachable_on_remote', lambda commit, repo_url, branch='': False
+        b4.ty,
+        'commit_reachable_on_remote',
+        lambda commit, repo_url, branch='', gitdir=None: False,
     )
     with b4.lockfile_nb(b4.ty._get_queue_lock_path()):
         with pytest.raises(b4.LockHeldError):
@@ -599,7 +605,9 @@ def test_process_queue_check_only(
     monkeypatch.chdir(repo)
     _queue_test_message()
     monkeypatch.setattr(
-        b4.ty, 'commit_reachable_on_remote', lambda commit, repo_url, branch='': True
+        b4.ty,
+        'commit_reachable_on_remote',
+        lambda commit, repo_url, branch='', gitdir=None: True,
     )
 
     def _no_send(dryrun: bool = False) -> Tuple[None, str]:
@@ -627,7 +635,9 @@ def test_process_queue_explicit_topdir(
     monkeypatch.chdir(repo)
     _queue_test_message()
     monkeypatch.setattr(
-        b4.ty, 'commit_reachable_on_remote', lambda commit, repo_url, branch='': True
+        b4.ty,
+        'commit_reachable_on_remote',
+        lambda commit, repo_url, branch='', gitdir=None: True,
     )
     outside = tmp_path / 'elsewhere'
     outside.mkdir()
@@ -665,7 +675,9 @@ def test_process_queue_finalizes_thanked(
 
     _queue_test_message()
     monkeypatch.setattr(
-        b4.ty, 'commit_reachable_on_remote', lambda commit, repo_url, branch='': True
+        b4.ty,
+        'commit_reachable_on_remote',
+        lambda commit, repo_url, branch='', gitdir=None: True,
     )
     monkeypatch.setattr(b4, 'get_smtp', lambda dryrun=False: (None, 't@example.com'))
     monkeypatch.setattr(b4, 'send_mail', lambda *args, **kwargs: 1)
@@ -738,7 +750,9 @@ def _series_status(identifier: str, change_id: str = 'test-change-id') -> str:
 
 def _mock_delivery(monkeypatch: pytest.MonkeyPatch) -> None:
     monkeypatch.setattr(
-        b4.ty, 'commit_reachable_on_remote', lambda commit, repo_url, branch='': True
+        b4.ty,
+        'commit_reachable_on_remote',
+        lambda commit, repo_url, branch='', gitdir=None: True,
     )
     monkeypatch.setattr(b4, 'get_smtp', lambda dryrun=False: (None, 't@example.com'))
     monkeypatch.setattr(b4, 'send_mail', lambda *args, **kwargs: 1)

-- 
2.53.0