Skip to content

Commit 5039df3

Browse files
committed
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.
1 parent fba59aa commit 5039df3

File tree

3 files changed

+5
-21
lines changed

3 files changed

+5
-21
lines changed

Diff for: git/objects/submodule/base.py

+2-18
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
unbare_repo,
3030
IterableList,
3131
)
32-
from git.util import HIDE_WINDOWS_KNOWN_ERRORS
3332

3433
import os.path as osp
3534

@@ -1060,28 +1059,13 @@ def remove(
10601059
import gc
10611060

10621061
gc.collect()
1063-
try:
1064-
rmtree(str(wtd))
1065-
except Exception as ex:
1066-
if HIDE_WINDOWS_KNOWN_ERRORS:
1067-
from unittest import SkipTest
1068-
1069-
raise SkipTest("FIXME: fails with: PermissionError\n {}".format(ex)) from ex
1070-
raise
1062+
rmtree(str(wtd))
10711063
# END delete tree if possible
10721064
# END handle force
10731065

10741066
if not dry_run and osp.isdir(git_dir):
10751067
self._clear_cache()
1076-
try:
1077-
rmtree(git_dir)
1078-
except Exception as ex:
1079-
if HIDE_WINDOWS_KNOWN_ERRORS:
1080-
from unittest import SkipTest
1081-
1082-
raise SkipTest(f"FIXME: fails with: PermissionError\n {ex}") from ex
1083-
else:
1084-
raise
1068+
rmtree(git_dir)
10851069
# end handle separate bare repository
10861070
# END handle module deletion
10871071

Diff for: test/test_docs.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,8 @@ def tearDown(self):
2121

2222
gc.collect()
2323

24-
# ACTUALLY skipped by git.objects.submodule.base.Submodule.remove, at the last
25-
# rmtree call (in "handle separate bare repository"), line 1082.
24+
# ACTUALLY skipped by git.util.rmtree (in local onerror function), from the last call to it via
25+
# git.objects.submodule.base.Submodule.remove (at "handle separate bare repository"), line 1068.
2626
#
2727
# @skipIf(HIDE_WINDOWS_KNOWN_ERRORS,
2828
# "FIXME: helper.wrapper fails with: PermissionError: [WinError 5] Access is denied: "

Diff for: test/test_submodule.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -458,7 +458,7 @@ def _do_base_tests(self, rwrepo):
458458
)
459459

460460
# ACTUALLY skipped by git.util.rmtree (in local onerror function), called via
461-
# git.objects.submodule.base.Submodule.remove at "method(mp)", line 1018.
461+
# git.objects.submodule.base.Submodule.remove at "method(mp)", line 1017.
462462
#
463463
# @skipIf(HIDE_WINDOWS_KNOWN_ERRORS,
464464
# "FIXME: fails with: PermissionError: [WinError 32] The process cannot access the file because"

0 commit comments

Comments
 (0)