From 83b8ebdbbed5ec894a2f89eb5e4bcf5d0bb14300 Mon Sep 17 00:00:00 2001 From: Gavin Mak Date: Sat, 7 Feb 2026 00:53:31 +0000 Subject: [PATCH] git_superproject: avoid re-initing bare repo Running sync with reftable on a files-backed workspace fails to re-init the superproject dir with: ``` fatal: could not open '.../.repo/exp-superproject/-superproject.git/refs/heads' for writing: Is a directory ``` Bug: 476209856 Change-Id: Ie8473d66069aafefa5661bd3ea8e73b2b27c6a38 Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/550981 Commit-Queue: Gavin Mak Reviewed-by: Mike Frysinger Tested-by: Gavin Mak --- git_superproject.py | 70 +++++++++++++++++++++++++--------- tests/test_git_superproject.py | 56 +++++++++++++++++++++++++++ 2 files changed, 109 insertions(+), 17 deletions(-) diff --git a/git_superproject.py b/git_superproject.py index 1ada173b7..12ac3dec6 100644 --- a/git_superproject.py +++ b/git_superproject.py @@ -23,9 +23,11 @@ Examples: """ import functools +import glob import hashlib import os import sys +import tempfile import time from typing import NamedTuple import urllib.parse @@ -34,6 +36,7 @@ from git_command import git_require from git_command import GitCommand from git_config import RepoConfig from git_refs import GitRefs +import platform_utils _SUPERPROJECT_GIT_NAME = "superproject.git" @@ -215,30 +218,63 @@ class Superproject: """ if not os.path.exists(self._superproject_path): os.mkdir(self._superproject_path) - if not self._quiet and not os.path.exists(self._work_git): + + if os.path.exists(self._work_git): + return True + + if not self._quiet: print( "%s: Performing initial setup for superproject; this might " "take several minutes." % self._work_git ) - cmd = ["init", "--bare", self._work_git_name] - p = GitCommand( - None, - cmd, - cwd=self._superproject_path, - capture_stdout=True, - capture_stderr=True, + + tmp_gitdir_prefix = ".tmp-superproject-initgitdir-" + tmp_gitdir = tempfile.mkdtemp( + prefix=tmp_gitdir_prefix, + dir=self._superproject_path, ) - retval = p.Wait() - if retval: - self._LogWarning( - "git init call failed, command: git {}, " - "return code: {}, stderr: {}", + tmp_git_name = os.path.basename(tmp_gitdir) + + try: + cmd = ["init", "--bare", tmp_git_name] + p = GitCommand( + None, cmd, - retval, - p.stderr, + cwd=self._superproject_path, + capture_stdout=True, + capture_stderr=True, ) - return False - return True + retval = p.Wait() + if retval: + self._LogWarning( + "git init call failed, command: git {}, " + "return code: {}, stderr: {}", + cmd, + retval, + p.stderr, + ) + return False + + platform_utils.rename(tmp_gitdir, self._work_git) + tmp_gitdir = None + return True + finally: + # Clean up the temporary directory created during the process, + # as well as any stale ones left over from previous attempts. + if tmp_gitdir and os.path.exists(tmp_gitdir): + platform_utils.rmtree(tmp_gitdir) + + age_threshold = 60 * 60 * 24 # 1 day in seconds + now = time.time() + for tmp_dir in glob.glob( + os.path.join(self._superproject_path, f"{tmp_gitdir_prefix}*") + ): + try: + mtime = os.path.getmtime(tmp_dir) + if now - mtime > age_threshold: + platform_utils.rmtree(tmp_dir) + except OSError: + pass def _Fetch(self): """Fetches a superproject for the manifest based on |_remote_url|. diff --git a/tests/test_git_superproject.py b/tests/test_git_superproject.py index 3ceb9320f..dbfb8385c 100644 --- a/tests/test_git_superproject.py +++ b/tests/test_git_superproject.py @@ -461,6 +461,62 @@ class SuperprojectTestCase(unittest.TestCase): "", ) + def test_Init_success(self): + """Test _Init succeeds and creates the work git dir.""" + self.assertFalse(os.path.exists(self._superproject._work_git)) + with mock.patch( + "git_superproject.GitCommand", autospec=True + ) as mock_git_command: + instance = mock_git_command.return_value + instance.Wait.return_value = 0 + + self.assertTrue(self._superproject._Init()) + + mock_git_command.assert_called_once() + args, kwargs = mock_git_command.call_args + self.assertEqual(args[1][:2], ["init", "--bare"]) + + tmp_git_name = args[1][2] + self.assertTrue( + tmp_git_name.startswith(".tmp-superproject-initgitdir-") + ) + self.assertTrue(os.path.exists(self._superproject._work_git)) + + tmp_git_path = os.path.join( + self._superproject._superproject_path, tmp_git_name + ) + self.assertFalse(os.path.exists(tmp_git_path)) + + def test_Init_already_exists(self): + """Test _Init returns early if the work git dir already exists.""" + os.mkdir(self._superproject._superproject_path) + os.mkdir(self._superproject._work_git) + with mock.patch( + "git_superproject.GitCommand", autospec=True + ) as mock_git_command: + self.assertTrue(self._superproject._Init()) + mock_git_command.assert_not_called() + + def test_Init_failure_cleans_up(self): + """Test _Init cleans up the temporary directory if 'git init' fails.""" + with mock.patch( + "git_superproject.GitCommand", autospec=True + ) as mock_git_command: + instance = mock_git_command.return_value + instance.Wait.return_value = 1 + instance.stderr = "mock git init failure" + + self.assertFalse(self._superproject._Init()) + mock_git_command.assert_called_once() + args, kwargs = mock_git_command.call_args + tmp_git_name = args[1][2] + + tmp_git_path = os.path.join( + self._superproject._superproject_path, tmp_git_name + ) + self.assertFalse(os.path.exists(tmp_git_path)) + self.assertFalse(os.path.exists(self._superproject._work_git)) + def test_Fetch(self): manifest = self.getXmlManifest( """