Skip to content

Commit

Permalink
Eliminate duplicate rmtree try-except logic
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
EliahKagan committed Oct 8, 2023
1 parent fba59aa commit 5039df3
Show file tree
Hide file tree
Showing 3 changed files with 5 additions and 21 deletions.
20 changes: 2 additions & 18 deletions git/objects/submodule/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
unbare_repo,
IterableList,
)
from git.util import HIDE_WINDOWS_KNOWN_ERRORS

import os.path as osp

Expand Down Expand Up @@ -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

Expand Down
4 changes: 2 additions & 2 deletions test/test_docs.py
Original file line number Diff line number Diff line change
Expand Up @@ -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: "
Expand Down
2 changes: 1 addition & 1 deletion test/test_submodule.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down

0 comments on commit 5039df3

Please sign in to comment.