From e3f38ffe1c3e37ec5a8f18d60961ddd0cd0f62f4 Mon Sep 17 00:00:00 2001 From: George Ogden Date: Thu, 27 Nov 2025 17:32:48 +0000 Subject: [PATCH 01/18] Move clone tests into dedicated file --- test/test_clone.py | 294 ++++++++++++++++++++++++++++++++++++++++++++- test/test_repo.py | 283 +------------------------------------------ 2 files changed, 294 insertions(+), 283 deletions(-) diff --git a/test/test_clone.py b/test/test_clone.py index 126ef0063..91b7d7621 100644 --- a/test/test_clone.py +++ b/test/test_clone.py @@ -1,12 +1,23 @@ # This module is part of GitPython and is released under the # 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/ +import os +import os.path as osp +import pathlib +import sys +import tempfile +from unittest import skip + +from git import GitCommandError, Repo +from git.exc import UnsafeOptionError, UnsafeProtocolError + +from test.lib import TestBase, with_rw_directory, with_rw_repo + from pathlib import Path import re import git - -from test.lib import TestBase, with_rw_directory +import pytest class TestClone(TestBase): @@ -29,3 +40,282 @@ def test_checkout_in_non_empty_dir(self, rw_dir): ) else: self.fail("GitCommandError not raised") + + @with_rw_directory + def test_clone_from_pathlib(self, rw_dir): + original_repo = Repo.init(osp.join(rw_dir, "repo")) + + Repo.clone_from(original_repo.git_dir, pathlib.Path(rw_dir) / "clone_pathlib") + + @with_rw_directory + def test_clone_from_pathlib_withConfig(self, rw_dir): + original_repo = Repo.init(osp.join(rw_dir, "repo")) + + cloned = Repo.clone_from( + original_repo.git_dir, + pathlib.Path(rw_dir) / "clone_pathlib_withConfig", + multi_options=[ + "--recurse-submodules=repo", + "--config core.filemode=false", + "--config submodule.repo.update=checkout", + "--config filter.lfs.clean='git-lfs clean -- %f'", + ], + allow_unsafe_options=True, + ) + + self.assertEqual(cloned.config_reader().get_value("submodule", "active"), "repo") + self.assertEqual(cloned.config_reader().get_value("core", "filemode"), False) + self.assertEqual(cloned.config_reader().get_value('submodule "repo"', "update"), "checkout") + self.assertEqual( + cloned.config_reader().get_value('filter "lfs"', "clean"), + "git-lfs clean -- %f", + ) + + def test_clone_from_with_path_contains_unicode(self): + with tempfile.TemporaryDirectory() as tmpdir: + unicode_dir_name = "\u0394" + path_with_unicode = os.path.join(tmpdir, unicode_dir_name) + os.makedirs(path_with_unicode) + + try: + Repo.clone_from( + url=self._small_repo_url(), + to_path=path_with_unicode, + ) + except UnicodeEncodeError: + self.fail("Raised UnicodeEncodeError") + + @with_rw_directory + @skip( + """The referenced repository was removed, and one needs to set up a new + password controlled repo under the org's control.""" + ) + def test_leaking_password_in_clone_logs(self, rw_dir): + password = "fakepassword1234" + try: + Repo.clone_from( + url="https://fakeuser:{}@fakerepo.example.com/testrepo".format(password), + to_path=rw_dir, + ) + except GitCommandError as err: + assert password not in str(err), "The error message '%s' should not contain the password" % err + # Working example from a blank private project. + Repo.clone_from( + url="https://gitlab+deploy-token-392045:mLWhVus7bjLsy8xj8q2V@gitlab.com/mercierm/test_git_python", + to_path=rw_dir, + ) + + @with_rw_repo("HEAD") + def test_clone_unsafe_options(self, rw_repo): + with tempfile.TemporaryDirectory() as tdir: + tmp_dir = pathlib.Path(tdir) + tmp_file = tmp_dir / "pwn" + unsafe_options = [ + f"--upload-pack='touch {tmp_file}'", + f"-u 'touch {tmp_file}'", + "--config=protocol.ext.allow=always", + "-c protocol.ext.allow=always", + ] + for unsafe_option in unsafe_options: + with self.assertRaises(UnsafeOptionError): + rw_repo.clone(tmp_dir, multi_options=[unsafe_option]) + assert not tmp_file.exists() + + unsafe_options = [ + {"upload-pack": f"touch {tmp_file}"}, + {"u": f"touch {tmp_file}"}, + {"config": "protocol.ext.allow=always"}, + {"c": "protocol.ext.allow=always"}, + ] + for unsafe_option in unsafe_options: + with self.assertRaises(UnsafeOptionError): + rw_repo.clone(tmp_dir, **unsafe_option) + assert not tmp_file.exists() + + @pytest.mark.xfail( + sys.platform == "win32", + reason=( + "File not created. A separate Windows command may be needed. This and the " + "currently passing test test_clone_unsafe_options must be adjusted in the " + "same way. Until then, test_clone_unsafe_options is unreliable on Windows." + ), + raises=AssertionError, + ) + @with_rw_repo("HEAD") + def test_clone_unsafe_options_allowed(self, rw_repo): + with tempfile.TemporaryDirectory() as tdir: + tmp_dir = pathlib.Path(tdir) + tmp_file = tmp_dir / "pwn" + unsafe_options = [ + f"--upload-pack='touch {tmp_file}'", + f"-u 'touch {tmp_file}'", + ] + for i, unsafe_option in enumerate(unsafe_options): + destination = tmp_dir / str(i) + assert not tmp_file.exists() + # The options will be allowed, but the command will fail. + with self.assertRaises(GitCommandError): + rw_repo.clone(destination, multi_options=[unsafe_option], allow_unsafe_options=True) + assert tmp_file.exists() + tmp_file.unlink() + + unsafe_options = [ + "--config=protocol.ext.allow=always", + "-c protocol.ext.allow=always", + ] + for i, unsafe_option in enumerate(unsafe_options): + destination = tmp_dir / str(i) + assert not destination.exists() + rw_repo.clone(destination, multi_options=[unsafe_option], allow_unsafe_options=True) + assert destination.exists() + + @with_rw_repo("HEAD") + def test_clone_safe_options(self, rw_repo): + with tempfile.TemporaryDirectory() as tdir: + tmp_dir = pathlib.Path(tdir) + options = [ + "--depth=1", + "--single-branch", + "-q", + ] + for option in options: + destination = tmp_dir / option + assert not destination.exists() + rw_repo.clone(destination, multi_options=[option]) + assert destination.exists() + + @with_rw_repo("HEAD") + def test_clone_from_unsafe_options(self, rw_repo): + with tempfile.TemporaryDirectory() as tdir: + tmp_dir = pathlib.Path(tdir) + tmp_file = tmp_dir / "pwn" + unsafe_options = [ + f"--upload-pack='touch {tmp_file}'", + f"-u 'touch {tmp_file}'", + "--config=protocol.ext.allow=always", + "-c protocol.ext.allow=always", + ] + for unsafe_option in unsafe_options: + with self.assertRaises(UnsafeOptionError): + Repo.clone_from(rw_repo.working_dir, tmp_dir, multi_options=[unsafe_option]) + assert not tmp_file.exists() + + unsafe_options = [ + {"upload-pack": f"touch {tmp_file}"}, + {"u": f"touch {tmp_file}"}, + {"config": "protocol.ext.allow=always"}, + {"c": "protocol.ext.allow=always"}, + ] + for unsafe_option in unsafe_options: + with self.assertRaises(UnsafeOptionError): + Repo.clone_from(rw_repo.working_dir, tmp_dir, **unsafe_option) + assert not tmp_file.exists() + + @pytest.mark.xfail( + sys.platform == "win32", + reason=( + "File not created. A separate Windows command may be needed. This and the " + "currently passing test test_clone_from_unsafe_options must be adjusted in the " + "same way. Until then, test_clone_from_unsafe_options is unreliable on Windows." + ), + raises=AssertionError, + ) + @with_rw_repo("HEAD") + def test_clone_from_unsafe_options_allowed(self, rw_repo): + with tempfile.TemporaryDirectory() as tdir: + tmp_dir = pathlib.Path(tdir) + tmp_file = tmp_dir / "pwn" + unsafe_options = [ + f"--upload-pack='touch {tmp_file}'", + f"-u 'touch {tmp_file}'", + ] + for i, unsafe_option in enumerate(unsafe_options): + destination = tmp_dir / str(i) + assert not tmp_file.exists() + # The options will be allowed, but the command will fail. + with self.assertRaises(GitCommandError): + Repo.clone_from( + rw_repo.working_dir, destination, multi_options=[unsafe_option], allow_unsafe_options=True + ) + assert tmp_file.exists() + tmp_file.unlink() + + unsafe_options = [ + "--config=protocol.ext.allow=always", + "-c protocol.ext.allow=always", + ] + for i, unsafe_option in enumerate(unsafe_options): + destination = tmp_dir / str(i) + assert not destination.exists() + Repo.clone_from( + rw_repo.working_dir, destination, multi_options=[unsafe_option], allow_unsafe_options=True + ) + assert destination.exists() + + @with_rw_repo("HEAD") + def test_clone_from_safe_options(self, rw_repo): + with tempfile.TemporaryDirectory() as tdir: + tmp_dir = pathlib.Path(tdir) + options = [ + "--depth=1", + "--single-branch", + "-q", + ] + for option in options: + destination = tmp_dir / option + assert not destination.exists() + Repo.clone_from(rw_repo.common_dir, destination, multi_options=[option]) + assert destination.exists() + + def test_clone_from_unsafe_protocol(self): + with tempfile.TemporaryDirectory() as tdir: + tmp_dir = pathlib.Path(tdir) + tmp_file = tmp_dir / "pwn" + urls = [ + f"ext::sh -c touch% {tmp_file}", + "fd::17/foo", + ] + for url in urls: + with self.assertRaises(UnsafeProtocolError): + Repo.clone_from(url, tmp_dir / "repo") + assert not tmp_file.exists() + + def test_clone_from_unsafe_protocol_allowed(self): + with tempfile.TemporaryDirectory() as tdir: + tmp_dir = pathlib.Path(tdir) + tmp_file = tmp_dir / "pwn" + urls = [ + f"ext::sh -c touch% {tmp_file}", + "fd::/foo", + ] + for url in urls: + # The URL will be allowed into the command, but the command will + # fail since we don't have that protocol enabled in the Git config file. + with self.assertRaises(GitCommandError): + Repo.clone_from(url, tmp_dir / "repo", allow_unsafe_protocols=True) + assert not tmp_file.exists() + + def test_clone_from_unsafe_protocol_allowed_and_enabled(self): + with tempfile.TemporaryDirectory() as tdir: + tmp_dir = pathlib.Path(tdir) + tmp_file = tmp_dir / "pwn" + urls = [ + f"ext::sh -c touch% {tmp_file}", + ] + allow_ext = [ + "--config=protocol.ext.allow=always", + ] + for url in urls: + # The URL will be allowed into the command, and the protocol is enabled, + # but the command will fail since it can't read from the remote repo. + assert not tmp_file.exists() + with self.assertRaises(GitCommandError): + Repo.clone_from( + url, + tmp_dir / "repo", + multi_options=allow_ext, + allow_unsafe_protocols=True, + allow_unsafe_options=True, + ) + assert tmp_file.exists() + tmp_file.unlink() diff --git a/test/test_repo.py b/test/test_repo.py index bfa1bbb78..dc2cfe7b1 100644 --- a/test/test_repo.py +++ b/test/test_repo.py @@ -14,7 +14,7 @@ import pickle import sys import tempfile -from unittest import mock, skip +from unittest import mock import pytest @@ -36,7 +36,7 @@ Submodule, Tree, ) -from git.exc import BadObject, UnsafeOptionError, UnsafeProtocolError +from git.exc import BadObject from git.repo.fun import touch from git.util import bin_to_hex, cwd, cygpath, join_path_native, rmfile, rmtree @@ -214,285 +214,6 @@ def test_date_format(self, rw_dir): # @-timestamp is the format used by git commit hooks. repo.index.commit("Commit messages", commit_date="@1400000000 +0000") - @with_rw_directory - def test_clone_from_pathlib(self, rw_dir): - original_repo = Repo.init(osp.join(rw_dir, "repo")) - - Repo.clone_from(original_repo.git_dir, pathlib.Path(rw_dir) / "clone_pathlib") - - @with_rw_directory - def test_clone_from_pathlib_withConfig(self, rw_dir): - original_repo = Repo.init(osp.join(rw_dir, "repo")) - - cloned = Repo.clone_from( - original_repo.git_dir, - pathlib.Path(rw_dir) / "clone_pathlib_withConfig", - multi_options=[ - "--recurse-submodules=repo", - "--config core.filemode=false", - "--config submodule.repo.update=checkout", - "--config filter.lfs.clean='git-lfs clean -- %f'", - ], - allow_unsafe_options=True, - ) - - self.assertEqual(cloned.config_reader().get_value("submodule", "active"), "repo") - self.assertEqual(cloned.config_reader().get_value("core", "filemode"), False) - self.assertEqual(cloned.config_reader().get_value('submodule "repo"', "update"), "checkout") - self.assertEqual( - cloned.config_reader().get_value('filter "lfs"', "clean"), - "git-lfs clean -- %f", - ) - - def test_clone_from_with_path_contains_unicode(self): - with tempfile.TemporaryDirectory() as tmpdir: - unicode_dir_name = "\u0394" - path_with_unicode = os.path.join(tmpdir, unicode_dir_name) - os.makedirs(path_with_unicode) - - try: - Repo.clone_from( - url=self._small_repo_url(), - to_path=path_with_unicode, - ) - except UnicodeEncodeError: - self.fail("Raised UnicodeEncodeError") - - @with_rw_directory - @skip( - """The referenced repository was removed, and one needs to set up a new - password controlled repo under the org's control.""" - ) - def test_leaking_password_in_clone_logs(self, rw_dir): - password = "fakepassword1234" - try: - Repo.clone_from( - url="https://fakeuser:{}@fakerepo.example.com/testrepo".format(password), - to_path=rw_dir, - ) - except GitCommandError as err: - assert password not in str(err), "The error message '%s' should not contain the password" % err - # Working example from a blank private project. - Repo.clone_from( - url="https://gitlab+deploy-token-392045:mLWhVus7bjLsy8xj8q2V@gitlab.com/mercierm/test_git_python", - to_path=rw_dir, - ) - - @with_rw_repo("HEAD") - def test_clone_unsafe_options(self, rw_repo): - with tempfile.TemporaryDirectory() as tdir: - tmp_dir = pathlib.Path(tdir) - tmp_file = tmp_dir / "pwn" - unsafe_options = [ - f"--upload-pack='touch {tmp_file}'", - f"-u 'touch {tmp_file}'", - "--config=protocol.ext.allow=always", - "-c protocol.ext.allow=always", - ] - for unsafe_option in unsafe_options: - with self.assertRaises(UnsafeOptionError): - rw_repo.clone(tmp_dir, multi_options=[unsafe_option]) - assert not tmp_file.exists() - - unsafe_options = [ - {"upload-pack": f"touch {tmp_file}"}, - {"u": f"touch {tmp_file}"}, - {"config": "protocol.ext.allow=always"}, - {"c": "protocol.ext.allow=always"}, - ] - for unsafe_option in unsafe_options: - with self.assertRaises(UnsafeOptionError): - rw_repo.clone(tmp_dir, **unsafe_option) - assert not tmp_file.exists() - - @pytest.mark.xfail( - sys.platform == "win32", - reason=( - "File not created. A separate Windows command may be needed. This and the " - "currently passing test test_clone_unsafe_options must be adjusted in the " - "same way. Until then, test_clone_unsafe_options is unreliable on Windows." - ), - raises=AssertionError, - ) - @with_rw_repo("HEAD") - def test_clone_unsafe_options_allowed(self, rw_repo): - with tempfile.TemporaryDirectory() as tdir: - tmp_dir = pathlib.Path(tdir) - tmp_file = tmp_dir / "pwn" - unsafe_options = [ - f"--upload-pack='touch {tmp_file}'", - f"-u 'touch {tmp_file}'", - ] - for i, unsafe_option in enumerate(unsafe_options): - destination = tmp_dir / str(i) - assert not tmp_file.exists() - # The options will be allowed, but the command will fail. - with self.assertRaises(GitCommandError): - rw_repo.clone(destination, multi_options=[unsafe_option], allow_unsafe_options=True) - assert tmp_file.exists() - tmp_file.unlink() - - unsafe_options = [ - "--config=protocol.ext.allow=always", - "-c protocol.ext.allow=always", - ] - for i, unsafe_option in enumerate(unsafe_options): - destination = tmp_dir / str(i) - assert not destination.exists() - rw_repo.clone(destination, multi_options=[unsafe_option], allow_unsafe_options=True) - assert destination.exists() - - @with_rw_repo("HEAD") - def test_clone_safe_options(self, rw_repo): - with tempfile.TemporaryDirectory() as tdir: - tmp_dir = pathlib.Path(tdir) - options = [ - "--depth=1", - "--single-branch", - "-q", - ] - for option in options: - destination = tmp_dir / option - assert not destination.exists() - rw_repo.clone(destination, multi_options=[option]) - assert destination.exists() - - @with_rw_repo("HEAD") - def test_clone_from_unsafe_options(self, rw_repo): - with tempfile.TemporaryDirectory() as tdir: - tmp_dir = pathlib.Path(tdir) - tmp_file = tmp_dir / "pwn" - unsafe_options = [ - f"--upload-pack='touch {tmp_file}'", - f"-u 'touch {tmp_file}'", - "--config=protocol.ext.allow=always", - "-c protocol.ext.allow=always", - ] - for unsafe_option in unsafe_options: - with self.assertRaises(UnsafeOptionError): - Repo.clone_from(rw_repo.working_dir, tmp_dir, multi_options=[unsafe_option]) - assert not tmp_file.exists() - - unsafe_options = [ - {"upload-pack": f"touch {tmp_file}"}, - {"u": f"touch {tmp_file}"}, - {"config": "protocol.ext.allow=always"}, - {"c": "protocol.ext.allow=always"}, - ] - for unsafe_option in unsafe_options: - with self.assertRaises(UnsafeOptionError): - Repo.clone_from(rw_repo.working_dir, tmp_dir, **unsafe_option) - assert not tmp_file.exists() - - @pytest.mark.xfail( - sys.platform == "win32", - reason=( - "File not created. A separate Windows command may be needed. This and the " - "currently passing test test_clone_from_unsafe_options must be adjusted in the " - "same way. Until then, test_clone_from_unsafe_options is unreliable on Windows." - ), - raises=AssertionError, - ) - @with_rw_repo("HEAD") - def test_clone_from_unsafe_options_allowed(self, rw_repo): - with tempfile.TemporaryDirectory() as tdir: - tmp_dir = pathlib.Path(tdir) - tmp_file = tmp_dir / "pwn" - unsafe_options = [ - f"--upload-pack='touch {tmp_file}'", - f"-u 'touch {tmp_file}'", - ] - for i, unsafe_option in enumerate(unsafe_options): - destination = tmp_dir / str(i) - assert not tmp_file.exists() - # The options will be allowed, but the command will fail. - with self.assertRaises(GitCommandError): - Repo.clone_from( - rw_repo.working_dir, destination, multi_options=[unsafe_option], allow_unsafe_options=True - ) - assert tmp_file.exists() - tmp_file.unlink() - - unsafe_options = [ - "--config=protocol.ext.allow=always", - "-c protocol.ext.allow=always", - ] - for i, unsafe_option in enumerate(unsafe_options): - destination = tmp_dir / str(i) - assert not destination.exists() - Repo.clone_from( - rw_repo.working_dir, destination, multi_options=[unsafe_option], allow_unsafe_options=True - ) - assert destination.exists() - - @with_rw_repo("HEAD") - def test_clone_from_safe_options(self, rw_repo): - with tempfile.TemporaryDirectory() as tdir: - tmp_dir = pathlib.Path(tdir) - options = [ - "--depth=1", - "--single-branch", - "-q", - ] - for option in options: - destination = tmp_dir / option - assert not destination.exists() - Repo.clone_from(rw_repo.common_dir, destination, multi_options=[option]) - assert destination.exists() - - def test_clone_from_unsafe_protocol(self): - with tempfile.TemporaryDirectory() as tdir: - tmp_dir = pathlib.Path(tdir) - tmp_file = tmp_dir / "pwn" - urls = [ - f"ext::sh -c touch% {tmp_file}", - "fd::17/foo", - ] - for url in urls: - with self.assertRaises(UnsafeProtocolError): - Repo.clone_from(url, tmp_dir / "repo") - assert not tmp_file.exists() - - def test_clone_from_unsafe_protocol_allowed(self): - with tempfile.TemporaryDirectory() as tdir: - tmp_dir = pathlib.Path(tdir) - tmp_file = tmp_dir / "pwn" - urls = [ - f"ext::sh -c touch% {tmp_file}", - "fd::/foo", - ] - for url in urls: - # The URL will be allowed into the command, but the command will - # fail since we don't have that protocol enabled in the Git config file. - with self.assertRaises(GitCommandError): - Repo.clone_from(url, tmp_dir / "repo", allow_unsafe_protocols=True) - assert not tmp_file.exists() - - def test_clone_from_unsafe_protocol_allowed_and_enabled(self): - with tempfile.TemporaryDirectory() as tdir: - tmp_dir = pathlib.Path(tdir) - tmp_file = tmp_dir / "pwn" - urls = [ - f"ext::sh -c touch% {tmp_file}", - ] - allow_ext = [ - "--config=protocol.ext.allow=always", - ] - for url in urls: - # The URL will be allowed into the command, and the protocol is enabled, - # but the command will fail since it can't read from the remote repo. - assert not tmp_file.exists() - with self.assertRaises(GitCommandError): - Repo.clone_from( - url, - tmp_dir / "repo", - multi_options=allow_ext, - allow_unsafe_protocols=True, - allow_unsafe_options=True, - ) - assert tmp_file.exists() - tmp_file.unlink() - @with_rw_repo("HEAD") def test_max_chunk_size(self, repo): class TestOutputStream(TestBase): From 24abf10dc2913f9c1674c6d60dd70c0ec775a6d4 Mon Sep 17 00:00:00 2001 From: George Ogden Date: Thu, 27 Nov 2025 17:39:15 +0000 Subject: [PATCH 02/18] Allow Pathlike urls and destinations when cloning --- git/repo/base.py | 6 ++++-- test/test_clone.py | 16 +++++++++++++++- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/git/repo/base.py b/git/repo/base.py index 6ea96aad2..fbed6e471 100644 --- a/git/repo/base.py +++ b/git/repo/base.py @@ -1362,8 +1362,10 @@ def _clone( odbt = kwargs.pop("odbt", odb_default_type) # When pathlib.Path or other class-based path is passed + if not isinstance(url, str): + url = url.__fspath__() if not isinstance(path, str): - path = str(path) + path = path.__fspath__() ## A bug win cygwin's Git, when `--bare` or `--separate-git-dir` # it prepends the cwd or(?) the `url` into the `path, so:: @@ -1380,7 +1382,7 @@ def _clone( multi = shlex.split(" ".join(multi_options)) if not allow_unsafe_protocols: - Git.check_unsafe_protocols(str(url)) + Git.check_unsafe_protocols(url) if not allow_unsafe_options: Git.check_unsafe_options(options=list(kwargs.keys()), unsafe_options=cls.unsafe_git_clone_options) if not allow_unsafe_options and multi_options: diff --git a/test/test_clone.py b/test/test_clone.py index 91b7d7621..489931458 100644 --- a/test/test_clone.py +++ b/test/test_clone.py @@ -1,6 +1,7 @@ # This module is part of GitPython and is released under the # 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/ +from dataclasses import dataclass import os import os.path as osp import pathlib @@ -45,7 +46,20 @@ def test_checkout_in_non_empty_dir(self, rw_dir): def test_clone_from_pathlib(self, rw_dir): original_repo = Repo.init(osp.join(rw_dir, "repo")) - Repo.clone_from(original_repo.git_dir, pathlib.Path(rw_dir) / "clone_pathlib") + Repo.clone_from(pathlib.Path(original_repo.git_dir), pathlib.Path(rw_dir) / "clone_pathlib") + + @with_rw_directory + def test_clone_from_pathlike(self, rw_dir): + original_repo = Repo.init(osp.join(rw_dir, "repo")) + + @dataclass + class PathLikeMock: + path: str + + def __fspath__(self) -> str: + return self.path + + Repo.clone_from(PathLikeMock(original_repo.git_dir), PathLikeMock(os.path.join(rw_dir, "clone_pathlike"))) @with_rw_directory def test_clone_from_pathlib_withConfig(self, rw_dir): From ad1ae5fea338d2a716506f46532932dc458f791a Mon Sep 17 00:00:00 2001 From: George Ogden Date: Thu, 27 Nov 2025 17:41:07 +0000 Subject: [PATCH 03/18] Simplify logic with direct path conversion --- git/index/typ.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/git/index/typ.py b/git/index/typ.py index 4bcb604ab..5fb1b9abc 100644 --- a/git/index/typ.py +++ b/git/index/typ.py @@ -58,9 +58,9 @@ def __init__(self, paths: Sequence[PathLike]) -> None: def __call__(self, stage_blob: Tuple[StageType, Blob]) -> bool: blob_pathlike: PathLike = stage_blob[1].path - blob_path: Path = blob_pathlike if isinstance(blob_pathlike, Path) else Path(blob_pathlike) + blob_path = Path(blob_pathlike) for pathlike in self.paths: - path: Path = pathlike if isinstance(pathlike, Path) else Path(pathlike) + path = Path(pathlike) # TODO: Change to use `PosixPath.is_relative_to` once Python 3.8 is no # longer supported. filter_parts = path.parts From 5d26325f59880864863b5e56a08aa0f83b623f2d Mon Sep 17 00:00:00 2001 From: George Ogden Date: Thu, 27 Nov 2025 17:46:13 +0000 Subject: [PATCH 04/18] Allow Pathlike paths when creating a git repo --- git/repo/base.py | 2 +- test/test_repo.py | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/git/repo/base.py b/git/repo/base.py index fbed6e471..b1b95ce42 100644 --- a/git/repo/base.py +++ b/git/repo/base.py @@ -223,7 +223,7 @@ def __init__( epath = epath or path or os.getcwd() if not isinstance(epath, str): - epath = str(epath) + epath = epath.__fspath__() if expand_vars and re.search(self.re_envvars, epath): warnings.warn( "The use of environment variables in paths is deprecated" diff --git a/test/test_repo.py b/test/test_repo.py index dc2cfe7b1..cd22430a7 100644 --- a/test/test_repo.py +++ b/test/test_repo.py @@ -3,6 +3,7 @@ # This module is part of GitPython and is released under the # 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/ +from dataclasses import dataclass import gc import glob import io @@ -105,6 +106,18 @@ def test_repo_creation_pathlib(self, rw_repo): r_from_gitdir = Repo(pathlib.Path(rw_repo.git_dir)) self.assertEqual(r_from_gitdir.git_dir, rw_repo.git_dir) + @with_rw_repo("0.3.2.1") + def test_repo_creation_pathlike(self, rw_repo): + @dataclass + class PathLikeMock: + path: str + + def __fspath__(self) -> str: + return self.path + + r_from_gitdir = Repo(PathLikeMock(rw_repo.git_dir)) + self.assertEqual(r_from_gitdir.git_dir, rw_repo.git_dir) + def test_description(self): txt = "Test repository" self.rorepo.description = txt From 59c3c8065a402c4cd8a71625f51b6792fdc04863 Mon Sep 17 00:00:00 2001 From: George Ogden Date: Thu, 27 Nov 2025 19:20:07 +0000 Subject: [PATCH 05/18] Fix missing path conversion --- git/repo/base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git/repo/base.py b/git/repo/base.py index b1b95ce42..be50300b5 100644 --- a/git/repo/base.py +++ b/git/repo/base.py @@ -219,7 +219,7 @@ def __init__( # Given how the tests are written, this seems more likely to catch Cygwin # git used from Windows than Windows git used from Cygwin. Therefore # changing to Cygwin-style paths is the relevant operation. - epath = cygpath(str(epath)) + epath = cygpath(epath if isinstance(epath, str) else epath.__fspath__()) epath = epath or path or os.getcwd() if not isinstance(epath, str): From 91d4cc5ea05df04c82fcfd3e35a6af2e903cc554 Mon Sep 17 00:00:00 2001 From: George Ogden Date: Fri, 28 Nov 2025 08:57:54 +0000 Subject: [PATCH 06/18] Use os.fspath instead of __fspath__ for reading paths --- git/config.py | 2 +- git/repo/base.py | 13 ++++--------- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/git/config.py b/git/config.py index 200c81bb7..e3081401d 100644 --- a/git/config.py +++ b/git/config.py @@ -634,7 +634,7 @@ def read(self) -> None: # type: ignore[override] self._read(file_path, file_path.name) else: # Assume a path if it is not a file-object. - file_path = cast(PathLike, file_path) + file_path = os.fspath(file_path) try: with open(file_path, "rb") as fp: file_ok = True diff --git a/git/repo/base.py b/git/repo/base.py index be50300b5..2d87a06b7 100644 --- a/git/repo/base.py +++ b/git/repo/base.py @@ -219,11 +219,9 @@ def __init__( # Given how the tests are written, this seems more likely to catch Cygwin # git used from Windows than Windows git used from Cygwin. Therefore # changing to Cygwin-style paths is the relevant operation. - epath = cygpath(epath if isinstance(epath, str) else epath.__fspath__()) + epath = cygpath(os.fspath(epath)) - epath = epath or path or os.getcwd() - if not isinstance(epath, str): - epath = epath.__fspath__() + epath = os.fspath(epath) if expand_vars and re.search(self.re_envvars, epath): warnings.warn( "The use of environment variables in paths is deprecated" @@ -1361,11 +1359,8 @@ def _clone( ) -> "Repo": odbt = kwargs.pop("odbt", odb_default_type) - # When pathlib.Path or other class-based path is passed - if not isinstance(url, str): - url = url.__fspath__() - if not isinstance(path, str): - path = path.__fspath__() + url = os.fspath(url) + path = os.fspath(path) ## A bug win cygwin's Git, when `--bare` or `--separate-git-dir` # it prepends the cwd or(?) the `url` into the `path, so:: From 0414bf78cfc7d1889121c414efa7841f57343984 Mon Sep 17 00:00:00 2001 From: George Ogden Date: Fri, 28 Nov 2025 21:28:17 +0000 Subject: [PATCH 07/18] Replace extra occurrences of str with fspath --- git/config.py | 2 +- git/index/base.py | 18 +++++++++--------- git/index/fun.py | 4 ++-- git/index/util.py | 2 +- git/objects/blob.py | 3 ++- git/objects/submodule/base.py | 10 +++++----- git/refs/reference.py | 3 ++- git/refs/symbolic.py | 18 +++++++++--------- git/repo/base.py | 4 ++-- git/util.py | 20 ++++++++++---------- test/lib/helper.py | 11 +++++++++++ test/test_clone.py | 11 +---------- test/test_index.py | 22 ++++++++++++---------- test/test_repo.py | 10 +--------- 14 files changed, 68 insertions(+), 70 deletions(-) diff --git a/git/config.py b/git/config.py index e3081401d..732f347f0 100644 --- a/git/config.py +++ b/git/config.py @@ -578,7 +578,7 @@ def _included_paths(self) -> List[Tuple[str, str]]: value, ) if self._repo.git_dir: - if fnmatch.fnmatchcase(str(self._repo.git_dir), value): + if fnmatch.fnmatchcase(os.fspath(self._repo.git_dir), value): paths += self.items(section) elif keyword == "onbranch": diff --git a/git/index/base.py b/git/index/base.py index 7cc9d3ade..905feb076 100644 --- a/git/index/base.py +++ b/git/index/base.py @@ -404,10 +404,10 @@ def _iter_expand_paths(self: "IndexFile", paths: Sequence[PathLike]) -> Iterator def raise_exc(e: Exception) -> NoReturn: raise e - r = str(self.repo.working_tree_dir) + r = os.fspath(self.repo.working_tree_dir) rs = r + os.sep for path in paths: - abs_path = str(path) + abs_path = os.fspath(path) if not osp.isabs(abs_path): abs_path = osp.join(r, path) # END make absolute path @@ -434,7 +434,7 @@ def raise_exc(e: Exception) -> NoReturn: # characters. if abs_path not in resolved_paths: for f in self._iter_expand_paths(glob.glob(abs_path)): - yield str(f).replace(rs, "") + yield os.fspath(f).replace(rs, "") continue # END glob handling try: @@ -569,7 +569,7 @@ def resolve_blobs(self, iter_blobs: Iterator[Blob]) -> "IndexFile": for blob in iter_blobs: stage_null_key = (blob.path, 0) if stage_null_key in self.entries: - raise ValueError("Path %r already exists at stage 0" % str(blob.path)) + raise ValueError("Path %r already exists at stage 0" % os.fspath(blob.path)) # END assert blob is not stage 0 already # Delete all possible stages. @@ -656,10 +656,10 @@ def _to_relative_path(self, path: PathLike) -> PathLike: return path if self.repo.bare: raise InvalidGitRepositoryError("require non-bare repository") - if not osp.normpath(str(path)).startswith(str(self.repo.working_tree_dir)): + if not osp.normpath(os.fspath(path)).startswith(os.fspath(self.repo.working_tree_dir)): raise ValueError("Absolute path %r is not in git repository at %r" % (path, self.repo.working_tree_dir)) result = os.path.relpath(path, self.repo.working_tree_dir) - if str(path).endswith(os.sep) and not result.endswith(os.sep): + if os.fspath(path).endswith(os.sep) and not result.endswith(os.sep): result += os.sep return result @@ -731,7 +731,7 @@ def _entries_for_paths( for path in paths: if osp.isabs(path): abspath = path - gitrelative_path = path[len(str(self.repo.working_tree_dir)) + 1 :] + gitrelative_path = path[len(os.fspath(self.repo.working_tree_dir)) + 1 :] else: gitrelative_path = path if self.repo.working_tree_dir: @@ -1359,11 +1359,11 @@ def make_exc() -> GitCommandError: try: self.entries[(co_path, 0)] except KeyError: - folder = str(co_path) + folder = os.fspath(co_path) if not folder.endswith("/"): folder += "/" for entry in self.entries.values(): - if str(entry.path).startswith(folder): + if os.fspath(entry.path).startswith(folder): p = entry.path self._write_path_to_stdin(proc, p, p, make_exc, fprogress, read_from_stdout=False) checked_out_files.append(p) diff --git a/git/index/fun.py b/git/index/fun.py index 0b3d79cf1..845221c61 100644 --- a/git/index/fun.py +++ b/git/index/fun.py @@ -87,7 +87,7 @@ def run_commit_hook(name: str, index: "IndexFile", *args: str) -> None: return env = os.environ.copy() - env["GIT_INDEX_FILE"] = safe_decode(str(index.path)) + env["GIT_INDEX_FILE"] = safe_decode(os.fspath(index.path)) env["GIT_EDITOR"] = ":" cmd = [hp] try: @@ -167,7 +167,7 @@ def write_cache( beginoffset = tell() write(entry.ctime_bytes) # ctime write(entry.mtime_bytes) # mtime - path_str = str(entry.path) + path_str = os.fspath(entry.path) path: bytes = force_bytes(path_str, encoding=defenc) plen = len(path) & CE_NAMEMASK # Path length assert plen == len(path), "Path %s too long to fit into index" % entry.path diff --git a/git/index/util.py b/git/index/util.py index e59cb609f..2d2422ab4 100644 --- a/git/index/util.py +++ b/git/index/util.py @@ -106,7 +106,7 @@ def git_working_dir(func: Callable[..., _T]) -> Callable[..., _T]: @wraps(func) def set_git_working_dir(self: "IndexFile", *args: Any, **kwargs: Any) -> _T: cur_wd = os.getcwd() - os.chdir(str(self.repo.working_tree_dir)) + os.chdir(os.fspath(self.repo.working_tree_dir)) try: return func(self, *args, **kwargs) finally: diff --git a/git/objects/blob.py b/git/objects/blob.py index 58de59642..f7d49c9cc 100644 --- a/git/objects/blob.py +++ b/git/objects/blob.py @@ -6,6 +6,7 @@ __all__ = ["Blob"] from mimetypes import guess_type +import os import sys if sys.version_info >= (3, 8): @@ -44,5 +45,5 @@ def mime_type(self) -> str: """ guesses = None if self.path: - guesses = guess_type(str(self.path)) + guesses = guess_type(os.fspath(self.path)) return guesses and guesses[0] or self.DEFAULT_MIME_TYPE diff --git a/git/objects/submodule/base.py b/git/objects/submodule/base.py index 5031a2e71..cc1f94c6c 100644 --- a/git/objects/submodule/base.py +++ b/git/objects/submodule/base.py @@ -352,7 +352,7 @@ def _clone_repo( module_abspath_dir = osp.dirname(module_abspath) if not osp.isdir(module_abspath_dir): os.makedirs(module_abspath_dir) - module_checkout_path = osp.join(str(repo.working_tree_dir), path) + module_checkout_path = osp.join(os.fspath(repo.working_tree_dir), path) if url.startswith("../"): remote_name = repo.active_branch.tracking_branch().remote_name @@ -541,7 +541,7 @@ def add( if sm.exists(): # Reretrieve submodule from tree. try: - sm = repo.head.commit.tree[str(path)] + sm = repo.head.commit.tree[os.fspath(path)] sm._name = name return sm except KeyError: @@ -1016,7 +1016,7 @@ def move(self, module_path: PathLike, configuration: bool = True, module: bool = return self # END handle no change - module_checkout_abspath = join_path_native(str(self.repo.working_tree_dir), module_checkout_path) + module_checkout_abspath = join_path_native(os.fspath(self.repo.working_tree_dir), module_checkout_path) if osp.isfile(module_checkout_abspath): raise ValueError("Cannot move repository onto a file: %s" % module_checkout_abspath) # END handle target files @@ -1313,7 +1313,7 @@ def set_parent_commit(self, commit: Union[Commit_ish, str, None], check: bool = # If check is False, we might see a parent-commit that doesn't even contain the # submodule anymore. in that case, mark our sha as being NULL. try: - self.binsha = pctree[str(self.path)].binsha + self.binsha = pctree[os.fspath(self.path)].binsha except KeyError: self.binsha = self.NULL_BIN_SHA @@ -1395,7 +1395,7 @@ def rename(self, new_name: str) -> "Submodule": destination_module_abspath = self._module_abspath(self.repo, self.path, new_name) source_dir = mod.git_dir # Let's be sure the submodule name is not so obviously tied to a directory. - if str(destination_module_abspath).startswith(str(mod.git_dir)): + if os.fspath(destination_module_abspath).startswith(os.fspath(mod.git_dir)): tmp_dir = self._module_abspath(self.repo, self.path, str(uuid.uuid4())) os.renames(source_dir, tmp_dir) source_dir = tmp_dir diff --git a/git/refs/reference.py b/git/refs/reference.py index e5d473779..0c4327225 100644 --- a/git/refs/reference.py +++ b/git/refs/reference.py @@ -3,6 +3,7 @@ __all__ = ["Reference"] +import os from git.util import IterableObj, LazyMixin from .symbolic import SymbolicReference, T_References @@ -65,7 +66,7 @@ def __init__(self, repo: "Repo", path: PathLike, check_path: bool = True) -> Non If ``False``, you can provide any path. Otherwise the path must start with the default path prefix of this type. """ - if check_path and not str(path).startswith(self._common_path_default + "/"): + if check_path and not os.fspath(path).startswith(self._common_path_default + "/"): raise ValueError(f"Cannot instantiate {self.__class__.__name__!r} from path {path}") self.path: str # SymbolicReference converts to string at the moment. super().__init__(repo, path) diff --git a/git/refs/symbolic.py b/git/refs/symbolic.py index 74bb1fe0a..c7db129d9 100644 --- a/git/refs/symbolic.py +++ b/git/refs/symbolic.py @@ -79,7 +79,7 @@ def __init__(self, repo: "Repo", path: PathLike, check_path: bool = False) -> No self.path = path def __str__(self) -> str: - return str(self.path) + return os.fspath(self.path) def __repr__(self) -> str: return '' % (self.__class__.__name__, self.path) @@ -103,7 +103,7 @@ def name(self) -> str: In case of symbolic references, the shortest assumable name is the path itself. """ - return str(self.path) + return os.fspath(self.path) @property def abspath(self) -> PathLike: @@ -178,7 +178,7 @@ def _check_ref_name_valid(ref_path: PathLike) -> None: """ previous: Union[str, None] = None one_before_previous: Union[str, None] = None - for c in str(ref_path): + for c in os.fspath(ref_path): if c in " ~^:?*[\\": raise ValueError( f"Invalid reference '{ref_path}': references cannot contain spaces, tildes (~), carets (^)," @@ -212,7 +212,7 @@ def _check_ref_name_valid(ref_path: PathLike) -> None: raise ValueError(f"Invalid reference '{ref_path}': references cannot end with a forward slash (/)") elif previous == "@" and one_before_previous is None: raise ValueError(f"Invalid reference '{ref_path}': references cannot be '@'") - elif any(component.endswith(".lock") for component in str(ref_path).split("/")): + elif any(component.endswith(".lock") for component in os.fspath(ref_path).split("/")): raise ValueError( f"Invalid reference '{ref_path}': references cannot have slash-separated components that end with" " '.lock'" @@ -235,7 +235,7 @@ def _get_ref_info_helper( tokens: Union[None, List[str], Tuple[str, str]] = None repodir = _git_dir(repo, ref_path) try: - with open(os.path.join(repodir, str(ref_path)), "rt", encoding="UTF-8") as fp: + with open(os.path.join(repodir, os.fspath(ref_path)), "rt", encoding="UTF-8") as fp: value = fp.read().rstrip() # Don't only split on spaces, but on whitespace, which allows to parse lines like: # 60b64ef992065e2600bfef6187a97f92398a9144 branch 'master' of git-server:/path/to/repo @@ -614,7 +614,7 @@ def to_full_path(cls, path: Union[PathLike, "SymbolicReference"]) -> PathLike: full_ref_path = path if not cls._common_path_default: return full_ref_path - if not str(path).startswith(cls._common_path_default + "/"): + if not os.fspath(path).startswith(cls._common_path_default + "/"): full_ref_path = "%s/%s" % (cls._common_path_default, path) return full_ref_path @@ -706,7 +706,7 @@ def _create( if not force and os.path.isfile(abs_ref_path): target_data = str(target) if isinstance(target, SymbolicReference): - target_data = str(target.path) + target_data = os.fspath(target.path) if not resolve: target_data = "ref: " + target_data with open(abs_ref_path, "rb") as fd: @@ -842,7 +842,7 @@ def _iter_items( # Read packed refs. for _sha, rela_path in cls._iter_packed_refs(repo): - if rela_path.startswith(str(common_path)): + if rela_path.startswith(os.fspath(common_path)): rela_paths.add(rela_path) # END relative path matches common path # END packed refs reading @@ -931,4 +931,4 @@ def from_path(cls: Type[T_References], repo: "Repo", path: PathLike) -> T_Refere def is_remote(self) -> bool: """:return: True if this symbolic reference points to a remote branch""" - return str(self.path).startswith(self._remote_common_path_default + "/") + return os.fspath(self.path).startswith(self._remote_common_path_default + "/") diff --git a/git/repo/base.py b/git/repo/base.py index 2d87a06b7..2fe18f48c 100644 --- a/git/repo/base.py +++ b/git/repo/base.py @@ -554,7 +554,7 @@ def tag(self, path: PathLike) -> TagReference: @staticmethod def _to_full_tag_path(path: PathLike) -> str: - path_str = str(path) + path_str = os.fspath(path) if path_str.startswith(TagReference._common_path_default + "/"): return path_str if path_str.startswith(TagReference._common_default + "/"): @@ -959,7 +959,7 @@ def is_dirty( if not submodules: default_args.append("--ignore-submodules") if path: - default_args.extend(["--", str(path)]) + default_args.extend(["--", os.fspath(path)]) if index: # diff index against HEAD. if osp.isfile(self.index.path) and len(self.git.diff("--cached", *default_args)): diff --git a/git/util.py b/git/util.py index 0aff5eb64..f18eb6e52 100644 --- a/git/util.py +++ b/git/util.py @@ -272,9 +272,9 @@ def stream_copy(source: BinaryIO, destination: BinaryIO, chunk_size: int = 512 * def join_path(a: PathLike, *p: PathLike) -> PathLike: R"""Join path tokens together similar to osp.join, but always use ``/`` instead of possibly ``\`` on Windows.""" - path = str(a) + path = os.fspath(a) for b in p: - b = str(b) + b = os.fspath(b) if not b: continue if b.startswith("/"): @@ -290,18 +290,18 @@ def join_path(a: PathLike, *p: PathLike) -> PathLike: if sys.platform == "win32": def to_native_path_windows(path: PathLike) -> PathLike: - path = str(path) + path = os.fspath(path) return path.replace("/", "\\") def to_native_path_linux(path: PathLike) -> str: - path = str(path) + path = os.fspath(path) return path.replace("\\", "/") to_native_path = to_native_path_windows else: # No need for any work on Linux. def to_native_path_linux(path: PathLike) -> str: - return str(path) + return os.fspath(path) to_native_path = to_native_path_linux @@ -372,7 +372,7 @@ def is_exec(fpath: str) -> bool: progs = [] if not path: path = os.environ["PATH"] - for folder in str(path).split(os.pathsep): + for folder in os.fspath(path).split(os.pathsep): folder = folder.strip('"') if folder: exe_path = osp.join(folder, program) @@ -397,7 +397,7 @@ def _cygexpath(drive: Optional[str], path: str) -> str: p = cygpath(p) elif drive: p = "/proc/cygdrive/%s/%s" % (drive.lower(), p) - p_str = str(p) # ensure it is a str and not AnyPath + p_str = os.fspath(p) # ensure it is a str and not AnyPath return p_str.replace("\\", "/") @@ -418,7 +418,7 @@ def _cygexpath(drive: Optional[str], path: str) -> str: def cygpath(path: str) -> str: """Use :meth:`git.cmd.Git.polish_url` instead, that works on any environment.""" - path = str(path) # Ensure is str and not AnyPath. + path = os.fspath(path) # Ensure is str and not AnyPath. # Fix to use Paths when 3.5 dropped. Or to be just str if only for URLs? if not path.startswith(("/cygdrive", "//", "/proc/cygdrive")): for regex, parser, recurse in _cygpath_parsers: @@ -438,7 +438,7 @@ def cygpath(path: str) -> str: def decygpath(path: PathLike) -> str: - path = str(path) + path = os.fspath(path) m = _decygpath_regex.match(path) if m: drive, rest_path = m.groups() @@ -497,7 +497,7 @@ def is_cygwin_git(git_executable: Union[None, PathLike]) -> bool: elif git_executable is None: return False else: - return _is_cygwin_git(str(git_executable)) + return _is_cygwin_git(os.fspath(git_executable)) def get_user_id() -> str: diff --git a/test/lib/helper.py b/test/lib/helper.py index 241d27341..e51f428e3 100644 --- a/test/lib/helper.py +++ b/test/lib/helper.py @@ -10,6 +10,7 @@ "with_rw_directory", "with_rw_repo", "with_rw_and_rw_remote_repo", + "PathLikeMock", "TestBase", "VirtualEnvironment", "TestCase", @@ -20,6 +21,7 @@ ] import contextlib +from dataclasses import dataclass from functools import wraps import gc import io @@ -49,6 +51,15 @@ _logger = logging.getLogger(__name__) + +@dataclass +class PathLikeMock: + path: str + + def __fspath__(self) -> str: + return self.path + + # { Routines diff --git a/test/test_clone.py b/test/test_clone.py index 489931458..143a3b51f 100644 --- a/test/test_clone.py +++ b/test/test_clone.py @@ -1,7 +1,6 @@ # This module is part of GitPython and is released under the # 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/ -from dataclasses import dataclass import os import os.path as osp import pathlib @@ -12,7 +11,7 @@ from git import GitCommandError, Repo from git.exc import UnsafeOptionError, UnsafeProtocolError -from test.lib import TestBase, with_rw_directory, with_rw_repo +from test.lib import TestBase, with_rw_directory, with_rw_repo, PathLikeMock from pathlib import Path import re @@ -51,14 +50,6 @@ def test_clone_from_pathlib(self, rw_dir): @with_rw_directory def test_clone_from_pathlike(self, rw_dir): original_repo = Repo.init(osp.join(rw_dir, "repo")) - - @dataclass - class PathLikeMock: - path: str - - def __fspath__(self) -> str: - return self.path - Repo.clone_from(PathLikeMock(original_repo.git_dir), PathLikeMock(os.path.join(rw_dir, "clone_pathlike"))) @with_rw_directory diff --git a/test/test_index.py b/test/test_index.py index 711b43a0b..c1db4166b 100644 --- a/test/test_index.py +++ b/test/test_index.py @@ -37,14 +37,7 @@ from git.objects import Blob from git.util import Actor, cwd, hex_to_bin, rmtree -from test.lib import ( - TestBase, - VirtualEnvironment, - fixture, - fixture_path, - with_rw_directory, - with_rw_repo, -) +from test.lib import TestBase, VirtualEnvironment, fixture, fixture_path, with_rw_directory, with_rw_repo, PathLikeMock HOOKS_SHEBANG = "#!/usr/bin/env sh\n" @@ -586,7 +579,7 @@ def mixed_iterator(): if type_id == 0: # path (str) yield entry.path elif type_id == 1: # path (PathLike) - yield Path(entry.path) + yield PathLikeMock(entry.path) elif type_id == 2: # blob yield Blob(rw_repo, entry.binsha, entry.mode, entry.path) elif type_id == 3: # BaseIndexEntry @@ -1198,7 +1191,7 @@ def test_commit_msg_hook_fail(self, rw_repo): raise AssertionError("Should have caught a HookExecutionError") @with_rw_repo("HEAD") - def test_index_add_pathlike(self, rw_repo): + def test_index_add_pathlib(self, rw_repo): git_dir = Path(rw_repo.git_dir) file = git_dir / "file.txt" @@ -1206,6 +1199,15 @@ def test_index_add_pathlike(self, rw_repo): rw_repo.index.add(file) + @with_rw_repo("HEAD") + def test_index_add_pathlike(self, rw_repo): + git_dir = Path(rw_repo.git_dir) + + file = git_dir / "file.txt" + file.touch() + + rw_repo.index.add(PathLikeMock(str(file))) + @with_rw_repo("HEAD") def test_index_add_non_normalized_path(self, rw_repo): git_dir = Path(rw_repo.git_dir) diff --git a/test/test_repo.py b/test/test_repo.py index cd22430a7..f089bf6b8 100644 --- a/test/test_repo.py +++ b/test/test_repo.py @@ -3,7 +3,6 @@ # This module is part of GitPython and is released under the # 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/ -from dataclasses import dataclass import gc import glob import io @@ -41,7 +40,7 @@ from git.repo.fun import touch from git.util import bin_to_hex, cwd, cygpath, join_path_native, rmfile, rmtree -from test.lib import TestBase, fixture, with_rw_directory, with_rw_repo +from test.lib import TestBase, fixture, with_rw_directory, with_rw_repo, PathLikeMock def iter_flatten(lol): @@ -108,13 +107,6 @@ def test_repo_creation_pathlib(self, rw_repo): @with_rw_repo("0.3.2.1") def test_repo_creation_pathlike(self, rw_repo): - @dataclass - class PathLikeMock: - path: str - - def __fspath__(self) -> str: - return self.path - r_from_gitdir = Repo(PathLikeMock(rw_repo.git_dir)) self.assertEqual(r_from_gitdir.git_dir, rw_repo.git_dir) From 3801505d1218242e853dda17e981e2a2fa795b0e Mon Sep 17 00:00:00 2001 From: George Ogden Date: Fri, 28 Nov 2025 21:43:09 +0000 Subject: [PATCH 08/18] Convert paths in constructors and large function calls --- git/index/base.py | 5 +++-- git/index/util.py | 2 +- git/refs/head.py | 2 ++ git/refs/log.py | 3 ++- git/refs/symbolic.py | 2 +- git/repo/fun.py | 1 + git/util.py | 2 +- 7 files changed, 11 insertions(+), 6 deletions(-) diff --git a/git/index/base.py b/git/index/base.py index 905feb076..cf7220ef8 100644 --- a/git/index/base.py +++ b/git/index/base.py @@ -652,14 +652,15 @@ def _to_relative_path(self, path: PathLike) -> PathLike: :raise ValueError: """ + path = os.fspath(path) if not osp.isabs(path): return path if self.repo.bare: raise InvalidGitRepositoryError("require non-bare repository") - if not osp.normpath(os.fspath(path)).startswith(os.fspath(self.repo.working_tree_dir)): + if not osp.normpath(path).startswith(os.fspath(self.repo.working_tree_dir)): raise ValueError("Absolute path %r is not in git repository at %r" % (path, self.repo.working_tree_dir)) result = os.path.relpath(path, self.repo.working_tree_dir) - if os.fspath(path).endswith(os.sep) and not result.endswith(os.sep): + if path.endswith(os.sep) and not result.endswith(os.sep): result += os.sep return result diff --git a/git/index/util.py b/git/index/util.py index 2d2422ab4..231634cd6 100644 --- a/git/index/util.py +++ b/git/index/util.py @@ -37,7 +37,7 @@ class TemporaryFileSwap: __slots__ = ("file_path", "tmp_file_path") def __init__(self, file_path: PathLike) -> None: - self.file_path = file_path + self.file_path = os.fspath(file_path) dirname, basename = osp.split(file_path) fd, self.tmp_file_path = tempfile.mkstemp(prefix=basename, dir=dirname) os.close(fd) diff --git a/git/refs/head.py b/git/refs/head.py index 683634451..03daa3973 100644 --- a/git/refs/head.py +++ b/git/refs/head.py @@ -8,6 +8,7 @@ __all__ = ["HEAD", "Head"] +import os from git.config import GitConfigParser, SectionConstraint from git.exc import GitCommandError from git.util import join_path @@ -48,6 +49,7 @@ class HEAD(SymbolicReference): commit: "Commit" def __init__(self, repo: "Repo", path: PathLike = _HEAD_NAME) -> None: + path = os.fspath(path) 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 8f2f2cd38..48bb02c60 100644 --- a/git/refs/log.py +++ b/git/refs/log.py @@ -4,6 +4,7 @@ __all__ = ["RefLog", "RefLogEntry"] from mmap import mmap +import os import os.path as osp import re import time as _time @@ -167,7 +168,7 @@ 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.""" - self._path = filepath + self._path = os.fspath(filepath) if filepath is not None: self._read_from_file() # END handle filepath diff --git a/git/refs/symbolic.py b/git/refs/symbolic.py index c7db129d9..24a72257d 100644 --- a/git/refs/symbolic.py +++ b/git/refs/symbolic.py @@ -76,7 +76,7 @@ class SymbolicReference: def __init__(self, repo: "Repo", path: PathLike, check_path: bool = False) -> None: self.repo = repo - self.path = path + self.path = os.fspath(path) def __str__(self) -> str: return os.fspath(self.path) diff --git a/git/repo/fun.py b/git/repo/fun.py index 1c995c6c6..1c7cfcb04 100644 --- a/git/repo/fun.py +++ b/git/repo/fun.py @@ -62,6 +62,7 @@ def is_git_dir(d: PathLike) -> bool: clearly indicates that we don't support it. There is the unlikely danger to throw if we see directories which just look like a worktree dir, but are none. """ + d = os.fspath(d) if osp.isdir(d): if (osp.isdir(osp.join(d, "objects")) or "GIT_OBJECT_DIRECTORY" in os.environ) and osp.isdir( osp.join(d, "refs") diff --git a/git/util.py b/git/util.py index f18eb6e52..5326af9d1 100644 --- a/git/util.py +++ b/git/util.py @@ -1011,7 +1011,7 @@ class LockFile: __slots__ = ("_file_path", "_owns_lock") def __init__(self, file_path: PathLike) -> None: - self._file_path = file_path + self._file_path = os.fspath(file_path) self._owns_lock = False def __del__(self) -> None: From 086e83239388988772e21ee820c23e59533382f7 Mon Sep 17 00:00:00 2001 From: George Ogden Date: Fri, 28 Nov 2025 21:48:38 +0000 Subject: [PATCH 09/18] Fix union type conversion to path --- git/refs/log.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git/refs/log.py b/git/refs/log.py index 48bb02c60..d1cc393f4 100644 --- a/git/refs/log.py +++ b/git/refs/log.py @@ -168,7 +168,7 @@ 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.""" - self._path = os.fspath(filepath) + self._path = None if filepath is None else os.fspath(filepath) if filepath is not None: self._read_from_file() # END handle filepath From b3908ed04815b1d89a000fc7824a804c37906365 Mon Sep 17 00:00:00 2001 From: George Ogden <38294960+George-Ogden@users.noreply.github.com> Date: Sat, 29 Nov 2025 11:56:57 +0000 Subject: [PATCH 10/18] Use converted file path Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- git/index/util.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git/index/util.py b/git/index/util.py index 231634cd6..80f0b014c 100644 --- a/git/index/util.py +++ b/git/index/util.py @@ -38,7 +38,7 @@ class TemporaryFileSwap: def __init__(self, file_path: PathLike) -> None: self.file_path = os.fspath(file_path) - dirname, basename = osp.split(file_path) + dirname, basename = osp.split(self.file_path) fd, self.tmp_file_path = tempfile.mkstemp(prefix=basename, dir=dirname) os.close(fd) with contextlib.suppress(OSError): # It may be that the source does not exist. From 50aea998641248f501735421ddc6165cbdb5d08c Mon Sep 17 00:00:00 2001 From: George Ogden <38294960+George-Ogden@users.noreply.github.com> Date: Sat, 29 Nov 2025 11:57:25 +0000 Subject: [PATCH 11/18] Remove redundant `fspath` Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- git/refs/symbolic.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git/refs/symbolic.py b/git/refs/symbolic.py index 24a72257d..7cf812416 100644 --- a/git/refs/symbolic.py +++ b/git/refs/symbolic.py @@ -79,7 +79,7 @@ def __init__(self, repo: "Repo", path: PathLike, check_path: bool = False) -> No self.path = os.fspath(path) def __str__(self) -> str: - return os.fspath(self.path) + return self.path def __repr__(self) -> str: return '' % (self.__class__.__name__, self.path) From 57a3af1ddc9b03f59a3e6d3f012c6043905763c0 Mon Sep 17 00:00:00 2001 From: George Ogden <38294960+George-Ogden@users.noreply.github.com> Date: Sat, 29 Nov 2025 11:57:55 +0000 Subject: [PATCH 12/18] Remove redundant `fspath` Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- git/refs/symbolic.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git/refs/symbolic.py b/git/refs/symbolic.py index 7cf812416..3cadb5061 100644 --- a/git/refs/symbolic.py +++ b/git/refs/symbolic.py @@ -103,7 +103,7 @@ def name(self) -> str: In case of symbolic references, the shortest assumable name is the path itself. """ - return os.fspath(self.path) + return self.path @property def abspath(self) -> PathLike: From df8087a2c90e25692eb8d4f09c8726fc78e21d05 Mon Sep 17 00:00:00 2001 From: George Ogden Date: Sat, 29 Nov 2025 12:24:01 +0000 Subject: [PATCH 13/18] Remove a large number of redundant fspaths --- git/index/base.py | 2 +- git/index/fun.py | 2 +- git/objects/submodule/base.py | 2 +- git/refs/symbolic.py | 4 ++-- git/repo/base.py | 6 +++--- git/util.py | 3 +-- test/test_index.py | 8 +++++--- 7 files changed, 14 insertions(+), 13 deletions(-) diff --git a/git/index/base.py b/git/index/base.py index cf7220ef8..2489949c1 100644 --- a/git/index/base.py +++ b/git/index/base.py @@ -1360,7 +1360,7 @@ def make_exc() -> GitCommandError: try: self.entries[(co_path, 0)] except KeyError: - folder = os.fspath(co_path) + folder = co_path if not folder.endswith("/"): folder += "/" for entry in self.entries.values(): diff --git a/git/index/fun.py b/git/index/fun.py index 845221c61..9832aea6b 100644 --- a/git/index/fun.py +++ b/git/index/fun.py @@ -87,7 +87,7 @@ def run_commit_hook(name: str, index: "IndexFile", *args: str) -> None: return env = os.environ.copy() - env["GIT_INDEX_FILE"] = safe_decode(os.fspath(index.path)) + env["GIT_INDEX_FILE"] = safe_decode(index.path) env["GIT_EDITOR"] = ":" cmd = [hp] try: diff --git a/git/objects/submodule/base.py b/git/objects/submodule/base.py index cc1f94c6c..ffc1d3595 100644 --- a/git/objects/submodule/base.py +++ b/git/objects/submodule/base.py @@ -1229,7 +1229,7 @@ def remove( wtd = mod.working_tree_dir del mod # Release file-handles (Windows). gc.collect() - rmtree(str(wtd)) + rmtree(wtd) # END delete tree if possible # END handle force diff --git a/git/refs/symbolic.py b/git/refs/symbolic.py index 3cadb5061..557e8f5b4 100644 --- a/git/refs/symbolic.py +++ b/git/refs/symbolic.py @@ -706,7 +706,7 @@ def _create( if not force and os.path.isfile(abs_ref_path): target_data = str(target) if isinstance(target, SymbolicReference): - target_data = os.fspath(target.path) + target_data = target.path if not resolve: target_data = "ref: " + target_data with open(abs_ref_path, "rb") as fd: @@ -931,4 +931,4 @@ def from_path(cls: Type[T_References], repo: "Repo", path: PathLike) -> T_Refere def is_remote(self) -> bool: """:return: True if this symbolic reference points to a remote branch""" - return os.fspath(self.path).startswith(self._remote_common_path_default + "/") + return self.path.startswith(self._remote_common_path_default + "/") diff --git a/git/repo/base.py b/git/repo/base.py index 2fe18f48c..e721aea40 100644 --- a/git/repo/base.py +++ b/git/repo/base.py @@ -126,7 +126,7 @@ class Repo: working_dir: PathLike """The working directory of the git command.""" - _working_tree_dir: Optional[PathLike] = None + _working_tree_dir: Optional[str] = None git_dir: PathLike """The ``.git`` repository directory.""" @@ -306,7 +306,7 @@ def __init__( self._working_tree_dir = None # END working dir handling - self.working_dir: PathLike = self._working_tree_dir or self.common_dir + self.working_dir: str = self._working_tree_dir or self.common_dir self.git = self.GitCommandWrapperType(self.working_dir) # Special handling, in special times. @@ -366,7 +366,7 @@ def description(self, descr: str) -> None: fp.write((descr + "\n").encode(defenc)) @property - def working_tree_dir(self) -> Optional[PathLike]: + def working_tree_dir(self) -> Optional[str]: """ :return: The working tree directory of our git repository. diff --git a/git/util.py b/git/util.py index 5326af9d1..94452ab17 100644 --- a/git/util.py +++ b/git/util.py @@ -397,8 +397,7 @@ def _cygexpath(drive: Optional[str], path: str) -> str: p = cygpath(p) elif drive: p = "/proc/cygdrive/%s/%s" % (drive.lower(), p) - p_str = os.fspath(p) # ensure it is a str and not AnyPath - return p_str.replace("\\", "/") + return p.replace("\\", "/") _cygpath_parsers: Tuple[Tuple[Pattern[str], Callable, bool], ...] = ( diff --git a/test/test_index.py b/test/test_index.py index c1db4166b..bca353748 100644 --- a/test/test_index.py +++ b/test/test_index.py @@ -579,12 +579,14 @@ def mixed_iterator(): if type_id == 0: # path (str) yield entry.path elif type_id == 1: # path (PathLike) + yield Path(entry.path) + elif type_id == 2: # path mock (PathLike) yield PathLikeMock(entry.path) - elif type_id == 2: # blob + elif type_id == 3: # blob yield Blob(rw_repo, entry.binsha, entry.mode, entry.path) - elif type_id == 3: # BaseIndexEntry + elif type_id == 4: # BaseIndexEntry yield BaseIndexEntry(entry[:4]) - elif type_id == 4: # IndexEntry + elif type_id == 5: # IndexEntry yield entry else: raise AssertionError("Invalid Type") From 17225612969dd68cdfa6dc6b7d5ea8d1956da533 Mon Sep 17 00:00:00 2001 From: George Ogden Date: Sat, 29 Nov 2025 13:31:22 +0000 Subject: [PATCH 14/18] Remove redundant str call --- git/repo/base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git/repo/base.py b/git/repo/base.py index e721aea40..72869c562 100644 --- a/git/repo/base.py +++ b/git/repo/base.py @@ -1386,7 +1386,7 @@ def _clone( proc = git.clone( multi, "--", - Git.polish_url(str(url)), + Git.polish_url(url), clone_path, with_extended_output=True, as_process=True, From 921ca8a1ddcfe84fce3d6f7f9b50a421cbee9012 Mon Sep 17 00:00:00 2001 From: George Ogden Date: Sat, 29 Nov 2025 13:37:12 +0000 Subject: [PATCH 15/18] Limit mypy version due to Cygwin errors --- test-requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test-requirements.txt b/test-requirements.txt index 75e9e81fa..552496ae4 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -1,7 +1,7 @@ coverage[toml] ddt >= 1.1.1, != 1.4.3 mock ; python_version < "3.8" -mypy +mypy<1.19.0 pre-commit pytest >= 7.3.1 pytest-cov From 12e15ba71a49652af33a8bb1556a24a19dd15c91 Mon Sep 17 00:00:00 2001 From: George Ogden Date: Sat, 29 Nov 2025 21:27:12 +0000 Subject: [PATCH 16/18] Validate every fspath with tests --- git/index/base.py | 9 ++++----- git/index/fun.py | 4 ++-- git/index/typ.py | 4 ++-- git/index/util.py | 6 +++--- git/objects/submodule/base.py | 10 +++++----- git/refs/head.py | 2 -- git/refs/log.py | 3 +-- git/refs/symbolic.py | 15 ++++++++------- git/repo/base.py | 14 ++++++++------ git/repo/fun.py | 1 - git/util.py | 13 +++++++------ test/test_index.py | 8 +++++--- test/test_refs.py | 21 ++++++++++++++++++++- test/test_repo.py | 10 ++++++++++ test/test_submodule.py | 6 +++++- 15 files changed, 80 insertions(+), 46 deletions(-) diff --git a/git/index/base.py b/git/index/base.py index 2489949c1..43def2f06 100644 --- a/git/index/base.py +++ b/git/index/base.py @@ -434,7 +434,7 @@ def raise_exc(e: Exception) -> NoReturn: # characters. if abs_path not in resolved_paths: for f in self._iter_expand_paths(glob.glob(abs_path)): - yield os.fspath(f).replace(rs, "") + yield str(f).replace(rs, "") continue # END glob handling try: @@ -569,7 +569,7 @@ def resolve_blobs(self, iter_blobs: Iterator[Blob]) -> "IndexFile": for blob in iter_blobs: stage_null_key = (blob.path, 0) if stage_null_key in self.entries: - raise ValueError("Path %r already exists at stage 0" % os.fspath(blob.path)) + raise ValueError("Path %r already exists at stage 0" % str(blob.path)) # END assert blob is not stage 0 already # Delete all possible stages. @@ -652,7 +652,6 @@ def _to_relative_path(self, path: PathLike) -> PathLike: :raise ValueError: """ - path = os.fspath(path) if not osp.isabs(path): return path if self.repo.bare: @@ -660,7 +659,7 @@ def _to_relative_path(self, path: PathLike) -> PathLike: if not osp.normpath(path).startswith(os.fspath(self.repo.working_tree_dir)): raise ValueError("Absolute path %r is not in git repository at %r" % (path, self.repo.working_tree_dir)) result = os.path.relpath(path, self.repo.working_tree_dir) - if path.endswith(os.sep) and not result.endswith(os.sep): + if os.fspath(path).endswith(os.sep) and not result.endswith(os.sep): result += os.sep return result @@ -1364,7 +1363,7 @@ def make_exc() -> GitCommandError: if not folder.endswith("/"): folder += "/" for entry in self.entries.values(): - if os.fspath(entry.path).startswith(folder): + if entry.path.startswith(folder): p = entry.path self._write_path_to_stdin(proc, p, p, make_exc, fprogress, read_from_stdout=False) checked_out_files.append(p) diff --git a/git/index/fun.py b/git/index/fun.py index 9832aea6b..629c19b1e 100644 --- a/git/index/fun.py +++ b/git/index/fun.py @@ -87,7 +87,7 @@ def run_commit_hook(name: str, index: "IndexFile", *args: str) -> None: return env = os.environ.copy() - env["GIT_INDEX_FILE"] = safe_decode(index.path) + env["GIT_INDEX_FILE"] = safe_decode(os.fspath(index.path)) env["GIT_EDITOR"] = ":" cmd = [hp] try: @@ -167,7 +167,7 @@ def write_cache( beginoffset = tell() write(entry.ctime_bytes) # ctime write(entry.mtime_bytes) # mtime - path_str = os.fspath(entry.path) + path_str = str(entry.path) path: bytes = force_bytes(path_str, encoding=defenc) plen = len(path) & CE_NAMEMASK # Path length assert plen == len(path), "Path %s too long to fit into index" % entry.path diff --git a/git/index/typ.py b/git/index/typ.py index 8273e5895..927633a9f 100644 --- a/git/index/typ.py +++ b/git/index/typ.py @@ -58,9 +58,9 @@ def __init__(self, paths: Sequence[PathLike]) -> None: def __call__(self, stage_blob: Tuple[StageType, Blob]) -> bool: blob_pathlike: PathLike = stage_blob[1].path - blob_path = Path(blob_pathlike) + blob_path: Path = blob_pathlike if isinstance(blob_pathlike, Path) else Path(blob_pathlike) for pathlike in self.paths: - path = Path(pathlike) + path: Path = pathlike if isinstance(pathlike, Path) else Path(pathlike) # TODO: Change to use `PosixPath.is_relative_to` once Python 3.8 is no # longer supported. filter_parts = path.parts diff --git a/git/index/util.py b/git/index/util.py index 80f0b014c..15eba0052 100644 --- a/git/index/util.py +++ b/git/index/util.py @@ -37,8 +37,8 @@ class TemporaryFileSwap: __slots__ = ("file_path", "tmp_file_path") def __init__(self, file_path: PathLike) -> None: - self.file_path = os.fspath(file_path) - dirname, basename = osp.split(self.file_path) + self.file_path = file_path + dirname, basename = osp.split(file_path) fd, self.tmp_file_path = tempfile.mkstemp(prefix=basename, dir=dirname) os.close(fd) with contextlib.suppress(OSError): # It may be that the source does not exist. @@ -106,7 +106,7 @@ def git_working_dir(func: Callable[..., _T]) -> Callable[..., _T]: @wraps(func) def set_git_working_dir(self: "IndexFile", *args: Any, **kwargs: Any) -> _T: cur_wd = os.getcwd() - os.chdir(os.fspath(self.repo.working_tree_dir)) + os.chdir(self.repo.working_tree_dir) try: return func(self, *args, **kwargs) finally: diff --git a/git/objects/submodule/base.py b/git/objects/submodule/base.py index ca6253883..36ec7c538 100644 --- a/git/objects/submodule/base.py +++ b/git/objects/submodule/base.py @@ -352,7 +352,7 @@ def _clone_repo( module_abspath_dir = osp.dirname(module_abspath) if not osp.isdir(module_abspath_dir): os.makedirs(module_abspath_dir) - module_checkout_path = osp.join(os.fspath(repo.working_tree_dir), path) + module_checkout_path = osp.join(repo.working_tree_dir, path) if url.startswith("../"): remote_name = cast("RemoteReference", repo.active_branch.tracking_branch()).remote_name @@ -1016,7 +1016,7 @@ def move(self, module_path: PathLike, configuration: bool = True, module: bool = return self # END handle no change - module_checkout_abspath = join_path_native(os.fspath(self.repo.working_tree_dir), module_checkout_path) + module_checkout_abspath = join_path_native(str(self.repo.working_tree_dir), module_checkout_path) if osp.isfile(module_checkout_abspath): raise ValueError("Cannot move repository onto a file: %s" % module_checkout_abspath) # END handle target files @@ -1229,7 +1229,7 @@ def remove( wtd = mod.working_tree_dir del mod # Release file-handles (Windows). gc.collect() - rmtree(wtd) + rmtree(str(wtd)) # END delete tree if possible # END handle force @@ -1313,7 +1313,7 @@ def set_parent_commit(self, commit: Union[Commit_ish, str, None], check: bool = # If check is False, we might see a parent-commit that doesn't even contain the # submodule anymore. in that case, mark our sha as being NULL. try: - self.binsha = pctree[os.fspath(self.path)].binsha + self.binsha = pctree[str(self.path)].binsha except KeyError: self.binsha = self.NULL_BIN_SHA @@ -1395,7 +1395,7 @@ def rename(self, new_name: str) -> "Submodule": destination_module_abspath = self._module_abspath(self.repo, self.path, new_name) source_dir = mod.git_dir # Let's be sure the submodule name is not so obviously tied to a directory. - if os.fspath(destination_module_abspath).startswith(os.fspath(mod.git_dir)): + if str(destination_module_abspath).startswith(str(mod.git_dir)): tmp_dir = self._module_abspath(self.repo, self.path, str(uuid.uuid4())) os.renames(source_dir, tmp_dir) source_dir = tmp_dir diff --git a/git/refs/head.py b/git/refs/head.py index 9959b889b..3c43993e7 100644 --- a/git/refs/head.py +++ b/git/refs/head.py @@ -8,7 +8,6 @@ __all__ = ["HEAD", "Head"] -import os from git.config import GitConfigParser, SectionConstraint from git.exc import GitCommandError from git.util import join_path @@ -45,7 +44,6 @@ class HEAD(SymbolicReference): __slots__ = () def __init__(self, repo: "Repo", path: PathLike = _HEAD_NAME) -> None: - path = os.fspath(path) 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 a5dfa6d20..4751cff99 100644 --- a/git/refs/log.py +++ b/git/refs/log.py @@ -4,7 +4,6 @@ __all__ = ["RefLog", "RefLogEntry"] from mmap import mmap -import os import os.path as osp import re import time as _time @@ -168,7 +167,7 @@ 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.""" - self._path = None if filepath is None else os.fspath(filepath) + self._path = filepath if filepath is not None: self._read_from_file() # END handle filepath diff --git a/git/refs/symbolic.py b/git/refs/symbolic.py index 77e4b98f2..a422fb78c 100644 --- a/git/refs/symbolic.py +++ b/git/refs/symbolic.py @@ -4,6 +4,7 @@ __all__ = ["SymbolicReference"] import os +from pathlib import Path from gitdb.exc import BadName, BadObject @@ -76,10 +77,10 @@ class SymbolicReference: def __init__(self, repo: "Repo", path: PathLike, check_path: bool = False) -> None: self.repo = repo - self.path = os.fspath(path) + self.path: PathLike = path def __str__(self) -> str: - return self.path + return os.fspath(self.path) def __repr__(self) -> str: return '' % (self.__class__.__name__, self.path) @@ -103,7 +104,7 @@ def name(self) -> str: In case of symbolic references, the shortest assumable name is the path itself. """ - return self.path + return os.fspath(self.path) @property def abspath(self) -> PathLike: @@ -212,7 +213,7 @@ def _check_ref_name_valid(ref_path: PathLike) -> None: raise ValueError(f"Invalid reference '{ref_path}': references cannot end with a forward slash (/)") elif previous == "@" and one_before_previous is None: raise ValueError(f"Invalid reference '{ref_path}': references cannot be '@'") - elif any(component.endswith(".lock") for component in os.fspath(ref_path).split("/")): + elif any(component.endswith(".lock") for component in Path(ref_path).parts): raise ValueError( f"Invalid reference '{ref_path}': references cannot have slash-separated components that end with" " '.lock'" @@ -235,7 +236,7 @@ def _get_ref_info_helper( tokens: Union[None, List[str], Tuple[str, str]] = None repodir = _git_dir(repo, ref_path) try: - with open(os.path.join(repodir, os.fspath(ref_path)), "rt", encoding="UTF-8") as fp: + with open(os.path.join(repodir, ref_path), "rt", encoding="UTF-8") as fp: value = fp.read().rstrip() # Don't only split on spaces, but on whitespace, which allows to parse lines like: # 60b64ef992065e2600bfef6187a97f92398a9144 branch 'master' of git-server:/path/to/repo @@ -706,7 +707,7 @@ def _create( if not force and os.path.isfile(abs_ref_path): target_data = str(target) if isinstance(target, SymbolicReference): - target_data = target.path + target_data = os.fspath(target.path) if not resolve: target_data = "ref: " + target_data with open(abs_ref_path, "rb") as fd: @@ -930,4 +931,4 @@ def from_path(cls: Type[T_References], repo: "Repo", path: PathLike) -> T_Refere def is_remote(self) -> bool: """:return: True if this symbolic reference points to a remote branch""" - return self.path.startswith(self._remote_common_path_default + "/") + return os.fspath(self.path).startswith(self._remote_common_path_default + "/") diff --git a/git/repo/base.py b/git/repo/base.py index d1af79620..1f543cc57 100644 --- a/git/repo/base.py +++ b/git/repo/base.py @@ -126,7 +126,8 @@ class Repo: working_dir: PathLike """The working directory of the git command.""" - _working_tree_dir: Optional[str] = None + # stored as string for easier processing, but annotated as path for clearer intention + _working_tree_dir: Optional[PathLike] = None git_dir: PathLike """The ``.git`` repository directory.""" @@ -215,13 +216,13 @@ def __init__( epath = path or os.getenv("GIT_DIR") if not epath: epath = os.getcwd() + epath = os.fspath(epath) if Git.is_cygwin(): # Given how the tests are written, this seems more likely to catch Cygwin # git used from Windows than Windows git used from Cygwin. Therefore # changing to Cygwin-style paths is the relevant operation. - epath = cygpath(os.fspath(epath)) + epath = cygpath(epath) - epath = os.fspath(epath) if expand_vars and re.search(self.re_envvars, epath): warnings.warn( "The use of environment variables in paths is deprecated" @@ -306,7 +307,7 @@ def __init__( self._working_tree_dir = None # END working dir handling - self.working_dir: str = self._working_tree_dir or self.common_dir + self.working_dir: PathLike = self._working_tree_dir or self.common_dir self.git = self.GitCommandWrapperType(self.working_dir) # Special handling, in special times. @@ -366,7 +367,7 @@ def description(self, descr: str) -> None: fp.write((descr + "\n").encode(defenc)) @property - def working_tree_dir(self) -> Optional[str]: + def working_tree_dir(self) -> Optional[PathLike]: """ :return: The working tree directory of our git repository. @@ -554,7 +555,7 @@ def tag(self, path: PathLike) -> TagReference: @staticmethod def _to_full_tag_path(path: PathLike) -> str: - path_str = os.fspath(path) + path_str = str(path) if path_str.startswith(TagReference._common_path_default + "/"): return path_str if path_str.startswith(TagReference._common_default + "/"): @@ -1355,6 +1356,7 @@ def _clone( ) -> "Repo": odbt = kwargs.pop("odbt", odb_default_type) + # url may be a path and this has no effect if it is a string url = os.fspath(url) path = os.fspath(path) diff --git a/git/repo/fun.py b/git/repo/fun.py index 49097c373..3f00e60ea 100644 --- a/git/repo/fun.py +++ b/git/repo/fun.py @@ -62,7 +62,6 @@ def is_git_dir(d: PathLike) -> bool: clearly indicates that we don't support it. There is the unlikely danger to throw if we see directories which just look like a worktree dir, but are none. """ - d = os.fspath(d) if osp.isdir(d): if (osp.isdir(osp.join(d, "objects")) or "GIT_OBJECT_DIRECTORY" in os.environ) and osp.isdir( osp.join(d, "refs") diff --git a/git/util.py b/git/util.py index e82ccdcfa..c3ffdd62b 100644 --- a/git/util.py +++ b/git/util.py @@ -36,7 +36,7 @@ import logging import os import os.path as osp -import pathlib +from pathlib import Path import platform import re import shutil @@ -397,7 +397,8 @@ def _cygexpath(drive: Optional[str], path: str) -> str: p = cygpath(p) elif drive: p = "/proc/cygdrive/%s/%s" % (drive.lower(), p) - return p.replace("\\", "/") + p_str = os.fspath(p) # ensure it is a str and not AnyPath + return p_str.replace("\\", "/") _cygpath_parsers: Tuple[Tuple[Pattern[str], Callable, bool], ...] = ( @@ -464,7 +465,7 @@ def _is_cygwin_git(git_executable: str) -> bool: # Just a name given, not a real path. uname_cmd = osp.join(git_dir, "uname") - if not (pathlib.Path(uname_cmd).is_file() and os.access(uname_cmd, os.X_OK)): + if not (Path(uname_cmd).is_file() and os.access(uname_cmd, os.X_OK)): _logger.debug(f"Failed checking if running in CYGWIN: {uname_cmd} is not an executable") _is_cygwin_cache[git_executable] = is_cygwin return is_cygwin @@ -496,7 +497,7 @@ def is_cygwin_git(git_executable: Union[None, PathLike]) -> bool: elif git_executable is None: return False else: - return _is_cygwin_git(os.fspath(git_executable)) + return _is_cygwin_git(str(git_executable)) def get_user_id() -> str: @@ -522,7 +523,7 @@ def expand_path(p: PathLike, expand_vars: bool = ...) -> str: def expand_path(p: Union[None, PathLike], expand_vars: bool = True) -> Optional[PathLike]: - if isinstance(p, pathlib.Path): + if isinstance(p, Path): return p.resolve() try: p = osp.expanduser(p) # type: ignore[arg-type] @@ -1010,7 +1011,7 @@ class LockFile: __slots__ = ("_file_path", "_owns_lock") def __init__(self, file_path: PathLike) -> None: - self._file_path = os.fspath(file_path) + self._file_path = file_path self._owns_lock = False def __del__(self) -> None: diff --git a/test/test_index.py b/test/test_index.py index bca353748..33490f907 100644 --- a/test/test_index.py +++ b/test/test_index.py @@ -582,11 +582,13 @@ def mixed_iterator(): yield Path(entry.path) elif type_id == 2: # path mock (PathLike) yield PathLikeMock(entry.path) - elif type_id == 3: # blob + elif type_id == 3: # path mock in a blob yield Blob(rw_repo, entry.binsha, entry.mode, entry.path) - elif type_id == 4: # BaseIndexEntry + elif type_id == 4: # blob + yield Blob(rw_repo, entry.binsha, entry.mode, entry.path) + elif type_id == 5: # BaseIndexEntry yield BaseIndexEntry(entry[:4]) - elif type_id == 5: # IndexEntry + elif type_id == 6: # IndexEntry yield entry else: raise AssertionError("Invalid Type") diff --git a/test/test_refs.py b/test/test_refs.py index 08096e69e..329515807 100644 --- a/test/test_refs.py +++ b/test/test_refs.py @@ -25,7 +25,7 @@ import git.refs as refs from git.util import Actor -from test.lib import TestBase, with_rw_repo +from test.lib import TestBase, with_rw_repo, PathLikeMock class TestRefs(TestBase): @@ -43,6 +43,25 @@ def test_from_path(self): self.assertRaises(ValueError, TagReference, self.rorepo, "refs/invalid/tag") # Works without path check. TagReference(self.rorepo, "refs/invalid/tag", check_path=False) + # Check remoteness + assert Reference(self.rorepo, "refs/remotes/origin").is_remote() + + def test_from_pathlike(self): + # Should be able to create any reference directly. + for ref_type in (Reference, Head, TagReference, RemoteReference): + for name in ("rela_name", "path/rela_name"): + full_path = ref_type.to_full_path(PathLikeMock(name)) + instance = ref_type.from_path(self.rorepo, PathLikeMock(full_path)) + assert isinstance(instance, ref_type) + # END for each name + # END for each type + + # Invalid path. + self.assertRaises(ValueError, TagReference, self.rorepo, "refs/invalid/tag") + # Works without path check. + TagReference(self.rorepo, PathLikeMock("refs/invalid/tag"), check_path=False) + # Check remoteness + assert Reference(self.rorepo, PathLikeMock("refs/remotes/origin")).is_remote() def test_tag_base(self): tag_object_refs = [] diff --git a/test/test_repo.py b/test/test_repo.py index f089bf6b8..2a92c2523 100644 --- a/test/test_repo.py +++ b/test/test_repo.py @@ -15,6 +15,7 @@ import sys import tempfile from unittest import mock +from pathlib import Path import pytest @@ -369,6 +370,15 @@ def test_is_dirty_with_path(self, rwrepo): assert rwrepo.is_dirty(path="doc") is False assert rwrepo.is_dirty(untracked_files=True, path="doc") is True + @with_rw_repo("HEAD") + def test_is_dirty_with_pathlib_and_pathlike(self, rwrepo): + with open(osp.join(rwrepo.working_dir, "git", "util.py"), "at") as f: + f.write("junk") + assert rwrepo.is_dirty(path=Path("git")) is True + assert rwrepo.is_dirty(path=PathLikeMock("git")) is True + assert rwrepo.is_dirty(path=Path("doc")) is False + assert rwrepo.is_dirty(path=PathLikeMock("doc")) is False + def test_head(self): self.assertEqual(self.rorepo.head.reference.object, self.rorepo.active_branch.object) diff --git a/test/test_submodule.py b/test/test_submodule.py index edff064c4..2bf0940c9 100644 --- a/test/test_submodule.py +++ b/test/test_submodule.py @@ -28,7 +28,7 @@ from git.repo.fun import find_submodule_git_dir, touch 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 +from test.lib import TestBase, with_rw_directory, with_rw_repo, PathLikeMock @contextlib.contextmanager @@ -175,6 +175,10 @@ def _do_base_tests(self, rwrepo): sma = Submodule.add(rwrepo, sm.name, sm.path) assert sma.path == sm.path + # Adding existing as pathlike + sma = Submodule.add(rwrepo, sm.name, PathLikeMock(sm.path)) + assert sma.path == sm.path + # No url and no module at path fails. self.assertRaises(ValueError, Submodule.add, rwrepo, "newsubm", "pathtorepo", url=None) From 8434967c010cc108d3a8a01d1db6d0c3971b0048 Mon Sep 17 00:00:00 2001 From: George Ogden Date: Sat, 29 Nov 2025 21:46:54 +0000 Subject: [PATCH 17/18] Fix type hints --- git/index/base.py | 10 +++++----- git/index/util.py | 4 ++-- git/objects/submodule/base.py | 2 +- git/refs/symbolic.py | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/git/index/base.py b/git/index/base.py index 43def2f06..93de7933c 100644 --- a/git/index/base.py +++ b/git/index/base.py @@ -404,7 +404,7 @@ def _iter_expand_paths(self: "IndexFile", paths: Sequence[PathLike]) -> Iterator def raise_exc(e: Exception) -> NoReturn: raise e - r = os.fspath(self.repo.working_tree_dir) + r = str(self.repo.working_tree_dir) rs = r + os.sep for path in paths: abs_path = os.fspath(path) @@ -656,7 +656,7 @@ def _to_relative_path(self, path: PathLike) -> PathLike: return path if self.repo.bare: raise InvalidGitRepositoryError("require non-bare repository") - if not osp.normpath(path).startswith(os.fspath(self.repo.working_tree_dir)): + if not osp.normpath(path).startswith(str(self.repo.working_tree_dir)): raise ValueError("Absolute path %r is not in git repository at %r" % (path, self.repo.working_tree_dir)) result = os.path.relpath(path, self.repo.working_tree_dir) if os.fspath(path).endswith(os.sep) and not result.endswith(os.sep): @@ -731,7 +731,7 @@ def _entries_for_paths( for path in paths: if osp.isabs(path): abspath = path - gitrelative_path = path[len(os.fspath(self.repo.working_tree_dir)) + 1 :] + gitrelative_path = path[len(str(self.repo.working_tree_dir)) + 1 :] else: gitrelative_path = path if self.repo.working_tree_dir: @@ -1036,7 +1036,7 @@ def remove( args.append("--") # Preprocess paths. - paths = self._items_to_rela_paths(items) + paths = list(map(os.fspath, self._items_to_rela_paths(items))) # type: ignore[arg-type] removed_paths = self.repo.git.rm(args, paths, **kwargs).splitlines() # Process output to gain proper paths. @@ -1363,7 +1363,7 @@ def make_exc() -> GitCommandError: if not folder.endswith("/"): folder += "/" for entry in self.entries.values(): - if entry.path.startswith(folder): + if os.fspath(entry.path).startswith(folder): p = entry.path self._write_path_to_stdin(proc, p, p, make_exc, fprogress, read_from_stdout=False) checked_out_files.append(p) diff --git a/git/index/util.py b/git/index/util.py index 15eba0052..982a5afb7 100644 --- a/git/index/util.py +++ b/git/index/util.py @@ -15,7 +15,7 @@ # typing ---------------------------------------------------------------------- -from typing import Any, Callable, TYPE_CHECKING, Optional, Type +from typing import Any, Callable, TYPE_CHECKING, Optional, Type, cast from git.types import Literal, PathLike, _T @@ -106,7 +106,7 @@ def git_working_dir(func: Callable[..., _T]) -> Callable[..., _T]: @wraps(func) def set_git_working_dir(self: "IndexFile", *args: Any, **kwargs: Any) -> _T: cur_wd = os.getcwd() - os.chdir(self.repo.working_tree_dir) + os.chdir(cast(PathLike, self.repo.working_tree_dir)) try: return func(self, *args, **kwargs) finally: diff --git a/git/objects/submodule/base.py b/git/objects/submodule/base.py index 36ec7c538..d183672db 100644 --- a/git/objects/submodule/base.py +++ b/git/objects/submodule/base.py @@ -352,7 +352,7 @@ def _clone_repo( module_abspath_dir = osp.dirname(module_abspath) if not osp.isdir(module_abspath_dir): os.makedirs(module_abspath_dir) - module_checkout_path = osp.join(repo.working_tree_dir, path) + module_checkout_path = osp.join(repo.working_tree_dir, path) # type: ignore[arg-type] if url.startswith("../"): remote_name = cast("RemoteReference", repo.active_branch.tracking_branch()).remote_name diff --git a/git/refs/symbolic.py b/git/refs/symbolic.py index a422fb78c..99af4f57c 100644 --- a/git/refs/symbolic.py +++ b/git/refs/symbolic.py @@ -236,7 +236,7 @@ def _get_ref_info_helper( tokens: Union[None, List[str], Tuple[str, str]] = None repodir = _git_dir(repo, ref_path) try: - with open(os.path.join(repodir, ref_path), "rt", encoding="UTF-8") as fp: + with open(os.path.join(repodir, ref_path), "rt", encoding="UTF-8") as fp: # type: ignore[arg-type] value = fp.read().rstrip() # Don't only split on spaces, but on whitespace, which allows to parse lines like: # 60b64ef992065e2600bfef6187a97f92398a9144 branch 'master' of git-server:/path/to/repo From 171062655e24b6a6ca1a3beab3c7679278350ab5 Mon Sep 17 00:00:00 2001 From: George Ogden Date: Mon, 1 Dec 2025 16:44:37 +0000 Subject: [PATCH 18/18] Add tests with non-ascii characters --- test/test_clone.py | 7 +++++++ test/test_index.py | 9 +++++++++ 2 files changed, 16 insertions(+) diff --git a/test/test_clone.py b/test/test_clone.py index 143a3b51f..2d00a9e79 100644 --- a/test/test_clone.py +++ b/test/test_clone.py @@ -52,6 +52,13 @@ def test_clone_from_pathlike(self, rw_dir): original_repo = Repo.init(osp.join(rw_dir, "repo")) Repo.clone_from(PathLikeMock(original_repo.git_dir), PathLikeMock(os.path.join(rw_dir, "clone_pathlike"))) + @with_rw_directory + def test_clone_from_pathlike_unicode_repr(self, rw_dir): + original_repo = Repo.init(osp.join(rw_dir, "repo-áēñöưḩ̣")) + Repo.clone_from( + PathLikeMock(original_repo.git_dir), PathLikeMock(os.path.join(rw_dir, "clone_pathlike-áēñöưḩ̣")) + ) + @with_rw_directory def test_clone_from_pathlib_withConfig(self, rw_dir): original_repo = Repo.init(osp.join(rw_dir, "repo")) diff --git a/test/test_index.py b/test/test_index.py index 33490f907..dcdc3b56d 100644 --- a/test/test_index.py +++ b/test/test_index.py @@ -1212,6 +1212,15 @@ def test_index_add_pathlike(self, rw_repo): rw_repo.index.add(PathLikeMock(str(file))) + @with_rw_repo("HEAD") + def test_index_add_pathlike_unicode(self, rw_repo): + git_dir = Path(rw_repo.git_dir) + + file = git_dir / "file-áēñöưḩ̣.txt" + file.touch() + + rw_repo.index.add(PathLikeMock(str(file))) + @with_rw_repo("HEAD") def test_index_add_non_normalized_path(self, rw_repo): git_dir = Path(rw_repo.git_dir)