Prevent leftover bare gitdirs after failed sync attempts

The gitdir for a project may be left in a state with bare=true due
to a previous failed sync. In this state, during a subsequent sync
attempt, repo will skip initializing the gitdir (since the directory
already exists) and directly attempt to checkout the worktree, which
will fail because the project is bare. To reduce the chance of this
happening, initialize the gitdir in a temp directory and move it once
it is ready.

Bug: 457478027
Change-Id: I4767494a3a54e7734174eae3a0d939fa9d174288
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/524203
Tested-by: Kaushik Lingarkar <[email protected]>
Commit-Queue: Kaushik Lingarkar <[email protected]>
Reviewed-by: Mike Frysinger <[email protected]>
Reviewed-by: Gavin Mak <[email protected]>

Author: quic-kaushikl

project.py

@@ -12,6 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import datetime
 import errno
 import filecmp
 import glob
@@ -3073,8 +3074,13 @@ def _FastForward(self, head, ffonly=False, quiet=True):
             raise GitError(f"{self.name} merge {head} ", project=self.name)
 
     def _InitGitDir(self, mirror_git=None, force_sync=False, quiet=False):
+        # Prefix for temporary directories created during gitdir initialization.
+        TMP_GITDIR_PREFIX = ".tmp-project-initgitdir-"
         init_git_dir = not os.path.exists(self.gitdir)
         init_obj_dir = not os.path.exists(self.objdir)
+        tmp_gitdir = None
+        curr_gitdir = self.gitdir
+        curr_config = self.config
         try:
             # Initialize the bare repository, which contains all of the objects.
             if init_obj_dir:
@@ -3094,27 +3100,33 @@ def _InitGitDir(self, mirror_git=None, force_sync=False, quiet=False):
             # well.
             if self.objdir != self.gitdir:
                 if init_git_dir:
-                    os.makedirs(self.gitdir)
+                    os.makedirs(os.path.dirname(self.gitdir), exist_ok=True)
+                    tmp_gitdir = tempfile.mkdtemp(
+                        prefix=TMP_GITDIR_PREFIX,
+                        dir=os.path.dirname(self.gitdir),
+                    )
+                    curr_config = GitConfig.ForRepository(
+                        gitdir=tmp_gitdir, defaults=self.manifest.globalConfig
+                    )
+                    curr_gitdir = tmp_gitdir
 
                 if init_obj_dir or init_git_dir:
                     self._ReferenceGitDir(
-                        self.objdir, self.gitdir, copy_all=True
+                        self.objdir, curr_gitdir, copy_all=True
                     )
                 try:
-                    self._CheckDirReference(self.objdir, self.gitdir)
+                    self._CheckDirReference(self.objdir, curr_gitdir)
                 except GitError as e:
                     if force_sync:
-                        logger.error(
-                            "Retrying clone after deleting %s", self.gitdir
-                        )
                         try:
-                            platform_utils.rmtree(os.path.realpath(self.gitdir))
-                            if self.worktree and os.path.exists(
-                                os.path.realpath(self.worktree)
-                            ):
-                                platform_utils.rmtree(
-                                    os.path.realpath(self.worktree)
-                                )
+                            rm_dirs = (
+                                tmp_gitdir,
+                                self.gitdir,
+                                self.worktree,
+                            )
+                            for d in rm_dirs:
+                                if d and os.path.exists(d):
+                                    platform_utils.rmtree(os.path.realpath(d))
                             return self._InitGitDir(
                                 mirror_git=mirror_git,
                                 force_sync=False,
@@ -3165,18 +3177,21 @@ def _expanded_ref_dirs():
                 m = self.manifest.manifestProject.config
                 for key in ["user.name", "user.email"]:
                     if m.Has(key, include_defaults=False):
-                        self.config.SetString(key, m.GetString(key))
+                        curr_config.SetString(key, m.GetString(key))
                 if not self.manifest.EnableGitLfs:
-                    self.config.SetString(
+                    curr_config.SetString(
                         "filter.lfs.smudge", "git-lfs smudge --skip -- %f"
                     )
-                    self.config.SetString(
+                    curr_config.SetString(
                         "filter.lfs.process", "git-lfs filter-process --skip"
                     )
-                self.config.SetBoolean(
+                curr_config.SetBoolean(
                     "core.bare", True if self.manifest.IsMirror else None
                 )
 
+                if tmp_gitdir:
+                    platform_utils.rename(tmp_gitdir, self.gitdir)
+                    tmp_gitdir = None
             if not init_obj_dir:
                 # The project might be shared (obj_dir already initialized), but
                 # such information is not available here. Instead of passing it,
@@ -3193,6 +3208,27 @@ def _expanded_ref_dirs():
             if init_git_dir and os.path.exists(self.gitdir):
                 platform_utils.rmtree(self.gitdir)
             raise
+        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 = datetime.timedelta(days=1)
+            now = datetime.datetime.now()
+            for tmp_dir in glob.glob(
+                os.path.join(
+                    os.path.dirname(self.gitdir), f"{TMP_GITDIR_PREFIX}*"
+                )
+            ):
+                try:
+                    mtime = datetime.datetime.fromtimestamp(
+                        os.path.getmtime(tmp_dir)
+                    )
+                    if now - mtime > age_threshold:
+                        platform_utils.rmtree(tmp_dir)
+                except OSError:
+                    pass
 
     def _UpdateHooks(self, quiet=False):
         if os.path.exists(self.objdir):