repo_synchronizer: don't ignore fetch errors (bug 2082293)

git_hg_sync/repo_synchronizer.py

@@ -62,20 +62,20 @@ def _commit_has_mercurial_metadata(self, repo: Repo, git_commit: str) -> bool:
     def fetch_all_from_remote(
         self, repo: Repo, remote: str, verbose: bool = False
     ) -> None:
-        try:
-            retry(
-                f"fetching changes and tags from {remote}",
-                lambda: self._log_git_execute(
+        def fetch() -> None:
+            try:
+                self._log_git_execute(
                     repo,
                     ["git", "-c", "cinnabar.graft=true", "fetch", "--tags", remote],
                     verbose,
-                ),
-            )
+                )
+            except GitCommandError as exc:
+                # can't fetch if repo is empty
+                if "fatal: couldn't find remote ref HEAD" not in exc.stderr:
+                    raise
+                logger.info(f"Remote {remote} is empty, nothing to fetch")
 
-        except GitCommandError as exc:
-            # can't fetch if repo is empty
-            if "fatal: couldn't find remote ref HEAD" in exc.stderr:
-                raise exc
+        retry(f"fetching changes and tags from {remote}", fetch)
 
         if remote.startswith("hg::"):
             retry(
@@ -95,25 +95,32 @@ def _log_git_execute(repo: Repo, command: list[str], verbose: bool = False) -> N
         if verbose:
             logger.info(f"Running `{' '.join(command)}` as PID {proc.pid}")
 
+            stderr_lines: list[bytes] = []
             stdout_thread = threading.Thread(
                 target=RepoSynchronizer._stream_output,
                 args=(proc.stdout, f"{proc.pid}/STDOUT"),
             )
             stderr_thread = threading.Thread(
                 target=RepoSynchronizer._stream_output,
-                args=(proc.stderr, f"{proc.pid}/STDERR"),
+                args=(proc.stderr, f"{proc.pid}/STDERR", stderr_lines),
             )
             stdout_thread.start()
             stderr_thread.start()
             stdout_thread.join()
             stderr_thread.join()
 
-            proc.wait()
+            # The stream has been consumed, so pass it on to be included in any
+            # GitCommandError.
+            proc.wait(stderr=b"".join(stderr_lines))
 
     @staticmethod
-    def _stream_output(stream: io.BufferedReader, label: str) -> None:
+    def _stream_output(
+        stream: io.BufferedReader, label: str, lines: list[bytes] | None = None
+    ) -> None:
         for line in iter(stream.readline, b""):
             logger.info(f"{label}: {line.decode().strip()}")
+            if lines is not None:
+                lines.append(line)
 
     def sync(
         self, destination_url: str, operations: list[SyncOperation], request_user: str
@@ -315,10 +322,7 @@ def _ensure_cinnabar_metadata(self, repo: Repo, destination_remote: str) -> None
             logger.debug("Cinnabar metadata already present, not updating")
             return
 
-        retry(
-            f"fetching commits from {destination_remote}",
-            lambda: self.fetch_all_from_remote(repo, destination_remote),
-        )
+        self.fetch_all_from_remote(repo, destination_remote)
 
     def _git2hg(self, repo: Repo, git_commit: str) -> str:
         return repo.git.cinnabar(["git2hg", git_commit]).strip()

tests/test_repo_synchronizer.py

@@ -323,3 +323,23 @@ def test_get_connection_and_queue(pulse_config: PulseConfig) -> None:
     assert connection.userid == pulse_config.userid
     assert connection.host == f"{pulse_config.host}:{pulse_config.port}"
     assert queue.name == pulse_config.queue
+
+
+@pytest.mark.parametrize("verbose", [False, True])
+def test_fetch_all_from_remote_empty(tmp_path: Path, verbose: bool) -> None:
+    empty_remote = Repo.init(tmp_path / "empty", bare=True)
+    clone = Repo.init(tmp_path / "clone", bare=True)
+    syncrepos = RepoSynchronizer(tmp_path / "clone", str(empty_remote.git_dir))
+
+    # Fetching from an empty repository is not an error.
+    syncrepos.fetch_all_from_remote(clone, str(empty_remote.git_dir), verbose)
+
+
+@pytest.mark.parametrize("verbose", [False, True])
+def test_fetch_all_from_remote_error(tmp_path: Path, verbose: bool) -> None:
+    clone = Repo.init(tmp_path / "clone", bare=True)
+    missing_remote = str(tmp_path / "missing")
+    syncrepos = RepoSynchronizer(tmp_path / "clone", missing_remote)
+
+    with pytest.raises(GitCommandError, match="does not appear to be a git repository"):
+        syncrepos.fetch_all_from_remote(clone, missing_remote, verbose)