git_refs, project: skip redundant git calls on unborn HEAD and warm refs

Read plain files-backend unborn HEAD symrefs directly before spawning
git symbolic-ref, update already-loaded bare_ref snapshots in
_InitAnyMRef when no external ref changes occurred, and reuse warm
bare_ref snapshots in GetRevisionId for non-tag refs.

Also skip redundant rev-parse and empty-index ls-files subprocesses in
_Reproject when HEAD is guaranteed to be an unborn local ref.

Bug: 564817989
Change-Id: I0da10cefb14753ce6e1e22e192d652790d59a5b0
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/649605
Tested-by: Gavin Mak <gavinmak@google.com>
Reviewed-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Gavin Mak <gavinmak@google.com>
This commit is contained in:
hungweichen authored and gerrit-scoped@luci-project-accounts.iam.gserviceaccount.com committed 2026-11-05 07:52:57 -08:00
1 parent 6596cb6499
commit b16e5f49d6
4 files changed
+542 -36

No files matched your search

+123 -16
View File
@@ -27,6 +27,8 @@ R_PUB = "refs/published/"
R_WORKTREE = "refs/worktree/"
R_WORKTREE_M = R_WORKTREE + "m/"
R_M = "refs/remotes/m/"
# Maximum symbolic-ref indirection depth (matches Git SYMREF_MAXDEPTH).
_SYMREF_MAXDEPTH = 5
class GitRefs:
@@ -76,6 +78,101 @@ class GitRefs:
except KeyError:
return ""
def UpdateSymref(self, name: str, target: str) -> None:
"""Update a symbolic reference in an already-loaded ref snapshot.
Args:
name: Full symbolic reference name (e.g. "refs/remotes/m/main").
target: Target reference name that the symref points to.
"""
if self._phyref is None:
return
if self._HasExternalRefChanges(name):
self._phyref = None
return
self._symref[name] = target
self._ResolveSymrefs()
parts = name.strip("/").split("/")
for idx in range(1, len(parts)):
anc = os.path.join(*parts[:idx])
self._TrackMtime(anc)
self._TrackMtime(os.path.join(*parts))
def _HasExternalRefChanges(self, updated_symref: str) -> bool:
"""Check whether refs outside |updated_symref| changed on disk.
Args:
updated_symref: Symbolic ref path just updated on disk.
Returns:
True if reftable is in use or any unrelated ref path changed.
"""
if os.path.exists(os.path.join(self._gitdir, "reftable")):
return True
if "packed-refs" not in self._mtime and os.path.exists(
os.path.join(self._gitdir, "packed-refs")
):
return True
parts = updated_symref.strip("/").split("/")
expected_child = {
"/".join(parts[:idx]): parts[idx] for idx in range(1, len(parts))
}
for tracked, mtime in self._mtime.items():
norm = tracked.replace(os.sep, "/")
if norm == updated_symref:
continue
full_path = os.path.join(self._gitdir, tracked)
try:
curr_mtime = os.path.getmtime(full_path)
except OSError:
return True
if norm in expected_child:
if curr_mtime != mtime:
try:
children = platform_utils.listdir(full_path)
except OSError:
return True
next_comp = expected_child[norm]
for child in children:
if child == next_comp:
continue
if os.path.join(tracked, child) not in self._mtime:
return True
elif curr_mtime != mtime:
return True
for anc, next_comp in expected_child.items():
anc_os = anc.replace("/", os.sep)
if anc_os not in self._mtime:
anc_full = os.path.join(self._gitdir, anc_os)
try:
if platform_utils.listdir(anc_full) != [next_comp]:
return True
except OSError:
return True
return False
def _ResolveSymrefs(self) -> None:
"""Resolve all symbolic references into _phyref up to max depth."""
for name in self._symref:
self._phyref.pop(name, None)
scan = self._symref
attempts = 0
while scan and attempts < _SYMREF_MAXDEPTH:
scan_next = {}
for name, dest in scan.items():
if dest in self._phyref:
self._phyref[name] = self._phyref[dest]
else:
scan_next[name] = dest
scan = scan_next
attempts += 1
def _EnsureLoaded(self):
if self._phyref is None or self._NeedUpdate():
self._LoadAll()
@@ -105,23 +202,17 @@ class GitRefs:
# --include-root-refs does not report an unborn HEAD.
self._ReadSymbolicRef(HEAD)
scan = self._symref
attempts = 0
while scan and attempts < 5:
scan_next = {}
for name, dest in scan.items():
if dest in self._phyref:
self._phyref[name] = self._phyref[dest]
else:
scan_next[name] = dest
scan = scan_next
attempts += 1
self._ResolveSymrefs()
self._RefreshMtimes()
self._TrackMtime(HEAD)
self._TrackMtime("config")
self._TrackMtime("packed-refs")
self._TrackTreeMtimes("refs")
self._TrackTreeMtimes("reftable")
def _RefreshMtimes(self) -> None:
"""Record current modification times for ref storage paths."""
self._mtime = {}
self._TrackMtime(HEAD)
self._TrackMtime("config")
self._TrackMtime("packed-refs")
self._TrackTreeMtimes("refs")
self._TrackTreeMtimes("reftable")
@staticmethod
def _IsNullRef(ref_id: str) -> bool:
@@ -165,6 +256,22 @@ class GitRefs:
def _ReadSymbolicRef(self, name: str) -> None:
"""Read a symbolic reference."""
if name == HEAD:
path = os.path.join(self._gitdir, HEAD)
try:
if not platform_utils.islink(path):
with open(
path, "r", encoding="utf-8", errors="replace"
) as fd:
line = fd.readline().strip()
if line.startswith("ref:"):
ref = line[4:].strip()
if ref and ref != R_HEADS + ".invalid":
self._symref[name] = ref
return
except OSError:
pass
p = git_command.GitCommand(
None,
["symbolic-ref", "-q", name],
+89 -20
View File
@@ -2133,11 +2133,12 @@ class Project:
# Tags may be annotated, in which case all_refs holds the tag object
# SHA rather than the peeled commit SHA. Fall through to ResolveCommit
# so GetRevisionId always returns a commit SHA.
if (
all_refs is not None
and not rev.startswith(git_refs.R_TAGS)
and rev in all_refs
):
is_tag = rev.startswith(
git_refs.R_TAGS
) or self.revisionExpr.startswith(git_refs.R_TAGS)
if all_refs is None and not is_tag and self.bare_ref.is_loaded:
all_refs = self.bare_ref.all
if all_refs is not None and not is_tag and rev in all_refs:
return all_refs[rev]
try:
@@ -4186,12 +4187,17 @@ class Project:
project=self.name,
)
try:
head_tree = self.work_git.rev_parse(
"-q", "--verify", "HEAD^{tree}", log_as_error=False
)
except error.GitError:
use_exec_work_git = isinstance(self.work_git, self._GitGetByExec)
unborn_head = use_exec_work_git and self.work_git.IsUnbornHead()
if unborn_head:
head_tree = None
else:
try:
head_tree = self.work_git.rev_parse(
"-q", "--verify", "HEAD^{tree}", log_as_error=False
)
except error.GitError:
head_tree = None
if head_tree:
p = git_command.GitCommand(
@@ -4207,6 +4213,8 @@ class Project:
project=self.name,
)
staged = p.stdout.split("\0")[:-1]
elif use_exec_work_git and not self.work_git.HasIndexFile():
staged = []
else:
p = git_command.GitCommand(
self,
@@ -4230,12 +4238,15 @@ class Project:
)
head = self.work_git.GetHead()
try:
head_oid = self.work_git.rev_parse(
"-q", "--verify", "HEAD", log_as_error=False
)
except error.GitError:
if unborn_head:
head_oid = None
else:
try:
head_oid = self.work_git.rev_parse(
"-q", "--verify", "HEAD", log_as_error=False
)
except error.GitError:
head_oid = None
env = os.environ.copy()
env.update(self.GetEnvVars())
@@ -4276,12 +4287,15 @@ class Project:
raise LocalSyncFail(msg, project=self.name)
new_head = self.work_git.GetHead()
try:
new_head_oid = self.work_git.rev_parse(
"-q", "--verify", "HEAD", log_as_error=False
)
except error.GitError:
if use_exec_work_git and self.work_git.IsUnbornHead(head=new_head):
new_head_oid = None
else:
try:
new_head_oid = self.work_git.rev_parse(
"-q", "--verify", "HEAD", log_as_error=False
)
except error.GitError:
new_head_oid = None
if new_head != head or new_head_oid != head_oid:
from_desc = (
@@ -4626,6 +4640,8 @@ class Project:
active_git.UpdateRef(ref, dst, message=msg, detach=True)
else:
active_git.symbolic_ref("-m", msg, ref, dst)
if active_git is self.bare_git:
self.bare_ref.UpdateSymref(ref, dst)
def _CheckDirReference(self, srcdir, destdir):
# Git worktrees don't use symlinks to share at all.
@@ -5262,6 +5278,59 @@ class Project:
)
return ref
def IsUnbornHead(self, head: Optional[str] = None) -> bool:
"""Check whether HEAD is guaranteed to be an unborn local ref.
Args:
head: Optional parsed HEAD symref target; if None, read from
the worktree's HEAD file on disk.
Returns:
True when the repository uses plain loose files and HEAD
points to a missing local ref; False otherwise.
"""
if "GIT_DIR" in os.environ or "GIT_COMMON_DIR" in os.environ:
return False
try:
dotgit = self.GetDotgitPath()
head_path = os.path.join(dotgit, git_refs.HEAD)
if not os.path.isdir(dotgit) or platform_utils.islink(
head_path
):
return False
if head is None:
with open(
head_path, encoding="utf-8", errors="replace"
) as fd:
head = self._ParseHead(fd.readline())
except (OSError, AssertionError):
return False
if not head or not head.startswith("refs/") or ".." in head:
return False
if any(ch in head for ch in "~^:?*[\\@{ "):
return False
return not any(
os.path.lexists(os.path.join(dotgit, rel))
for rel in ("packed-refs", "reftable", "commondir", head)
)
def HasIndexFile(self) -> bool:
"""Check whether the worktree may have a Git index file.
Returns:
False only when the worktree's .git/index file is confirmed
absent; True otherwise (including custom env overrides).
"""
if "GIT_INDEX_FILE" in os.environ or "GIT_DIR" in os.environ:
return True
try:
dotgit = self.GetDotgitPath()
return not os.path.isdir(dotgit) or os.path.lexists(
os.path.join(dotgit, "index")
)
except (OSError, AssertionError):
return True
def ResolveCommit(self, revision: str) -> str:
"""Resolve |revision| to a commit without option ambiguity."""
cmdv = ["--verify", "--quiet"]
+162
View File
@@ -244,3 +244,165 @@ def test_updates_when_storage_backend_toggles(tmp_path):
_run(repo, "refs", "migrate", "--ref-format=files")
_run(repo, "branch", "files-branch")
assert refs.get("refs/heads/files-branch") == head
@pytest.mark.parametrize("reftable", [False, True])
def test_unborn_head_resolution(
tmp_path: Path, reftable: bool, monkeypatch: pytest.MonkeyPatch
) -> None:
"""Unborn HEAD reads files backend directly and uses git on reftable."""
if reftable and not utils_for_test.supports_reftable():
pytest.skip("reftable not supported")
repo = os.path.join(tmp_path, "unborn")
ref_format = "reftable" if reftable else "files"
utils_for_test.init_git_tree(repo, ref_format=ref_format)
_run(repo, "symbolic-ref", "HEAD", "refs/heads/unborn-main")
refs = git_refs.GitRefs(os.path.join(repo, ".git"))
seen_symbolic_ref = []
class SpyGitCommand(git_command.GitCommand):
def __init__(
self,
proj: Any,
cmdv: List[str],
*args: Any,
**kwargs: Any,
) -> None:
if cmdv and cmdv[0] == "symbolic-ref":
seen_symbolic_ref.append(cmdv)
super().__init__(proj, cmdv, *args, **kwargs)
monkeypatch.setattr(git_command, "GitCommand", SpyGitCommand)
assert refs.symref("HEAD") == "refs/heads/unborn-main"
if not reftable:
assert seen_symbolic_ref == []
@pytest.mark.parametrize("reftable", [False, True])
def test_update_symref_keeps_snapshot_warm(
tmp_path: Path, reftable: bool
) -> None:
"""UpdateSymref stays warm on files backend and reloads on reftable."""
if reftable and not utils_for_test.supports_reftable():
pytest.skip("reftable not supported")
repo = _init_repo(tmp_path, reftable=reftable)
branch = _run(repo, "symbolic-ref", "--short", "HEAD")
head = _run(repo, "rev-parse", "HEAD")
refs = git_refs.GitRefs(os.path.join(repo, ".git"))
assert refs.get(f"refs/heads/{branch}") == head
_run(repo, "symbolic-ref", "refs/remotes/m/main", f"refs/heads/{branch}")
refs.UpdateSymref("refs/remotes/m/main", f"refs/heads/{branch}")
if reftable:
assert not refs.is_loaded
assert refs.symref("refs/remotes/m/main") == f"refs/heads/{branch}"
assert refs.get("refs/remotes/m/main") == head
else:
with mock.patch.object(
refs,
"_LoadAll",
side_effect=AssertionError("expected warm GitRefs snapshot"),
):
assert refs.symref("refs/remotes/m/main") == f"refs/heads/{branch}"
assert refs.get("refs/remotes/m/main") == head
def test_update_symref_invalidates_when_external_ref_changed(
tmp_path: Path,
) -> None:
"""External ref writes before UpdateSymref invalidate instead of masking."""
repo = _init_repo(tmp_path, reftable=False)
branch = _run(repo, "symbolic-ref", "--short", "HEAD")
head = _run(repo, "rev-parse", "HEAD")
refs = git_refs.GitRefs(os.path.join(repo, ".git"))
assert refs.get(f"refs/heads/{branch}") == head
# External branch write happens before symbolic-ref + UpdateSymref.
_run(repo, "branch", "external-topic")
_run(repo, "symbolic-ref", "refs/remotes/m/main", f"refs/heads/{branch}")
refs.UpdateSymref("refs/remotes/m/main", f"refs/heads/{branch}")
assert not refs.is_loaded
assert refs.get("refs/heads/external-topic") == head
assert refs.get("refs/remotes/m/main") == head
# External sibling ref in the same directory is also detected.
_run(repo, "update-ref", "refs/remotes/m/sibling", head)
_run(
repo,
"symbolic-ref",
"refs/remotes/m/main",
"refs/heads/external-topic",
)
refs.UpdateSymref("refs/remotes/m/main", "refs/heads/external-topic")
assert not refs.is_loaded
assert refs.get("refs/remotes/m/sibling") == head
def test_update_symref_self_or_cyclic_clears_phyref(tmp_path: Path) -> None:
"""Self or cyclic symrefs clear any prior physical SHA from _phyref."""
repo = _init_repo(tmp_path, reftable=False)
head = _run(repo, "rev-parse", "HEAD")
gitdir = os.path.join(repo, ".git")
_run(repo, "update-ref", "refs/remotes/m/main", head)
_run(repo, "update-ref", "refs/remotes/m/other", head)
refs = git_refs.GitRefs(gitdir)
assert refs.get("refs/remotes/m/main") == head
assert refs.get("refs/remotes/m/other") == head
_run(repo, "symbolic-ref", "refs/remotes/m/main", "refs/remotes/m/main")
refs.UpdateSymref("refs/remotes/m/main", "refs/remotes/m/main")
assert refs.get("refs/remotes/m/main") == ""
_run(repo, "symbolic-ref", "refs/remotes/m/main", "refs/remotes/m/other")
refs.UpdateSymref("refs/remotes/m/main", "refs/remotes/m/other")
assert refs.get("refs/remotes/m/main") == head
_run(repo, "symbolic-ref", "refs/remotes/m/other", "refs/remotes/m/main")
refs.UpdateSymref("refs/remotes/m/other", "refs/remotes/m/main")
assert refs.get("refs/remotes/m/main") == ""
assert refs.get("refs/remotes/m/other") == ""
def test_update_symref_propagates_chained_symrefs(tmp_path: Path) -> None:
"""Re-pointing an intermediate symref updates chained symrefs cleanly."""
repo = _init_repo(tmp_path, reftable=False)
branch = _run(repo, "symbolic-ref", "--short", "HEAD")
c1 = _run(repo, "rev-parse", "HEAD")
Path(os.path.join(repo, "a")).write_text("2", encoding="utf-8")
_run(repo, "add", "a")
_run(repo, "commit", "-q", "-m", "second")
c2 = _run(repo, "rev-parse", "HEAD")
_run(repo, "update-ref", "refs/heads/first", c1)
# Point HEAD at unborn refs/remotes/m/main before warming GitRefs.
_run(repo, "symbolic-ref", "HEAD", "refs/remotes/m/main")
refs = git_refs.GitRefs(os.path.join(repo, ".git"))
assert refs.symref("HEAD") == "refs/remotes/m/main"
assert refs.get("HEAD") == ""
_run(repo, "symbolic-ref", "refs/remotes/m/main", "refs/heads/first")
refs.UpdateSymref("refs/remotes/m/main", "refs/heads/first")
_run(repo, "symbolic-ref", "refs/remotes/m/alias", "refs/remotes/m/main")
refs.UpdateSymref("refs/remotes/m/alias", "refs/remotes/m/main")
assert refs.get("HEAD") == c1
assert refs.get("refs/remotes/m/alias") == c1
_run(repo, "symbolic-ref", "refs/remotes/m/main", f"refs/heads/{branch}")
refs.UpdateSymref("refs/remotes/m/main", f"refs/heads/{branch}")
with mock.patch.object(
refs,
"_LoadAll",
side_effect=AssertionError("expected warm GitRefs snapshot"),
):
assert refs.get("refs/remotes/m/main") == c2
assert refs.get("refs/remotes/m/alias") == c2
assert refs.get("HEAD") == c2
+168
View File
@@ -3845,6 +3845,16 @@ class ReprojectCmdTests(unittest.TestCase):
proj.work_git.rev_parse.side_effect = error.GitError("unborn")
mock_run = self._reproject(proj)
mock_run.assert_called_once()
self.assertEqual(
[
mock.call(
"-q", "--verify", "HEAD^{tree}", log_as_error=False
),
mock.call("-q", "--verify", "HEAD", log_as_error=False),
mock.call("-q", "--verify", "HEAD", log_as_error=False),
],
proj.work_git.rev_parse.call_args_list,
)
def test_reproject_rejects_an_operation_in_progress(self) -> None:
"""Test an unfinished rebase fails the sync before the command runs."""
@@ -4298,6 +4308,113 @@ class ReprojectCmdTests(unittest.TestCase):
self.assertTrue(proj.config.GetBoolean("custom.reprojected"))
def test_reproject_missing_head_tree_keeps_symmetric_head_oid(
self,
) -> None:
"""Born HEAD with missing HEAD^{tree} still compares HEAD cleanly."""
with utils_for_test.TempGitTree() as tempdir:
proj = self._get_project(tempdir)
proj.work_git.GetHead.return_value = "refs/heads/main"
def _rev_parse(*args: str, **_kwargs: Any) -> str:
if "HEAD^{tree}" in args:
raise error.GitError("missing tree object")
return self.HEAD_ID
proj.work_git.rev_parse.side_effect = _rev_parse
mock_run = self._reproject(proj)
mock_run.assert_called_once()
def test_get_revision_id_reuses_loaded_bare_ref(self) -> None:
"""Test GetRevisionId reuses warm bare_ref and peels tag revisions."""
with utils_for_test.TempGitTree() as tempdir:
subprocess.run(
[
"git",
"-C",
tempdir,
"commit",
"--allow-empty",
"-q",
"-m",
"init",
],
check=True,
)
commit_sha = subprocess.check_output(
["git", "-C", tempdir, "rev-parse", "HEAD"], text=True
).strip()
subprocess.run(
[
"git",
"-C",
tempdir,
"update-ref",
"refs/remotes/origin/main",
commit_sha,
],
check=True,
)
subprocess.run(
[
"git",
"-C",
tempdir,
"tag",
"-a",
"-m",
"tag v1.0",
"v1.0",
commit_sha,
],
check=True,
)
tag_sha = subprocess.check_output(
["git", "-C", tempdir, "rev-parse", "refs/tags/v1.0"], text=True
).strip()
subprocess.run(
[
"git",
"-C",
tempdir,
"update-ref",
"refs/remotes/origin/tags/v1.0",
tag_sha,
],
check=True,
)
proj = _create_mock_project(tempdir, use_local_gitdirs=True)
proj.revisionId = None
mock_remote = mock.MagicMock()
proj.GetRemote = mock.MagicMock(return_value=mock_remote)
proj.bare_git.ResolveCommit.return_value = commit_sha
# Warm bare_ref via public API.
self.assertEqual(
proj.bare_ref.all["refs/remotes/origin/main"], commit_sha
)
proj.revisionExpr = "main"
mock_remote.ToLocal.return_value = "refs/remotes/origin/main"
self.assertEqual(proj.GetRevisionId(), commit_sha)
proj.bare_git.ResolveCommit.assert_not_called()
proj.revisionExpr = "refs/tags/v1.0"
mock_remote.ToLocal.return_value = "refs/tags/v1.0"
self.assertEqual(proj.GetRevisionId(), commit_sha)
proj.bare_git.ResolveCommit.assert_called_once_with(
"refs/tags/v1.0"
)
# Custom refspec mapping refs/tags/* elsewhere still peels tags.
proj.bare_git.ResolveCommit.reset_mock()
mock_remote.ToLocal.return_value = "refs/remotes/origin/tags/v1.0"
self.assertEqual(proj.GetRevisionId(), commit_sha)
proj.bare_git.ResolveCommit.assert_called_once_with(
"refs/remotes/origin/tags/v1.0"
)
class ReprojectCmdGitTests(unittest.TestCase):
"""Tests running the reprojectcmd contract against a real Git checkout."""
@@ -4609,6 +4726,57 @@ class ReprojectCmdGitTests(unittest.TestCase):
self.assertTrue(clean, errors)
self.assertFalse(os.path.exists(marker))
def test_sync_local_half_rejects_staged_changes_on_unborn_head(
self,
) -> None:
"""Test real staged file on unborn HEAD is rejected before reproject."""
with tempfile.TemporaryDirectory() as topdir:
proj, _ = self._make_client(topdir, self.READ_TREE)
staged_path = os.path.join(proj.worktree, "staged.txt")
with open(staged_path, "w", encoding="utf-8") as fp:
fp.write("staged\n")
self._git(proj.worktree, "add", "staged.txt")
clean, errors = self._sync(proj)
self.assertFalse(clean)
self.assertTrue(
any("staged.txt" in str(err) for err in errors), errors
)
def test_reproject_unborn_empty_skips_redundant_git_checks(self) -> None:
"""Clean unborn files-backend checkout runs only post-check diff."""
with tempfile.TemporaryDirectory() as topdir, mock.patch.dict(
os.environ, {"GIT_DEFAULT_REF_FORMAT": "files"}
):
proj, revid = self._make_client(topdir, self.READ_TREE)
seen_cmds: List[List[str]] = []
class SpyGitCommand(git_command.GitCommand):
def __init__(
self,
proj_arg: Any,
cmdv: List[str],
*args: Any,
**kwargs: Any,
) -> None:
seen_cmds.append(list(cmdv))
super().__init__(proj_arg, cmdv, *args, **kwargs)
with mock.patch.object(git_command, "GitCommand", SpyGitCommand):
proj._Reproject(revid)
self.assertEqual(
[
[
"diff-index",
"--cached",
"--name-only",
f"{revid}^{{tree}}",
]
],
seen_cmds,
)
class DownloadPatchSetTests(unittest.TestCase):
"""Tests for Project.DownloadPatchSet."""