From 07d21e6bde9bd7efdfb8f25f2ed23f023daa5c1f Mon Sep 17 00:00:00 2001
From: Mike Frysinger <vapier@google.com>
Date: Thu, 18 Nov 2021 15:53:17 -0500
Subject: [PATCH] project: initialize new manifests in temp dirs

If initializing the manifest fails for any reason, don't leave it in
a half complete state.  This can cause problems if/when the user tries
to reinit because different codepaths will be taken.  For example, if
we initialize manifests.git and don't finish probing the remote to see
what default branch it uses, we end up always using "master" even if
that isn't what the remote uses.

To avoid all of this, use .tmp dirs when initializing, and rename to
the final path only after we complete all the right steps.

We should roll this out to all projects we clone, but start with the
manifest project for now.

Bug: https://crbug.com/gerrit/13526
Bug: https://crbug.com/gerrit/15805
Change-Id: I0214338de69ee11e090285c6b0b211052804af06
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/343539
Reviewed-by: LaMont Jones <lamontjones@google.com>
Tested-by: Mike Frysinger <vapier@google.com>
---
 project.py | 42 +++++++++++++++++++++++++++++++++++++-----
 1 file changed, 37 insertions(+), 5 deletions(-)

diff --git a/project.py b/project.py
index 2b57a5fb..ab449f00 100644
--- a/project.py
+++ b/project.py
@@ -2794,6 +2794,35 @@ class Project(object):
         else:
           raise
 
+  def _InitialCheckoutStart(self):
+    """Called when checking out a project for the first time.
+
+    This will use temporary non-visible paths so we can be safely interrupted
+    without leaving incomplete state behind.
+    """
+    paths = [f'{x}.tmp' for x in (self.relpath, self.worktree, self.gitdir, self.objdir)]
+    for p in paths:
+      platform_utils.rmtree(p, ignore_errors=True)
+    self.UpdatePaths(*paths)
+
+  def _InitialCheckoutFinalizeNetworkHalf(self):
+    """Finalize the object dirs after network syncing works."""
+    # Once the network half finishes, we can move the objects into the right
+    # place by removing the ".tmp" suffix on the dirs.
+    platform_utils.rmtree(self.gitdir[:-4], ignore_errors=True)
+    os.rename(self.gitdir, self.gitdir[:-4])
+    self.UpdatePaths(self.relpath, self.worktree, self.gitdir[:-4], self.objdir[:-4])
+
+  def _InitialCheckoutFinalizeLocalHalf(self):
+    """Finalize the initial checkout and make it available."""
+    assert self.gitdir == self.objdir
+    # Once the local half finishes, we can move the manifest dir into the right
+    # place by removing the ".tmp" suffix on the dirs.
+    platform_utils.rmtree(self.worktree[:-4], ignore_errors=True)
+    os.rename(self.worktree, self.worktree[:-4])
+    self.UpdatePaths(
+        self.relpath[:-4], self.worktree[:-4], self.gitdir, self.objdir)
+
   def _InitGitWorktree(self):
     """Init the project using git worktrees."""
     self.bare_git.worktree('prune')
@@ -3680,6 +3709,8 @@ class ManifestProject(MetaProject):
               (GitConfig.ForUser().UrlInsteadOf(manifest_url),),
               file=sys.stderr)
 
+      self._InitialCheckoutStart()
+
       # The manifest project object doesn't keep track of the path on the
       # server where this git is located, so let's save that here.
       mirrored_manifest_git = None
@@ -3839,16 +3870,14 @@ class ManifestProject(MetaProject):
           partial_clone_exclude=self.manifest.PartialCloneExclude):
         r = self.GetRemote()
         print('fatal: cannot obtain manifest %s' % r.url, file=sys.stderr)
-
-        # Better delete the manifest git dir if we created it; otherwise next
-        # time (when user fixes problems) we won't go through the "is_new" logic.
-        if is_new:
-          platform_utils.rmtree(self.gitdir)
         return False
 
       if manifest_branch:
         self.MetaBranchSwitch(submodules=submodules)
 
+      if is_new:
+        self._InitialCheckoutFinalizeNetworkHalf()
+
       syncbuf = SyncBuffer(self.config)
       self.Sync_LocalHalf(syncbuf, submodules=submodules)
       syncbuf.Finish()
@@ -3871,6 +3900,9 @@ class ManifestProject(MetaProject):
       with open(dest, 'wb') as f:
         f.write(manifest_data)
 
+    if is_new:
+      self._InitialCheckoutFinalizeLocalHalf()
+
     try:
       self.manifest.Link(manifest_name)
     except ManifestParseError as e: