From 5ca0fbae5c8f1ae112ddd0d4647d3399fe610501 Mon Sep 17 00:00:00 2001 From: jcole-crowdstrike Date: Fri, 1 Mar 2024 11:55:32 -0800 Subject: [PATCH 01/20] Updating regex pattern to handle unicode whitespaces. Replacing the \s whitespace characters with normal spaces (" ") to prevent breaking on unicode whitespace characters (e.g., NBSP). Without this, if a branch name contains a unicode whitespace character that falls under \s, then the branch name will be truncated. --- git/remote.py | 2 +- test/test_remote.py | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/git/remote.py b/git/remote.py index 6ce720ee3..6e841949f 100644 --- a/git/remote.py +++ b/git/remote.py @@ -325,7 +325,7 @@ class FetchInfo(IterableObj): ERROR, ) = [1 << x for x in range(8)] - _re_fetch_result = re.compile(r"^\s*(.) (\[[\w\s\.$@]+\]|[\w\.$@]+)\s+(.+) -> ([^\s]+)( \(.*\)?$)?") + _re_fetch_result = re.compile(r"^ *(.) (\[[\w \.$@]+\]|[\w\.$@]+) +(.+) -> ([^ ]+)( \(.*\)?$)?") _flag_map: Dict[flagKeyLiteral, int] = { "!": ERROR, diff --git a/test/test_remote.py b/test/test_remote.py index c0bd11f80..63eababac 100644 --- a/test/test_remote.py +++ b/test/test_remote.py @@ -1002,6 +1002,21 @@ def test_push_unsafe_options_allowed(self, rw_repo): assert tmp_file.exists() tmp_file.unlink() + @with_rw_and_rw_remote_repo("0.1.6") + def test_fetch_unsafe_branch_name(self, rw_repo, remote_repo): + # Create branch with a name containing a NBSP + bad_branch_name = f"branch_with_{chr(160)}_nbsp" + Head.create(remote_repo, bad_branch_name) + + # Fetch and get branches + remote = rw_repo.remote("origin") + branches = remote.fetch() + + # Test for truncated branch name in branches + assert f"origin/{bad_branch_name}" in [b.name for b in branches] + + # Cleanup branch + Head.delete(remote_repo, bad_branch_name) class TestTimeouts(TestBase): @with_rw_repo("HEAD", bare=False) From 827e986ec686b168e65f3bbad526d6a863191031 Mon Sep 17 00:00:00 2001 From: jcole-crowdstrike Date: Fri, 1 Mar 2024 13:51:33 -0800 Subject: [PATCH 02/20] Updating spacing per linting test. Adding a second blank line after the newly added test case. --- test/test_remote.py | 1 + 1 file changed, 1 insertion(+) diff --git a/test/test_remote.py b/test/test_remote.py index 63eababac..35af8172d 100644 --- a/test/test_remote.py +++ b/test/test_remote.py @@ -1018,6 +1018,7 @@ def test_fetch_unsafe_branch_name(self, rw_repo, remote_repo): # Cleanup branch Head.delete(remote_repo, bad_branch_name) + class TestTimeouts(TestBase): @with_rw_repo("HEAD", bare=False) def test_timeout_funcs(self, repo): From 9f226fc47683038acd3304dd7d59be18984d3737 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Mon, 4 Mar 2024 19:38:10 -0500 Subject: [PATCH 03/20] Remove unneeded annotation on __slots__ variable The correct type is inferred, and no other __slots__ variable in the codebase has an annotation. --- git/cmd.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git/cmd.py b/git/cmd.py index c7cec48d7..8550830aa 100644 --- a/git/cmd.py +++ b/git/cmd.py @@ -749,7 +749,7 @@ class CatFileContentStream: rest to ensure the underlying stream continues to work. """ - __slots__: Tuple[str, ...] = ("_stream", "_nbr", "_size") + __slots__ = ("_stream", "_nbr", "_size") def __init__(self, size: int, stream: IO[bytes]) -> None: self._stream = stream From e984bfebd9f1934b9810ac6cccb1bb803c64dd21 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Mon, 4 Mar 2024 20:02:47 -0500 Subject: [PATCH 04/20] Fix some underindented portions of docstrings A few docstrings had parts that were meant to be indented the usual four spaces beyond the surrounding indentation, but were indented only three spaces beyond it instead. --- git/cmd.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/git/cmd.py b/git/cmd.py index 8550830aa..3d5992dbd 100644 --- a/git/cmd.py +++ b/git/cmd.py @@ -850,10 +850,10 @@ def __init__(self, working_dir: Union[None, PathLike] = None): """Initialize this instance with: :param working_dir: - Git directory we should work in. If ``None``, we always work in the current - directory as returned by :func:`os.getcwd`. - This is meant to be the working tree directory if available, or the - ``.git`` directory in case of bare repositories. + Git directory we should work in. If ``None``, we always work in the current + directory as returned by :func:`os.getcwd`. + This is meant to be the working tree directory if available, or the + ``.git`` directory in case of bare repositories. """ super().__init__() self._working_dir = expand_path(working_dir) @@ -1103,8 +1103,8 @@ def execute( :raise git.exc.GitCommandError: :note: - If you add additional keyword arguments to the signature of this method, - you must update the ``execute_kwargs`` variable housed in this module. + If you add additional keyword arguments to the signature of this method, you + must update the ``execute_kwargs`` variable housed in this module. """ # Remove password for the command if present. redacted_command = remove_password_if_present(command) @@ -1438,7 +1438,7 @@ def _call_process( turns into:: - git rev-list max-count 10 --header master + git rev-list max-count 10 --header master :return: Same as :meth:`execute`. If no args are given, used :meth:`execute`'s From 5d7e55b8dd2abaacd5397860e6efd7bd4215fe12 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Mon, 4 Mar 2024 20:11:40 -0500 Subject: [PATCH 05/20] Add return-type annotation on __init__ methods Although sometimes found unintuitive, the reutrn type of a class's __init__ method is best annotated as None, as in other functions that return implicitly or by operand-less return statements. Note that this is only a minor improvement, effectively just a style fix, because mypy treats __init__ specially and, *when at least one parameter is annotated*, its return type is implicitly None rather than implicitly Any like other functions. All the __init__ methods modified here did already have one or more annotated parameters. However, having __init__ methods without the return type makes it easier to introduce a bug where an __init__ method with no parameters besides self -- which itself should almost always be unannotated and is properly inferred -- is written and the return annotation needed to get mypy to regard it as statically typed, so it checks it at all, is omitted. (It is also inelegant when one considers the meaning of __init__ and the distinction between it and __new__.) This commit does not add any return type annotations to functions in the test suite, since the test suite doesn't currently use static typing. Further reading: - https://peps.python.org/pep-0484/#the-meaning-of-annotations - https://github.com/python/mypy/issues/604 - https://github.com/gitpython-developers/GitPython/pull/1755#issuecomment-1837419098 --- git/cmd.py | 2 +- git/objects/base.py | 2 +- git/objects/submodule/root.py | 2 +- git/refs/head.py | 2 +- git/refs/log.py | 2 +- git/refs/symbolic.py | 2 +- git/util.py | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-) diff --git a/git/cmd.py b/git/cmd.py index 3d5992dbd..731d0fab8 100644 --- a/git/cmd.py +++ b/git/cmd.py @@ -846,7 +846,7 @@ def __del__(self) -> None: self._stream.read(bytes_left + 1) # END handle incomplete read - def __init__(self, working_dir: Union[None, PathLike] = None): + def __init__(self, working_dir: Union[None, PathLike] = None) -> None: """Initialize this instance with: :param working_dir: diff --git a/git/objects/base.py b/git/objects/base.py index e78420e8a..2b8dd0ff6 100644 --- a/git/objects/base.py +++ b/git/objects/base.py @@ -55,7 +55,7 @@ class Object(LazyMixin): type: Union[Lit_commit_ish, None] = None - def __init__(self, repo: "Repo", binsha: bytes): + def __init__(self, repo: "Repo", binsha: bytes) -> None: """Initialize an object by identifying it by its binary sha. All keyword arguments will be set on demand if ``None``. diff --git a/git/objects/submodule/root.py b/git/objects/submodule/root.py index ac0f7ad94..3268d73a4 100644 --- a/git/objects/submodule/root.py +++ b/git/objects/submodule/root.py @@ -56,7 +56,7 @@ class RootModule(Submodule): k_root_name = "__ROOT__" - def __init__(self, repo: "Repo"): + def __init__(self, repo: "Repo") -> None: # repo, binsha, mode=None, path=None, name = None, parent_commit=None, url=None, ref=None) super().__init__( repo, diff --git a/git/refs/head.py b/git/refs/head.py index d546cb4ef..f6020f461 100644 --- a/git/refs/head.py +++ b/git/refs/head.py @@ -44,7 +44,7 @@ class HEAD(SymbolicReference): __slots__ = () - def __init__(self, repo: "Repo", path: PathLike = _HEAD_NAME): + def __init__(self, repo: "Repo", path: PathLike = _HEAD_NAME) -> None: if path != self._HEAD_NAME: raise ValueError("HEAD instance must point to %r, got %r" % (self._HEAD_NAME, path)) super().__init__(repo, path) diff --git a/git/refs/log.py b/git/refs/log.py index 7aefeb4e6..f98f56f11 100644 --- a/git/refs/log.py +++ b/git/refs/log.py @@ -164,7 +164,7 @@ def __new__(cls, filepath: Union[PathLike, None] = None) -> "RefLog": inst = super().__new__(cls) return inst - def __init__(self, filepath: Union[PathLike, None] = None): + def __init__(self, filepath: Union[PathLike, None] = None) -> None: """Initialize this instance with an optional filepath, from which we will initialize our data. The path is also used to write changes back using the :meth:`write` method.""" diff --git a/git/refs/symbolic.py b/git/refs/symbolic.py index 142952e06..16aada0a7 100644 --- a/git/refs/symbolic.py +++ b/git/refs/symbolic.py @@ -74,7 +74,7 @@ class SymbolicReference: _remote_common_path_default = "refs/remotes" _id_attribute_ = "name" - def __init__(self, repo: "Repo", path: PathLike, check_path: bool = False): + def __init__(self, repo: "Repo", path: PathLike, check_path: bool = False) -> None: self.repo = repo self.path = path diff --git a/git/util.py b/git/util.py index 52f9dbdad..d272dc53c 100644 --- a/git/util.py +++ b/git/util.py @@ -917,7 +917,7 @@ class Stats: __slots__ = ("total", "files") - def __init__(self, total: Total_TD, files: Dict[PathLike, Files_TD]): + def __init__(self, total: Total_TD, files: Dict[PathLike, Files_TD]) -> None: self.total = total self.files = files From 4810491cb9d30d05e3ed73e55fe803fcf7ae0b53 Mon Sep 17 00:00:00 2001 From: jcole-crowdstrike Date: Mon, 4 Mar 2024 20:33:24 -0800 Subject: [PATCH 06/20] Fixing Windows encoding issue. Stdout is being encoded as Windows-1251 instead of UTF-8 due to passing universal_newlines as True to subprocess. --- git/remote.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/git/remote.py b/git/remote.py index 6e841949f..6cb33bede 100644 --- a/git/remote.py +++ b/git/remote.py @@ -891,7 +891,7 @@ def _get_fetch_info_from_stderr( None, progress_handler, finalizer=None, - decode_streams=False, + decode_streams=True, kill_after_timeout=kill_after_timeout, ) @@ -1068,7 +1068,7 @@ def fetch( Git.check_unsafe_options(options=list(kwargs.keys()), unsafe_options=self.unsafe_git_fetch_options) proc = self.repo.git.fetch( - "--", self, *args, as_process=True, with_stdout=False, universal_newlines=True, v=verbose, **kwargs + "--", self, *args, as_process=True, with_stdout=False, v=verbose, **kwargs ) res = self._get_fetch_info_from_stderr(proc, progress, kill_after_timeout=kill_after_timeout) if hasattr(self.repo.odb, "update_cache"): From 938f2725eb137492a459b4210ae58565e26e4fd5 Mon Sep 17 00:00:00 2001 From: jcole-crowdstrike Date: Tue, 5 Mar 2024 07:37:36 -0800 Subject: [PATCH 07/20] Setting universal_newlines=False explicitly in fetch() and pull(). This fix avoids encoding problems on Windows, as was done for fetch() in the previous commit. However, here it is being made more explicit and adds the same fix for the pull function. --- git/remote.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/git/remote.py b/git/remote.py index 6cb33bede..bbe1aea97 100644 --- a/git/remote.py +++ b/git/remote.py @@ -1068,7 +1068,7 @@ def fetch( Git.check_unsafe_options(options=list(kwargs.keys()), unsafe_options=self.unsafe_git_fetch_options) proc = self.repo.git.fetch( - "--", self, *args, as_process=True, with_stdout=False, v=verbose, **kwargs + "--", self, *args, as_process=True, with_stdout=False, universal_newlines=False, v=verbose, **kwargs ) res = self._get_fetch_info_from_stderr(proc, progress, kill_after_timeout=kill_after_timeout) if hasattr(self.repo.odb, "update_cache"): @@ -1122,7 +1122,7 @@ def pull( Git.check_unsafe_options(options=list(kwargs.keys()), unsafe_options=self.unsafe_git_pull_options) proc = self.repo.git.pull( - "--", self, refspec, with_stdout=False, as_process=True, universal_newlines=True, v=True, **kwargs + "--", self, refspec, with_stdout=False, as_process=True, universal_newlines=False, v=True, **kwargs ) res = self._get_fetch_info_from_stderr(proc, progress, kill_after_timeout=kill_after_timeout) if hasattr(self.repo.odb, "update_cache"): From 8b8c76a707155e32e7538136053ff00c3231cc92 Mon Sep 17 00:00:00 2001 From: jcole-crowdstrike Date: Fri, 8 Mar 2024 12:45:33 -0800 Subject: [PATCH 08/20] Fixing stripping issue causing passing tests to be interpreted as failures Removing this block of code that strips away nonprintable ASCII characters, as it is causing some tests to fail inappropriately. Successful tests are identified by the suffix ", done", which is being removed from some messages due to this aforementioned block of code. The codeblock probably intends to just strip away traililng non-ASCII characters from strings, but does not break, and so will instead continue to iterate through the string and then strip away both printable/nonprintable ASCII; this is causing the loss of the ", done" because anytime a nonprintable character is anywhere but the end of the string then the entire section of string after the character is truncated. Looking back through commits, this codeblock was added in 882ebb153e14488b275e374ccebcdda1dea22dd7 as a workaround after the addition of universal_newlines. Seeing as we have removed universal_newlines in the previous commit, we can also remove this codeblock. In future, if something likw this is needed, then maybe a join that concatenates all printable ASCII characters would serve better (e.g., "".join(b for b in line if b >= 32)), instead of trying to cut out the nonprintable chars. --- git/util.py | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/git/util.py b/git/util.py index 52f9dbdad..63c9b1e99 100644 --- a/git/util.py +++ b/git/util.py @@ -611,20 +611,6 @@ def _parse_progress_line(self, line: AnyStr) -> None: self.error_lines.append(self._cur_line) return - # Find escape characters and cut them away - regex will not work with - # them as they are non-ASCII. As git might expect a tty, it will send them. - last_valid_index = None - for i, c in enumerate(reversed(line_str)): - if ord(c) < 32: - # its a slice index - last_valid_index = -i - 1 - # END character was non-ASCII - # END for each character in line - if last_valid_index is not None: - line_str = line_str[:last_valid_index] - # END cut away invalid part - line_str = line_str.rstrip() - cur_count, max_count = None, None match = self.re_op_relative.match(line_str) if match is None: From dd8ee4fabb8ae7921cf53b95b195f4d2186f136f Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Mon, 11 Mar 2024 12:06:18 -0400 Subject: [PATCH 09/20] Start fixing venv test fixture pip toml bug This is not yet a usable fix, because venv.create only supports upgrade_deps on Python 3.9 and higher. --- test/lib/helper.py | 6 +++--- test/test_index.py | 2 +- test/test_installation.py | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/test/lib/helper.py b/test/lib/helper.py index 26469ed5d..cf7b37b08 100644 --- a/test/lib/helper.py +++ b/test/lib/helper.py @@ -403,13 +403,13 @@ class VirtualEnvironment: __slots__ = ("_env_dir",) - def __init__(self, env_dir, *, with_pip): + def __init__(self, env_dir, *, need_pip): if os.name == "nt": self._env_dir = osp.realpath(env_dir) - venv.create(self.env_dir, symlinks=False, with_pip=with_pip) + venv.create(self.env_dir, symlinks=False, with_pip=need_pip, upgrade_deps=need_pip) else: self._env_dir = env_dir - venv.create(self.env_dir, symlinks=True, with_pip=with_pip) + venv.create(self.env_dir, symlinks=True, with_pip=need_pip, upgrade_deps=need_pip) @property def env_dir(self): diff --git a/test/test_index.py b/test/test_index.py index fa64b82a2..206023bfe 100644 --- a/test/test_index.py +++ b/test/test_index.py @@ -1060,7 +1060,7 @@ def test_hook_uses_shell_not_from_cwd(self, rw_dir, case): # from a venv may not run when copied outside of it, and a global interpreter # won't run when copied to a different location if it was installed from the # Microsoft Store. So we make a new venv in rw_dir and use its interpreter. - venv = VirtualEnvironment(rw_dir, with_pip=False) + venv = VirtualEnvironment(rw_dir, need_pip=False) shutil.copy(venv.python, Path(rw_dir, shell_name)) shutil.copy(fixture_path("polyglot"), hook_path("polyglot", repo.git_dir)) payload = Path(rw_dir, "payload.txt") diff --git a/test/test_installation.py b/test/test_installation.py index ae6472e98..38c0c45b6 100644 --- a/test/test_installation.py +++ b/test/test_installation.py @@ -64,7 +64,7 @@ def test_installation(self, rw_dir): @staticmethod def _set_up_venv(rw_dir): - venv = VirtualEnvironment(rw_dir, with_pip=True) + venv = VirtualEnvironment(rw_dir, need_pip=True) os.symlink( os.path.dirname(os.path.dirname(__file__)), venv.sources, From a262a06634ccb616f061fdae7cf67b00a92f0ba6 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Mon, 11 Mar 2024 13:16:15 -0400 Subject: [PATCH 10/20] Upgrade test fixture pip in venv without upgrade_deps Because the upgrade_deps parameter to venv.create, as well as related functionality such as the EnvBuilder.upgrade_dependencies method that it uses, are only available starting in Python 3.9. This also puts the name of the VirtualEnvironment.__init__ parameter for setting up pip in the test fixture virtual environment back from need_pip to with_pip, which may be more intuitive. --- test/lib/helper.py | 15 ++++++++++++--- test/test_index.py | 2 +- test/test_installation.py | 2 +- 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/test/lib/helper.py b/test/lib/helper.py index cf7b37b08..27586c2b0 100644 --- a/test/lib/helper.py +++ b/test/lib/helper.py @@ -10,6 +10,8 @@ import logging import os import os.path as osp +import subprocess +import sys import tempfile import textwrap import time @@ -403,13 +405,20 @@ class VirtualEnvironment: __slots__ = ("_env_dir",) - def __init__(self, env_dir, *, need_pip): + def __init__(self, env_dir, *, with_pip): if os.name == "nt": self._env_dir = osp.realpath(env_dir) - venv.create(self.env_dir, symlinks=False, with_pip=need_pip, upgrade_deps=need_pip) + venv.create(self.env_dir, symlinks=False, with_pip=with_pip) else: self._env_dir = env_dir - venv.create(self.env_dir, symlinks=True, with_pip=need_pip, upgrade_deps=need_pip) + venv.create(self.env_dir, symlinks=True, with_pip=with_pip) + + if with_pip: + # The upgrade_deps parameter to venv.create is 3.9+ only, so do it this way. + command = [self.python, "-m", "pip", "install", "--upgrade", "pip"] + if sys.version_info < (3, 12): + command.append("setuptools") + subprocess.check_output(command) @property def env_dir(self): diff --git a/test/test_index.py b/test/test_index.py index 206023bfe..fa64b82a2 100644 --- a/test/test_index.py +++ b/test/test_index.py @@ -1060,7 +1060,7 @@ def test_hook_uses_shell_not_from_cwd(self, rw_dir, case): # from a venv may not run when copied outside of it, and a global interpreter # won't run when copied to a different location if it was installed from the # Microsoft Store. So we make a new venv in rw_dir and use its interpreter. - venv = VirtualEnvironment(rw_dir, need_pip=False) + venv = VirtualEnvironment(rw_dir, with_pip=False) shutil.copy(venv.python, Path(rw_dir, shell_name)) shutil.copy(fixture_path("polyglot"), hook_path("polyglot", repo.git_dir)) payload = Path(rw_dir, "payload.txt") diff --git a/test/test_installation.py b/test/test_installation.py index 38c0c45b6..ae6472e98 100644 --- a/test/test_installation.py +++ b/test/test_installation.py @@ -64,7 +64,7 @@ def test_installation(self, rw_dir): @staticmethod def _set_up_venv(rw_dir): - venv = VirtualEnvironment(rw_dir, need_pip=True) + venv = VirtualEnvironment(rw_dir, with_pip=True) os.symlink( os.path.dirname(os.path.dirname(__file__)), venv.sources, From 0b7f15945cc5adeb79ca9e25abafdbf8c565d716 Mon Sep 17 00:00:00 2001 From: Jirka Date: Mon, 11 Mar 2024 09:54:30 +0100 Subject: [PATCH 11/20] lint: replace `flake8` with `ruff` check --- .flake8 | 26 -------------------------- .pre-commit-config.yaml | 13 ++++++------- README.md | 2 +- pyproject.toml | 38 +++++++++++++++++++++++++++++++++++++- requirements-dev.txt | 2 -- 5 files changed, 44 insertions(+), 37 deletions(-) delete mode 100644 .flake8 diff --git a/.flake8 b/.flake8 deleted file mode 100644 index 1cc049a69..000000000 --- a/.flake8 +++ /dev/null @@ -1,26 +0,0 @@ -[flake8] - -show-source = True -count = True -statistics = True - -# E266 = too many leading '#' for block comment -# E731 = do not assign a lambda expression, use a def -# TC002 = move third party import to TYPE_CHECKING -# TC, TC2 = flake8-type-checking - -# select = C,E,F,W ANN, TC, TC2 # to enable code. Disabled if not listed, including builtin codes -enable-extensions = TC, TC2 # only needed for extensions not enabled by default - -ignore = E266, E731 - -exclude = .tox, .venv, build, dist, doc, git/ext/ - -rst-roles = # for flake8-RST-docstrings - attr, class, func, meth, mod, obj, ref, term, var # used by sphinx - -min-python-version = 3.7.0 - -# for `black` compatibility -max-line-length = 120 -extend-ignore = E203, W503 diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 1ac5baa00..cd5f58441 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -14,14 +14,13 @@ repos: name: black (format) exclude: ^git/ext/ -- repo: https://github.com/PyCQA/flake8 - rev: 6.1.0 +- repo: https://github.com/astral-sh/ruff-pre-commit + rev: v0.3.0 hooks: - - id: flake8 - additional_dependencies: - - flake8-bugbear==23.9.16 - - flake8-comprehensions==3.14.0 - - flake8-typing-imports==1.14.0 + #- id: ruff-format # todo: eventually replace Black with Ruff for consistency + # args: ["--preview"] + - id: ruff + args: ["--fix"] exclude: ^doc|^git/ext/ - repo: https://github.com/shellcheck-py/shellcheck-py diff --git a/README.md b/README.md index 7faeae23b..1e4a59d7f 100644 --- a/README.md +++ b/README.md @@ -187,7 +187,7 @@ The same linting, and running tests on all the different supported Python versio Specific tools: - Configurations for `mypy`, `pytest`, `coverage.py`, and `black` are in `./pyproject.toml`. -- Configuration for `flake8` is in the `./.flake8` file. +- Configuration for `ruff` is in the `pyproject.toml` file. Orchestration tools: diff --git a/pyproject.toml b/pyproject.toml index 7109389d7..230a9cf3a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -29,7 +29,6 @@ warn_unreachable = true show_error_codes = true implicit_reexport = true # strict = true - # TODO: Remove when 'gitdb' is fully annotated. exclude = ["^git/ext/gitdb"] [[tool.mypy.overrides]] @@ -47,3 +46,40 @@ omit = ["*/git/ext/*"] line-length = 120 target-version = ["py37"] extend-exclude = "git/ext/gitdb" + +[tool.ruff] +target-version = "py37" +line-length = 120 +# Exclude a variety of commonly ignored directories. +exclude = [ + "git/ext/", + "doc", + "build", + "dist", +] +# Enable Pyflakes `E` and `F` codes by default. +lint.select = [ + "E", + "W", # see: https://pypi.org/project/pycodestyle + "F", # see: https://pypi.org/project/pyflakes +# "I", #see: https://pypi.org/project/isort/ +# "S", # see: https://pypi.org/project/flake8-bandit +# "UP", # see: https://docs.astral.sh/ruff/rules/#pyupgrade-up +] +lint.extend-select = [ + "A", # see: https://pypi.org/project/flake8-builtins + "B", # see: https://pypi.org/project/flake8-bugbear + "C4", # see: https://pypi.org/project/flake8-comprehensions + "TCH004", # see: https://docs.astral.sh/ruff/rules/runtime-import-in-type-checking-block/ +] +lint.ignore = [ + "E203", "W503" +] +lint.ignore-init-module-imports = true +lint.unfixable = ["F401"] + +#[tool.ruff.lint.per-file-ignores] +#"setup.py" = ["ANN202", "ANN401"] +#"docs/source/conf.py" = ["A001", "D103"] +#"src/**" = ["ANN401"] +#"tests/**" = ["S101", "ANN001", "ANN201", "ANN202", "ANN401"] diff --git a/requirements-dev.txt b/requirements-dev.txt index e3030c597..69a79d13d 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -3,7 +3,5 @@ # libraries for additional local testing/linting - to be added to test-requirements.txt when all pass -flake8-type-checking;python_version>="3.8" # checks for TYPE_CHECKING only imports - pytest-icdiff # pytest-profiling From aa9298a37f1bbdecfbfe9b13026c81374abdc2d7 Mon Sep 17 00:00:00 2001 From: Jirka Date: Mon, 11 Mar 2024 11:10:00 +0100 Subject: [PATCH 12/20] fixing lints / noqa --- git/index/base.py | 8 ++++---- git/objects/submodule/base.py | 2 +- git/objects/util.py | 2 +- git/refs/symbolic.py | 4 ++-- pyproject.toml | 12 +++++------- 5 files changed, 13 insertions(+), 15 deletions(-) diff --git a/git/index/base.py b/git/index/base.py index 249144d54..985b1bccf 100644 --- a/git/index/base.py +++ b/git/index/base.py @@ -248,7 +248,7 @@ def write( # Make sure we have our entries read before getting a write lock. # Otherwise it would be done when streaming. # This can happen if one doesn't change the index, but writes it right away. - self.entries + self.entries # noqa: B018 lfd = LockedFD(file_path or self._file_path) stream = lfd.open(write=True, stream=True) @@ -397,7 +397,7 @@ def from_tree(cls, repo: "Repo", *treeish: Treeish, **kwargs: Any) -> "IndexFile with TemporaryFileSwap(join_path_native(repo.git_dir, "index")): repo.git.read_tree(*arg_list, **kwargs) index = cls(repo, tmp_index) - index.entries # Force it to read the file as we will delete the temp-file. + index.entries # noqa: B018 # Force it to read the file as we will delete the temp-file. return index # END index merge handling @@ -1339,7 +1339,7 @@ def handle_stderr(proc: "Popen[bytes]", iter_checked_out_files: Iterable[PathLik # Make sure we have our entries loaded before we start checkout_index, which # will hold a lock on it. We try to get the lock as well during our entries # initialization. - self.entries + self.entries # noqa: B018 args.append("--stdin") kwargs["as_process"] = True @@ -1379,7 +1379,7 @@ def handle_stderr(proc: "Popen[bytes]", iter_checked_out_files: Iterable[PathLik self._flush_stdin_and_wait(proc, ignore_stdout=True) except GitCommandError: # Without parsing stdout we don't know what failed. - raise CheckoutError( + raise CheckoutError( # noqa: B904 "Some files could not be checked out from the index, probably because they didn't exist.", failed_files, [], diff --git a/git/objects/submodule/base.py b/git/objects/submodule/base.py index cdd7c8e1b..b3e681e94 100644 --- a/git/objects/submodule/base.py +++ b/git/objects/submodule/base.py @@ -1445,7 +1445,7 @@ def exists(self) -> bool: try: try: - self.path + self.path # noqa: B018 return True except Exception: return False diff --git a/git/objects/util.py b/git/objects/util.py index 6f4e7d087..71eb9c230 100644 --- a/git/objects/util.py +++ b/git/objects/util.py @@ -439,7 +439,7 @@ def _list_traverse( if not as_edge: out: IterableList[Union["Commit", "Submodule", "Tree", "Blob"]] = IterableList(id) - out.extend(self.traverse(as_edge=as_edge, *args, **kwargs)) + out.extend(self.traverse(as_edge=as_edge, *args, **kwargs)) # noqa: B026 return out # Overloads in subclasses (mypy doesn't allow typing self: subclass). # Union[IterableList['Commit'], IterableList['Submodule'], IterableList[Union['Submodule', 'Tree', 'Blob']]] diff --git a/git/refs/symbolic.py b/git/refs/symbolic.py index 16aada0a7..465acf872 100644 --- a/git/refs/symbolic.py +++ b/git/refs/symbolic.py @@ -496,7 +496,7 @@ def is_valid(self) -> bool: valid object or reference. """ try: - self.object + self.object # noqa: B018 except (OSError, ValueError): return False else: @@ -510,7 +510,7 @@ def is_detached(self) -> bool: instead to another reference. """ try: - self.ref + self.ref # noqa: B018 return False except TypeError: return True diff --git a/pyproject.toml b/pyproject.toml index 230a9cf3a..af0e52ca4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -67,19 +67,17 @@ lint.select = [ # "UP", # see: https://docs.astral.sh/ruff/rules/#pyupgrade-up ] lint.extend-select = [ - "A", # see: https://pypi.org/project/flake8-builtins + #"A", # see: https://pypi.org/project/flake8-builtins "B", # see: https://pypi.org/project/flake8-bugbear "C4", # see: https://pypi.org/project/flake8-comprehensions "TCH004", # see: https://docs.astral.sh/ruff/rules/runtime-import-in-type-checking-block/ ] lint.ignore = [ - "E203", "W503" + "E203", + "E731", # Do not assign a `lambda` expression, use a `def` ] lint.ignore-init-module-imports = true lint.unfixable = ["F401"] -#[tool.ruff.lint.per-file-ignores] -#"setup.py" = ["ANN202", "ANN401"] -#"docs/source/conf.py" = ["A001", "D103"] -#"src/**" = ["ANN401"] -#"tests/**" = ["S101", "ANN001", "ANN201", "ANN202", "ANN401"] +[tool.ruff.lint.per-file-ignores] +"test/**" = ["B018"] From 5f889c4137d1d4442ab3ceb8e45c3c03cafded1a Mon Sep 17 00:00:00 2001 From: Jirka Date: Mon, 11 Mar 2024 11:18:06 +0100 Subject: [PATCH 13/20] try: from typing import Literal --- git/objects/blob.py | 6 +++++- git/objects/commit.py | 7 ++++++- git/objects/submodule/base.py | 7 ++++++- git/objects/tag.py | 5 ++++- git/objects/tree.py | 7 ++++++- 5 files changed, 27 insertions(+), 5 deletions(-) diff --git a/git/objects/blob.py b/git/objects/blob.py index 253ceccb5..4035c3e7c 100644 --- a/git/objects/blob.py +++ b/git/objects/blob.py @@ -6,7 +6,11 @@ from mimetypes import guess_type from . import base -from git.types import Literal + +try: + from typing import Literal +except ImportError: + from typing_extensions import Literal __all__ = ("Blob",) diff --git a/git/objects/commit.py b/git/objects/commit.py index 06ab0898b..dcb3be695 100644 --- a/git/objects/commit.py +++ b/git/objects/commit.py @@ -44,7 +44,12 @@ Dict, ) -from git.types import PathLike, Literal +from git.types import PathLike + +try: + from typing import Literal +except ImportError: + from typing_extensions import Literal if TYPE_CHECKING: from git.repo import Repo diff --git a/git/objects/submodule/base.py b/git/objects/submodule/base.py index b3e681e94..e5933b116 100644 --- a/git/objects/submodule/base.py +++ b/git/objects/submodule/base.py @@ -44,7 +44,12 @@ from typing import Callable, Dict, Mapping, Sequence, TYPE_CHECKING, cast from typing import Any, Iterator, Union -from git.types import Commit_ish, Literal, PathLike, TBD +from git.types import Commit_ish, PathLike, TBD + +try: + from typing import Literal +except ImportError: + from typing_extensions import Literal if TYPE_CHECKING: from git.index import IndexFile diff --git a/git/objects/tag.py b/git/objects/tag.py index f455c55fc..d8815e436 100644 --- a/git/objects/tag.py +++ b/git/objects/tag.py @@ -16,7 +16,10 @@ from typing import List, TYPE_CHECKING, Union -from git.types import Literal +try: + from typing import Literal +except ImportError: + from typing_extensions import Literal if TYPE_CHECKING: from git.repo import Repo diff --git a/git/objects/tree.py b/git/objects/tree.py index a506bba7d..731ab5fa1 100644 --- a/git/objects/tree.py +++ b/git/objects/tree.py @@ -31,7 +31,12 @@ TYPE_CHECKING, ) -from git.types import PathLike, Literal +from git.types import PathLike + +try: + from typing import Literal +except ImportError: + from typing_extensions import Literal if TYPE_CHECKING: from git.repo import Repo From 517f83aae949511fcc0696d4b67392d024acded1 Mon Sep 17 00:00:00 2001 From: Jirka Date: Mon, 11 Mar 2024 21:33:19 +0100 Subject: [PATCH 14/20] lint: switch Black with `ruff-format` --- .github/workflows/lint.yml | 2 -- .pre-commit-config.yaml | 18 ++---------------- Makefile | 2 +- pyproject.toml | 5 ----- test-requirements.txt | 1 - tox.ini | 2 -- 6 files changed, 3 insertions(+), 27 deletions(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index d805124fc..a32fb6c4e 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -16,5 +16,3 @@ jobs: - uses: pre-commit/action@v3.0.1 with: extra_args: --all-files --hook-stage manual - env: - SKIP: black-format diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index cd5f58441..60bbe3518 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,24 +1,10 @@ repos: -- repo: https://github.com/psf/black-pre-commit-mirror - rev: 23.9.1 - hooks: - - id: black - alias: black-check - name: black (check) - args: [--check, --diff] - exclude: ^git/ext/ - stages: [manual] - - - id: black - alias: black-format - name: black (format) - exclude: ^git/ext/ - repo: https://github.com/astral-sh/ruff-pre-commit rev: v0.3.0 hooks: - #- id: ruff-format # todo: eventually replace Black with Ruff for consistency - # args: ["--preview"] + - id: ruff-format + exclude: ^git/ext/ - id: ruff args: ["--fix"] exclude: ^doc|^git/ext/ diff --git a/Makefile b/Makefile index 839dc9f78..71a370ef2 100644 --- a/Makefile +++ b/Makefile @@ -4,7 +4,7 @@ all: @awk -F: '/^[[:alpha:]].*:/ && !/^all:/ {print $$1}' Makefile lint: - SKIP=black-format pre-commit run --all-files --hook-stage manual + SKIP=pre-commit run --all-files --hook-stage manual clean: rm -rf build/ dist/ .eggs/ .tox/ diff --git a/pyproject.toml b/pyproject.toml index af0e52ca4..eb57cc7b7 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -42,11 +42,6 @@ source = ["git"] include = ["*/git/*"] omit = ["*/git/ext/*"] -[tool.black] -line-length = 120 -target-version = ["py37"] -extend-exclude = "git/ext/gitdb" - [tool.ruff] target-version = "py37" line-length = 120 diff --git a/test-requirements.txt b/test-requirements.txt index 7cfb977a1..e1f5e2ed4 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -1,4 +1,3 @@ -black coverage[toml] ddt >= 1.1.1, != 1.4.3 mock ; python_version < "3.8" diff --git a/tox.ini b/tox.ini index f9ac25b78..28b7b147f 100644 --- a/tox.ini +++ b/tox.ini @@ -12,8 +12,6 @@ commands = pytest --color=yes {posargs} [testenv:lint] description = Lint via pre-commit base_python = py{39,310,311,312,38,37} -set_env = - SKIP = black-format commands = pre-commit run --all-files --hook-stage manual [testenv:mypy] From 2d0158c2c147dae90f9bbceeead1c4f322e90436 Mon Sep 17 00:00:00 2001 From: Jirka Date: Mon, 11 Mar 2024 21:36:25 +0100 Subject: [PATCH 15/20] apply `ruff-format` --- git/cmd.py | 72 ++++++++++++++++++--------------------------- git/compat.py | 18 ++++-------- git/index/fun.py | 6 ++-- git/objects/fun.py | 6 ++-- git/objects/tree.py | 2 +- git/objects/util.py | 12 +++----- git/remote.py | 11 +++---- git/util.py | 9 ++---- 8 files changed, 52 insertions(+), 84 deletions(-) diff --git a/git/cmd.py b/git/cmd.py index 731d0fab8..915f46a05 100644 --- a/git/cmd.py +++ b/git/cmd.py @@ -495,9 +495,8 @@ def refresh(cls, path: Union[None, PathLike] = None) -> bool: if mode in quiet: pass elif mode in warn or mode in error: - err = ( - dedent( - """\ + err = dedent( + """\ %s All git commands will error until this is rectified. @@ -510,16 +509,14 @@ def refresh(cls, path: Union[None, PathLike] = None) -> bool: Example: export %s=%s """ - ) - % ( - err, - cls._refresh_env_var, - "|".join(quiet), - "|".join(warn), - "|".join(error), - cls._refresh_env_var, - quiet[0], - ) + ) % ( + err, + cls._refresh_env_var, + "|".join(quiet), + "|".join(warn), + "|".join(error), + cls._refresh_env_var, + quiet[0], ) if mode in warn: @@ -527,9 +524,8 @@ def refresh(cls, path: Union[None, PathLike] = None) -> bool: else: raise ImportError(err) else: - err = ( - dedent( - """\ + err = dedent( + """\ %s environment variable has been set but it has been set with an invalid value. Use only the following values: @@ -537,13 +533,11 @@ def refresh(cls, path: Union[None, PathLike] = None) -> bool: - %s: for a warning message (logging level CRITICAL, displayed by default) - %s: for a raised exception """ - ) - % ( - cls._refresh_env_var, - "|".join(quiet), - "|".join(warn), - "|".join(error), - ) + ) % ( + cls._refresh_env_var, + "|".join(quiet), + "|".join(warn), + "|".join(error), ) raise ImportError(err) @@ -565,13 +559,11 @@ def is_cygwin(cls) -> bool: @overload @classmethod - def polish_url(cls, url: str, is_cygwin: Literal[False] = ...) -> str: - ... + def polish_url(cls, url: str, is_cygwin: Literal[False] = ...) -> str: ... @overload @classmethod - def polish_url(cls, url: str, is_cygwin: Union[None, bool] = None) -> str: - ... + def polish_url(cls, url: str, is_cygwin: Union[None, bool] = None) -> str: ... @classmethod def polish_url(cls, url: str, is_cygwin: Union[None, bool] = None) -> PathLike: @@ -932,8 +924,7 @@ def execute( command: Union[str, Sequence[Any]], *, as_process: Literal[True], - ) -> "AutoInterrupt": - ... + ) -> "AutoInterrupt": ... @overload def execute( @@ -942,8 +933,7 @@ def execute( *, as_process: Literal[False] = False, stdout_as_string: Literal[True], - ) -> Union[str, Tuple[int, str, str]]: - ... + ) -> Union[str, Tuple[int, str, str]]: ... @overload def execute( @@ -952,8 +942,7 @@ def execute( *, as_process: Literal[False] = False, stdout_as_string: Literal[False] = False, - ) -> Union[bytes, Tuple[int, bytes, str]]: - ... + ) -> Union[bytes, Tuple[int, bytes, str]]: ... @overload def execute( @@ -963,8 +952,7 @@ def execute( with_extended_output: Literal[False], as_process: Literal[False], stdout_as_string: Literal[True], - ) -> str: - ... + ) -> str: ... @overload def execute( @@ -974,8 +962,7 @@ def execute( with_extended_output: Literal[False], as_process: Literal[False], stdout_as_string: Literal[False], - ) -> bytes: - ... + ) -> bytes: ... def execute( self, @@ -1387,8 +1374,9 @@ def __call__(self, **kwargs: Any) -> "Git": return self @overload - def _call_process(self, method: str, *args: None, **kwargs: None) -> str: - ... # If no args were given, execute the call with all defaults. + def _call_process( + self, method: str, *args: None, **kwargs: None + ) -> str: ... # If no args were given, execute the call with all defaults. @overload def _call_process( @@ -1398,14 +1386,12 @@ def _call_process( as_process: Literal[True], *args: Any, **kwargs: Any, - ) -> "Git.AutoInterrupt": - ... + ) -> "Git.AutoInterrupt": ... @overload def _call_process( self, method: str, *args: Any, **kwargs: Any - ) -> Union[str, bytes, Tuple[int, Union[str, bytes], str], "Git.AutoInterrupt"]: - ... + ) -> Union[str, bytes, Tuple[int, Union[str, bytes], str], "Git.AutoInterrupt"]: ... def _call_process( self, method: str, *args: Any, **kwargs: Any diff --git a/git/compat.py b/git/compat.py index 7753fe8b2..e64c645c7 100644 --- a/git/compat.py +++ b/git/compat.py @@ -69,13 +69,11 @@ @overload -def safe_decode(s: None) -> None: - ... +def safe_decode(s: None) -> None: ... @overload -def safe_decode(s: AnyStr) -> str: - ... +def safe_decode(s: AnyStr) -> str: ... def safe_decode(s: Union[AnyStr, None]) -> Optional[str]: @@ -91,13 +89,11 @@ def safe_decode(s: Union[AnyStr, None]) -> Optional[str]: @overload -def safe_encode(s: None) -> None: - ... +def safe_encode(s: None) -> None: ... @overload -def safe_encode(s: AnyStr) -> bytes: - ... +def safe_encode(s: AnyStr) -> bytes: ... def safe_encode(s: Optional[AnyStr]) -> Optional[bytes]: @@ -113,13 +109,11 @@ def safe_encode(s: Optional[AnyStr]) -> Optional[bytes]: @overload -def win_encode(s: None) -> None: - ... +def win_encode(s: None) -> None: ... @overload -def win_encode(s: AnyStr) -> bytes: - ... +def win_encode(s: AnyStr) -> bytes: ... def win_encode(s: Optional[AnyStr]) -> Optional[bytes]: diff --git a/git/index/fun.py b/git/index/fun.py index 58335739e..beca67d3f 100644 --- a/git/index/fun.py +++ b/git/index/fun.py @@ -286,9 +286,9 @@ def read_cache( # 4 bytes length of chunk # Repeated 0 - N times extension_data = stream.read(~0) - assert ( - len(extension_data) > 19 - ), "Index Footer was not at least a sha on content as it was only %i bytes in size" % len(extension_data) + assert len(extension_data) > 19, ( + "Index Footer was not at least a sha on content as it was only %i bytes in size" % len(extension_data) + ) content_sha = extension_data[-20:] diff --git a/git/objects/fun.py b/git/objects/fun.py index 22b99cb6b..5bd8a3d62 100644 --- a/git/objects/fun.py +++ b/git/objects/fun.py @@ -152,13 +152,11 @@ def _find_by_name(tree_data: MutableSequence[EntryTupOrNone], name: str, is_dir: @overload -def _to_full_path(item: None, path_prefix: str) -> None: - ... +def _to_full_path(item: None, path_prefix: str) -> None: ... @overload -def _to_full_path(item: EntryTup, path_prefix: str) -> EntryTup: - ... +def _to_full_path(item: EntryTup, path_prefix: str) -> EntryTup: ... def _to_full_path(item: EntryTupOrNone, path_prefix: str) -> EntryTupOrNone: diff --git a/git/objects/tree.py b/git/objects/tree.py index 731ab5fa1..3964b016c 100644 --- a/git/objects/tree.py +++ b/git/objects/tree.py @@ -188,7 +188,7 @@ class Tree(IndexObject, git_diff.Diffable, util.Traversable, util.Serializable): _map_id_to_type: Dict[int, Type[IndexObjUnion]] = { commit_id: Submodule, blob_id: Blob, - symlink_id: Blob + symlink_id: Blob, # Tree ID added once Tree is defined. } diff --git a/git/objects/util.py b/git/objects/util.py index 71eb9c230..26a34f94c 100644 --- a/git/objects/util.py +++ b/git/objects/util.py @@ -620,8 +620,7 @@ def list_traverse(self: T_TIobj, *args: Any, **kwargs: Any) -> IterableList[T_TI return super()._list_traverse(*args, **kwargs) @overload # type: ignore - def traverse(self: T_TIobj) -> Iterator[T_TIobj]: - ... + def traverse(self: T_TIobj) -> Iterator[T_TIobj]: ... @overload def traverse( @@ -633,8 +632,7 @@ def traverse( visit_once: bool, ignore_self: Literal[True], as_edge: Literal[False], - ) -> Iterator[T_TIobj]: - ... + ) -> Iterator[T_TIobj]: ... @overload def traverse( @@ -646,8 +644,7 @@ def traverse( visit_once: bool, ignore_self: Literal[False], as_edge: Literal[True], - ) -> Iterator[Tuple[Union[T_TIobj, None], T_TIobj]]: - ... + ) -> Iterator[Tuple[Union[T_TIobj, None], T_TIobj]]: ... @overload def traverse( @@ -659,8 +656,7 @@ def traverse( visit_once: bool, ignore_self: Literal[True], as_edge: Literal[True], - ) -> Iterator[Tuple[T_TIobj, T_TIobj]]: - ... + ) -> Iterator[Tuple[T_TIobj, T_TIobj]]: ... def traverse( self: T_TIobj, diff --git a/git/remote.py b/git/remote.py index fd4de7100..b63cfc208 100644 --- a/git/remote.py +++ b/git/remote.py @@ -93,22 +93,19 @@ def add_progress( @overload -def to_progress_instance(progress: None) -> RemoteProgress: - ... +def to_progress_instance(progress: None) -> RemoteProgress: ... @overload -def to_progress_instance(progress: Callable[..., Any]) -> CallableRemoteProgress: - ... +def to_progress_instance(progress: Callable[..., Any]) -> CallableRemoteProgress: ... @overload -def to_progress_instance(progress: RemoteProgress) -> RemoteProgress: - ... +def to_progress_instance(progress: RemoteProgress) -> RemoteProgress: ... def to_progress_instance( - progress: Union[Callable[..., Any], RemoteProgress, None] + progress: Union[Callable[..., Any], RemoteProgress, None], ) -> Union[RemoteProgress, CallableRemoteProgress]: """Given the `progress` return a suitable object derived from :class:`~git.util.RemoteProgress`.""" diff --git a/git/util.py b/git/util.py index 9d451eee2..27751f687 100644 --- a/git/util.py +++ b/git/util.py @@ -441,13 +441,11 @@ def decygpath(path: PathLike) -> str: @overload -def is_cygwin_git(git_executable: None) -> Literal[False]: - ... +def is_cygwin_git(git_executable: None) -> Literal[False]: ... @overload -def is_cygwin_git(git_executable: PathLike) -> bool: - ... +def is_cygwin_git(git_executable: PathLike) -> bool: ... def is_cygwin_git(git_executable: Union[None, PathLike]) -> bool: @@ -494,8 +492,7 @@ def finalize_process(proc: Union[subprocess.Popen, "Git.AutoInterrupt"], **kwarg @overload -def expand_path(p: None, expand_vars: bool = ...) -> None: - ... +def expand_path(p: None, expand_vars: bool = ...) -> None: ... @overload From 1b8812a968a76f47acdf80bc1d15e6b0dd5d84bf Mon Sep 17 00:00:00 2001 From: Jirka Date: Tue, 12 Mar 2024 21:08:54 +0100 Subject: [PATCH 16/20] drop `make lint` --- Makefile | 5 +---- README.md | 3 --- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/Makefile b/Makefile index 71a370ef2..d4f9acf87 100644 --- a/Makefile +++ b/Makefile @@ -1,11 +1,8 @@ -.PHONY: all lint clean release force_release +.PHONY: all clean release force_release all: @awk -F: '/^[[:alpha:]].*:/ && !/^all:/ {print $$1}' Makefile -lint: - SKIP=pre-commit run --all-files --hook-stage manual - clean: rm -rf build/ dist/ .eggs/ .tox/ diff --git a/README.md b/README.md index 1e4a59d7f..33e093945 100644 --- a/README.md +++ b/README.md @@ -165,9 +165,6 @@ To lint, and apply automatic code formatting, run: pre-commit run --all-files ``` -- Linting without modifying code can be done with: `make lint` -- Auto-formatting without other lint checks can be done with: `black .` - To typecheck, run: ```bash From 395b70ae2fdf01590f157e94d96e7ddac7893ba9 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Tue, 12 Mar 2024 12:12:27 -0400 Subject: [PATCH 17/20] Very slightly improve readme presentation - Add a missing period. - Indicate sh rather than bash as as the language for syntax highlighting of shell commands that don't need bash. I had held off on making that second change in previous revisions because it would have involved either introducing an inconsistency or editing the section giving the deprecated signature-checking instructions. That section was removed in 2671167 (#1823). (This also wraps a paragraph where the immediately surrounding text was wrapped, but that should not affect rendered text, and broader consistency improvements to Markdown wrapping style are not done.) --- README.md | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index 33e093945..458162212 100644 --- a/README.md +++ b/README.md @@ -17,7 +17,7 @@ probably the skills to scratch that itch of mine: implement `git` in a way that If you like the idea and want to learn more, please head over to [gitoxide](https://github.com/Byron/gitoxide), an implementation of 'git' in [Rust](https://www.rust-lang.org). -*(Please note that `gitoxide` is not currently available for use in Python, and that Rust is required)* +*(Please note that `gitoxide` is not currently available for use in Python, and that Rust is required.)* ## GitPython @@ -39,9 +39,9 @@ The project is open to contributions of all kinds, as well as new maintainers. ### REQUIREMENTS -GitPython needs the `git` executable to be installed on the system and available in your `PATH` for most operations. -If it is not in your `PATH`, you can help GitPython find it by setting -the `GIT_PYTHON_GIT_EXECUTABLE=` environment variable. +GitPython needs the `git` executable to be installed on the system and available in your +`PATH` for most operations. If it is not in your `PATH`, you can help GitPython find it +by setting the `GIT_PYTHON_GIT_EXECUTABLE=` environment variable. - Git (1.7.x or newer) - Python >= 3.7 @@ -57,7 +57,7 @@ GitPython and its required package dependencies can be installed in any of the f To obtain and install a copy [from PyPI](https://pypi.org/project/GitPython/), run: -```bash +```sh pip install GitPython ``` @@ -67,7 +67,7 @@ pip install GitPython If you have downloaded the source code, run this from inside the unpacked `GitPython` directory: -```bash +```sh pip install . ``` @@ -75,7 +75,7 @@ pip install . To clone the [the GitHub repository](https://github.com/gitpython-developers/GitPython) from source to work on the code, you can do it like so: -```bash +```sh git clone https://github.com/gitpython-developers/GitPython cd GitPython ./init-tests-after-clone.sh @@ -85,7 +85,7 @@ On Windows, `./init-tests-after-clone.sh` can be run in a Git Bash shell. If you are cloning [your own fork](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/about-forks), then replace the above `git clone` command with one that gives the URL of your fork. Or use this [`gh`](https://cli.github.com/) command (assuming you have `gh` and your fork is called `GitPython`): -```bash +```sh gh repo clone GitPython ``` @@ -93,7 +93,7 @@ Having cloned the repo, create and activate your [virtual environment](https://d Then make an [editable install](https://pip.pypa.io/en/stable/topics/local-project-installs/#editable-installs): -```bash +```sh pip install -e ".[test]" ``` @@ -105,7 +105,7 @@ In rare cases, you may want to work on GitPython and one or both of its [gitdb]( If you want to do that *and* you want the versions in GitPython's git submodules to be used, then pass `-e git/ext/gitdb` and/or `-e git/ext/gitdb/gitdb/ext/smmap` to `pip install`. This can be done in any order, and in separate `pip install` commands or the same one, so long as `-e` appears before *each* path. For example, you can install GitPython, gitdb, and smmap editably in the currently active virtual environment this way: -```bash +```sh pip install -e ".[test]" -e git/ext/gitdb -e git/ext/gitdb/gitdb/ext/smmap ``` @@ -141,13 +141,13 @@ you will encounter test failures. Ensure testing libraries are installed. This is taken care of already if you installed with: -```bash +```sh pip install -e ".[test]" ``` Otherwise, you can run: -```bash +```sh pip install -r test-requirements.txt ``` @@ -155,19 +155,19 @@ pip install -r test-requirements.txt To test, run: -```bash +```sh pytest ``` To lint, and apply automatic code formatting, run: -```bash +```sh pre-commit run --all-files ``` To typecheck, run: -```bash +```sh mypy -p git ``` From 3a6ee9e0e478eea5f6defe2b851a7ca6c74976ca Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Tue, 12 Mar 2024 12:31:06 -0400 Subject: [PATCH 18/20] Make installation instructions more consistent If people who want to run the tests didn't install the test extra, they can still install that extra. This simplifies the instructions accordingly. test-requirements.txt is still mentioned near the beginning in case people want to look at it to see dependencies. But the changed code is the only place where the instructions had still said to do anything with those files. A possible disadvantage of this change is that in the rare case that someone following those instructions to run the tests locally didn't do an editable installation, then installing with the extra shouldn't be done editably either. But this doesn't seem like a likely problem, and the new text has an example command with -e to clarify the kind of command whose effect is augmented here. Because of that subtlety, it is not obvious that this change is really justified for purposes of clarity. However, this also helps prepare for a time when test-requirements.txt won't exist anymore, as may happen when the project definition is made fully declarative (see discussion in comments in #1716). --- README.md | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 458162212..ead8093f8 100644 --- a/README.md +++ b/README.md @@ -145,11 +145,8 @@ Ensure testing libraries are installed. This is taken care of already if you ins pip install -e ".[test]" ``` -Otherwise, you can run: - -```sh -pip install -r test-requirements.txt -``` +If you had installed with a command like `pip install -e .` instead, you can still run +the above command to add the testing dependencies. #### Test commands From 826234384d38126130388102355a274a353bfade Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Tue, 12 Mar 2024 12:50:41 -0400 Subject: [PATCH 19/20] Update readme for recent tooling changes This also reorganizes the "Specific tools" list, since they are all configured in pyproject.toml now (only flake8 was not before, and it was removed in favor of ruff in #1862). In doing so, I've also added brief parenthesized phrases to characterize what each of these four tools is for, so readers don't have to look around as much to understand most of the tooling GitPython has set up. --- README.md | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index ead8093f8..30af532db 100644 --- a/README.md +++ b/README.md @@ -156,12 +156,14 @@ To test, run: pytest ``` -To lint, and apply automatic code formatting, run: +To lint, and apply some linting fixes as well as automatic code formatting, run: ```sh pre-commit run --all-files ``` +This includes the linting and autoformatting done by Ruff, as well as some other checks. + To typecheck, run: ```sh @@ -170,7 +172,7 @@ mypy -p git #### CI (and tox) -The same linting, and running tests on all the different supported Python versions, will be performed: +Style and formatting checks, and running tests on all the different supported Python versions, will be performed: - Upon submitting a pull request. - On each push, *if* you have a fork with GitHub Actions enabled. @@ -178,10 +180,12 @@ The same linting, and running tests on all the different supported Python versio #### Configuration files -Specific tools: +Specific tools are all configured in the `./pyproject.toml` file: -- Configurations for `mypy`, `pytest`, `coverage.py`, and `black` are in `./pyproject.toml`. -- Configuration for `ruff` is in the `pyproject.toml` file. +- `pytest` (test runner) +- `coverage.py` (code coverage) +- `ruff` (linter and formatter) +- `mypy` (type checker) Orchestration tools: From b059cd580b71b44488bf3cc77000a6dea3cb1898 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Tue, 12 Mar 2024 13:27:34 -0400 Subject: [PATCH 20/20] Have tox skip linting unless requested, for now This is to make it so simple `tox` usage has the expected property of leaving all source code files in the working tree unchanged. Linting how sometimes performs auto-fixes since #1862, and the pre-commit command in tox.ini, which had also run `black --check`, will do even more file editing due to the changes in #1865. The bifurcation for black into separate mutating and non-mutating hooks, introduced in 5d8ddd9 (#1693), was not carried over into Ruff autoformatting in #1865. But also it: - Was not necessarily a good approach, and likely should not be preserved in any form. It was an unusual and unintuitive use of pre-commit. (It can be brought back if no better approach is found, though.) - Was done to avoid a situation where it was nontrivial to set up necessary dependencies for linting in the GitPython virtual environment itself, because flake8 and its various plugins would have to be installed. They were not listed in any existing or newly introduced extra (for example, they were not added to test-requirements.txt) in part in the hope that they would all be replaced by Ruff, which happened in #1862. - Already did not achieve its goal as of #1862, since it was (probably rightly) not extended to Ruff linting to use/omit --fix. Now that Ruff is being used, people can run `pip install ruff` in a virtual environment, then run the `ruff` command however they like. This takes the place of multiple tools and plugins. The situation with the tox "lint" environment is thus now similar to that of the tox "html" environment when it was added in e6ec6c8 (#1667), until it was improved in f094909 (#1693) to run with proper isolation. --- tox.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index 28b7b147f..6e02e5aee 100644 --- a/tox.ini +++ b/tox.ini @@ -1,6 +1,6 @@ [tox] requires = tox>=4 -env_list = py{37,38,39,310,311,312}, lint, mypy, html +env_list = py{37,38,39,310,311,312}, mypy, html [testenv] description = Run unit tests