project: read HEAD directly in-memory to avoid subprocesses
Benchmark:
* repo upload frameworks/base: 2.84s -> 0.50s (-82.3%, 5.6x faster)
* repo upload (3,045 projects): 7.66s -> 5.24s (-31.6%, 2.42s saved)
From repo's inception through v2.56, GetHead() directly read the
`.git/HEAD` file in Python. In commit 52bab0ba ("project: Use git
rev-parse to read HEAD"), this was replaced with git subprocess calls
on the premise that git provides a dedicated command. However, in
large multi-project workspaces (such as Android with 3,000+ projects),
spawning thousands of git processes introduced severe latency
regressions during `repo upload` and `repo status`.
Furthermore, switching to subprocesses broke detached HEADs and
unborn branches (fixed in commits 7f7d70ef and 8c3585f3 by re-adding
the v2.56 file-reading logic as an error recovery fallback).
This patch restores fast in-memory reading as the primary path, while
adding modern defensive safeguards:
* Symbolic refs (`ref: refs/heads/...`): Strips whitespace and tabs
and returns the ref directly in memory.
* Detached HEAD: Validates 40-char SHA-1 and 64-char SHA-256 commit
hashes via git_config.IsId(), normalizing to lowercase.
* Symlinks: Detects filesystem symlinks via os.path.islink() and
safely falls back to git symbolic-ref.
* Fallback: Catches (OSError, AssertionError) and transparently falls
back to native git commands for reftables, unexpected layouts, or
filesystem errors. Unifies recovery fallback parsing with the fast
path (CRLF/tabs, lowercase hashes, and consistent RelPath errors).
In addition, this change substantially expands test coverage in
tests/test_project.py, adding comprehensive unit tests for symbolic
refs, whitespace/tabs, CRLF line endings, SHA-1, SHA-256, uppercase
hash normalization, symlinks, corrupted worktrees, and fallback
robustness.
Test: ./run_tests tests/test_project.py
Change-Id: Ib5c2530117c6939e4b9293feda81aa745c003c6a
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/623001
Commit-Queue: James Hawkins <jhawkins@google.com>
Reviewed-by: Brian Gan <brgan@google.com>
Tested-by: James Hawkins <jhawkins@google.com>
diff --git a/project.py b/project.py
index a73c7d4..45a3aa8 100644
--- a/project.py
+++ b/project.py
@@ -4464,8 +4464,52 @@
return dotgit if subpath is None else os.path.join(dotgit, subpath)
+ @staticmethod
+ def _ParseHead(line: str) -> Optional[str]:
+ """Parse the content of a .git/HEAD file.
+
+ Handles both symbolic refs (e.g. 'ref: refs/heads/...') and raw
+ commit IDs (40-hex SHA-1 or 64-hex SHA-256).
+
+ Returns:
+ The ref name (e.g. 'refs/heads/main') or lowercase commit hash
+ if valid, or None if empty or invalid.
+ """
+ line = line.strip()
+ if line.startswith("ref:"):
+ ref = line[4:].strip()
+ # Ensure the ref is not empty, pure whitespace, or the
+ # "refs/heads/.invalid" placeholder used for unborn branches,
+ # empty repositories, or when the reftables backend is used
+ # (which will be the default in Git 3.0).
+ if not ref or ref == R_HEADS + ".invalid":
+ return None
+ return ref
+ else:
+ # Normalize commit IDs to canonical lowercase hexadecimal,
+ # matching the output format of `git rev-parse`.
+ line_lower = line.lower()
+ if IsId(line_lower):
+ return line_lower
+ return None
+
def GetHead(self):
"""Return the ref that HEAD points to."""
+ path = None
+ try:
+ # Catch AssertionError raised by GetDotgitPath when worktree
+ # .git pointer file is malformed (e.g. missing 'gitdir:').
+ path = self.GetDotgitPath(subpath=HEAD)
+ if not platform_utils.islink(path):
+ with open(
+ path, "r", encoding="utf-8", errors="replace"
+ ) as fd:
+ ref = self._ParseHead(fd.readline())
+ if ref:
+ return ref
+ except (OSError, AssertionError):
+ pass
+
try:
return self.symbolic_ref("-q", HEAD, log_as_error=False)
except GitError:
@@ -4485,22 +4529,23 @@
# Fallback to direct file reading for compatibility with broken
# repos, e.g. if HEAD points to an unborn branch.
- path = self.GetDotgitPath(subpath=HEAD)
+ if not path:
+ raise NoManifestException(
+ self._project.RelPath(local=False), str(e)
+ )
try:
- with open(path) as fd:
- line = fd.readline()
+ with open(
+ path, "r", encoding="utf-8", errors="replace"
+ ) as fd:
+ ref = self._ParseHead(fd.readline())
except OSError:
- raise NoManifestException(path, str(e))
- try:
- line = line.decode()
- except AttributeError:
- pass
- if line.startswith("ref: "):
- ref = line[5:-1]
- else:
- ref = line[:-1]
- if ref == R_HEADS + ".invalid":
- raise NoManifestException(path, str(e))
+ raise NoManifestException(
+ self._project.RelPath(local=False), str(e)
+ )
+ if not ref:
+ raise NoManifestException(
+ self._project.RelPath(local=False), str(e)
+ )
return ref
def SetHead(self, ref, message=None):
diff --git a/tests/test_project.py b/tests/test_project.py
index e716b1a..cd184f8 100644
--- a/tests/test_project.py
+++ b/tests/test_project.py
@@ -409,6 +409,228 @@
).strip()
self.assertEqual(expected, fakeproj.work_git.GetHead())
+ def test_parse_head(self) -> None:
+ """Verify _ParseHead parses refs, hashes, whitespace, and invalid
+ refs.
+ """
+ with tempfile.TemporaryDirectory(prefix="repo-tests") as tempdir:
+ fakeproj = FakeProject(tempdir)
+ work_git = fakeproj.work_git
+
+ # Standard symbolic ref
+ self.assertEqual(
+ work_git._ParseHead("ref: refs/heads/main\n"),
+ "refs/heads/main",
+ )
+
+ # Tabs and extra whitespace
+ self.assertEqual(
+ work_git._ParseHead("ref:\t refs/heads/branch \r\n"),
+ "refs/heads/branch",
+ )
+
+ # Reftables placeholder should return None
+ self.assertIsNone(work_git._ParseHead("ref: refs/heads/.invalid\n"))
+
+ # Empty or whitespace-only symbolic refs should return None
+ self.assertIsNone(work_git._ParseHead("ref:\n"))
+ self.assertIsNone(work_git._ParseHead("ref: \t \r\n"))
+
+ # 40-character SHA-1
+ sha1 = "0123456789abcdef0123456789abcdef01234567"
+ self.assertEqual(work_git._ParseHead(f"{sha1}\n"), sha1)
+
+ # Uppercase SHA-1 normalized to lowercase
+ sha_upper = "4B825DC642CB6EB9A060E54BF8D69288FBEE4904"
+ self.assertEqual(
+ work_git._ParseHead(f"{sha_upper}\r\n"), sha_upper.lower()
+ )
+
+ # 64-character SHA-256
+ sha256 = "0123456789abcdef" * 4
+ self.assertEqual(work_git._ParseHead(f"{sha256}\n"), sha256)
+
+ # 40-character string with invalid hex characters (e.g. 'g'-'z')
+ invalid_sha = "0123456789abcdef0123456789abcdef0123456z"
+ self.assertIsNone(work_git._ParseHead(f"{invalid_sha}\n"))
+
+ # Empty or unparseable lines
+ self.assertIsNone(work_git._ParseHead(""))
+ self.assertIsNone(work_git._ParseHead(" \n"))
+ self.assertIsNone(work_git._ParseHead("corrupted-not-a-hash"))
+
+ def test_get_head_in_memory_fast_path(self) -> None:
+ """Verify GetHead reads HEAD in-memory without spawning subprocesses."""
+ with tempfile.TemporaryDirectory(prefix="repo-tests") as tempdir:
+ fakeproj = FakeProject(tempdir)
+ os.makedirs(fakeproj.gitdir, exist_ok=True)
+ head_file = os.path.join(fakeproj.gitdir, "HEAD")
+
+ # 1. Standard symbolic ref (on a branch)
+ with open(head_file, "w", encoding="utf-8", newline="") as fp:
+ fp.write("ref: refs/heads/feature-branch\n")
+
+ with mock.patch.object(
+ fakeproj.work_git, "symbolic_ref"
+ ) as mock_sym, mock.patch.object(
+ fakeproj.work_git, "rev_parse"
+ ) as mock_parse:
+ self.assertEqual(
+ fakeproj.work_git.GetHead(), "refs/heads/feature-branch"
+ )
+ mock_sym.assert_not_called()
+ mock_parse.assert_not_called()
+
+ # 2. Whitespace, tabs, and CRLF handling
+ with open(head_file, "w", encoding="utf-8", newline="") as fp:
+ fp.write("ref:\t refs/heads/feature-branch \r\n")
+
+ with mock.patch.object(
+ fakeproj.work_git, "symbolic_ref"
+ ) as mock_sym, mock.patch.object(
+ fakeproj.work_git, "rev_parse"
+ ) as mock_parse:
+ self.assertEqual(
+ fakeproj.work_git.GetHead(), "refs/heads/feature-branch"
+ )
+ mock_sym.assert_not_called()
+ mock_parse.assert_not_called()
+
+ # 3. Detached HEAD with 40-character SHA-1
+ fake_sha1 = "0123456789abcdef0123456789abcdef01234567"
+ with open(head_file, "w", encoding="utf-8", newline="") as fp:
+ fp.write(f"{fake_sha1}\n")
+
+ with mock.patch.object(
+ fakeproj.work_git, "symbolic_ref"
+ ) as mock_sym, mock.patch.object(
+ fakeproj.work_git, "rev_parse"
+ ) as mock_parse:
+ self.assertEqual(fakeproj.work_git.GetHead(), fake_sha1)
+ mock_sym.assert_not_called()
+ mock_parse.assert_not_called()
+
+ # 4. Detached HEAD with 64-character SHA-256
+ fake_sha256 = "0123456789abcdef" * 4
+ with open(head_file, "w", encoding="utf-8", newline="") as fp:
+ fp.write(f"{fake_sha256}\n")
+
+ with mock.patch.object(
+ fakeproj.work_git, "symbolic_ref"
+ ) as mock_sym, mock.patch.object(
+ fakeproj.work_git, "rev_parse"
+ ) as mock_parse:
+ self.assertEqual(fakeproj.work_git.GetHead(), fake_sha256)
+ mock_sym.assert_not_called()
+ mock_parse.assert_not_called()
+
+ # 5. Uppercase SHA normalized to lowercase
+ fake_upper = fake_sha1.upper()
+ with open(head_file, "w", encoding="utf-8", newline="") as fp:
+ fp.write(f"{fake_upper}\n")
+
+ with mock.patch.object(
+ fakeproj.work_git, "symbolic_ref"
+ ) as mock_sym, mock.patch.object(
+ fakeproj.work_git, "rev_parse"
+ ) as mock_parse:
+ self.assertEqual(fakeproj.work_git.GetHead(), fake_sha1)
+ mock_sym.assert_not_called()
+ mock_parse.assert_not_called()
+
+ def test_get_head_symlink_and_fallback(self) -> None:
+ """Verify GetHead handles symlinks and invalid files via fallback."""
+ with tempfile.TemporaryDirectory(prefix="repo-tests") as tempdir:
+ fakeproj = FakeProject(tempdir)
+ os.makedirs(fakeproj.gitdir, exist_ok=True)
+ head_file = os.path.join(fakeproj.gitdir, "HEAD")
+
+ # Symlink HEAD should fall back to symbolic_ref
+ platform_utils.symlink("refs/heads/main", head_file)
+ with mock.patch.object(
+ fakeproj.work_git,
+ "symbolic_ref",
+ return_value="refs/heads/main",
+ ) as mock_sym:
+ self.assertEqual(fakeproj.work_git.GetHead(), "refs/heads/main")
+ mock_sym.assert_called_once()
+
+ def test_get_head_worktree_corrupted_fallback(self) -> None:
+ """Verify GetHead raises NoManifestException on corrupted worktrees."""
+ with tempfile.TemporaryDirectory(prefix="repo-tests") as tempdir:
+ dotgit = os.path.join(tempdir, ".git")
+ with open(dotgit, "w", encoding="utf-8", newline="") as fp:
+ fp.write("malformed without gitdir prefix\n")
+ fakeproj = FakeProject(tempdir)
+ with self.assertRaises(error.NoManifestException) as cm:
+ fakeproj.work_git.GetHead()
+ self.assertEqual(cm.exception.path, fakeproj.RelPath(local=False))
+
+ def test_get_head_fallback_robustness(self) -> None:
+ """Verify GetHead fallback handles CRLF, tabs, and lowercase hashes."""
+ with tempfile.TemporaryDirectory(prefix="repo-tests") as tempdir:
+ fakeproj = FakeProject(tempdir)
+ os.makedirs(fakeproj.gitdir, exist_ok=True)
+ head_file = os.path.join(fakeproj.gitdir, "HEAD")
+
+ # 1. Fallback strips tabs, extra whitespace, and CRLF
+ with open(head_file, "w", encoding="utf-8", newline="") as fp:
+ fp.write("ref:\t refs/heads/fallback-branch\r\n")
+
+ with mock.patch("platform_utils.islink", return_value=True):
+ with mock.patch.object(
+ fakeproj.work_git,
+ "symbolic_ref",
+ side_effect=error.GitError("sym error"),
+ ), mock.patch.object(
+ fakeproj.work_git,
+ "rev_parse",
+ side_effect=error.GitError("parse error"),
+ ):
+ self.assertEqual(
+ fakeproj.work_git.GetHead(),
+ "refs/heads/fallback-branch",
+ )
+
+ # 2. Fallback normalizes uppercase hashes to lowercase
+ sha_upper = "4B825DC642CB6EB9A060E54BF8D69288FBEE4904"
+ with open(head_file, "w", encoding="utf-8", newline="") as fp:
+ fp.write(f"{sha_upper}\r\n")
+
+ with mock.patch("platform_utils.islink", return_value=True):
+ with mock.patch.object(
+ fakeproj.work_git,
+ "symbolic_ref",
+ side_effect=error.GitError("sym error"),
+ ), mock.patch.object(
+ fakeproj.work_git,
+ "rev_parse",
+ side_effect=error.GitError("parse error"),
+ ):
+ self.assertEqual(
+ fakeproj.work_git.GetHead(), sha_upper.lower()
+ )
+
+ # 3. Fallback raises NoManifestException with RelPath on .invalid
+ with open(head_file, "w", encoding="utf-8", newline="") as fp:
+ fp.write("ref: refs/heads/.invalid\r\n")
+
+ with mock.patch("platform_utils.islink", return_value=True):
+ with mock.patch.object(
+ fakeproj.work_git,
+ "symbolic_ref",
+ side_effect=error.GitError("sym error"),
+ ), mock.patch.object(
+ fakeproj.work_git,
+ "rev_parse",
+ side_effect=error.GitError("parse error"),
+ ):
+ with self.assertRaises(error.NoManifestException) as cm:
+ fakeproj.work_git.GetHead()
+ self.assertEqual(
+ cm.exception.path, fakeproj.RelPath(local=False)
+ )
+
def _get_derived_subproject_url(self, submodule_url):
with tempfile.TemporaryDirectory(prefix="repo-tests") as tempdir:
