Merge pull request #1700 from EliahKagan/untangle-skips

Byron · web-flow · commit d77fadd15702 · 2023-10-10T09:56:43.000+02:00
Fix bugs affecting exception wrapping in rmtree callback
diff --git a/git/objects/submodule/base.py b/git/objects/submodule/base.py
@@ -1,44 +1,37 @@
-# need a dict to set bloody .name field
 from io import BytesIO
 import logging
 import os
+import os.path as osp
 import stat
 import uuid
 
 import git
 from git.cmd import Git
-from git.compat import (
- defenc,
- is_win,
-)
-from git.config import SectionConstraint, GitConfigParser, cp
+from git.compat import defenc, is_win
+from git.config import GitConfigParser, SectionConstraint, cp
 from git.exc import (
+ BadName,
 InvalidGitRepositoryError,
 NoSuchPathError,
 RepositoryDirtyError,
- BadName,
 )
 from git.objects.base import IndexObject, Object
 from git.objects.util import TraversableIterableObj
-
 from git.util import (
- join_path_native,
- to_native_path_linux,
+ IterableList,
 RemoteProgress,
+ join_path_native,
 rmtree,
+ to_native_path_linux,
 unbare_repo,
- IterableList,
 )
-from git.util import HIDE_WINDOWS_KNOWN_ERRORS
-
-import os.path as osp
 
 from .util import (
+ SubmoduleConfigParser,
+ find_first_remote_branch,
 mkhead,
 sm_name,
 sm_section,
- SubmoduleConfigParser,
- find_first_remote_branch,
 )
 
 
@@ -1060,28 +1053,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/git/util.py b/git/util.py
@@ -5,24 +5,24 @@
 # the BSD License: https://opensource.org/license/bsd-3-clause/
 
 from abc import abstractmethod
-import os.path as osp
-from .compat import is_win
 import contextlib
 from functools import wraps
 import getpass
 import logging
 import os
+import os.path as osp
+import pathlib
 import platform
-import subprocess
 import re
 import shutil
 import stat
-from sys import maxsize
+import subprocess
+import sys
 import time
 from urllib.parse import urlsplit, urlunsplit
 import warnings
 
-# from git.objects.util import Traversable
+from .compat import is_win
 
 # typing ---------------------------------------------------------
 
@@ -42,22 +42,17 @@
 Tuple,
 TypeVar,
 Union,
- cast,
 TYPE_CHECKING,
+ cast,
 overload,
 )
 
-import pathlib
-
 if TYPE_CHECKING:
 from git.remote import Remote
 from git.repo.base import Repo
 from git.config import GitConfigParser, SectionConstraint
 from git import Git
 
