project: check upstream ref ancestry for non-shallow clones In _CheckForImmutableRevision, commit d9cc0a15 restricted upstream ref validation strictly to superprojects (if use_superproject) to prevent shallow clones with an upstream attribute from failing ancestry checks and falling back to full clones. However, restricting this check exclusively to superprojects broke non-shallow projects with pinned immutable revisions (such as in Smart Sync, `repo sync -t <BUILD_ID>`, or pinned manifests): When the target commit already exists in the local Git object store (for instance, prefetched into refs/prefetch/ by a background daemon or via shared object dirs), _CheckForImmutableRevision returned True without verifying that the local tracking ref (refs/remotes/<remote>/<upstream>) is present and reaches the revision. As a result, _RemoteFetch skipped fetching the upstream branch, leaving the local tracking ref stale or missing. Subsequent `repo start` branches tracking that remote branch diverged, causing `repo upload` to attempt uploading all intermediate commits between the stale tracking ref and HEAD. Restore upstream ancestry validation in _CheckForImmutableRevision for non-shallow projects when upstream is specified. Also pass the sync depth into _CheckForImmutableRevision call sites so shallow checkouts continue to skip upstream verification and avoid triggering un-shallow fallbacks. Test: PYTHONPATH=. pytest tests/test_project.py Change-Id: Ib5bdf41810bb06c8ed053447209ddc2e488f3913 Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/624361 Tested-by: Rahul Yadav <yadavrah@google.com> Commit-Queue: Rahul Yadav <yadavrah@google.com> Reviewed-by: Gavin Mak <gavinmak@google.com>
diff --git a/project.py b/project.py index 75f91ab..2f72bb0 100644 --- a/project.py +++ b/project.py
@@ -1623,11 +1623,7 @@ # If the project has been manually unshallowed (e.g. via # `git fetch --unshallow`), don't re-shallow it during sync. - if ( - depth - and not is_new - and not os.path.exists(os.path.join(self.gitdir, "shallow")) - ): + if depth and not is_new and not self._HasShallow(): depth = None if depth and clone_filter_for_depth: @@ -1663,17 +1659,15 @@ ) else: # See if we can skip the standard network fetch entirely. - has_shallow = os.path.exists(os.path.join(self.gitdir, "shallow")) + has_shallow = self._HasShallow() skip_fetch = ( optimized_fetch and IsId(self.revisionExpr) and self._CheckForImmutableRevision( - use_superproject=use_superproject + use_superproject=use_superproject, + depth=depth, ) - and ( - has_shallow - or (not depth and not self._SharingProjectHasShallow()) - ) + and (has_shallow or not self._IsShallow(depth)) ) if not skip_fetch: @@ -2797,19 +2791,22 @@ return None def _CheckForImmutableRevision( - self, use_superproject: Optional[bool] = None + self, + use_superproject: Optional[bool] = None, + depth: Optional[int] = None, ) -> bool: try: # if revision (sha or tag) is not present then following function # throws an error. revs = [f"{self.revisionExpr}^0"] upstream_rev = None - use_superproject_for_upstream = self.upstream and ( - self._UseSuperprojectForUpstream(use_superproject) + verify_upstream = self._ShouldVerifyUpstream( + use_superproject=use_superproject, + depth=depth, ) - # Only check upstream when using superproject. - if use_superproject_for_upstream: + # Ensure the local upstream tracking ref also exists in the ODB. + if verify_upstream: upstream_rev = self.GetRemote().ToLocal(self.upstream) revs.append(upstream_rev) @@ -2821,9 +2818,8 @@ log_as_error=False, ) - # Only verify upstream relationship for superproject scenarios - # without affecting plain usage. - if use_superproject_for_upstream: + # Verify revision is an ancestor of the upstream tracking ref. + if verify_upstream: self.bare_git.merge_base( "--is-ancestor", self.revisionExpr, @@ -2835,6 +2831,31 @@ # There is no such persistent revision. We have to fetch it. return False + def _HasShallow(self) -> bool: + """Check if this project has a shallow file in its gitdir.""" + return bool( + self.gitdir and os.path.exists(os.path.join(self.gitdir, "shallow")) + ) + + def _IsShallow(self, depth: Optional[int] = None) -> bool: + """Check if the project is shallow or sharing shallow objects.""" + return bool( + self._HasShallow() or self._SharingProjectHasShallow() or depth + ) + + def _ShouldVerifyUpstream( + self, + use_superproject: Optional[bool] = None, + depth: Optional[int] = None, + ) -> bool: + """Whether to verify upstream ancestry during immutable revision + check.""" + if not (IsId(self.revisionExpr) and self.upstream): + return False + if self._UseSuperprojectForUpstream(use_superproject): + return True + return not self._IsShallow(depth) + def _SharingProjectHasShallow(self) -> bool: """Check if another project sharing this objdir has a "shallow" file. @@ -2848,18 +2869,14 @@ ) for proj in other_projects: if proj.objdir == self.objdir and proj.gitdir != self.gitdir: - if os.path.exists(os.path.join(proj.gitdir, "shallow")): + if proj._HasShallow(): return True return False def _UseSuperprojectForUpstream( self, use_superproject: Optional[bool] = None ) -> bool: - """Whether to include upstream in the immutability check. - - The upstream ancestry check is only meaningful for projects - that participate in a superproject relationship. - """ + """Whether to check upstream for superprojects.""" return git_superproject.UseSuperproject(use_superproject, self.manifest) def _FetchArchive(self, tarpath, cwd=None): @@ -3060,15 +3077,11 @@ tag_name = upstream[len(R_TAGS) :] if is_sha1 or tag_name is not None: - has_shallow = os.path.exists( - os.path.join(self.gitdir, "shallow") - ) + has_shallow = self._HasShallow() if self._CheckForImmutableRevision( - use_superproject=use_superproject - ) and ( - has_shallow - or (not depth and not self._SharingProjectHasShallow()) - ): + use_superproject=use_superproject, + depth=depth, + ) and (has_shallow or not self._IsShallow(depth)): if verbose: print( "Skipped fetching project %s (already have " @@ -3134,7 +3147,7 @@ # have shallow objects or not. Tell git to unshallow all fetched # refs. Don't do this with projects that don't have shallow # objects, since it is less efficient. - if os.path.exists(os.path.join(self.gitdir, "shallow")): + if self._HasShallow(): cmd.append("--depth=2147483647") # Use clone-depth="1" as a heuristic for repositories containing @@ -3377,7 +3390,8 @@ # got what we wanted, else trigger a second run of all # refs. if not self._CheckForImmutableRevision( - use_superproject=use_superproject + use_superproject=use_superproject, + depth=depth, ): # Sync the current branch only with depth set to None. # We always pass depth=None down to avoid infinite recursion. @@ -4942,6 +4956,16 @@ # before manifest.xml has been linked into .repo/. return False + def _ShouldVerifyUpstream( + self, + use_superproject: Optional[bool] = None, + depth: Optional[int] = None, + ) -> bool: + """MetaProjects (manifest repo and repo itself) do not verify upstream + ancestry. + """ + return False + @property def HasChanges(self): """Has the remote received new commits not yet checked out?"""
diff --git a/tests/test_project.py b/tests/test_project.py index c2db279..af296c2 100644 --- a/tests/test_project.py +++ b/tests/test_project.py
@@ -1630,6 +1630,18 @@ self.assertFalse(fakeproj._SharingProjectHasShallow()) self.assertFalse(os.path.exists(manifest_path)) + def test_should_verify_upstream_metaproject_returns_false( + self, + ) -> None: + """MetaProjects never verify upstream ancestry.""" + with utils_for_test.TempGitTree() as tempdir: + fakeproj = self.setUpManifest(tempdir) + fakeproj.revisionExpr = "4f8a3c0000000000000000000000000000000000" + fakeproj.upstream = "refs/heads/main" + self.assertFalse( + fakeproj._ShouldVerifyUpstream(use_superproject=False) + ) + def test_sync_use_local_gitdirs_worktree_conflict(self): """Test that --use-local-gitdirs conflicts with --worktree.""" with utils_for_test.TempGitTree() as tempdir: @@ -2236,6 +2248,167 @@ self.assertTrue(res) mock_git_cmd.assert_not_called() + def test_check_immutable_revision_plain_project_upstream_ancestor( + self, + ) -> None: + """Non-shallow projects verify upstream ancestry for immutable + revisions.""" + with utils_for_test.TempGitTree() as tempdir: + proj = _create_mock_project(tempdir) + proj.bare_git = project.Project._GitGetByExec( + proj, bare=True, gitdir=proj.gitdir + ) + proj.upstream = "refs/heads/main" + proj.work_git.config("remote.origin.url", "http://example.com/repo") + proj.work_git.config( + "remote.origin.fetch", "+refs/heads/*:refs/remotes/origin/*" + ) + + test_file = os.path.join(tempdir, "file.txt") + with open(test_file, "w") as f: + f.write("commit1") + proj.work_git.add("file.txt") + proj.work_git.commit("-m", "commit 1") + commit1 = proj.work_git.rev_parse("HEAD") + + proj.work_git.update_ref("refs/remotes/origin/main", commit1) + + with open(test_file, "w") as f: + f.write("commit2") + proj.work_git.add("file.txt") + proj.work_git.commit("-m", "commit 2") + commit2 = proj.work_git.rev_parse("HEAD") + + # 1. When revision is commit1 and upstream tracking ref is at + # commit2: commit1 is ancestor of origin/main -> True. + proj.revisionExpr = commit1 + proj.work_git.update_ref("refs/remotes/origin/main", commit2) + self.assertTrue( + proj._CheckForImmutableRevision(use_superproject=False) + ) + + # 2. When revision is commit2 and upstream tracking ref is at + # commit1 (behind): commit2 is NOT an ancestor -> False. + proj.revisionExpr = commit2 + proj.work_git.update_ref("refs/remotes/origin/main", commit1) + self.assertFalse( + proj._CheckForImmutableRevision(use_superproject=False) + ) + + # 3. In shallow mode (depth passed), upstream ancestry is skipped: + # commit2 exists in ODB, so shallow returns True even if upstream + # is behind. + self.assertTrue( + proj._CheckForImmutableRevision(use_superproject=False, depth=1) + ) + + # 4. If gitdir has shallow file, shallow check also skips upstream + # ancestry. + shallow_file = os.path.join(proj.gitdir, "shallow") + with open(shallow_file, "w") as f: + f.write("") + self.assertTrue( + proj._CheckForImmutableRevision(use_superproject=False) + ) + os.unlink(shallow_file) + + # 5. Tag revisions skip upstream ancestry verification: tag + # commits are immutable and do not track an upstream branch. + proj.revisionExpr = "refs/tags/v1.0" + proj.work_git.tag("-a", "v1.0", "-m", "tag v1.0", commit2) + self.assertTrue( + proj._CheckForImmutableRevision(use_superproject=False) + ) + + def test_sync_network_half_stale_upstream_fetches(self) -> None: + """Sync_NetworkHalf does not skip fetch when upstream ref is behind.""" + with utils_for_test.TempGitTree() as tempdir: + proj = _create_mock_project(tempdir) + proj.bare_git = project.Project._GitGetByExec( + proj, bare=True, gitdir=proj.gitdir + ) + proj.upstream = "refs/heads/main" + proj.work_git.config("remote.origin.url", "http://example.com/repo") + proj.work_git.config( + "remote.origin.fetch", "+refs/heads/*:refs/remotes/origin/*" + ) + + test_file = os.path.join(tempdir, "file.txt") + with open(test_file, "w") as f: + f.write("commit1") + proj.work_git.add("file.txt") + proj.work_git.commit("-m", "commit 1") + commit1 = proj.work_git.rev_parse("HEAD") + + with open(test_file, "w") as f: + f.write("commit2") + proj.work_git.add("file.txt") + proj.work_git.commit("-m", "commit 2") + commit2 = proj.work_git.rev_parse("HEAD") + + # Upstream ref is at commit1 (behind commit2). + proj.work_git.update_ref("refs/remotes/origin/main", commit1) + proj.revisionExpr = commit2 + + proj._InitRemote = mock.MagicMock() + proj._InitMRef = mock.MagicMock() + proj._RemoteFetch = mock.MagicMock( + return_value=project.SyncNetworkHalfResult(True) + ) + + res = proj.Sync_NetworkHalf(optimized_fetch=True) + self.assertTrue(res.success) + proj._RemoteFetch.assert_called_once() + + def test_should_verify_upstream(self) -> None: + """Test _ShouldVerifyUpstream conditions.""" + sha = "4f8a3c0000000000000000000000000000000000" + with utils_for_test.TempGitTree() as tempdir: + proj = self._get_project(tempdir, revisionExpr=sha) + proj.upstream = "refs/heads/main" + + # SHA revision with upstream on non-shallow project -> True. + self.assertTrue(proj._ShouldVerifyUpstream(use_superproject=False)) + + # Not a SHA (e.g. tag or branch name) -> False. + proj.revisionExpr = "refs/tags/v1.0" + self.assertFalse(proj._ShouldVerifyUpstream(use_superproject=False)) + + # SHA revision but no upstream -> False. + proj.revisionExpr = sha + proj.upstream = None + self.assertFalse(proj._ShouldVerifyUpstream(use_superproject=False)) + + # Shallow with depth -> False. + proj.upstream = "refs/heads/main" + self.assertFalse( + proj._ShouldVerifyUpstream(use_superproject=False, depth=1) + ) + + # Shallow with .git/shallow file -> False. + os.makedirs(proj.gitdir, exist_ok=True) + with open(os.path.join(proj.gitdir, "shallow"), "w") as f: + f.write("") + self.assertFalse(proj._ShouldVerifyUpstream(use_superproject=False)) + + def test_is_shallow_and_has_shallow(self) -> None: + """Test _HasShallow and _IsShallow helpers.""" + with utils_for_test.TempGitTree() as tempdir: + proj = self._get_project(tempdir) + self.assertFalse(proj._HasShallow()) + self.assertFalse(proj._IsShallow()) + + # depth makes _IsShallow True. + self.assertTrue(proj._IsShallow(depth=1)) + self.assertFalse(proj._HasShallow()) + + # shallow file in gitdir makes both True. + os.makedirs(proj.gitdir, exist_ok=True) + with open(os.path.join(proj.gitdir, "shallow"), "w") as f: + f.write("") + self.assertTrue(proj._HasShallow()) + self.assertTrue(proj._IsShallow()) + def test_remote_fetch_sha1_dest_branch_not_fetched(self) -> None: """Test _RemoteFetch ignores dest-branch for SHA-1 revisions.""" sha = "4f8a3c0000000000000000000000000000000000"