project: preserve -c optimization when revision is a SHA-1

Avoid disabling --current-branch when syncing a SHA-1 revision without
an explicit project upstream (e.g., smart tags). Resolve a fallback
upstream from dest-branch or manifest defaults so -c only fetches the
target branch.

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

project.py

@@ -2857,6 +2857,18 @@ class Project:
 
         return True
 
+    def _GetUpstreamFallback(self) -> Optional[str]:
+        """Resolve a fallback upstream ref when revisionExpr is a SHA-1."""
+        for cand in (
+            self.dest_branch,
+            self.manifest.default.upstreamExpr,
+            self.manifest.default.destBranchExpr,
+            self.manifest.default.revisionExpr,
+        ):
+            if cand and not IsId(cand):
+                return cand
+        return None
+
     def _RemoteFetch(
         self,
         name=None,
@@ -2890,14 +2902,31 @@ class Project:
             current_branch_only = True
 
         is_sha1 = IsId(self.revisionExpr)
+        upstream = self.upstream
 
         if current_branch_only:
+            if is_sha1 and not depth:
+                # When syncing a specific commit and --depth is not set:
+                # * if upstream is explicitly specified and is not a sha1, fetch
+                #   only upstream as users expect only upstream to be fetch.
+                #   Note: The commit might not be in upstream in which case the
+                #   sync will fail.
+                # * otherwise, fetch all branches to make sure we end up with
+                #   the specific commit.
+                if not upstream:
+                    upstream = self._GetUpstreamFallback()
+
+                if upstream:
+                    current_branch_only = not IsId(upstream)
+                else:
+                    current_branch_only = False
+
             if self.revisionExpr.startswith(R_TAGS):
                 # This is a tag and its commit id should never change.
                 tag_name = self.revisionExpr[len(R_TAGS) :]
-            elif self.upstream and self.upstream.startswith(R_TAGS):
+            elif upstream and upstream.startswith(R_TAGS):
                 # This is a tag and its commit id should never change.
-                tag_name = self.upstream[len(R_TAGS) :]
+                tag_name = upstream[len(R_TAGS) :]
 
             if is_sha1 or tag_name is not None:
                 has_shallow = os.path.exists(
@@ -2915,18 +2944,6 @@ class Project:
                             "persistent ref)" % self.name
                         )
                     return True
-            if is_sha1 and not depth:
-                # When syncing a specific commit and --depth is not set:
-                # * if upstream is explicitly specified and is not a sha1, fetch
-                #   only upstream as users expect only upstream to be fetch.
-                #   Note: The commit might not be in upstream in which case the
-                #   sync will fail.
-                # * otherwise, fetch all branches to make sure we end up with
-                #   the specific commit.
-                if self.upstream:
-                    current_branch_only = not IsId(self.upstream)
-                else:
-                    current_branch_only = False
 
         if not name:
             name = self.remote.name
@@ -3038,11 +3055,11 @@ class Project:
             # Shallow checkout of a specific commit, fetch from that commit and
             # not the heads only as the commit might be deeper in the history.
             spec.append(branch)
-            if self.upstream:
-                spec.append(self.upstream)
+            if upstream:
+                spec.append(upstream)
         else:
             if is_sha1:
-                branch = self.upstream
+                branch = upstream
             if branch is not None and branch.strip():
                 if not branch.startswith("refs/"):
                     branch = R_HEADS + branch

tests/test_project.py

@@ -1045,13 +1045,20 @@ class StatelessSyncTests(unittest.TestCase):
 class SyncOptimizationTests(unittest.TestCase):
     """Tests for sync optimization logic involving shallow clones."""
 
-    def _get_project(self, tempdir, depth=None):
+    def _get_project(
+        self,
+        tempdir: str,
+        depth: Optional[int] = None,
+        revisionExpr: Optional[str] = None,
+    ) -> project.Project:
+        if revisionExpr is None:
+            revisionExpr = "0123456789abcdef0123456789abcdef01234567"
         proj = _create_mock_project(
             tempdir,
             depth=depth,
             gitdir=os.path.join(tempdir, "gitdir"),
             objdir=os.path.join(tempdir, "objdir"),
-            revisionExpr="0123456789abcdef0123456789abcdef01234567",
+            revisionExpr=revisionExpr,
         )
         proj._CheckForImmutableRevision = mock.MagicMock(return_value=True)
         proj.DeleteWorktree = mock.MagicMock()
@@ -1278,6 +1285,124 @@ class SyncOptimizationTests(unittest.TestCase):
                 self.assertTrue(res)
                 mock_git_cmd.assert_not_called()
 
+    def test_remote_fetch_sha1_upstream_fallback(self) -> None:
+        """Test _RemoteFetch resolves upstream fallback for SHA-1 revisions."""
+        sha = "4f8a3c0000000000000000000000000000000000"
+        with utils_for_test.TempGitTree() as tempdir:
+            proj = self._get_project(tempdir, revisionExpr=sha)
+            proj._CheckForImmutableRevision.side_effect = [False, True]
+            proj.upstream = None
+            proj.dest_branch = "my-dest-branch"
+
+            mock_remote = mock.MagicMock()
+            mock_remote.name = "origin"
+
+            def _to_local(r: str) -> str:
+                if r.startswith("refs/heads/"):
+                    return "refs/remotes/origin/" + r[11:]
+                return r
+
+            mock_remote.ToLocal.side_effect = _to_local
+            mock_remote.PreConnectFetch.return_value = True
+            proj.GetRemote = mock.MagicMock(return_value=mock_remote)
+
+            with mock.patch("project.GitCommand") as mock_git_cmd:
+                mock_cmd_instance = mock.MagicMock()
+                mock_cmd_instance.Wait.return_value = 0
+                mock_git_cmd.return_value = mock_cmd_instance
+
+                res = proj._RemoteFetch(current_branch_only=True)
+
+                self.assertTrue(res)
+                mock_git_cmd.assert_called_once()
+                cmd_args = mock_git_cmd.call_args[0][1]
+                self.assertIn(
+                    "+refs/heads/my-dest-branch:"
+                    "refs/remotes/origin/my-dest-branch",
+                    cmd_args,
+                )
+                self.assertNotIn(
+                    "+refs/heads/*:refs/remotes/origin/*", cmd_args
+                )
+
+    def test_remote_fetch_sha1_manifest_default_fallback(self) -> None:
+        """Test _RemoteFetch upstream fallback from manifest defaults."""
+        sha = "4f8a3c0000000000000000000000000000000000"
+        with utils_for_test.TempGitTree() as tempdir:
+            proj = self._get_project(tempdir, revisionExpr=sha)
+            proj._CheckForImmutableRevision.side_effect = [False, True]
+            proj.upstream = None
+            proj.dest_branch = None
+            proj.manifest.default.upstreamExpr = "manifest-upstream"
+
+            mock_remote = mock.MagicMock()
+            mock_remote.name = "origin"
+
+            def _to_local(r: str) -> str:
+                if r.startswith("refs/heads/"):
+                    return "refs/remotes/origin/" + r[11:]
+                return r
+
+            mock_remote.ToLocal.side_effect = _to_local
+            mock_remote.PreConnectFetch.return_value = True
+            proj.GetRemote = mock.MagicMock(return_value=mock_remote)
+
+            with mock.patch("project.GitCommand") as mock_git_cmd:
+                mock_cmd_instance = mock.MagicMock()
+                mock_cmd_instance.Wait.return_value = 0
+                mock_git_cmd.return_value = mock_cmd_instance
+
+                res = proj._RemoteFetch(current_branch_only=True)
+
+                self.assertTrue(res)
+                mock_git_cmd.assert_called_once()
+                cmd_args = mock_git_cmd.call_args[0][1]
+                self.assertIn(
+                    "+refs/heads/manifest-upstream:"
+                    "refs/remotes/origin/manifest-upstream",
+                    cmd_args,
+                )
+                self.assertNotIn(
+                    "+refs/heads/*:refs/remotes/origin/*", cmd_args
+                )
+
+    def test_remote_fetch_sha1_tag_fallback(self) -> None:
+        """Test _RemoteFetch resolves upstream fallback to tag correctly."""
+        sha = "4f8a3c0000000000000000000000000000000000"
+        with utils_for_test.TempGitTree() as tempdir:
+            proj = self._get_project(tempdir, revisionExpr=sha)
+            proj._CheckForImmutableRevision.side_effect = [False, True]
+            proj.upstream = None
+            proj.dest_branch = "refs/tags/v1.0"
+
+            mock_remote = mock.MagicMock()
+            mock_remote.name = "origin"
+
+            def _to_local(r: str) -> str:
+                if r.startswith("refs/tags/"):
+                    return "refs/tags/" + r[10:]
+                return r
+
+            mock_remote.ToLocal.side_effect = _to_local
+            mock_remote.PreConnectFetch.return_value = True
+            proj.GetRemote = mock.MagicMock(return_value=mock_remote)
+
+            with mock.patch("project.GitCommand") as mock_git_cmd:
+                mock_cmd_instance = mock.MagicMock()
+                mock_cmd_instance.Wait.return_value = 0
+                mock_git_cmd.return_value = mock_cmd_instance
+
+                res = proj._RemoteFetch(current_branch_only=True)
+
+                self.assertTrue(res)
+                mock_git_cmd.assert_called_once()
+                cmd_args = mock_git_cmd.call_args[0][1]
+                self.assertIn("tag", cmd_args)
+                self.assertIn("v1.0", cmd_args)
+                self.assertNotIn(
+                    "+refs/heads/*:refs/remotes/origin/*", cmd_args
+                )
+
 
 class GetEnvVarsTests(unittest.TestCase):
     """Tests for GetEnvVars project environment variable generation."""