repo: tolerate a transiently missing stable branch

The automated release process deletes the stable branch and then
recreates it. A client that fetches during that window can't find it:
`repo init` fails to resolve --repo-rev, and the daily self-update in
`repo sync` fails with "Cannot fetch repo tool".

In the launcher, if --repo-rev doesn't resolve after the initial fetch,
wait 10 seconds and refetch once before giving up. `resolve_repo_rev`
gains a `report_errors` flag so this first check doesn't print an error
that the refetch then fixes. Bump the launcher version to 2.69 so
existing users pick this up.

For fetches of the repo project itself (sync's self-update and
`repo selfupdate`), wait 10 seconds instead of 4 before retrying. The
delay is now a `RETRY_SLEEP_INITIAL_SEC` class attribute that
`RepoProject` overrides, so other projects keep the 4 second delay. This
replaces the unused `retry_sleep_initial_sec` parameter of
`_RemoteFetch`.

Bug: 566270786
Change-Id: I23cb810258522b5aa9eae9d7013dbb7904ebcad8
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/643265
Tested-by: Brian Gan <brgan@google.com>
Commit-Queue: Brian Gan <brgan@google.com>
Reviewed-by: Gavin Mak <gavinmak@google.com>

project.py

@@ -559,6 +559,9 @@ class RemoteSpec:
 
 
 class Project:
+    # How long to wait before the first retry of a failed fetch.
+    RETRY_SLEEP_INITIAL_SEC = 4.0
+
     # These objects can be shared between several working trees.
     @property
     def shareable_dirs(self):
@@ -3353,7 +3356,6 @@ class Project:
         force_sync=False,
         clone_filter=None,
         retry_fetches=2,
-        retry_sleep_initial_sec=4.0,
         retry_exp_factor=2.0,
     ) -> bool:
         tag_name = None
@@ -3546,7 +3548,7 @@ class Project:
 
         # At least one retry minimum due to git remote prune.
         retry_fetches = max(retry_fetches, 2)
-        retry_cur_sleep = retry_sleep_initial_sec
+        retry_cur_sleep = self.RETRY_SLEEP_INITIAL_SEC
         ok = prune_tried = False
         try:
             for try_n in range(retry_fetches):
@@ -5488,6 +5490,10 @@ class MetaProject(Project):
 class RepoProject(MetaProject):
     """The MetaProject for repo itself."""
 
+    # The automated release process deletes the stable branch and then
+    # recreates it, so wait long enough for it to come back before retrying.
+    RETRY_SLEEP_INITIAL_SEC = 10.0
+
     @property
     def LastFetch(self):
         try:

repo

@@ -26,7 +26,7 @@ import platform
 import shlex
 import subprocess
 import sys
-from typing import NamedTuple
+from typing import NamedTuple, Tuple
 
 
 # These should never be newer than the main.py version since this needs to be a
@@ -129,7 +129,7 @@ if not REPO_REV:
 BUG_URL = "https://issues.gerritcodereview.com/issues/new?component=1370071"
 
 # increment this whenever we make important changes to this script
-VERSION = (2, 65)
+VERSION = (2, 69)
 
 # increment this if the MAINTAINER_KEYS block is modified
 KEYRING_VERSION = (2, 3)
@@ -220,6 +220,10 @@ repodir = ".repo"  # name of repo's private directory
 S_repo = "repo"  # special repo repository
 S_manifests = "manifests"  # special manifest repository
 REPO_MAIN = S_repo + "/main.py"  # main script
+# The automated release process deletes the stable branch and then recreates it
+# at the new release, so a fetch can briefly miss it.  Wait this long and
+# refetch once before giving up on --repo-rev.
+REV_REFETCH_DELAY_SEC = 10
 
 
 import errno
@@ -228,6 +232,7 @@ import optparse
 import re
 import shutil
 import stat
+import time
 import urllib.error
 import urllib.request
 
@@ -631,6 +636,7 @@ def _Init(args):
         dst = dst_final + ".tmp"
         shutil.rmtree(dst, ignore_errors=True)
         _Clone(url, dst, opt.clone_bundle, opt.quiet, opt.verbose)
