mirrors/git-repo
1
0
Fork 0
mirror of https://gerrit.googlesource.com/git-repo synced 2026-09-24 15:50:35 +00:00
Code Issues Projects Releases Wiki Activity

project: check upstream ref ancestry for non-shallow clones

Browse Source 
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>
This commit is contained in:
Rahul Yadav
2026-09-03 02:15:23 -07:00
committed by gerrit-scoped@luci-project-accounts.iam.gserviceaccount.com
parent 0ea57e2eed
commit 127e4dbcd0
2 changed files with 232 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files
project.py
+59 -35
View File
@@ -1623,11 +1623,7 @@ class Project:
# 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 @@ class Project:
)
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
)
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 not skip_fetch:
@@ -2797,19 +2791,22 @@ class Project:
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 @@ class Project:
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 @@ class Project:
# 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 @@ class Project:
)
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 @@ class Project:
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 @@ class Project:
# 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 @@ class Project:
# 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 @@ class MetaProject(Project):
# 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?"""
tests/test_project.py
+173
View File
@@ -1630,6 +1630,18 @@ class ManifestPropertiesFetchedCorrectly(unittest.TestCase):
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 @@ class SyncOptimizationTests(unittest.TestCase):
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"