- # from git.objects.base import IndexObject
-
-
 from .types import (
 Literal,
 SupportsIndex,
@@ -75,7 +70,6 @@
 
 # ---------------------------------------------------------------------
 
-
 from gitdb.util import ( # NOQA @IgnorePep8
 make_sha,
 LockedFD, # @UnusedImport
@@ -88,7 +82,6 @@
 hex_to_bin, # @UnusedImport
 )
 
-
 # NOTE: Some of the unused imports might be used/imported by others.
 # Handle once test-cases are back up and running.
 # Most of these are unused here, but are for use by git-python modules so these
@@ -116,14 +109,33 @@
 
 log = logging.getLogger(__name__)
 
-# types############################################################
+
+def _read_env_flag(name: str, default: bool) -> bool:
+ try:
+ value = os.environ[name]
+ except KeyError:
+ return default
+
+ log.warning(
+ "The %s environment variable is deprecated. Its effect has never been documented and changes without warning.",
+ name,
+ )
+
+ adjusted_value = value.strip().lower()
+
+ if adjusted_value in{"", "0", "false", "no"}:
+ return False
+ if adjusted_value in{"1", "true", "yes"}:
+ return True
+ log.warning("%s has unrecognized value %r, treating as %r.", name, value, default)
+ return default
 
 
 #: We need an easy way to see if Appveyor TCs start failing,
 #: so the errors marked with this var are considered "acknowledged" ones, awaiting remedy,
 #: till then, we wish to hide them.
-HIDE_WINDOWS_KNOWN_ERRORS = is_win and os.environ.get("HIDE_WINDOWS_KNOWN_ERRORS", True)
-HIDE_WINDOWS_FREEZE_ERRORS = is_win and os.environ.get("HIDE_WINDOWS_FREEZE_ERRORS", True)
+HIDE_WINDOWS_KNOWN_ERRORS = is_win and _read_env_flag("HIDE_WINDOWS_KNOWN_ERRORS", True)
+HIDE_WINDOWS_FREEZE_ERRORS = is_win and _read_env_flag("HIDE_WINDOWS_FREEZE_ERRORS", True)
 
 #{Utility Methods
 
@@ -177,25 +189,29 @@ def patch_env(name: str, value: str) -> Generator[None, None, None]:
 
 
 def rmtree(path: PathLike) -> None:
- """Remove the given recursively.
+ """Remove the given directory tree recursively.
 
- :note: we use shutil rmtree but adjust its behaviour to see whether files that
- couldn't be deleted are read-only. Windows will not remove them in that case"""
+ :note: We use :func:`shutil.rmtree` but adjust its behaviour to see whether files that
+ couldn't be deleted are read-only. Windows will not remove them in that case."""
 
- def onerror(func: Callable, path: PathLike, exc_info: str) -> None:
- # Is the error an access error ?
+ def handler(function: Callable, path: PathLike, _excinfo: Any) -> None:
+ """Callback for :func:`shutil.rmtree`. Works either as ``onexc`` or ``onerror``."""
+ # Is the error an access error?
 os.chmod(path, stat.S_IWUSR)
 
 try:
- func(path) # Will scream if still not possible to delete.
- except Exception as ex:
+ function(path)
+ except PermissionError as ex:
 if HIDE_WINDOWS_KNOWN_ERRORS:
 from unittest import SkipTest
 
- raise SkipTest("FIXME: fails with: PermissionError\n{}".format(ex)) from ex
+ raise SkipTest(f"FIXME: fails with: PermissionError\n{ex}") from ex
 raise
 
- return shutil.rmtree(path, False, onerror)
+ if sys.version_info >= (3, 12):
+ shutil.rmtree(path, onexc=handler)
+ else:
+ shutil.rmtree(path, onerror=handler)
 
 
 def rmfile(path: PathLike) -> None:
@@ -995,7 +1011,7 @@ def __init__(
 self,
 file_path: PathLike,
 check_interval_s: float = 0.3,
- max_block_time_s: int = maxsize,
+ max_block_time_s: int = sys.maxsize,
 ) -> None:
 """Configure the instance
 
diff --git a/test-requirements.txt b/test-requirements.txt
@@ -7,4 +7,5 @@ pre-commit
 pytest
 pytest-cov
 pytest-instafail
+pytest-subtests
 pytest-sugar
diff --git a/test/test_docs.py b/test/test_docs.py
@@ -21,7 +21,10 @@ def tearDown(self):
 
 gc.collect()
 
- # @skipIf(HIDE_WINDOWS_KNOWN_ERRORS, ## ACTUALLY skipped by `git.submodule.base#L869`.
+ # 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 1062.
+ #
+ # @skipIf(HIDE_WINDOWS_KNOWN_ERRORS,
 # "FIXME: helper.wrapper fails with: PermissionError: [WinError 5] Access is denied: "
 # "'C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\test_work_tree_unsupportedryfa60di\\master_repo\\.git\\objects\\pack\\pack-bc9e0787aef9f69e1591ef38ea0a6f566ec66fe3.idx") # noqa E501
 @with_rw_directory
diff --git a/test/test_submodule.py b/test/test_submodule.py
@@ -457,7 +457,10 @@ def _do_base_tests(self, rwrepo):
 True,
 )
 
- # @skipIf(HIDE_WINDOWS_KNOWN_ERRORS, ## ACTUALLY skipped by `git.submodule.base#L869`.
+ # ACTUALLY skipped by git.util.rmtree (in local onerror function), called via
+ # git.objects.submodule.base.Submodule.remove at "method(mp)", line 1011.
+ #
+ # @skipIf(HIDE_WINDOWS_KNOWN_ERRORS,
 # "FIXME: fails with: PermissionError: [WinError 32] The process cannot access the file because"
 # "it is being used by another process: "
 # "'C:\\Users\\ankostis\\AppData\\Local\\Temp\\tmp95c3z83bnon_bare_test_base_rw\\git\\ext\\gitdb\\gitdb\\ext\\smmap'") # noqa E501
@@ -819,9 +822,11 @@ def test_git_submodules_and_add_sm_with_new_commit(self, rwdir):
 assert commit_sm.binsha == sm_too.binsha
 assert sm_too.binsha != sm.binsha
 
- # @skipIf(HIDE_WINDOWS_KNOWN_ERRORS, ## ACTUALLY skipped by `git.submodule.base#L869`.
- # "FIXME: helper.wrapper fails with: PermissionError: [WinError 5] Access is denied: "
- # "'C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\test_work_tree_unsupportedryfa60di\\master_repo\\.git\\objects\\pack\\pack-bc9e0787aef9f69e1591ef38ea0a6f566ec66fe3.idx") # noqa E501
+ @pytest.mark.xfail(
+ HIDE_WINDOWS_KNOWN_ERRORS,
+ reason='"The process cannot access the file because it is being used by another process" on call to sm.move',
+ raises=PermissionError,
+ )
 @with_rw_directory
 def test_git_submodule_compatibility(self, rwdir):
 parent = git.Repo.init(osp.join(rwdir, "parent"))
diff --git a/test/test_util.py b/test/test_util.py

-Original file line number
+Diff line change
@@ @@ -1,44 +1,37 @@ @@
 -# need a dict to set bloody .name field
 fromioimportBytesIO
 importlogging
 importos
 +importos.pathasosp
 importstat
 importuuid
 importgit
 fromgit.cmdimportGit
 -fromgit.compatimport (
 -defenc,
 -is_win,
 -)
 -fromgit.configimportSectionConstraint, GitConfigParser, cp
 +fromgit.compatimportdefenc, is_win
 +fromgit.configimportGitConfigParser, SectionConstraint, cp
 fromgit.excimport (
 +BadName,
 InvalidGitRepositoryError,
 NoSuchPathError,
 RepositoryDirtyError,
 -BadName,
+)
 fromgit.objects.baseimportIndexObject, Object
 fromgit.objects.utilimportTraversableIterableObj
+-
 fromgit.utilimport (
 -join_path_native,
 -to_native_path_linux,
 +IterableList,
 RemoteProgress,
 +join_path_native,
 rmtree,
 +to_native_path_linux,
 unbare_repo,
 -IterableList,
+)
 -fromgit.utilimportHIDE_WINDOWS_KNOWN_ERRORS
+-
 -importos.pathasosp
 from .utilimport (
 +SubmoduleConfigParser,
 +find_first_remote_branch,
 mkhead,
 sm_name,
 sm_section,
 -SubmoduleConfigParser,
 -find_first_remote_branch,
+)
 importgc
 gc.collect()
 -try:
 -rmtree(str(wtd))
 -exceptExceptionasex:
 -ifHIDE_WINDOWS_KNOWN_ERRORS:
 -fromunittestimportSkipTest
+-
 -raiseSkipTest("FIXME: fails with: PermissionError\n{}".format(ex)) fromex
 -raise
 +rmtree(str(wtd))
 # END delete tree if possible
 # END handle force
 ifnotdry_runandosp.isdir(git_dir):
 self._clear_cache()
 -try:
 -rmtree(git_dir)
 -exceptExceptionasex:
 -ifHIDE_WINDOWS_KNOWN_ERRORS:
 -fromunittestimportSkipTest
+-
 -raiseSkipTest(f"FIXME: fails with: PermissionError\n{ex}") fromex
 -else:
 -raise
 +rmtree(git_dir)
 # end handle separate bare repository
 # END handle module deletion
-Original file line number
+Diff line change
 # the BSD License: https://opensource.org/license/bsd-3-clause/
 fromabcimportabstractmethod
 -importos.pathasosp
 -from .compatimportis_win
 importcontextlib
 fromfunctoolsimportwraps
 importgetpass
 importlogging
 importos
 +importos.pathasosp
 +importpathlib
 importplatform
 -importsubprocess
 importre
 importshutil
 importstat
 -fromsysimportmaxsize
 +importsubprocess
 +importsys
 importtime
 fromurllib.parseimporturlsplit, urlunsplit
 importwarnings
 -# from git.objects.util import Traversable
 +from.compatimportis_win
 # typing ---------------------------------------------------------
 Tuple,
 TypeVar,
 Union,
 -cast,
 TYPE_CHECKING,
 +cast,
 overload,
+)
 -importpathlib
+-
 ifTYPE_CHECKING:
 fromgit.remoteimportRemote
 fromgit.repo.baseimportRepo
 fromgit.configimportGitConfigParser, SectionConstraint
 fromgitimportGit
 -# from git.objects.base import IndexObject
+-
+-
 from .typesimport (
 Literal,
 SupportsIndex,
 # ---------------------------------------------------------------------
+-
 fromgitdb.utilimport ( # NOQA @IgnorePep8
 make_sha,
 LockedFD, # @UnusedImport
 hex_to_bin, # @UnusedImport
+)
+-
 # NOTE: Some of the unused imports might be used/imported by others.
 # Handle once test-cases are back up and running.
 # Most of these are unused here, but are for use by git-python modules so these
 log=logging.getLogger(__name__)
 -# types############################################################
++
 +def_read_env_flag(name: str, default: bool) ->bool:
 +try:
 +value=os.environ[name]
 +exceptKeyError:
 +returndefault
++
 +log.warning(
 +"The %s environment variable is deprecated. Its effect has never been documented and changes without warning.",
 +name,
 + )
++
 +adjusted_value=value.strip().lower()
++
 +ifadjusted_valuein{"", "0", "false", "no"}:
 +returnFalse
 +ifadjusted_valuein{"1", "true", "yes"}:
 +returnTrue
 +log.warning("%s has unrecognized value %r, treating as %r.", name, value, default)
 +returndefault
 #: We need an easy way to see if Appveyor TCs start failing,
 #: so the errors marked with this var are considered "acknowledged" ones, awaiting remedy,
 #: till then, we wish to hide them.
 -HIDE_WINDOWS_KNOWN_ERRORS=is_winandos.environ.get("HIDE_WINDOWS_KNOWN_ERRORS", True)
 -HIDE_WINDOWS_FREEZE_ERRORS=is_winandos.environ.get("HIDE_WINDOWS_FREEZE_ERRORS", True)
 +HIDE_WINDOWS_KNOWN_ERRORS=is_winand_read_env_flag("HIDE_WINDOWS_KNOWN_ERRORS", True)
 +HIDE_WINDOWS_FREEZE_ERRORS=is_winand_read_env_flag("HIDE_WINDOWS_FREEZE_ERRORS", True)
 #{Utility Methods
 defrmtree(path: PathLike) ->None:
 -"""Remove the given recursively.
 +"""Remove the given directory tree recursively.
 - :note: we use shutilrmtree but adjust its behaviour to see whether files that
 - couldn't be deleted are read-only. Windows will not remove them in that case"""
 + :note: We use :func:`shutil.rmtree` but adjust its behaviour to see whether files that
 + couldn't be deleted are read-only. Windows will not remove them in that case."""
 -defonerror(func: Callable, path: PathLike, exc_info: str) ->None:
 -# Is the error an access error ?
 +defhandler(function: Callable, path: PathLike, _excinfo: Any) ->None:
 +"""Callback for :func:`shutil.rmtree`. Works either as ``onexc`` or ``onerror``."""
 +# Is the error an access error?
 os.chmod(path, stat.S_IWUSR)
 try:
 -func(path)# Will scream if still not possible to delete.
 -exceptExceptionasex:
 +function(path)
 +exceptPermissionErrorasex:
 ifHIDE_WINDOWS_KNOWN_ERRORS:
 fromunittestimportSkipTest
 -raiseSkipTest("FIXME: fails with: PermissionError\n{}".format(ex)) fromex
 +raiseSkipTest(f"FIXME: fails with: PermissionError\n{ex}") fromex
 raise
 -returnshutil.rmtree(path, False, onerror)
 +ifsys.version_info>= (3, 12):
 +shutil.rmtree(path, onexc=handler)
 +else:
 +shutil.rmtree(path, onerror=handler)
 defrmfile(path: PathLike) ->None:
 self,
 file_path: PathLike,
 check_interval_s: float=0.3,
 -max_block_time_s: int=maxsize,
 +max_block_time_s: int=sys.maxsize,
  ) ->None:
 """Configure the instance