Repository navigation
Commit 5039df3
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
3 files changed
Lines changed: 5 additions & 21 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -29,7 +29,6 @@ | |||
| 29 | 29 | unbare_repo, | |
| 30 | 30 | IterableList, | |
| 31 | 31 | ) | |
| 32 | - from git.util import HIDE_WINDOWS_KNOWN_ERRORS | ||
| 33 | 32 | ||
| 34 | 33 | import os.path as osp | |
| 35 | 34 | ||
@@ -1060,28 +1059,13 @@ def remove( | |||
| 1060 | 1059 | import gc | |
| 1061 | 1060 | ||
| 1062 | 1061 | 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)) | ||
| 1071 | 1063 | # END delete tree if possible | |
| 1072 | 1064 | # END handle force | |
| 1073 | 1065 | ||
| 1074 | 1066 | if not dry_run and osp.isdir(git_dir): | |
| 1075 | 1067 | 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) | ||
| 1085 | 1069 | # end handle separate bare repository | |
| 1086 | 1070 | # END handle module deletion | |
| 1087 | 1071 | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -21,8 +21,8 @@ def tearDown(self): | |||
| 21 | 21 | ||
| 22 | 22 | gc.collect() | |
| 23 | 23 | ||
| 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. | ||
| 26 | 26 | # | |
| 27 | 27 | # @skipIf(HIDE_WINDOWS_KNOWN_ERRORS, | |
| 28 | 28 | # "FIXME: helper.wrapper fails with: PermissionError: [WinError 5] Access is denied: " | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -458,7 +458,7 @@ def _do_base_tests(self, rwrepo): | |||
| 458 | 458 | ) | |
| 459 | 459 | ||
| 460 | 460 | # 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. | ||
| 462 | 462 | # | |
| 463 | 463 | # @skipIf(HIDE_WINDOWS_KNOWN_ERRORS, | |
| 464 | 464 | # "FIXME: fails with: PermissionError: [WinError 32] The process cannot access the file because" | |
0 commit comments