From 5039df3560d321af1746bbecbeb1b2838daf7f91 Mon Sep 17 00:00:00 2001
From: Eliah Kagan <degeneracypressure@gmail.com>
Date: Sun, 8 Oct 2023 06:31:27 -0400
Subject: [PATCH] Eliminate duplicate rmtree try-except logic

In git.util.rmtree, exceptions are caught and conditionally
(depending on the value of HIDE_WINDOWS_KNOWN_ERRORS) reraised
wrapped in a unittest.SkipTest exception. Although this logic is
part of git.util.rmtree itself, two of the calls to that rmtree
function contain this same logic.

This is not quite a refactoring: because SkipTest derives from
Exception, and Exception rather than PermissionError is being
caught including in the duplicated logic, duplicated logic where
git.util.rmtree was called added another layer of indirection in
the chained exceptions leading back to the original that was raised
in an unsuccessful attempt to delete a file or directory in rmtree.
However, that appeared unintended and may be considered a minor
bug. The new behavior, differing only subtly, is preferable.
---
 git/objects/submodule/base.py | 20 ++------------------
 test/test_docs.py             |  4 ++--
 test/test_submodule.py        |  2 +-
 3 files changed, 5 insertions(+), 21 deletions(-)

diff --git a/git/objects/submodule/base.py b/git/objects/submodule/base.py
index c7e7856f0..6fe946084 100644
--- a/git/objects/submodule/base.py
+++ b/git/objects/submodule/base.py
@@ -29,7 +29,6 @@
     unbare_repo,
     IterableList,
 )
-from git.util import HIDE_WINDOWS_KNOWN_ERRORS
 
 import os.path as osp
 
@@ -1060,28 +1059,13 @@ def remove(
                     import gc
 
                     gc.collect()
-                    try:
-                        rmtree(str(wtd))
-                    except Exception as ex:
-                        if HIDE_WINDOWS_KNOWN_ERRORS:
-                            from unittest import SkipTest
-
-                            raise SkipTest("FIXME: fails with: PermissionError\n  {}".format(ex)) from ex
-                        raise
+                    rmtree(str(wtd))
                 # END delete tree if possible
             # END handle force
 
             if not dry_run and osp.isdir(git_dir):
                 self._clear_cache()
-                try:
-                    rmtree(git_dir)
-                except Exception as ex:
-                    if HIDE_WINDOWS_KNOWN_ERRORS:
-                        from unittest import SkipTest
-
-                        raise SkipTest(f"FIXME: fails with: PermissionError\n  {ex}") from ex
-                    else:
-                        raise
+                rmtree(git_dir)
             # end handle separate bare repository
         # END handle module deletion
 
diff --git a/test/test_docs.py b/test/test_docs.py
index b38ac31b0..f17538aeb 100644
--- a/test/test_docs.py
+++ b/test/test_docs.py
@@ -21,8 +21,8 @@ def tearDown(self):
 
         gc.collect()
 
-    # ACTUALLY skipped by git.objects.submodule.base.Submodule.remove, at the last
-    # rmtree call (in "handle separate bare repository"), line 1082.
+    # ACTUALLY skipped by git.util.rmtree (in local onerror function), from the last call to it via
+    # git.objects.submodule.base.Submodule.remove (at "handle separate bare repository"), line 1068.
     #
     # @skipIf(HIDE_WINDOWS_KNOWN_ERRORS,
     #         "FIXME: helper.wrapper fails with: PermissionError: [WinError 5] Access is denied: "
diff --git a/test/test_submodule.py b/test/test_submodule.py
index 1c105c816..318a5afde 100644
--- a/test/test_submodule.py
+++ b/test/test_submodule.py
@@ -458,7 +458,7 @@ def _do_base_tests(self, rwrepo):
         )
 
     # ACTUALLY skipped by git.util.rmtree (in local onerror function), called via
-    # git.objects.submodule.base.Submodule.remove at "method(mp)", line 1018.
+    # git.objects.submodule.base.Submodule.remove at "method(mp)", line 1017.
     #
     # @skipIf(HIDE_WINDOWS_KNOWN_ERRORS,
     #         "FIXME: fails with: PermissionError: [WinError 32] The process cannot access the file because"