From c01fe76f2351d06671296bffcd98a3f50543d785 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sun, 29 Oct 2023 16:34:23 -0400 Subject: [PATCH 1/2] Deprecate compat.is_, rewriting all uses Major changes: - Add a comment in the git.compat module above the definitions of is_win, is_posix, and is_darwin stating that they are deprecated and recommending that os.name (or, where applicable, sys.platform) be used directly instead for clarity (and sometimes accuracy). - Remove all uses of is_win and is_posix in git/ and test/, replacing them with `os.name == "nt"` and `os.name == "posix"`, respectively. There were no uses of is_darwin to be replaced. Although it had been used at one time, the last reference to it appears to have been removed in 4545762 (#1295). This doesn't emit a DeprecationWarning when those attributes are accessed in the git.compat module. (That might be valuable thing to do in the future, if the git.compat module is to remain non-deprecated overall.) Two related less consequential changes are also included: - Improve ordering and grouping of imports, in modules where they were already being changed as a result of no longer needing to import is_ (usually is_win) from git.compat. - Make minor revisions to a few comments, for readability. (This is in addition to somewhat more substantial revisions of comments where rewording was related to replacing uses of is_.) --- .github/workflows/cygwin-test.yml | 2 +- .github/workflows/pythonpackage.yml | 2 +- git/cmd.py | 64 ++++++++++++++--------------- git/compat.py | 11 +++++ git/config.py | 19 +++------ git/index/fun.py | 22 ++++------ git/index/util.py | 8 +--- git/objects/submodule/base.py | 7 ++-- git/repo/base.py | 22 ++++------ git/util.py | 36 ++++++++++------ test/lib/helper.py | 10 ++--- test/test_base.py | 3 +- test/test_git.py | 10 +++-- test/test_index.py | 9 ++-- test/test_installation.py | 3 +- test/test_submodule.py | 15 +++---- test/test_util.py | 3 +- 17 files changed, 118 insertions(+), 128 deletions(-) diff --git a/.github/workflows/cygwin-test.yml b/.github/workflows/cygwin-test.yml index 89c03a394..96728e51a 100644 --- a/.github/workflows/cygwin-test.yml +++ b/.github/workflows/cygwin-test.yml @@ -72,7 +72,7 @@ jobs: python --version python -c 'import sys; print(sys.platform)' python -c 'import os; print(os.name)' - python -c 'import git; print(git.compat.is_win)' + python -c 'import git; print(git.compat.is_win)' # NOTE: Deprecated. Use os.name directly. - name: Test with pytest run: | diff --git a/.github/workflows/pythonpackage.yml b/.github/workflows/pythonpackage.yml index 2dd97183b..35fecf16c 100644 --- a/.github/workflows/pythonpackage.yml +++ b/.github/workflows/pythonpackage.yml @@ -63,7 +63,7 @@ jobs: python --version python -c 'import sys; print(sys.platform)' python -c 'import os; print(os.name)' - python -c 'import git; print(git.compat.is_win)' + python -c 'import git; print(git.compat.is_win)' # NOTE: Deprecated. Use os.name directly. - name: Check types with mypy run: | diff --git a/git/cmd.py b/git/cmd.py index fd39a8eeb..f052c72cf 100644 --- a/git/cmd.py +++ b/git/cmd.py @@ -17,19 +17,21 @@ import threading from textwrap import dedent -from git.compat import ( - defenc, - force_bytes, - safe_decode, - is_posix, - is_win, +from git.compat import defenc, force_bytes, safe_decode +from git.exc import ( + CommandError, + GitCommandError, + GitCommandNotFound, + UnsafeOptionError, + UnsafeProtocolError, ) -from git.exc import CommandError -from git.util import is_cygwin_git, cygpath, expand_path, remove_password_if_present, patch_env - -from .exc import GitCommandError, GitCommandNotFound, UnsafeOptionError, UnsafeProtocolError -from .util import ( +from git.util import ( LazyMixin, + cygpath, + expand_path, + is_cygwin_git, + patch_env, + remove_password_if_present, stream_copy, ) @@ -180,14 +182,13 @@ def pump_stream( t.start() threads.append(t) - ## FIXME: Why Join?? Will block if `stdin` needs feeding... - # + # FIXME: Why join? Will block if stdin needs feeding... for t in threads: t.join(timeout=kill_after_timeout) if t.is_alive(): if isinstance(process, Git.AutoInterrupt): process._terminate() - else: # Don't want to deal with the other case + else: # Don't want to deal with the other case. raise RuntimeError( "Thread join() timed out in cmd.handle_process_output()." f" kill_after_timeout={kill_after_timeout} seconds" @@ -197,11 +198,11 @@ def pump_stream( "error: process killed because it timed out." f" kill_after_timeout={kill_after_timeout} seconds" ) if not decode_streams and isinstance(p_stderr, BinaryIO): - # Assume stderr_handler needs binary input + # Assume stderr_handler needs binary input. error_str = cast(str, error_str) error_str = error_str.encode() # We ignore typing on the next line because mypy does not like - # the way we inferred that stderr takes str or bytes + # the way we inferred that stderr takes str or bytes. stderr_handler(error_str) # type: ignore if finalizer: @@ -228,14 +229,12 @@ def dict_to_slots_and__excluded_are_none(self: object, d: Mapping[str, Any], exc ## -- End Utilities -- @} -# value of Windows process creation flag taken from MSDN -CREATE_NO_WINDOW = 0x08000000 - -## CREATE_NEW_PROCESS_GROUP is needed to allow killing it afterwards, -# see https://docs.python.org/3/library/subprocess.html#subprocess.Popen.send_signal -PROC_CREATIONFLAGS = ( - CREATE_NO_WINDOW | subprocess.CREATE_NEW_PROCESS_GROUP if is_win else 0 # type: ignore[attr-defined] -) # mypy error if not Windows. +if os.name == "nt": + # CREATE_NEW_PROCESS_GROUP is needed to allow killing it afterwards. See: + # https://docs.python.org/3/library/subprocess.html#subprocess.Popen.send_signal + PROC_CREATIONFLAGS = subprocess.CREATE_NO_WINDOW | subprocess.CREATE_NEW_PROCESS_GROUP +else: + PROC_CREATIONFLAGS = 0 class Git(LazyMixin): @@ -551,7 +550,7 @@ def _terminate(self) -> None: # For some reason, providing None for stdout/stderr still prints something. This is why # we simply use the shell and redirect to nul. Slower than CreateProcess. The question # is whether we really want to see all these messages. It's annoying no matter what. - if is_win: + if os.name == "nt": call( ("TASKKILL /F /T /PID %s 2>nul 1>nul" % str(proc.pid)), shell=True, @@ -967,7 +966,7 @@ def execute( if inline_env is not None: env.update(inline_env) - if is_win: + if os.name == "nt": cmd_not_found_exception = OSError if kill_after_timeout is not None: raise GitCommandError( @@ -999,11 +998,11 @@ def execute( env=env, cwd=cwd, bufsize=-1, - stdin=istream or DEVNULL, + stdin=(istream or DEVNULL), stderr=PIPE, stdout=stdout_sink, shell=shell, - close_fds=is_posix, # Unsupported on Windows. + close_fds=(os.name == "posix"), # Unsupported on Windows. universal_newlines=universal_newlines, creationflags=PROC_CREATIONFLAGS, **subprocess_kwargs, @@ -1073,7 +1072,7 @@ def kill_process(pid: int) -> None: ) if not universal_newlines: stderr_value = stderr_value.encode(defenc) - # strip trailing "\n" + # Strip trailing "\n". if stdout_value.endswith(newline) and strip_newline_in_stdout: # type: ignore stdout_value = stdout_value[:-1] if stderr_value.endswith(newline): # type: ignore @@ -1147,11 +1146,11 @@ def update_environment(self, **kwargs: Any) -> Dict[str, Union[str, None]]: """ old_env = {} for key, value in kwargs.items(): - # set value if it is None + # Set value if it is None. if value is not None: old_env[key] = self._environment.get(key) self._environment[key] = value - # remove key from environment if its value is None + # Remove key from environment if its value is None. elif key in self._environment: old_env[key] = self._environment[key] del self._environment[key] @@ -1330,7 +1329,8 @@ def _parse_object_header(self, header_line: str) -> Tuple[str, str, int]: :return: (hex_sha, type_string, size_as_int) :raise ValueError: If the header contains indication for an error due to - incorrect input sha""" + incorrect input sha + """ tokens = header_line.split() if len(tokens) != 3: if not tokens: diff --git a/git/compat.py b/git/compat.py index f17e52f7b..545bf65b4 100644 --- a/git/compat.py +++ b/git/compat.py @@ -34,9 +34,20 @@ # --------------------------------------------------------------------------- +# DEPRECATED attributes providing shortcuts to operating system checks based on os.name. +# +# - is_win and is_posix are deprecated because it is clearer, and helps avoid bugs, to +# write out the os.name checks explicitly. For example, is_win is False on Cygwin, but +# is often assumed to be True. +# +# - is_darwin is deprecated because it is always False on all systems, as os.name is +# never "darwin". For macOS, you can check for sys.platform == "darwin". (As on other +# Unix-like systems, os.name == "posix" on macOS. This is also the case on Cygwin.) +# is_win: bool = os.name == "nt" is_posix = os.name == "posix" is_darwin = os.name == "darwin" + defenc = sys.getfilesystemencoding() diff --git a/git/config.py b/git/config.py index 2cb057021..0846bbf94 100644 --- a/git/config.py +++ b/git/config.py @@ -7,28 +7,21 @@ """Module containing module parser implementation able to properly read and write configuration files.""" -import sys import abc +import configparser as cp +import fnmatch from functools import wraps import inspect from io import BufferedReader, IOBase import logging import os +import os.path as osp import re -import fnmatch - -from git.compat import ( - defenc, - force_text, - is_win, -) +import sys +from git.compat import defenc, force_text from git.util import LockFile -import os.path as osp - -import configparser as cp - # typing------------------------------------------------------- from typing import ( @@ -250,7 +243,7 @@ def items_all(self) -> List[Tuple[str, List[_T]]]: def get_config_path(config_level: Lit_config_levels) -> str: # We do not support an absolute path of the gitconfig on Windows. # Use the global config instead. - if is_win and config_level == "system": + if os.name == "nt" and config_level == "system": config_level = "global" if config_level == "system": diff --git a/git/index/fun.py b/git/index/fun.py index a35990d6d..fe8f373d8 100644 --- a/git/index/fun.py +++ b/git/index/fun.py @@ -2,8 +2,9 @@ # NOTE: Autodoc hates it if this is a docstring. from io import BytesIO -from pathlib import Path import os +import os.path as osp +from pathlib import Path from stat import ( S_IFDIR, S_IFLNK, @@ -16,26 +17,17 @@ import subprocess from git.cmd import PROC_CREATIONFLAGS, handle_process_output -from git.compat import ( - defenc, - force_text, - force_bytes, - is_posix, - is_win, - safe_decode, -) -from git.exc import UnmergedEntriesError, HookExecutionError +from git.compat import defenc, force_bytes, force_text, safe_decode +from git.exc import HookExecutionError, UnmergedEntriesError from git.objects.fun import ( - tree_to_stream, traverse_tree_recursive, traverse_trees_recursive, + tree_to_stream, ) from git.util import IndexFileSHA1Writer, finalize_process from gitdb.base import IStream from gitdb.typ import str_tree_type -import os.path as osp - from .typ import BaseIndexEntry, IndexEntry, CE_NAMEMASK, CE_STAGESHIFT from .util import pack, unpack @@ -96,7 +88,7 @@ def run_commit_hook(name: str, index: "IndexFile", *args: str) -> None: env["GIT_EDITOR"] = ":" cmd = [hp] try: - if is_win and not _has_file_extension(hp): + if os.name == "nt" and not _has_file_extension(hp): # Windows only uses extensions to determine how to open files # (doesn't understand shebangs). Try using bash to run the hook. relative_hp = Path(hp).relative_to(index.repo.working_dir).as_posix() @@ -108,7 +100,7 @@ def run_commit_hook(name: str, index: "IndexFile", *args: str) -> None: stdout=subprocess.PIPE, stderr=subprocess.PIPE, cwd=index.repo.working_dir, - close_fds=is_posix, + close_fds=(os.name == "posix"), creationflags=PROC_CREATIONFLAGS, ) except Exception as ex: diff --git a/git/index/util.py b/git/index/util.py index 08e49d860..5c2270abd 100644 --- a/git/index/util.py +++ b/git/index/util.py @@ -2,15 +2,11 @@ from functools import wraps import os +import os.path as osp import struct import tempfile from types import TracebackType -from git.compat import is_win - -import os.path as osp - - # typing ---------------------------------------------------------------------- from typing import Any, Callable, TYPE_CHECKING, Optional, Type @@ -58,7 +54,7 @@ def __exit__( exc_tb: Optional[TracebackType], ) -> bool: if osp.isfile(self.tmp_file_path): - if is_win and osp.exists(self.file_path): + if os.name == "nt" and osp.exists(self.file_path): os.remove(self.file_path) os.rename(self.tmp_file_path, self.file_path) diff --git a/git/objects/submodule/base.py b/git/objects/submodule/base.py index 1516306ec..d88524ecb 100644 --- a/git/objects/submodule/base.py +++ b/git/objects/submodule/base.py @@ -7,7 +7,7 @@ import git from git.cmd import Git -from git.compat import defenc, is_win +from git.compat import defenc from git.config import GitConfigParser, SectionConstraint, cp from git.exc import ( BadName, @@ -353,9 +353,8 @@ def _write_git_file_and_module_config(cls, working_tree_dir: PathLike, module_ab """ git_file = osp.join(working_tree_dir, ".git") rela_path = osp.relpath(module_abspath, start=working_tree_dir) - if is_win: - if osp.isfile(git_file): - os.remove(git_file) + if os.name == "nt" and osp.isfile(git_file): + os.remove(git_file) with open(git_file, "wb") as fp: fp.write(("gitdir: %s" % rela_path).encode(defenc)) diff --git a/git/repo/base.py b/git/repo/base.py index 4790ea4e7..bbbf242db 100644 --- a/git/repo/base.py +++ b/git/repo/base.py @@ -6,24 +6,21 @@ from __future__ import annotations +import gc import logging import os +import os.path as osp +from pathlib import Path import re import shlex import warnings -from pathlib import Path - +import gitdb from gitdb.db.loose import LooseObjectDB - from gitdb.exc import BadObject from git.cmd import Git, handle_process_output -from git.compat import ( - defenc, - safe_decode, - is_win, -) +from git.compat import defenc, safe_decode from git.config import GitConfigParser from git.db import GitCmdObjectDB from git.exc import ( @@ -43,7 +40,6 @@ expand_path, remove_password_if_present, ) -import os.path as osp from .fun import ( rev_parse, @@ -52,8 +48,6 @@ touch, find_worktree_git_dir, ) -import gc -import gitdb # typing ------------------------------------------------------ @@ -322,10 +316,10 @@ def close(self) -> None: # they are collected by the garbage collector, thus preventing deletion. # TODO: Find these references and ensure they are closed and deleted # synchronously rather than forcing a gc collection. - if is_win: + if os.name == "nt": gc.collect() gitdb.util.mman.collect() - if is_win: + if os.name == "nt": gc.collect() def __eq__(self, rhs: object) -> bool: @@ -571,7 +565,7 @@ def _get_config_path(self, config_level: Lit_config_levels, git_dir: Optional[Pa git_dir = self.git_dir # We do not support an absolute path of the gitconfig on Windows. # Use the global config instead. - if is_win and config_level == "system": + if os.name == "nt" and config_level == "system": config_level = "global" if config_level == "system": diff --git a/git/util.py b/git/util.py index bd1fbe247..be267fc9f 100644 --- a/git/util.py +++ b/git/util.py @@ -22,8 +22,6 @@ from urllib.parse import urlsplit, urlunsplit import warnings -from .compat import is_win - # typing --------------------------------------------------------- from typing import ( @@ -110,7 +108,18 @@ log = logging.getLogger(__name__) -def _read_env_flag(name: str, default: bool) -> bool: +def _read_win_env_flag(name: str, default: bool) -> bool: + """Read a boolean flag from an environment variable on Windows. + + :return: + On Windows, the flag, or the ``default`` value if absent or ambiguous. + On all other operating systems, ``False``. + + :note: This only accesses the environment on Windows. + """ + if os.name != "nt": + return False + try: value = os.environ[name] except KeyError: @@ -134,8 +143,8 @@ def _read_env_flag(name: str, default: bool) -> bool: #: 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 _read_env_flag("HIDE_WINDOWS_KNOWN_ERRORS", True) -HIDE_WINDOWS_FREEZE_ERRORS = is_win and _read_env_flag("HIDE_WINDOWS_FREEZE_ERRORS", True) +HIDE_WINDOWS_KNOWN_ERRORS = _read_win_env_flag("HIDE_WINDOWS_KNOWN_ERRORS", True) +HIDE_WINDOWS_FREEZE_ERRORS = _read_win_env_flag("HIDE_WINDOWS_FREEZE_ERRORS", True) # { Utility Methods @@ -220,7 +229,7 @@ def handler(function: Callable, path: PathLike, _excinfo: Any) -> None: def rmfile(path: PathLike) -> None: """Ensure file deleted also on *Windows* where read-only files need special treatment.""" if osp.isfile(path): - if is_win: + if os.name == "nt": os.chmod(path, 0o777) os.remove(path) @@ -260,7 +269,7 @@ def join_path(a: PathLike, *p: PathLike) -> PathLike: return path -if is_win: +if os.name == "nt": def to_native_path_windows(path: PathLike) -> PathLike: path = str(path) @@ -308,9 +317,12 @@ def assure_directory_exists(path: PathLike, is_file: bool = False) -> bool: def _get_exe_extensions() -> Sequence[str]: PATHEXT = os.environ.get("PATHEXT", None) - return ( - tuple(p.upper() for p in PATHEXT.split(os.pathsep)) if PATHEXT else (".BAT", "COM", ".EXE") if is_win else ("") - ) + if PATHEXT: + return tuple(p.upper() for p in PATHEXT.split(os.pathsep)) + elif os.name == "nt": + return (".BAT", "COM", ".EXE") + else: + return () def py_where(program: str, path: Optional[PathLike] = None) -> List[str]: @@ -418,8 +430,8 @@ def is_cygwin_git(git_executable: PathLike) -> bool: def is_cygwin_git(git_executable: Union[None, PathLike]) -> bool: - if is_win: - # is_win is only True on native Windows systems. On Cygwin, os.name == "posix". + if os.name == "nt": + # This is Windows-native Python, since Cygwin has os.name == "posix". return False if git_executable is None: diff --git a/test/lib/helper.py b/test/lib/helper.py index 8725cd13f..950ca3c0b 100644 --- a/test/lib/helper.py +++ b/test/lib/helper.py @@ -10,17 +10,15 @@ import io import logging import os +import os.path as osp import tempfile import textwrap import time import unittest -from git.compat import is_win -from git.util import rmtree, cwd import gitdb -import os.path as osp - +from git.util import rmtree, cwd TestCase = unittest.TestCase SkipTest = unittest.SkipTest @@ -177,7 +175,7 @@ def git_daemon_launched(base_path, ip, port): gd = None try: - if is_win: + if os.name == "nt": # On MINGW-git, daemon exists in Git\mingw64\libexec\git-core\, # but if invoked as 'git daemon', it detaches from parent `git` cmd, # and then CANNOT DIE! @@ -201,7 +199,7 @@ def git_daemon_launched(base_path, ip, port): as_process=True, ) # Yes, I know... fortunately, this is always going to work if sleep time is just large enough. - time.sleep(0.5 * (1 + is_win)) + time.sleep(1.0 if os.name == "nt" else 0.5) except Exception as ex: msg = textwrap.dedent( """ diff --git a/test/test_base.py b/test/test_base.py index e4704c7d8..302d696fe 100644 --- a/test/test_base.py +++ b/test/test_base.py @@ -11,7 +11,6 @@ from git import Repo from git.objects import Blob, Tree, Commit, TagObject -from git.compat import is_win from git.objects.util import get_object_type_by_name from test.lib import TestBase as _TestBase, with_rw_repo, with_rw_and_rw_remote_repo from git.util import hex_to_bin, HIDE_WINDOWS_FREEZE_ERRORS @@ -131,7 +130,7 @@ def test_add_unicode(self, rw_repo): with open(file_path, "wb") as fp: fp.write(b"something") - if is_win: + if os.name == "nt": # On Windows, there is no way this works, see images on: # https://github.com/gitpython-developers/GitPython/issues/147#issuecomment-68881897 # Therefore, it must be added using the Python implementation. diff --git a/test/test_git.py b/test/test_git.py index 06f8f5c97..b85be6359 100644 --- a/test/test_git.py +++ b/test/test_git.py @@ -23,7 +23,6 @@ import ddt from git import Git, refresh, GitCommandError, GitCommandNotFound, Repo, cmd -from git.compat import is_win from git.util import cwd, finalize_process from test.lib import TestBase, fixture_path, with_rw_directory @@ -139,7 +138,7 @@ def test_it_executes_git_and_returns_result(self): def test_it_executes_git_not_from_cwd(self): with TemporaryDirectory() as tmpdir: - if is_win: + if os.name == "nt": # Copy an actual binary executable that is not git. other_exe_path = os.path.join(os.getenv("WINDIR"), "system32", "hostname.exe") impostor_path = os.path.join(tmpdir, "git.exe") @@ -154,7 +153,10 @@ def test_it_executes_git_not_from_cwd(self): with cwd(tmpdir): self.assertRegex(self.git.execute(["git", "version"]), r"^git version\b") - @skipUnless(is_win, "The regression only affected Windows, and this test logic is OS-specific.") + @skipUnless( + os.name == "nt", + "The regression only affected Windows, and this test logic is OS-specific.", + ) def test_it_avoids_upcasing_unrelated_environment_variable_names(self): old_name = "28f425ca_d5d8_4257_b013_8d63166c8158" if old_name == old_name.upper(): @@ -268,7 +270,7 @@ def test_refresh(self): self.assertRaises(GitCommandNotFound, refresh, "yada") # Test a good path refresh. - which_cmd = "where" if is_win else "command -v" + which_cmd = "where" if os.name == "nt" else "command -v" path = os.popen("{0} git".format(which_cmd)).read().strip().split("\n")[0] refresh(path) diff --git a/test/test_index.py b/test/test_index.py index 3357dc880..b9d1e78ab 100644 --- a/test/test_index.py +++ b/test/test_index.py @@ -25,7 +25,6 @@ GitCommandError, CheckoutError, ) -from git.compat import is_win from git.exc import HookExecutionError, InvalidGitRepositoryError from git.index.fun import hook_path from git.index.typ import BaseIndexEntry, IndexEntry @@ -43,9 +42,9 @@ def _found_in(cmd, directory): return path and Path(path).parent == Path(directory) -is_win_without_bash = is_win and not shutil.which("bash.exe") +is_win_without_bash = os.name == "nt" and not shutil.which("bash.exe") -is_win_with_wsl_bash = is_win and _found_in( +is_win_with_wsl_bash = os.name == "nt" and _found_in( cmd="bash.exe", directory=Path(os.getenv("WINDIR")) / "System32", ) @@ -614,7 +613,7 @@ def mixed_iterator(): self.assertNotEqual(entries[0].hexsha, null_hex_sha) # Add symlink. - if not is_win: + if os.name != "nt": for target in ("/etc/nonexisting", "/etc/passwd", "/etc"): basename = "my_real_symlink" @@ -672,7 +671,7 @@ def mixed_iterator(): index.checkout(fake_symlink_path) # On Windows, we will never get symlinks. - if is_win: + if os.name == "nt": # Symlinks should contain the link as text (which is what a # symlink actually is). with open(fake_symlink_path, "rt") as fd: diff --git a/test/test_installation.py b/test/test_installation.py index e7774d29d..8e5f55d28 100644 --- a/test/test_installation.py +++ b/test/test_installation.py @@ -6,7 +6,6 @@ import subprocess import sys -from git.compat import is_win from test.lib import TestBase from test.lib.helper import with_rw_directory @@ -15,7 +14,7 @@ class TestInstallation(TestBase): def setUp_venv(self, rw_dir): self.venv = rw_dir subprocess.run([sys.executable, "-m", "venv", self.venv], stdout=subprocess.PIPE) - bin_name = "Scripts" if is_win else "bin" + bin_name = "Scripts" if os.name == "nt" else "bin" self.python = os.path.join(self.venv, bin_name, "python") self.pip = os.path.join(self.venv, bin_name, "pip") self.sources = os.path.join(self.venv, "src") diff --git a/test/test_submodule.py b/test/test_submodule.py index f63db1495..f706769c0 100644 --- a/test/test_submodule.py +++ b/test/test_submodule.py @@ -3,17 +3,17 @@ import contextlib import os -import shutil -import tempfile +import os.path as osp from pathlib import Path +import shutil import sys +import tempfile from unittest import mock, skipUnless import pytest import git from git.cmd import Git -from git.compat import is_win from git.config import GitConfigParser, cp from git.exc import ( GitCommandError, @@ -25,11 +25,8 @@ from git.objects.submodule.base import Submodule from git.objects.submodule.root import RootModule, RootUpdateProgress from git.repo.fun import find_submodule_git_dir, touch -from test.lib import TestBase, with_rw_repo -from test.lib import with_rw_directory -from git.util import HIDE_WINDOWS_KNOWN_ERRORS -from git.util import to_native_path_linux, join_path_native -import os.path as osp +from git.util import HIDE_WINDOWS_KNOWN_ERRORS, join_path_native, to_native_path_linux +from test.lib import TestBase, with_rw_directory, with_rw_repo @contextlib.contextmanager @@ -1040,7 +1037,7 @@ def test_branch_renames(self, rw_dir): assert sm_mod.commit() == sm_pfb.commit, "Now head should have been reset" assert sm_mod.head.ref.name == sm_pfb.name - @skipUnless(is_win, "Specifically for Windows.") + @skipUnless(os.name == "nt", "Specifically for Windows.") def test_to_relative_path_with_super_at_root_drive(self): class Repo: working_tree_dir = "D:\\" diff --git a/test/test_util.py b/test/test_util.py index d345247b1..4ef98fd73 100644 --- a/test/test_util.py +++ b/test/test_util.py @@ -20,7 +20,6 @@ import pytest from git.cmd import dashify -from git.compat import is_win from git.objects.util import ( altz_to_utctz_str, from_timestamp, @@ -364,7 +363,7 @@ def test_blocking_lock_file(self): self.assertRaises(IOError, wait_lock._obtain_lock) elapsed = time.time() - start extra_time = 0.02 - if is_win or sys.platform == "cygwin": + if os.name == "nt" or sys.platform == "cygwin": extra_time *= 6 # NOTE: Indeterministic failures without this... self.assertLess(elapsed, wait_time + extra_time) From 562bdee8110267f2861625f40aa403a72f000def Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sat, 4 Nov 2023 17:33:58 -0400 Subject: [PATCH 2/2] Fix compat.is_darwin This fixes compat.is_darwin to use sys.platform instead of os.name so that it is True on macOS systems instead of always being False. The deprecation rationale in compat.py is accordingly adjusted. Together with c01fe76, this largely addresses #1731. --- git/compat.py | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/git/compat.py b/git/compat.py index 545bf65b4..629be075a 100644 --- a/git/compat.py +++ b/git/compat.py @@ -36,17 +36,14 @@ # DEPRECATED attributes providing shortcuts to operating system checks based on os.name. # -# - is_win and is_posix are deprecated because it is clearer, and helps avoid bugs, to -# write out the os.name checks explicitly. For example, is_win is False on Cygwin, but -# is often assumed to be True. +# These are deprecated because it is clearer, and helps avoid bugs, to write out the +# os.name or sys.platform checks explicitly, especially in cases where it matters which +# is used. For example, is_win is False on Cygwin, but is often assumed True. To detect +# Cygwin, use sys.platform == "cygwin". (Also, in the past, is_darwin was unreliable.) # -# - is_darwin is deprecated because it is always False on all systems, as os.name is -# never "darwin". For macOS, you can check for sys.platform == "darwin". (As on other -# Unix-like systems, os.name == "posix" on macOS. This is also the case on Cygwin.) -# -is_win: bool = os.name == "nt" +is_win = os.name == "nt" is_posix = os.name == "posix" -is_darwin = os.name == "darwin" +is_darwin = sys.platform == "darwin" defenc = sys.getfilesystemencoding()