From f7a24df00cdde9b1c2d51e5d0b03e8a36d655f76 Mon Sep 17 00:00:00 2001 From: Gavin Mak Date: Thu, 4 Jun 2026 21:54:33 +0000 Subject: [PATCH] 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 Reviewed-by: Mike Frysinger Tested-by: Gavin Mak --- project.py | 97 +++++++++++++++++++++++++++++-------------- tests/test_project.py | 24 +++++++++++ 2 files changed, 89 insertions(+), 32 deletions(-) diff --git a/project.py b/project.py index 9e8d8605a..58366e90c 100644 --- a/project.py +++ b/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( - "warning: %s: Removing dirty project: uncommitted changes " - "lost.", - self.RelPath(local=False), - ) + if is_corrupted: + logger.warning( + "warning: %s: Removing corrupted project.", + rel_path, + ) + else: + logger.warning( + "warning: %s: Removing dirty project: " + "uncommitted changes lost.", + rel_path, + ) else: - msg = ( - "error: %s: Cannot remove project: uncommitted " - "changes are present.\n" % self.RelPath(local=False) - ) - logger.error(msg) - raise DeleteDirtyWorktreeError(msg, project=self.name) + if is_corrupted: + msg = ( + f"error: {rel_path}: Cannot remove project: " + "project is corrupted.\n" + ) + 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()[ - 0 - ] - assert output.startswith("worktree "), output - commondir = output[9:] - # Walk each of the git worktrees to see where they point. - configs = os.path.join(commondir, "worktrees") - for name in os.listdir(configs): - gitdir = os.path.join(configs, name, "gitdir") - with open(gitdir) as fp: - relpath = fp.read().strip() - # Resolve the checkout path and see if it matches this project. - fullpath = os.path.realpath( - os.path.join(configs, name, relpath) + try: + output = self.bare_git.worktree( + "list", "--porcelain" + ).splitlines()[0] + assert output.startswith("worktree "), output + commondir = output[9:] + # Walk each of the git worktrees to see where they point. + configs = os.path.join(commondir, "worktrees") + if os.path.exists(configs): + for name in os.listdir(configs): + gitdir = os.path.join(configs, name, "gitdir") + with open(gitdir) as fp: + relpath = fp.read().strip() + # 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) diff --git a/tests/test_project.py b/tests/test_project.py index af8bea4f0..dd24afa04 100644 --- a/tests/test_project.py +++ b/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,