project: handle corrupted projects in DeleteWorktree

Catch GitError from IsDirty() during DeleteWorktree so corrupted
projects can be cleanly wiped instead of crashing.

Bug: 515415221
Change-Id: Ic03ae77c30a722f9aa06f2220747251e0aea4ab7
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/591001
Commit-Queue: Gavin Mak <gavinmak@google.com>
Reviewed-by: Mike Frysinger <vapier@google.com>
Tested-by: Gavin Mak <gavinmak@google.com>

project.py

@@ -1999,28 +1999,51 @@ class Project:
 
         Args:
             verbose: Whether to show verbose messages.
-            force: Always delete tree even if dirty.
+            force: Always delete tree even if dirty or corrupted.
 
         Returns:
             True if the worktree was completely cleaned out.
         """
-        if self.IsDirty():
+        is_dirty = False
+        is_corrupted = False
+        try:
+            is_dirty = self.IsDirty()
+        except GitError:
+            is_corrupted = True
+
+        rel_path = self.RelPath(local=False)
+
+        if is_dirty or is_corrupted:
             if force:
-                logger.warning(
+                if is_corrupted:
-                    "warning: %s: Removing dirty project: uncommitted changes "
+                    logger.warning(
-                    "lost.",
+                        "warning: %s: Removing corrupted project.",
-                    self.RelPath(local=False),
+                        rel_path,
-                )
+                    )
+                else:
+                    logger.warning(
+                        "warning: %s: Removing dirty project: "
+                        "uncommitted changes lost.",
+                        rel_path,
+                    )
             else:
-                msg = (
+                if is_corrupted:
-                    "error: %s: Cannot remove project: uncommitted "
+                    msg = (
-                    "changes are present.\n" % self.RelPath(local=False)
+                        f"error: {rel_path}: Cannot remove project: "
-                )
+                        "project is corrupted.\n"
-                logger.error(msg)
+                    )
-                raise DeleteDirtyWorktreeError(msg, project=self.name)
+                    logger.error(msg)
+                    raise DeleteWorktreeError(msg, project=self.name)
+                else:
+                    msg = (
+                        f"error: {rel_path}: Cannot remove project: "
+                        "uncommitted changes are present.\n"
+                    )
+                    logger.error(msg)
+                    raise DeleteDirtyWorktreeError(msg, project=self.name)
 
         if verbose:
-            print(f"{self.RelPath(local=False)}: Deleting obsolete checkout.")
+            print(f"{rel_path}: Deleting obsolete checkout.")
 
         # Unlock and delink from the main worktree.  We don't use git's worktree
         # remove because it will recursively delete projects -- we handle that
@@ -2028,23 +2051,33 @@ class Project:
         if self.use_git_worktrees:
             needle = os.path.realpath(self.gitdir)
             # Find the git worktree commondir under .repo/worktrees/.
-            output = self.bare_git.worktree("list", "--porcelain").splitlines()[
+            try:
-                0
+                output = self.bare_git.worktree(
-            ]
+                    "list", "--porcelain"
-            assert output.startswith("worktree "), output
+                ).splitlines()[0]
-            commondir = output[9:]
+                assert output.startswith("worktree "), output
-            # Walk each of the git worktrees to see where they point.
+                commondir = output[9:]
-            configs = os.path.join(commondir, "worktrees")
+                # Walk each of the git worktrees to see where they point.
-            for name in os.listdir(configs):
+                configs = os.path.join(commondir, "worktrees")
-                gitdir = os.path.join(configs, name, "gitdir")
+                if os.path.exists(configs):
-                with open(gitdir) as fp:
+                    for name in os.listdir(configs):
-                    relpath = fp.read().strip()
+                        gitdir = os.path.join(configs, name, "gitdir")
-                # Resolve the checkout path and see if it matches this project.
+                        with open(gitdir) as fp:
-                fullpath = os.path.realpath(
+                            relpath = fp.read().strip()
-                    os.path.join(configs, name, relpath)
+                        # Resolve the checkout path and see if it
+                        # matches this project.
+                        fullpath = os.path.realpath(
+                            os.path.join(configs, name, relpath)
+                        )
+                        if fullpath == needle:
+                            platform_utils.rmtree(os.path.join(configs, name))
+            except GitError as e:
+                logger.warning(
+                    "warning: %s: Failed to list worktrees, skipping worktree "
+                    "cleanup: %s",
+                    rel_path,
+                    e,
                 )
-                if fullpath == needle:
-                    platform_utils.rmtree(os.path.join(configs, name))
 
         # Delete the .git directory first, so we're less likely to have a
         # partially working git repository around. There shouldn't be any git
@@ -2065,7 +2098,7 @@ class Project:
                 logger.error(
                     "error: %s: Failed to delete obsolete checkout; remove "
                     "manually, then run `repo sync -l`.",
-                    self.RelPath(local=False),
+                    rel_path,
                 )
                 raise DeleteWorktreeError(aggregate_errors=[e])
 
@@ -2129,7 +2162,7 @@ class Project:
                 logger.error(
                     "%s: Failed to delete obsolete checkout.\n",
                     "       Remove manually, then run `repo sync -l`.",
-                    self.RelPath(local=False),
+                    rel_path,
                 )
                 raise DeleteWorktreeError(aggregate_errors=errors)
 

tests/test_project.py

@@ -742,6 +742,30 @@ class ManifestPropertiesFetchedCorrectly(unittest.TestCase):
             result = fakeproj.Sync(use_local_gitdirs=True, mirror=True)
             self.assertFalse(result)
 
+    def test_delete_worktree_corrupted(self):
+        """Test DeleteWorktree gracefully handles corrupted projects."""
+        for use_git_worktrees in (False, True):
+            with self.subTest(use_git_worktrees=use_git_worktrees):
+                with utils_for_test.TempGitTree() as tempdir:
+                    proj = _create_mock_project(tempdir)
+                    os.makedirs(os.path.join(tempdir, "worktree"))
+                    os.makedirs(os.path.join(tempdir, "gitdir"))
+                    proj.worktree = os.path.join(tempdir, "worktree")
+                    proj.gitdir = os.path.join(tempdir, "gitdir")
+                    proj.use_git_worktrees = use_git_worktrees
+
+                    with mock.patch.object(
+                        proj,
+                        "IsDirty",
+                        side_effect=error.GitError("mock error"),
+                    ):
+                        with self.assertRaises(project.DeleteWorktreeError):
+                            proj.DeleteWorktree(force=False)
+
+                        self.assertTrue(proj.DeleteWorktree(force=True))
+                        self.assertFalse(os.path.exists(proj.worktree))
+                        self.assertFalse(os.path.exists(proj.gitdir))
+
 
 def _create_mock_project(
     tempdir,