+        _RefetchIfRevMissing(url, dst, rev, opt.quiet, opt.verbose)
 
         remote_ref, rev = check_repo_rev(
             dst, rev, opt.repo_verify, quiet=opt.quiet
@@ -1005,7 +1011,43 @@ def _Clone(url, cwd, clone_bundle, quiet, verbose):
     _Fetch(url, cwd, "origin", quiet, verbose)
 
 
-def resolve_repo_rev(cwd, committish):
+def _RefetchIfRevMissing(
+    url: str, cwd: str, rev: str, quiet: bool, verbose: bool
+) -> None:
+    """Refetch once if |rev| doesn't resolve.
+
+    This doesn't fail if |rev| still doesn't resolve; check_repo_rev reports
+    that.
+
+    Any unresolvable |rev| is retried, not just branches.  Telling a branch
+    apart from a tag or commit isn't possible while it's missing, and the only
+    cost for a bad --repo-rev is a short delay before the same error.
+
+    Args:
+        url: The URL of the repo source.
+        cwd: The git checkout to fetch into.
+        rev: The --repo-rev to wait for.
+        quiet: Whether to suppress fetch output.
+        verbose: Whether to show verbose fetch output.
+    """
+    try:
+        resolve_repo_rev(cwd, rev, report_errors=False)
+        return
+    except CloneFailure:
+        pass
+    # Print even with --quiet since this explains the delay.
+    print(
+        f'repo: warning: unable to resolve "{rev}"; '
+        f"refetching in {REV_REFETCH_DELAY_SEC} seconds",
+        file=sys.stderr,
+    )
+    time.sleep(REV_REFETCH_DELAY_SEC)
+    _Fetch(url, cwd, "origin", quiet, verbose)
+
+
+def resolve_repo_rev(
+    cwd: str, committish: str, report_errors: bool = True
+) -> Tuple[str, str]:
     """Figure out what REPO_REV represents.
 
     We support:
@@ -1016,6 +1058,7 @@ def resolve_repo_rev(cwd, committish):
     Args:
         cwd: The git checkout to run in.
         committish: The REPO_REV argument to resolve.
+        report_errors: Whether to print an error if it can't be resolved.
 
     Returns:
         A tuple of (remote ref, commit) as makes sense for the committish.
@@ -1034,17 +1077,18 @@ def resolve_repo_rev(cwd, committish):
         )
         return None if ret.returncode else ret.stdout.strip()
 
+    def error(msg: str) -> CloneFailure:
+        if report_errors:
+            print(f"repo: error: {msg}", file=sys.stderr)
+        return CloneFailure()
+
     # An explicit branch.
     if committish.startswith("refs/heads/"):
         remote_ref = committish
         committish = committish[len("refs/heads/") :]
         rev = resolve("refs/remotes/origin/%s" % committish)
         if rev is None:
-            print(
-                f'repo: error: unknown branch "{committish}"',
-                file=sys.stderr,
-            )
-            raise CloneFailure()
+            raise error(f'unknown branch "{committish}"')
         return (remote_ref, rev)
 
     # An explicit tag.
@@ -1053,11 +1097,7 @@ def resolve_repo_rev(cwd, committish):
         committish = committish[len("refs/tags/") :]
         rev = resolve(remote_ref)
         if rev is None:
-            print(
-                f'repo: error: unknown tag "{committish}"',
-                file=sys.stderr,
-            )
-            raise CloneFailure()
+            raise error(f'unknown tag "{committish}"')
         return (remote_ref, rev)
 
     # See if it's a short branch name.
@@ -1076,11 +1116,7 @@ def resolve_repo_rev(cwd, committish):
         return (rev, rev)
 
     # Give up!
-    print(
-        f'repo: error: unable to resolve "{committish}"',
-        file=sys.stderr,
-    )
-    raise CloneFailure()
+    raise error(f'unable to resolve "{committish}"')
 
 
 def verify_rev(cwd, remote_ref, rev, quiet):

tests/test_project.py

@@ -15,6 +15,7 @@
 """Unittests for the project.py module."""
 
 import contextlib
+import io
 import os
 from pathlib import Path
 import shutil
@@ -2113,6 +2114,35 @@ def test_metaproject_has_changes(
         assert proj.HasChanges is expected
 
 
+@pytest.mark.parametrize(
+    "project_cls,expected_sleep",
+    (
+        (project.ManifestProject, 4.0),
+        # Repo waits longer for a briefly deleted stable branch to come back.
+        (project.RepoProject, 10.0),
+    ),
+    ids=lambda v: v.__name__ if isinstance(v, type) else None,
+)
+def test_remote_fetch_retry_sleep(
+    project_cls: Type[project.MetaProject], expected_sleep: float
+) -> None:
+    """A failed fetch waits RETRY_SLEEP_INITIAL_SEC before retrying."""
+    with utils_for_test.TempGitTree() as tempdir:
+        proj = _create_meta_project(tempdir, project_cls)
+        proj.config.GetRemote("origin").ResetFetch()
+        proj.revisionExpr = "refs/heads/stable"
+        with mock.patch.object(project, "GitCommand") as mock_git:
+            # What git does while stable is deleted.
+            mock_git.return_value.Wait.return_value = 128
+            mock_git.return_value.stdout = (
+                "fatal: couldn't find remote ref refs/heads/stable\n"
+            )
+            with mock.patch.object(project.time, "sleep") as mock_sleep:
+                assert not proj._RemoteFetch(output_redir=io.StringIO())
+
+        assert mock_sleep.call_args_list == [mock.call(expected_sleep)]
+
+
 @pytest.mark.parametrize(
     "state,expect_failure",
     [

tests/test_wrapper.py

@@ -515,6 +515,62 @@ class TestResolveRepoRev:
         with pytest.raises(repo_wrapper.CloneFailure):
             repo_wrapper.resolve_repo_rev(git_checkout.git_dir, "boooooooya")
 
+    @pytest.mark.parametrize(
+        "committish",
+        ["refs/heads/unknown", "refs/tags/unknown", "boooooooya"],
+    )
+    @pytest.mark.parametrize("report_errors", [True, False])
+    def test_report_errors(
+        self,
+        repo_wrapper: wrapper.Wrapper,
+        git_checkout: GitCheckout,
+        capsys: pytest.CaptureFixture,
+        committish: str,
+        report_errors: bool,
+    ) -> None:
+        """Check errors are only printed when requested."""
+        with pytest.raises(repo_wrapper.CloneFailure):
+            repo_wrapper.resolve_repo_rev(
+                git_checkout.git_dir, committish, report_errors=report_errors
+            )
+        assert ("repo: error:" in capsys.readouterr().err) == report_errors
+
+
+class TestRefetchIfRevMissing:
+    """Check _RefetchIfRevMissing behavior."""
+
+    @pytest.mark.parametrize("missing", [False, True])
+    def test_refetch(
+        self,
+        repo_wrapper: wrapper.Wrapper,
+        capsys: pytest.CaptureFixture,
+        missing: bool,
+    ) -> None:
+        """Check it refetches once only if the rev doesn't resolve."""
+        mock_resolve = mock.Mock(
+            side_effect=repo_wrapper.CloneFailure() if missing else None
+        )
+        with mock.patch.object(repo_wrapper, "_Fetch") as mock_fetch:
+            with mock.patch.object(repo_wrapper.time, "sleep") as mock_sleep:
+                with mock.patch.object(
+                    repo_wrapper, "resolve_repo_rev", mock_resolve
+                ):
+                    repo_wrapper._RefetchIfRevMissing(
+                        "url", "/dst", "stable", False, False
+                    )
+
+        # It resolves quietly; check_repo_rev reports the error.
+        mock_resolve.assert_called_once_with(
+            "/dst", "stable", report_errors=False
+        )
+        fetches = int(missing)
+        fetch = mock.call("url", "/dst", "origin", False, False)
+        assert mock_fetch.call_args_list == [fetch] * fetches
+        delay = repo_wrapper.REV_REFETCH_DELAY_SEC
+        assert mock_sleep.call_args_list == [mock.call(delay)] * fetches
+        warnings = capsys.readouterr().err.count(f"refetching in {delay} ")
+        assert warnings == fetches
+
 
 class TestCheckRepoVerify:
     """Check check_repo_verify behavior."""