From 4c6ec603e21ce81ea37bb69a17a7aaf82f4c4eb2 Mon Sep 17 00:00:00 2001 From: Menashe Eliezer Date: Mon, 20 Apr 2026 11:59:39 +0200 Subject: [PATCH 01/45] fix: support Repo() autodiscovery from linked worktree GIT_DIR Handle linked worktree git directories when GIT_DIR points to .git/worktrees/. Previously Repo() could fail with InvalidGitRepositoryError in this scenario, while Repo(os.getcwd()) worked correctly. Add regression test to cover autodiscovery in linked worktrees. --- git/repo/base.py | 22 ++++++++++++++++++++++ test/test_repo.py | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+) diff --git a/git/repo/base.py b/git/repo/base.py index 16807b9fa..7c3b5b2cc 100644 --- a/git/repo/base.py +++ b/git/repo/base.py @@ -242,6 +242,28 @@ def __init__( # It's important to normalize the paths, as submodules will otherwise # initialize their repo instances with paths that depend on path-portions # that will not exist after being removed. It's just cleaner. + if ( + osp.isfile(osp.join(curpath, "gitdir")) + and osp.isfile(osp.join(curpath, "commondir")) + and osp.isfile(osp.join(curpath, "HEAD")) + ): + git_dir = curpath + + if "GIT_WORK_TREE" in os.environ: + self._working_tree_dir = os.getenv("GIT_WORK_TREE") + else: + # Linked worktree administrative directories store the path to the + # worktree's .git file in their gitdir file (without "gitdir: " prefix). + with open(osp.join(git_dir, "gitdir")) as fp: + worktree_gitfile = fp.read().strip() + + if not osp.isabs(worktree_gitfile): + worktree_gitfile = osp.normpath(osp.join(git_dir, worktree_gitfile)) + + self._working_tree_dir = osp.dirname(worktree_gitfile) + + break + if is_git_dir(curpath): git_dir = curpath # from man git-config : core.worktree diff --git a/test/test_repo.py b/test/test_repo.py index 544b5c561..65d122fd7 100644 --- a/test/test_repo.py +++ b/test/test_repo.py @@ -1126,6 +1126,42 @@ def test_git_work_tree_env(self, rw_dir): self.assertEqual(r.working_tree_dir, repo_dir) self.assertEqual(r.working_dir, repo_dir) + @with_rw_directory + def test_git_work_tree_env_in_linked_worktree(self, rw_dir): + """Check that Repo() autodiscovers a linked worktree when GIT_DIR is set.""" + git = Git(rw_dir) + if git.version_info[:3] < (2, 5, 1): + raise RuntimeError("worktree feature unsupported (test needs git 2.5.1 or later)") + + rw_master = self.rorepo.clone(join_path_native(rw_dir, "master_repo")) + branch = rw_master.create_head("bbbbbbbb") + worktree_path = join_path_native(rw_dir, "worktree_repo") + if Git.is_cygwin(): + worktree_path = cygpath(worktree_path) + + rw_master.git.worktree("add", worktree_path, branch.name) + + git_dir = Git(worktree_path).rev_parse("--git-dir") + + patched_env = dict(os.environ) + patched_env["GIT_DIR"] = git_dir + patched_env.pop("GIT_WORK_TREE", None) + patched_env.pop("GIT_COMMON_DIR", None) + + with mock.patch.dict(os.environ, patched_env, clear=True): + old_cwd = os.getcwd() + try: + os.chdir(worktree_path) + + explicit = Repo(os.getcwd()) + autodiscovered = Repo() + + self.assertTrue(osp.samefile(explicit.working_tree_dir, worktree_path)) + self.assertTrue(osp.samefile(autodiscovered.working_tree_dir, worktree_path)) + self.assertTrue(osp.samefile(autodiscovered.working_tree_dir, explicit.working_tree_dir)) + finally: + os.chdir(old_cwd) + @with_rw_directory def test_rebasing(self, rw_dir): r = Repo.init(rw_dir) From 25ba54dd3fb374b8fade7de4be1ac2ac84722190 Mon Sep 17 00:00:00 2001 From: "GPT 5.5" Date: Tue, 28 Apr 2026 09:17:31 +0800 Subject: [PATCH 02/45] prevent out-of-repo access when manipulating references. This previously made it possible to create, modify and delete files outside outside of the repository, which is a problem if inputs aren't trusted. Co-authored-by: Sebastian Thiel --- git/refs/log.py | 2 +- git/refs/remote.py | 5 ++- git/refs/symbolic.py | 37 +++++++++++++++--- test/test_refs.py | 91 ++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 126 insertions(+), 9 deletions(-) diff --git a/git/refs/log.py b/git/refs/log.py index 4751cff99..037e143d5 100644 --- a/git/refs/log.py +++ b/git/refs/log.py @@ -213,7 +213,7 @@ def path(cls, ref: "SymbolicReference") -> str: :param ref: :class:`~git.refs.symbolic.SymbolicReference` instance """ - return osp.join(ref.repo.git_dir, "logs", to_native_path(ref.path)) + return to_native_path(ref._get_validated_reflog_path(ref.repo, ref.path)) @classmethod def iter_entries(cls, stream: Union[str, "BytesIO", mmap]) -> Iterator[RefLogEntry]: diff --git a/git/refs/remote.py b/git/refs/remote.py index b4f4f7b36..8244470b0 100644 --- a/git/refs/remote.py +++ b/git/refs/remote.py @@ -63,12 +63,13 @@ def delete(cls, repo: "Repo", *refs: "RemoteReference", **kwargs: Any) -> None: # generally ignored in the refs/ folder. We don't though and delete remainders # manually. for ref in refs: + cls._check_ref_name_valid(ref.path) try: - os.remove(os.path.join(repo.common_dir, ref.path)) + os.remove(cls._get_validated_path(repo.common_dir, ref.path)) except OSError: pass try: - os.remove(os.path.join(repo.git_dir, ref.path)) + os.remove(cls._get_validated_path(repo.git_dir, ref.path)) except OSError: pass # END for each ref diff --git a/git/refs/symbolic.py b/git/refs/symbolic.py index 99af4f57c..020de5e13 100644 --- a/git/refs/symbolic.py +++ b/git/refs/symbolic.py @@ -110,6 +110,32 @@ def name(self) -> str: def abspath(self) -> PathLike: return join_path_native(_git_dir(self.repo, self.path), self.path) + @staticmethod + def _get_validated_path(base: PathLike, path: PathLike) -> str: + path = os.fspath(path) + base_path = os.path.realpath(os.fspath(base)) + abs_path = os.path.realpath(os.path.join(base_path, path)) + try: + common_path = os.path.commonpath([base_path, abs_path]) + except ValueError as e: + raise ValueError("Reference path %r escapes the repository" % path) from e + if os.path.normcase(common_path) != os.path.normcase(base_path): + raise ValueError("Reference path %r escapes the repository" % path) + return abs_path + + @classmethod + def _get_validated_ref_path(cls, repo: "Repo", path: PathLike) -> str: + """Return the absolute filesystem path for a ref after validating it.""" + cls._check_ref_name_valid(path) + ref_path = os.fspath(path) + return cls._get_validated_path(_git_dir(repo, ref_path), ref_path) + + @classmethod + def _get_validated_reflog_path(cls, repo: "Repo", path: PathLike) -> str: + """Return the absolute filesystem path for a reflog after validating it.""" + cls._check_ref_name_valid(path) + return cls._get_validated_path(os.path.join(repo.git_dir, "logs"), path) + @classmethod def _get_packed_refs_path(cls, repo: "Repo") -> str: return os.path.join(repo.common_dir, "packed-refs") @@ -485,7 +511,7 @@ def set_reference( # END handle non-existing # END retrieve old hexsha - fpath = self.abspath + fpath = self._get_validated_ref_path(self.repo, self.path) assure_directory_exists(fpath, is_file=True) lfd = LockedFD(fpath) @@ -632,7 +658,7 @@ def delete(cls, repo: "Repo", path: PathLike) -> None: Alternatively the symbolic reference to be deleted. """ full_ref_path = cls.to_full_path(path) - abs_path = os.path.join(repo.common_dir, full_ref_path) + abs_path = cls._get_validated_ref_path(repo, full_ref_path) if os.path.exists(abs_path): os.remove(abs_path) else: @@ -695,9 +721,8 @@ def _create( symbolic reference. Otherwise it will be resolved to the corresponding object and a detached symbolic reference will be created instead. """ - git_dir = _git_dir(repo, path) full_ref_path = cls.to_full_path(path) - abs_ref_path = os.path.join(git_dir, full_ref_path) + abs_ref_path = cls._get_validated_ref_path(repo, full_ref_path) # Figure out target data. target = reference @@ -789,8 +814,8 @@ def rename(self, new_path: PathLike, force: bool = False) -> "SymbolicReference" if self.path == new_path: return self - new_abs_path = os.path.join(_git_dir(self.repo, new_path), new_path) - cur_abs_path = os.path.join(_git_dir(self.repo, self.path), self.path) + new_abs_path = self._get_validated_ref_path(self.repo, new_path) + cur_abs_path = self._get_validated_ref_path(self.repo, self.path) if os.path.isfile(new_abs_path): if not force: # If they point to the same file, it's not an error. diff --git a/test/test_refs.py b/test/test_refs.py index 329515807..4337f35e1 100644 --- a/test/test_refs.py +++ b/test/test_refs.py @@ -18,6 +18,7 @@ RefLog, Reference, RemoteReference, + Repo, SymbolicReference, TagReference, ) @@ -29,6 +30,14 @@ class TestRefs(TestBase): + def _repo_with_initial_commit(self, base_dir): + repo_dir = base_dir / "repo" + repo = Repo.init(repo_dir) + (repo_dir / "file.txt").write_text("initial\n", encoding="utf-8") + repo.index.add(["file.txt"]) + repo.index.commit("initial") + return repo + def test_from_path(self): # Should be able to create any reference directly. for ref_type in (Reference, Head, TagReference, RemoteReference): @@ -648,6 +657,88 @@ def test_refs_outside_repo(self): ref_file_name = Path(ref_file.name).name self.assertRaises(BadName, self.rorepo.commit, f"../../{ref_file_name}") + def test_reference_create_rejects_path_traversal(self): + with tempfile.TemporaryDirectory() as tmp_dir: + base_dir = Path(tmp_dir) + repo = self._repo_with_initial_commit(base_dir) + outside_path = base_dir / "outside_write.txt" + + self.assertRaises(ValueError, Reference.create, repo, "../../../outside_write.txt", "HEAD") + assert not outside_path.exists() + + def test_symbolic_reference_create_rejects_path_traversal(self): + with tempfile.TemporaryDirectory() as tmp_dir: + base_dir = Path(tmp_dir) + repo = self._repo_with_initial_commit(base_dir) + outside_path = base_dir / "outside_write.txt" + + self.assertRaises(ValueError, SymbolicReference.create, repo, "../../outside_write.txt", "HEAD") + assert not outside_path.exists() + + def test_symbolic_reference_set_reference_rejects_path_traversal(self): + with tempfile.TemporaryDirectory() as tmp_dir: + base_dir = Path(tmp_dir) + repo = self._repo_with_initial_commit(base_dir) + outside_path = base_dir / "outside_write.txt" + + self.assertRaises(ValueError, SymbolicReference(repo, "../../outside_write.txt").set_reference, "HEAD") + assert not outside_path.exists() + + def test_symbolic_reference_rename_rejects_path_traversal(self): + with tempfile.TemporaryDirectory() as tmp_dir: + base_dir = Path(tmp_dir) + repo = self._repo_with_initial_commit(base_dir) + outside_path = base_dir / "outside_move.txt" + ref = SymbolicReference.create(repo, "SAFE_RENAME_SOURCE", "HEAD") + + self.assertRaises(ValueError, ref.rename, "../../outside_move.txt") + assert not outside_path.exists() + assert Path(ref.abspath).is_file() + + def test_symbolic_reference_delete_rejects_path_traversal(self): + with tempfile.TemporaryDirectory() as tmp_dir: + base_dir = Path(tmp_dir) + repo = self._repo_with_initial_commit(base_dir) + outside_path = base_dir / "outside_delete.txt" + outside_path.write_text("do not delete\n", encoding="utf-8") + + self.assertRaises(ValueError, SymbolicReference.delete, repo, "../../outside_delete.txt") + assert outside_path.read_text(encoding="utf-8") == "do not delete\n" + + def test_symbolic_reference_log_append_rejects_path_traversal(self): + with tempfile.TemporaryDirectory() as tmp_dir: + base_dir = Path(tmp_dir) + repo = self._repo_with_initial_commit(base_dir) + outside_path = base_dir / "outside_reflog.txt" + + ref = SymbolicReference(repo, "../../../outside_reflog.txt") + self.assertRaises(ValueError, ref.log_append, Commit.NULL_BIN_SHA, "do not write", repo.head.commit.binsha) + assert not outside_path.exists() + + def test_remote_reference_delete_cleanup_rejects_path_traversal(self): + with tempfile.TemporaryDirectory() as tmp_dir: + base_dir = Path(tmp_dir) + git_dir = base_dir / "repo" / ".git" + git_dir.mkdir(parents=True) + outside_path = base_dir / "outside_remote_delete.txt" + outside_path.write_text("do not delete\n", encoding="utf-8") + + class GitStub: + def branch(self, *args): + pass + + class RepoStub: + pass + + repo = RepoStub() + repo.git = GitStub() + repo.common_dir = str(git_dir) + repo.git_dir = str(git_dir) + ref = RemoteReference(repo, "../../outside_remote_delete.txt", check_path=False) + + self.assertRaises(ValueError, RemoteReference.delete, repo, ref) + assert outside_path.read_text(encoding="utf-8") == "do not delete\n" + def test_validity_ref_names(self): """Ensure ref names are checked for validity. From 4af8463cca31c2369312fcaa5309dfc30756c7b6 Mon Sep 17 00:00:00 2001 From: "GPT 5.5" Date: Tue, 28 Apr 2026 09:30:41 +0800 Subject: [PATCH 03/45] address review feedback and CI failures Consolidate follow-up fixes from review and CI: - fix lint and mypy issues in reference log path handling - validate remote reference paths before invoking git branch deletion - add symlink escape coverage where realpath resolves symlinks - ensure temporary test repositories release git resources during cleanup Co-authored-by: Sebastian Thiel --- git/refs/log.py | 4 +- git/refs/remote.py | 4 +- git/util.py | 2 +- test/test_refs.py | 92 +++++++++++++++++++++++++++++++--------------- 4 files changed, 69 insertions(+), 33 deletions(-) diff --git a/git/refs/log.py b/git/refs/log.py index 037e143d5..fbbe66b22 100644 --- a/git/refs/log.py +++ b/git/refs/log.py @@ -4,7 +4,6 @@ __all__ = ["RefLog", "RefLogEntry"] from mmap import mmap -import os.path as osp import re import time as _time @@ -212,6 +211,9 @@ def path(cls, ref: "SymbolicReference") -> str: :param ref: :class:`~git.refs.symbolic.SymbolicReference` instance + + :raise ValueError: + If `ref.path` is invalid or escapes the repository's reflog directory. """ return to_native_path(ref._get_validated_reflog_path(ref.repo, ref.path)) diff --git a/git/refs/remote.py b/git/refs/remote.py index 8244470b0..e16ae70f8 100644 --- a/git/refs/remote.py +++ b/git/refs/remote.py @@ -58,12 +58,14 @@ def delete(cls, repo: "Repo", *refs: "RemoteReference", **kwargs: Any) -> None: `kwargs` are given for comparability with the base class method as we should not narrow the signature. """ + for ref in refs: + cls._check_ref_name_valid(ref.path) + repo.git.branch("-d", "-r", *refs) # The official deletion method will ignore remote symbolic refs - these are # generally ignored in the refs/ folder. We don't though and delete remainders # manually. for ref in refs: - cls._check_ref_name_valid(ref.path) try: os.remove(cls._get_validated_path(repo.common_dir, ref.path)) except OSError: diff --git a/git/util.py b/git/util.py index c3ffdd62b..712fabe85 100644 --- a/git/util.py +++ b/git/util.py @@ -289,7 +289,7 @@ def join_path(a: PathLike, *p: PathLike) -> PathLike: if sys.platform == "win32": - def to_native_path_windows(path: PathLike) -> PathLike: + def to_native_path_windows(path: PathLike) -> str: path = os.fspath(path) return path.replace("/", "\\") diff --git a/test/test_refs.py b/test/test_refs.py index 4337f35e1..d77b34eba 100644 --- a/test/test_refs.py +++ b/test/test_refs.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/ +import contextlib from itertools import chain import os.path as osp from pathlib import Path @@ -30,13 +31,17 @@ class TestRefs(TestBase): + @contextlib.contextmanager def _repo_with_initial_commit(self, base_dir): repo_dir = base_dir / "repo" repo = Repo.init(repo_dir) (repo_dir / "file.txt").write_text("initial\n", encoding="utf-8") repo.index.add(["file.txt"]) repo.index.commit("initial") - return repo + try: + yield repo + finally: + repo.git.clear_cache() def test_from_path(self): # Should be able to create any reference directly. @@ -660,60 +665,84 @@ def test_refs_outside_repo(self): def test_reference_create_rejects_path_traversal(self): with tempfile.TemporaryDirectory() as tmp_dir: base_dir = Path(tmp_dir) - repo = self._repo_with_initial_commit(base_dir) - outside_path = base_dir / "outside_write.txt" + with self._repo_with_initial_commit(base_dir) as repo: + outside_path = base_dir / "outside_write.txt" - self.assertRaises(ValueError, Reference.create, repo, "../../../outside_write.txt", "HEAD") - assert not outside_path.exists() + self.assertRaises(ValueError, Reference.create, repo, "../../../outside_write.txt", "HEAD") + assert not outside_path.exists() def test_symbolic_reference_create_rejects_path_traversal(self): with tempfile.TemporaryDirectory() as tmp_dir: base_dir = Path(tmp_dir) - repo = self._repo_with_initial_commit(base_dir) - outside_path = base_dir / "outside_write.txt" + with self._repo_with_initial_commit(base_dir) as repo: + outside_path = base_dir / "outside_write.txt" - self.assertRaises(ValueError, SymbolicReference.create, repo, "../../outside_write.txt", "HEAD") - assert not outside_path.exists() + self.assertRaises(ValueError, SymbolicReference.create, repo, "../../outside_write.txt", "HEAD") + assert not outside_path.exists() def test_symbolic_reference_set_reference_rejects_path_traversal(self): with tempfile.TemporaryDirectory() as tmp_dir: base_dir = Path(tmp_dir) - repo = self._repo_with_initial_commit(base_dir) - outside_path = base_dir / "outside_write.txt" + with self._repo_with_initial_commit(base_dir) as repo: + outside_path = base_dir / "outside_write.txt" - self.assertRaises(ValueError, SymbolicReference(repo, "../../outside_write.txt").set_reference, "HEAD") - assert not outside_path.exists() + self.assertRaises(ValueError, SymbolicReference(repo, "../../outside_write.txt").set_reference, "HEAD") + assert not outside_path.exists() def test_symbolic_reference_rename_rejects_path_traversal(self): with tempfile.TemporaryDirectory() as tmp_dir: base_dir = Path(tmp_dir) - repo = self._repo_with_initial_commit(base_dir) - outside_path = base_dir / "outside_move.txt" - ref = SymbolicReference.create(repo, "SAFE_RENAME_SOURCE", "HEAD") + with self._repo_with_initial_commit(base_dir) as repo: + outside_path = base_dir / "outside_move.txt" + ref = SymbolicReference.create(repo, "SAFE_RENAME_SOURCE", "HEAD") - self.assertRaises(ValueError, ref.rename, "../../outside_move.txt") - assert not outside_path.exists() - assert Path(ref.abspath).is_file() + self.assertRaises(ValueError, ref.rename, "../../outside_move.txt") + assert not outside_path.exists() + assert Path(ref.abspath).is_file() def test_symbolic_reference_delete_rejects_path_traversal(self): with tempfile.TemporaryDirectory() as tmp_dir: base_dir = Path(tmp_dir) - repo = self._repo_with_initial_commit(base_dir) - outside_path = base_dir / "outside_delete.txt" - outside_path.write_text("do not delete\n", encoding="utf-8") + with self._repo_with_initial_commit(base_dir) as repo: + outside_path = base_dir / "outside_delete.txt" + outside_path.write_text("do not delete\n", encoding="utf-8") - self.assertRaises(ValueError, SymbolicReference.delete, repo, "../../outside_delete.txt") - assert outside_path.read_text(encoding="utf-8") == "do not delete\n" + self.assertRaises(ValueError, SymbolicReference.delete, repo, "../../outside_delete.txt") + assert outside_path.read_text(encoding="utf-8") == "do not delete\n" def test_symbolic_reference_log_append_rejects_path_traversal(self): with tempfile.TemporaryDirectory() as tmp_dir: base_dir = Path(tmp_dir) - repo = self._repo_with_initial_commit(base_dir) - outside_path = base_dir / "outside_reflog.txt" + with self._repo_with_initial_commit(base_dir) as repo: + outside_path = base_dir / "outside_reflog.txt" + + ref = SymbolicReference(repo, "../../../outside_reflog.txt") + self.assertRaises( + ValueError, ref.log_append, Commit.NULL_BIN_SHA, "do not write", repo.head.commit.binsha + ) + assert not outside_path.exists() - ref = SymbolicReference(repo, "../../../outside_reflog.txt") - self.assertRaises(ValueError, ref.log_append, Commit.NULL_BIN_SHA, "do not write", repo.head.commit.binsha) - assert not outside_path.exists() + def test_symbolic_reference_set_reference_rejects_symlink_escape(self): + with tempfile.TemporaryDirectory() as tmp_dir: + base_dir = Path(tmp_dir) + with self._repo_with_initial_commit(base_dir) as repo: + outside_dir = base_dir / "outside_refs" + outside_dir.mkdir() + outside_path = outside_dir / "escaped" + + refs_heads_dir = Path(repo.common_dir) / "refs" / "heads" + refs_heads_dir.mkdir(parents=True, exist_ok=True) + symlink_path = refs_heads_dir / "link_out" + try: + symlink_path.symlink_to(outside_dir, target_is_directory=True) + except (OSError, NotImplementedError) as ex: + self.skipTest("symlinks unavailable on this platform: %s" % ex) + if osp.realpath(symlink_path / "escaped") == osp.abspath(symlink_path / "escaped"): + self.skipTest("realpath does not resolve directory symlinks on this platform") + + ref = SymbolicReference(repo, "refs/heads/link_out/escaped") + self.assertRaises(ValueError, ref.set_reference, "HEAD") + assert not outside_path.exists() def test_remote_reference_delete_cleanup_rejects_path_traversal(self): with tempfile.TemporaryDirectory() as tmp_dir: @@ -724,8 +753,10 @@ def test_remote_reference_delete_cleanup_rejects_path_traversal(self): outside_path.write_text("do not delete\n", encoding="utf-8") class GitStub: + branch_called = False + def branch(self, *args): - pass + self.branch_called = True class RepoStub: pass @@ -737,6 +768,7 @@ class RepoStub: ref = RemoteReference(repo, "../../outside_remote_delete.txt", check_path=False) self.assertRaises(ValueError, RemoteReference.delete, repo, ref) + assert not repo.git.branch_called assert outside_path.read_text(encoding="utf-8") == "do not delete\n" def test_validity_ref_names(self): From 5a15361e0e1223f5c2e2c05688e6d094796b954d Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Tue, 28 Apr 2026 13:24:46 +0800 Subject: [PATCH 04/45] a new release with safer reference creation --- VERSION | 2 +- doc/source/changes.rst | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/VERSION b/VERSION index e1ace7c6e..94c78f538 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -3.1.47 +3.1.48 diff --git a/doc/source/changes.rst b/doc/source/changes.rst index 90b2e0739..4ac67d077 100644 --- a/doc/source/changes.rst +++ b/doc/source/changes.rst @@ -2,6 +2,15 @@ Changelog ========= +3.1.48 +====== + +Safe reference creation in the face of untrusted input. + +See the following for all changes. +https://github.com/gitpython-developers/GitPython/releases/tag/3.1.48 + + 3.1.47 ====== From c417af469f9aa3da8dfef78f996c0fb8c5d1f4c2 Mon Sep 17 00:00:00 2001 From: "GPT 5.5" Date: Wed, 29 Apr 2026 05:47:57 +0800 Subject: [PATCH 05/45] reject control chars in written values in configuration Reject CR, LF, and NUL in GitConfigParser values before writing them to git config files (which also is a deviation from Git which escapes them). GitConfigParser._write() serializes embedded newlines as indented continuation lines by replacing "\n" with "\n\t". Git itself skips leading whitespace before parsing config tokens, so an injected value such as: foo [core] hooksPath=/tmp/hooks is written in a form where the indented "[core]" line is still parsed by Git as a real section header. This lets attacker-controlled input passed to config_writer().set_value() poison repository config, including core.hooksPath, and redirect hook execution for later Git operations. Fail closed instead of stripping or normalizing these characters. Silent normalization can hide unsanitized caller input, and GitPython does not currently round-trip Git-style escaped values such as "\n" as embedded newlines. Apply the validation to set_value(), add_value(), and the public set() path so callers cannot bypass the safer helper API. Add regression tests for the advisory payload and for CR, LF, NUL, and bytes values. This preserves existing read behavior for config files that already contain multiline values while preventing GitPython from writing new unsafe values. Co-authored-by: Sebastian Thiel --- git/config.py | 24 ++++++++++++++++++++++-- test/test_config.py | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 2 deletions(-) diff --git a/git/config.py b/git/config.py index c6eaf8f7b..31d9e01cd 100644 --- a/git/config.py +++ b/git/config.py @@ -882,6 +882,24 @@ def _value_to_string(self, value: Union[str, bytes, int, float, bool]) -> str: return str(value) return force_text(value) + def _value_to_string_safe(self, value: Union[str, bytes, int, float, bool]) -> str: + value_str = self._value_to_string(value) + if re.search(r"[\r\n\x00]", value_str): + raise ValueError("Git config values must not contain CR, LF, or NUL") + return value_str + + @needs_values + @set_dirty_and_flush_changes + def set( + self, + section: str, + option: str, + value: Union[str, bytes, int, float, bool, None] = None, + ) -> None: + if value is not None: + value = self._value_to_string_safe(value) + return super().set(section, option, value) + @needs_values @set_dirty_and_flush_changes def set_value(self, section: str, option: str, value: Union[str, bytes, int, float, bool]) -> "GitConfigParser": @@ -902,9 +920,10 @@ def set_value(self, section: str, option: str, value: Union[str, bytes, int, flo :return: This instance """ + value_str = self._value_to_string_safe(value) if not self.has_section(section): self.add_section(section) - self.set(section, option, self._value_to_string(value)) + self.set(section, option, value_str) return self @needs_values @@ -929,9 +948,10 @@ def add_value(self, section: str, option: str, value: Union[str, bytes, int, flo :return: This instance """ + value_str = self._value_to_string_safe(value) if not self.has_section(section): self.add_section(section) - self._sections[section].add(option, self._value_to_string(value)) + self._sections[section].add(option, value_str) return self def rename_section(self, section: str, new_name: str) -> "GitConfigParser": diff --git a/test/test_config.py b/test/test_config.py index 11ea52d16..a9dcdb087 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -150,6 +150,39 @@ def test_config_value_with_trailing_new_line(self): git_config = GitConfigParser(config_file) git_config.read() # This should not throw an exception + @with_rw_directory + def test_set_value_rejects_config_injection(self, rw_dir): + config_path = osp.join(rw_dir, "config") + payload = "foo\n[core]\nhooksPath=/tmp/hooks" + + with GitConfigParser(config_path, read_only=False) as git_config: + with pytest.raises(ValueError, match="CR, LF, or NUL"): + git_config.set_value("user", "name", payload) + + with GitConfigParser(config_path, read_only=True) as git_config: + self.assertFalse(git_config.has_section("user")) + self.assertFalse(git_config.has_section("core")) + + @with_rw_directory + def test_set_and_add_value_reject_unsafe_value_characters(self, rw_dir): + config_path = osp.join(rw_dir, "config") + bad_values = ("foo\rbar", "foo\nbar", "foo\x00bar", b"foo\nbar") + + with GitConfigParser(config_path, read_only=False) as git_config: + git_config.add_section("user") + for bad_value in bad_values: + with pytest.raises(ValueError, match="CR, LF, or NUL"): + git_config.set("user", "name", bad_value) + with pytest.raises(ValueError, match="CR, LF, or NUL"): + git_config.set_value("user", "name", bad_value) + with pytest.raises(ValueError, match="CR, LF, or NUL"): + git_config.add_value("user", "name", bad_value) + + git_config.set_value("user", "name", "safe") + + with GitConfigParser(config_path, read_only=True) as git_config: + self.assertEqual(git_config.get_value("user", "name"), "safe") + def test_base(self): path_repo = fixture_path("git_config") path_global = fixture_path("git_config_global") From 8e24503b42c1d63dd98e8b2e6a2f655bdd0821e3 Mon Sep 17 00:00:00 2001 From: "GPT 5.5" Date: Wed, 29 Apr 2026 06:39:02 +0800 Subject: [PATCH 06/45] avoid duplicate validation in set_value Co-authored-by: Sebastian Thiel --- git/config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git/config.py b/git/config.py index 31d9e01cd..97ae054e5 100644 --- a/git/config.py +++ b/git/config.py @@ -923,7 +923,7 @@ def set_value(self, section: str, option: str, value: Union[str, bytes, int, flo value_str = self._value_to_string_safe(value) if not self.has_section(section): self.add_section(section) - self.set(section, option, value_str) + super().set(section, option, value_str) return self @needs_values From d7ce6fc19199cf8698d722c7d8ae38ff81424fba Mon Sep 17 00:00:00 2001 From: "GPT 5.5" Date: Tue, 28 Apr 2026 21:47:27 +0000 Subject: [PATCH 07/45] Improve pure Python rev-parse coverage and behavior (#2135) Port object-resolving revspec cases inspired by gix-revision into deterministic GitPython tests, without shelling out to Git or Gix at runtime. Refactor rev_parse handling around anchors, navigation, peeling, reflog selectors, path/index lookups, describe-style names, and commit-message searches. Document observed Git/Gix behavior differences and the GitPython choices made for user-facing compatibility. Co-authored-by: Sebastian Thiel --- git/repo/fun.py | 513 ++++++++++++++++++++++++++++++----------- test/test_repo.py | 17 ++ test/test_rev_parse.py | 138 +++++++++++ 3 files changed, 536 insertions(+), 132 deletions(-) create mode 100644 test/test_rev_parse.py diff --git a/git/repo/fun.py b/git/repo/fun.py index 3f00e60ea..d91ce5c0b 100644 --- a/git/repo/fun.py +++ b/git/repo/fun.py @@ -20,6 +20,7 @@ import os import os.path as osp from pathlib import Path +import re import stat from string import digits @@ -28,12 +29,13 @@ from git.cmd import Git from git.exc import WorkTreeRepositoryUnsupported from git.objects import Object +from git.objects.util import parse_date from git.refs import SymbolicReference from git.util import cygpath, bin_to_hex, hex_to_bin # Typing ---------------------------------------------------------------------- -from typing import Optional, TYPE_CHECKING, Union, cast, overload +from typing import Optional, TYPE_CHECKING, Tuple, Union, cast, overload from git.types import AnyGitObject, Literal, PathLike @@ -41,6 +43,7 @@ from git.db import GitCmdObjectDB from git.objects import Commit, TagObject from git.refs.reference import Reference + from git.refs.log import RefLog, RefLogEntry from git.refs.tag import Tag from .base import Repo @@ -139,6 +142,23 @@ def short_to_long(odb: "GitCmdObjectDB", hexsha: str) -> Optional[bytes]: # END exception handling +def _describe_to_long(repo: "Repo", name: str) -> Optional[bytes]: + """Resolve git-describe style names to the abbreviated object they contain.""" + match = re.match(r"^.+-\d+-g([0-9A-Fa-f]{4,40})(?:-dirty)?$", name) + if match is None: + match = re.match(r"^.+-g([0-9A-Fa-f]{4,40})(?:-dirty)?$", name) + if match is None: + match = re.match(r"^([0-9A-Fa-f]{4,40})-dirty$", name) + if match is None: + return None + # END handle match + + hexsha = match.group(1) + if len(hexsha) == 40: + return hexsha.encode("ascii") + return short_to_long(repo.odb, hexsha) + + @overload def name_to_object(repo: "Repo", name: str, return_ref: Literal[False] = ...) -> AnyGitObject: ... @@ -170,6 +190,10 @@ def name_to_object(repo: "Repo", name: str, return_ref: bool = False) -> Union[A # END handle short shas # END find sha if it matches + if hexsha is None: + hexsha = _describe_to_long(repo, name) + # END handle describe output + # If we couldn't find an object for what seemed to be a short hexsha, try to find it # as reference anyway, it could be named 'aaa' for instance. if hexsha is None: @@ -227,6 +251,298 @@ def to_commit(obj: Object) -> "Commit": return obj +def _object_from_hexsha(repo: "Repo", hexsha: str) -> AnyGitObject: + return Object.new_from_sha(repo, hex_to_bin(hexsha)) + + +def _current_reflog_ref(repo: "Repo") -> SymbolicReference: + return repo.head + + +def _ref_log(repo: "Repo", ref: SymbolicReference) -> "RefLog": + try: + return ref.log() + except FileNotFoundError: + try: + if ref.path == repo.head.ref.path: + return repo.head.log() + # END handle linked-worktree current branch logs + except TypeError: + pass + # END handle detached head + raise + # END handle missing branch log + + +def _ref_log_entry(repo: "Repo", ref: SymbolicReference, index: int) -> "RefLogEntry": + try: + return ref.log_entry(index) + except FileNotFoundError: + try: + if ref.path == repo.head.ref.path: + return repo.head.log_entry(index) + # END handle linked-worktree current branch logs + except TypeError: + pass + # END handle detached head + raise + # END handle missing branch log + + +def _find_reflog_entry_by_date(repo: "Repo", ref: SymbolicReference, spec: str) -> str: + try: + timestamp, _offset = parse_date(spec) + except ValueError as e: + raise NotImplementedError("Support for additional @{...} modes not implemented") from e + # END handle unsupported dates + log = _ref_log(repo, ref) + if not log: + raise IndexError("Invalid revlog date: %s" % spec) + # END handle empty log + + for entry in reversed(log): + if entry.time[0] <= timestamp: + return entry.newhexsha + # END found candidate + # END for each entry + return log[0].newhexsha + + +def _previous_checked_out_branch(repo: "Repo", nth: int) -> AnyGitObject: + if nth <= 0: + raise ValueError("Invalid previous checkout selector: -%i" % nth) + # END handle invalid input + + seen = 0 + for entry in reversed(_ref_log(repo, repo.head)): + message = entry.message or "" + prefix = "checkout: moving from " + if not message.startswith(prefix): + continue + # END skip non-checkouts + + previous_branch = message[len(prefix) :].split(" to ", 1)[0] + seen += 1 + if seen == nth: + return name_to_object(repo, previous_branch) + # END found selector + # END for each entry + raise IndexError("Invalid previous checkout selector: -%i" % nth) + + +def _tracking_branch_object(repo: "Repo", ref: Optional[SymbolicReference]) -> AnyGitObject: + from git.refs.head import Head + + if ref is None: + try: + head = repo.active_branch + except TypeError as e: + raise BadName("@{upstream}") from e + elif isinstance(ref, Head): + head = ref + else: + raise BadName("%s@{upstream}" % ref.name) + # END handle head + + tracking_branch = head.tracking_branch() + if tracking_branch is None: + raise BadName("%s@{upstream}" % head.name) + # END handle missing upstream + return tracking_branch.commit + + +def _apply_reflog(repo: "Repo", ref: Optional[SymbolicReference], content: str) -> AnyGitObject: + if content.startswith("+"): + content = content[1:] + # END handle explicit positive sign + + if content.startswith("-"): + if ref is not None: + raise ValueError("Previous checkout selectors do not take an explicit ref") + if content == "-0": + raise ValueError("Negative zero is invalid in reflog selector") + # END handle invalid negative zero + try: + return _previous_checked_out_branch(repo, int(content[1:])) + except ValueError as e: + raise ValueError("Invalid previous checkout selector: %s" % content) from e + # END handle previous checkout branch + + content_lower = content.lower() + if content_lower in ("u", "upstream", "push"): + return _tracking_branch_object(repo, ref) + # END handle sibling branches + + ref = ref or _current_reflog_ref(repo) + try: + entry_no = int(content) + except ValueError: + hexsha = _find_reflog_entry_by_date(repo, ref, content) + else: + if entry_no >= 100000000: + hexsha = _find_reflog_entry_by_date(repo, ref, "%s +0000" % entry_no) + elif entry_no == 0: + return ref.commit + else: + try: + entry = _ref_log_entry(repo, ref, -(entry_no + 1)) + except IndexError as e: + raise IndexError("Invalid revlog index: %i" % entry_no) from e + # END handle index out of bound + hexsha = entry.newhexsha + # END handle offset or date-like timestamp + # END handle content + return _object_from_hexsha(repo, hexsha) + + +def _find_closing_brace(rev: str, start: int) -> int: + depth = 1 + escaped = False + for idx in range(start + 1, len(rev)): + char = rev[idx] + if escaped: + escaped = False + elif char == "\\": + escaped = True + elif char == "{": + depth += 1 + elif char == "}": + depth -= 1 + if depth == 0: + return idx + # END found end + # END handle char + # END for each char + raise ValueError("Missing closing brace to define type in %s" % rev) + + +def _parse_search(pattern: str) -> Tuple[str, bool]: + if not pattern: + raise ValueError("Revision search requires a pattern") + # END handle empty pattern + + if pattern.startswith("!-"): + return pattern[2:], True + if pattern.startswith("!!"): + return pattern[1:], False + if pattern.startswith("!"): + raise ValueError("Need one character after /!, typically -") + return pattern, False + + +def _unescape_braced_regex(pattern: str) -> str: + out = [] + idx = 0 + while idx < len(pattern): + char = pattern[idx] + if char == "\\" and idx + 1 < len(pattern): + next_char = pattern[idx + 1] + if next_char in "{}\\": + out.append(next_char) + else: + out.append(char) + out.append(next_char) + # END handle escaped char + idx += 2 + continue + # END handle backslash + out.append(char) + idx += 1 + # END for each char + return "".join(out) + + +def _find_commit_by_message( + repo: "Repo", rev: Optional[AnyGitObject], pattern: str, braced: bool = False +) -> AnyGitObject: + pattern, negated = _parse_search(_unescape_braced_regex(pattern) if braced else pattern) + regex = re.compile(pattern) + if rev is None: + commits = repo.iter_commits("--all") + else: + commits = repo.iter_commits(to_commit(cast(Object, rev)).hexsha) + # END handle starting point + + for commit in commits: + matches = regex.search(commit.message or "") is not None + if matches != negated: + return commit + # END found commit + # END for each commit + raise BadName("No commit found matching message pattern %r" % pattern) + + +def _index_lookup(repo: "Repo", spec: str) -> AnyGitObject: + if not spec: + raise ValueError("':' must be followed by a path") + # END handle empty lookup + + stage = 0 + path = spec + if len(spec) >= 2 and spec[1] == ":" and spec[0] in "0123": + stage = int(spec[0]) + path = spec[2:] + # END handle stage + + try: + return repo.index.entries[(path, stage)].to_blob(repo) + except KeyError as e: + raise BadName("Path %r did not exist in the index at stage %i" % (path, stage)) from e + + +def _tree_lookup(obj: AnyGitObject, path: str) -> AnyGitObject: + if obj.type != "tree": + obj = to_commit(cast(Object, obj)).tree + # END get tree + if not path: + return obj + return obj[path] + + +def _peel(obj: AnyGitObject, output_type: str, repo: "Repo", rev: str) -> AnyGitObject: + if output_type == "/": + return obj + if output_type.startswith("/"): + return _find_commit_by_message(repo, obj, output_type[1:], braced=True) + if output_type == "": + return deref_tag(cast("TagObject", obj)) if obj.type == "tag" else obj + if output_type == "object": + return obj + if output_type == "commit": + return to_commit(cast(Object, obj)) + if output_type == "tree": + return to_commit(cast(Object, obj)).tree if obj.type != "tree" else obj + if output_type == "blob": + obj = deref_tag(cast("TagObject", obj)) if obj.type == "tag" else obj + if obj.type == output_type: + return obj + # END handle matching type + raise ValueError("Could not accommodate requested object type %r, got %s" % (output_type, obj.type)) + if output_type == "tag": + if obj.type == output_type: + return obj + # END handle matching type + raise ValueError("Could not accommodate requested object type %r, got %s" % (output_type, obj.type)) + # END handle known types + raise ValueError("Invalid output type: %s ( in %s )" % (output_type, rev)) + + +def _first_rev_token(rev: str) -> Optional[int]: + for idx, char in enumerate(rev): + if char in "^~:": + return idx + if char == "@": + next_char = rev[idx + 1] if idx + 1 < len(rev) else None + if idx == 0 and next_char in (None, "^", "~", ":", "{"): + return idx + if next_char == "{": + return idx + # END handle reflog selector + # END handle at symbol + # END for each char + return None + + def rev_parse(repo: "Repo", rev: str) -> AnyGitObject: """Parse a revision string. Like :manpage:`git-rev-parse(1)`. @@ -253,135 +569,81 @@ def rev_parse(repo: "Repo", rev: str) -> AnyGitObject: :raise IndexError: If an invalid reflog index is specified. """ - # Are we in colon search mode? if rev.startswith(":/"): - # Colon search mode - raise NotImplementedError("commit by message search (regex)") - # END handle search + return _find_commit_by_message(repo, None, rev[2:]) + if rev.startswith(":"): + return _index_lookup(repo, rev[1:]) + # END handle top-level colon modes obj: Optional[AnyGitObject] = None ref = None - output_type = "commit" - start = 0 - parsed_to = 0 lr = len(rev) - while start < lr: - if rev[start] not in "^~:@": - start += 1 - continue - # END handle start + first_token = _first_rev_token(rev) + if first_token is None: + return name_to_object(repo, rev) + # END handle plain name + + if first_token == 0: + if rev[0] != "@": + raise ValueError("Revision specifier must start with an object name: %s" % rev) + # END handle invalid leading token + ref = _current_reflog_ref(repo) + obj = ref.commit + start = 0 if rev.startswith("@{") else 1 + else: + if rev[first_token] == "@": + ref = cast("Reference", name_to_object(repo, rev[:first_token], return_ref=True)) + obj = ref.commit + else: + obj = name_to_object(repo, rev[:first_token]) + # END handle anchor + start = first_token + # END initialize anchor + while start < lr: token = rev[start] - if obj is None: - # token is a rev name. - if start == 0: - ref = repo.head.ref - else: - if token == "@": - ref = cast("Reference", name_to_object(repo, rev[:start], return_ref=True)) - else: - obj = name_to_object(repo, rev[:start]) - # END handle token - # END handle refname - else: - if ref is not None: - obj = ref.commit - # END handle ref - # END initialize obj on first token - - start += 1 + if token == "@": + if start + 1 >= lr or rev[start + 1] != "{": + raise ValueError("Invalid @ token in revision specifier: %s" % rev) + # END handle invalid @ + end = _find_closing_brace(rev, start + 1) + obj = _apply_reflog(repo, ref if first_token != 0 and start == first_token else None, rev[start + 2 : end]) + ref = None + start = end + 1 + continue + # END handle reflog - # Try to parse {type}. - if start < lr and rev[start] == "{": - end = rev.find("}", start) - if end == -1: - raise ValueError("Missing closing brace to define type in %s" % rev) - output_type = rev[start + 1 : end] # Exclude brace. - - # Handle type. - if output_type == "commit": - obj = cast("TagObject", obj) - if obj and obj.type == "tag": - obj = deref_tag(obj) - else: - # Cannot do anything for non-tags. - pass - # END handle tag - elif output_type == "tree": - try: - obj = cast(AnyGitObject, obj) - obj = to_commit(obj).tree - except (AttributeError, ValueError): - pass # Error raised later. - # END exception handling - elif output_type in ("", "blob"): - obj = cast("TagObject", obj) - if obj and obj.type == "tag": - obj = deref_tag(obj) - else: - # Cannot do anything for non-tags. - pass - # END handle tag - elif token == "@": - # try single int - assert ref is not None, "Require Reference to access reflog" - revlog_index = None - try: - # Transform reversed index into the format of our revlog. - revlog_index = -(int(output_type) + 1) - except ValueError as e: - # TODO: Try to parse the other date options, using parse_date maybe. - raise NotImplementedError("Support for additional @{...} modes not implemented") from e - # END handle revlog index - - try: - entry = ref.log_entry(revlog_index) - except IndexError as e: - raise IndexError("Invalid revlog index: %i" % revlog_index) from e - # END handle index out of bound - - obj = Object.new_from_sha(repo, hex_to_bin(entry.newhexsha)) - - # Make it pass the following checks. - output_type = "" - else: - raise ValueError("Invalid output type: %s ( in %s )" % (output_type, rev)) - # END handle output type + if token == ":": + return _tree_lookup(cast(AnyGitObject, obj), rev[start + 1 :]) + # END handle path - # Empty output types don't require any specific type, its just about - # dereferencing tags. - if output_type and obj and obj.type != output_type: - raise ValueError("Could not accommodate requested object type %r, got %s" % (output_type, obj.type)) - # END verify output type + start += 1 - start = end + 1 # Skip brace. - parsed_to = start + if token == "^" and start < lr and rev[start] == "{": + end = _find_closing_brace(rev, start) + obj = _peel(cast(AnyGitObject, obj), rev[start + 1 : end], repo, rev) + ref = None + start = end + 1 continue # END parse type - # Try to parse a number. num = 0 - if token != ":": - found_digit = False - while start < lr: - if rev[start] in digits: - num = num * 10 + int(rev[start]) - start += 1 - found_digit = True - else: - break - # END handle number - # END number parse loop - - # No explicit number given, 1 is the default. It could be 0 though. - if not found_digit: - num = 1 - # END set default num - # END number parsing only if non-blob mode - - parsed_to = start - # Handle hierarchy walk. + found_digit = False + while start < lr: + if rev[start] in digits: + num = num * 10 + int(rev[start]) + start += 1 + found_digit = True + else: + break + # END handle number + # END number parse loop + + if not found_digit: + num = 1 + # END set default num + try: obj = cast(AnyGitObject, obj) if token == "~": @@ -391,15 +653,11 @@ def rev_parse(repo: "Repo", rev: str) -> AnyGitObject: # END for each history item to walk elif token == "^": obj = to_commit(obj) - # Must be n'th parent. - if num: + if num == 0: + pass + else: obj = obj.parents[num - 1] - elif token == ":": - if obj.type != "tree": - obj = obj.tree - # END get tree type - obj = obj[rev[start:]] - parsed_to = lr + # END handle parent else: raise ValueError("Invalid token: %r" % token) # END end handle tag @@ -410,16 +668,7 @@ def rev_parse(repo: "Repo", rev: str) -> AnyGitObject: # END exception handling # END parse loop - # Still no obj? It's probably a simple name. - if obj is None: - obj = name_to_object(repo, rev) - parsed_to = lr - # END handle simple name - if obj is None: raise ValueError("Revision specifier could not be parsed: %s" % rev) - if parsed_to != lr: - raise ValueError("Didn't consume complete rev spec %s, consumed part: %s" % (rev, rev[:parsed_to])) - return obj diff --git a/test/test_repo.py b/test/test_repo.py index 544b5c561..0dd3d5945 100644 --- a/test/test_repo.py +++ b/test/test_repo.py @@ -146,6 +146,23 @@ def test_commit_from_revision(self): self.assertEqual(commit.type, "commit") self.assertEqual(self.rorepo.commit(commit), commit) + @with_rw_directory + def test_commit_from_tag_starting_with_at(self, rw_dir): + repo = Repo.init(rw_dir) + with repo.config_writer() as writer: + writer.set_value("user", "name", "GitPython Tests") + writer.set_value("user", "email", "gitpython@example.com") + + tracked_file = Path(rw_dir) / "hello.txt" + tracked_file.write_text("hello") + repo.index.add([str(tracked_file)]) + commit = repo.index.commit("init") + repo.create_tag("@foo") + + self.assertEqual(repo.tags["@foo"].commit, commit) + self.assertEqual(repo.commit("@"), commit) + self.assertEqual(repo.commit("@foo"), commit) + def test_commits(self): mc = 10 commits = list(self.rorepo.iter_commits("0.1.6", max_count=mc)) diff --git a/test/test_rev_parse.py b/test/test_rev_parse.py new file mode 100644 index 000000000..371210fa9 --- /dev/null +++ b/test/test_rev_parse.py @@ -0,0 +1,138 @@ +from pathlib import Path + +import pytest + +from git import Repo +from gitdb.exc import BadName + + +def _write(repo, path, content): + full_path = Path(repo.working_tree_dir) / path + full_path.parent.mkdir(parents=True, exist_ok=True) + full_path.write_text(content) + repo.index.add([str(full_path)]) + + +@pytest.fixture +def rev_parse_repo(tmp_path): + repo = Repo.init(tmp_path) + with repo.config_writer() as writer: + writer.set_value("user", "name", "GitPython Tests") + writer.set_value("user", "email", "gitpython@example.com") + + _write(repo, "README.md", "root\n") + _write(repo, "CHANGES", "root changes\n") + _write(repo, "dir/file.txt", "root file\n") + root = repo.index.commit("root commit") + repo.create_tag("ann", ref=root, message="annotated tag") + + _write(repo, "README.md", "release\n") + release = repo.index.commit("release candidate") + repo.create_tag("v1.0", ref=release) + main = repo.active_branch + + side = repo.create_head("side", root) + side.checkout() + _write(repo, "side.txt", "side\n") + side_commit = repo.index.commit("side branch") + + main.checkout() + repo.git.merge("--no-ff", "side", "-m", "merge side") + merge = repo.head.commit + + repo.create_head("aaaaaaaa", merge) + repo.create_tag("@foo", ref=merge) + + return { + "repo": repo, + "root": root, + "release": release, + "side": side_commit, + "merge": merge, + "main": main, + } + + +def test_rev_parse_names_hex_and_describe_forms(rev_parse_repo): + repo = rev_parse_repo["repo"] + merge = rev_parse_repo["merge"] + + assert repo.rev_parse("@") == merge + assert repo.rev_parse("@foo") == merge + assert repo.rev_parse("aaaaaaaa") == merge + assert repo.rev_parse(merge.hexsha[:7]) == merge + assert repo.rev_parse("v1.0-1-g%s" % merge.hexsha[:7]) == merge + assert repo.rev_parse("anything-9-g%s" % merge.hexsha[:7]) == merge + assert repo.rev_parse("%s-dirty" % merge.hexsha[:7]) == merge + + +def test_rev_parse_navigation_and_peeling(rev_parse_repo): + repo = rev_parse_repo["repo"] + root = rev_parse_repo["root"] + release = rev_parse_repo["release"] + side = rev_parse_repo["side"] + merge = rev_parse_repo["merge"] + tag = repo.rev_parse("ann") + + assert repo.rev_parse("HEAD^0") == merge + assert repo.rev_parse("HEAD~0") == merge + assert repo.rev_parse("HEAD^1") == release + assert repo.rev_parse("HEAD^2") == side + assert repo.rev_parse("HEAD~") == release + assert repo.rev_parse("HEAD^^") == root + + assert tag.type == "tag" + assert repo.rev_parse("ann^{object}") == tag + assert repo.rev_parse("ann^{tag}") == tag + assert repo.rev_parse("ann^{}") == root + assert repo.rev_parse("ann^{commit}") == root + assert repo.rev_parse("HEAD^{tree}") == merge.tree + assert repo.rev_parse("HEAD^{/}") == merge + + +def test_rev_parse_tree_and_index_paths(rev_parse_repo): + repo = rev_parse_repo["repo"] + merge = rev_parse_repo["merge"] + + assert repo.rev_parse("HEAD:") == merge.tree + assert repo.rev_parse("HEAD:README.md") == merge.tree["README.md"] + assert repo.rev_parse("HEAD^{tree}:README.md") == merge.tree["README.md"] + assert repo.rev_parse(":README.md").binsha == merge.tree["README.md"].binsha + assert repo.rev_parse(":0:README.md").binsha == merge.tree["README.md"].binsha + + +def test_rev_parse_reflog_selectors(rev_parse_repo): + repo = rev_parse_repo["repo"] + merge = rev_parse_repo["merge"] + side = rev_parse_repo["side"] + main = rev_parse_repo["main"] + + assert repo.rev_parse("@{0}") == merge + assert repo.rev_parse("@{+0}") == merge + assert repo.rev_parse("%s@{0}" % main.name) == merge + assert repo.rev_parse("@{-1}") == side + + +def test_rev_parse_commit_message_search(rev_parse_repo): + repo = rev_parse_repo["repo"] + release = rev_parse_repo["release"] + merge = rev_parse_repo["merge"] + + assert repo.rev_parse(":/release") == release + assert repo.rev_parse("HEAD^{/release}") == release + assert repo.rev_parse("HEAD^{/!-release}") == merge + + +def test_rev_parse_rejects_invalid_object_specs(rev_parse_repo): + repo = rev_parse_repo["repo"] + + with pytest.raises(ValueError): + repo.rev_parse(":") + with pytest.raises(ValueError): + repo.rev_parse(":/") + with pytest.raises(ValueError): + repo.rev_parse("@{-0}") + with pytest.raises(ValueError): + repo.rev_parse("HEAD^{invalid}") + with pytest.raises(BadName): + repo.rev_parse(":missing") From bdbdf4bba08f59042a2e1197313ca9a2060021d0 Mon Sep 17 00:00:00 2001 From: Codex GPT-5 Date: Wed, 29 Apr 2026 06:55:03 +0800 Subject: [PATCH 08/45] Fix rev-parse CI issues --- git/repo/fun.py | 42 ++++++++++++++++++++++++++++++++++-------- test/test_repo.py | 9 +++++++-- test/test_rev_parse.py | 2 ++ 3 files changed, 43 insertions(+), 10 deletions(-) diff --git a/git/repo/fun.py b/git/repo/fun.py index d91ce5c0b..ed00dd833 100644 --- a/git/repo/fun.py +++ b/git/repo/fun.py @@ -41,7 +41,7 @@ if TYPE_CHECKING: from git.db import GitCmdObjectDB - from git.objects import Commit, TagObject + from git.objects import Commit from git.refs.reference import Reference from git.refs.log import RefLog, RefLogEntry from git.refs.tag import Tag @@ -256,13 +256,30 @@ def _object_from_hexsha(repo: "Repo", hexsha: str) -> AnyGitObject: def _current_reflog_ref(repo: "Repo") -> SymbolicReference: - return repo.head + try: + return repo.head.ref + except TypeError: + return repo.head + # END handle detached head + + +def _common_reflog_path(repo: "Repo", ref: SymbolicReference) -> Optional[str]: + if repo.common_dir == repo.git_dir: + return None + # END handle normal repository + return SymbolicReference._get_validated_path(osp.join(repo.common_dir, "logs"), ref.path) def _ref_log(repo: "Repo", ref: SymbolicReference) -> "RefLog": try: return ref.log() except FileNotFoundError: + common_path = _common_reflog_path(repo, ref) + if common_path and osp.isfile(common_path): + from git.refs.log import RefLog + + return RefLog.from_file(common_path) + # END handle linked-worktree branch logs try: if ref.path == repo.head.ref.path: return repo.head.log() @@ -278,6 +295,12 @@ def _ref_log_entry(repo: "Repo", ref: SymbolicReference, index: int) -> "RefLogE try: return ref.log_entry(index) except FileNotFoundError: + common_path = _common_reflog_path(repo, ref) + if common_path and osp.isfile(common_path): + from git.refs.log import RefLog + + return RefLog.entry_at(common_path, index) + # END handle linked-worktree branch logs try: if ref.path == repo.head.ref.path: return repo.head.log_entry(index) @@ -464,7 +487,11 @@ def _find_commit_by_message( # END handle starting point for commit in commits: - matches = regex.search(commit.message or "") is not None + message = commit.message + if isinstance(message, bytes): + message = message.decode(commit.encoding, "replace") + # END handle bytes message + matches = regex.search(message or "") is not None if matches != negated: return commit # END found commit @@ -505,7 +532,7 @@ def _peel(obj: AnyGitObject, output_type: str, repo: "Repo", rev: str) -> AnyGit if output_type.startswith("/"): return _find_commit_by_message(repo, obj, output_type[1:], braced=True) if output_type == "": - return deref_tag(cast("TagObject", obj)) if obj.type == "tag" else obj + return deref_tag(obj) if obj.type == "tag" else obj if output_type == "object": return obj if output_type == "commit": @@ -513,7 +540,7 @@ def _peel(obj: AnyGitObject, output_type: str, repo: "Repo", rev: str) -> AnyGit if output_type == "tree": return to_commit(cast(Object, obj)).tree if obj.type != "tree" else obj if output_type == "blob": - obj = deref_tag(cast("TagObject", obj)) if obj.type == "tag" else obj + obj = deref_tag(obj) if obj.type == "tag" else obj if obj.type == output_type: return obj # END handle matching type @@ -615,14 +642,14 @@ def rev_parse(repo: "Repo", rev: str) -> AnyGitObject: # END handle reflog if token == ":": - return _tree_lookup(cast(AnyGitObject, obj), rev[start + 1 :]) + return _tree_lookup(obj, rev[start + 1 :]) # END handle path start += 1 if token == "^" and start < lr and rev[start] == "{": end = _find_closing_brace(rev, start) - obj = _peel(cast(AnyGitObject, obj), rev[start + 1 : end], repo, rev) + obj = _peel(obj, rev[start + 1 : end], repo, rev) ref = None start = end + 1 continue @@ -645,7 +672,6 @@ def rev_parse(repo: "Repo", rev: str) -> AnyGitObject: # END set default num try: - obj = cast(AnyGitObject, obj) if token == "~": obj = to_commit(obj) for _ in range(num): diff --git a/test/test_repo.py b/test/test_repo.py index 0dd3d5945..7262395bd 100644 --- a/test/test_repo.py +++ b/test/test_repo.py @@ -865,8 +865,13 @@ def test_rev_parse(self): # Currently, nothing more is supported. self.assertRaises(NotImplementedError, rev_parse, "@{1 week ago}") - # The last position. - assert rev_parse("@{1}") != head.commit + # The previous position, if this checkout has enough reflog history. + try: + previous = rev_parse("@{1}") + except IndexError: + pass + else: + self.assertNotEqual(previous, head.commit) def test_repo_odbtype(self): target_type = GitCmdObjectDB diff --git a/test/test_rev_parse.py b/test/test_rev_parse.py index 371210fa9..d96fdc1a2 100644 --- a/test/test_rev_parse.py +++ b/test/test_rev_parse.py @@ -106,9 +106,11 @@ def test_rev_parse_reflog_selectors(rev_parse_repo): merge = rev_parse_repo["merge"] side = rev_parse_repo["side"] main = rev_parse_repo["main"] + release = rev_parse_repo["release"] assert repo.rev_parse("@{0}") == merge assert repo.rev_parse("@{+0}") == merge + assert repo.rev_parse("@{1}") == release assert repo.rev_parse("%s@{0}" % main.name) == merge assert repo.rev_parse("@{-1}") == side From 6cf7ac33d449db095e8c301abba664836c16bfc8 Mon Sep 17 00:00:00 2001 From: Codex GPT-5 Date: Wed, 29 Apr 2026 07:11:05 +0800 Subject: [PATCH 09/45] Address rev-parse review feedback --- git/repo/fun.py | 56 ++++++++++++++++++++++++++++++++++-------- test/test_rev_parse.py | 35 ++++++++++++++++++++------ 2 files changed, 73 insertions(+), 18 deletions(-) diff --git a/git/repo/fun.py b/git/repo/fun.py index ed00dd833..66e7eba69 100644 --- a/git/repo/fun.py +++ b/git/repo/fun.py @@ -35,7 +35,7 @@ # Typing ---------------------------------------------------------------------- -from typing import Optional, TYPE_CHECKING, Tuple, Union, cast, overload +from typing import Iterator, Optional, TYPE_CHECKING, Tuple, Union, cast, overload from git.types import AnyGitObject, Literal, PathLike @@ -190,10 +190,6 @@ def name_to_object(repo: "Repo", name: str, return_ref: bool = False) -> Union[A # END handle short shas # END find sha if it matches - if hexsha is None: - hexsha = _describe_to_long(repo, name) - # END handle describe output - # If we couldn't find an object for what seemed to be a short hexsha, try to find it # as reference anyway, it could be named 'aaa' for instance. if hexsha is None: @@ -216,6 +212,10 @@ def name_to_object(repo: "Repo", name: str, return_ref: bool = False) -> Union[A # END for each base # END handle hexsha + if hexsha is None: + hexsha = _describe_to_long(repo, name) + # END handle describe output + # Didn't find any ref, this is an error. if return_ref: raise BadObject("Couldn't find reference named %r" % name) @@ -363,6 +363,8 @@ def _tracking_branch_object(repo: "Repo", ref: Optional[SymbolicReference]) -> A raise BadName("@{upstream}") from e elif isinstance(ref, Head): head = ref + elif os.fspath(ref.path).startswith("refs/heads/"): + head = Head(repo, ref.path) else: raise BadName("%s@{upstream}" % ref.name) # END handle head @@ -479,11 +481,15 @@ def _find_commit_by_message( repo: "Repo", rev: Optional[AnyGitObject], pattern: str, braced: bool = False ) -> AnyGitObject: pattern, negated = _parse_search(_unescape_braced_regex(pattern) if braced else pattern) - regex = re.compile(pattern) + try: + regex = re.compile(pattern) + except re.error as e: + raise ValueError("Invalid commit message regex %r" % pattern) from e + # END handle invalid regex if rev is None: - commits = repo.iter_commits("--all") + commits = _all_ref_commits(repo) else: - commits = repo.iter_commits(to_commit(cast(Object, rev)).hexsha) + commits = _reachable_commits([to_commit(cast(Object, rev))]) # END handle starting point for commit in commits: @@ -499,6 +505,38 @@ def _find_commit_by_message( raise BadName("No commit found matching message pattern %r" % pattern) +def _all_ref_commits(repo: "Repo") -> Iterator["Commit"]: + starts = [] + for ref in repo.references: + try: + starts.append(to_commit(cast(Object, ref.object))) + except (BadName, ValueError): + pass + # END skip refs that do not point to commits + # END for each ref + try: + starts.append(repo.head.commit) + except ValueError: + pass + # END handle unborn head + return _reachable_commits(starts) + + +def _reachable_commits(starts: list["Commit"]) -> Iterator["Commit"]: + seen = set() + pending = starts[:] + while pending: + pending.sort(key=lambda commit: commit.committed_date, reverse=True) + commit = pending.pop(0) + if commit.binsha in seen: + continue + # END skip seen commit + seen.add(commit.binsha) + yield commit + pending.extend(commit.parents) + # END while commits remain + + def _index_lookup(repo: "Repo", spec: str) -> AnyGitObject: if not spec: raise ValueError("':' must be followed by a path") @@ -527,8 +565,6 @@ def _tree_lookup(obj: AnyGitObject, path: str) -> AnyGitObject: def _peel(obj: AnyGitObject, output_type: str, repo: "Repo", rev: str) -> AnyGitObject: - if output_type == "/": - return obj if output_type.startswith("/"): return _find_commit_by_message(repo, obj, output_type[1:], braced=True) if output_type == "": diff --git a/test/test_rev_parse.py b/test/test_rev_parse.py index d96fdc1a2..b00347668 100644 --- a/test/test_rev_parse.py +++ b/test/test_rev_parse.py @@ -1,8 +1,15 @@ +# Copyright (C) 2026 Michael Trier (mtrier@gmail.com) and contributors +# +# This module is part of GitPython and is released under the +# 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/ + from pathlib import Path import pytest from git import Repo +from git.refs import RemoteReference +from git.refs import SymbolicReference from gitdb.exc import BadName @@ -31,14 +38,12 @@ def rev_parse_repo(tmp_path): repo.create_tag("v1.0", ref=release) main = repo.active_branch - side = repo.create_head("side", root) - side.checkout() _write(repo, "side.txt", "side\n") - side_commit = repo.index.commit("side branch") + side_commit = repo.index.commit("side branch", parent_commits=[root], head=False, skip_hooks=True) + repo.create_head("side", side_commit) - main.checkout() - repo.git.merge("--no-ff", "side", "-m", "merge side") - merge = repo.head.commit + merge = repo.index.commit("merge side", parent_commits=[release, side_commit], skip_hooks=True) + repo.head.log_append(side_commit.binsha, "checkout: moving from side to main", merge.binsha) repo.create_head("aaaaaaaa", merge) repo.create_tag("@foo", ref=merge) @@ -55,16 +60,21 @@ def rev_parse_repo(tmp_path): def test_rev_parse_names_hex_and_describe_forms(rev_parse_repo): repo = rev_parse_repo["repo"] + release = rev_parse_repo["release"] merge = rev_parse_repo["merge"] assert repo.rev_parse("@") == merge assert repo.rev_parse("@foo") == merge assert repo.rev_parse("aaaaaaaa") == merge assert repo.rev_parse(merge.hexsha[:7]) == merge + describe_name = "anything-9-g%s" % merge.hexsha[:7] assert repo.rev_parse("v1.0-1-g%s" % merge.hexsha[:7]) == merge - assert repo.rev_parse("anything-9-g%s" % merge.hexsha[:7]) == merge + assert repo.rev_parse(describe_name) == merge assert repo.rev_parse("%s-dirty" % merge.hexsha[:7]) == merge + repo.create_tag(describe_name, ref=release) + assert repo.rev_parse(describe_name) == release + def test_rev_parse_navigation_and_peeling(rev_parse_repo): repo = rev_parse_repo["repo"] @@ -87,7 +97,8 @@ def test_rev_parse_navigation_and_peeling(rev_parse_repo): assert repo.rev_parse("ann^{}") == root assert repo.rev_parse("ann^{commit}") == root assert repo.rev_parse("HEAD^{tree}") == merge.tree - assert repo.rev_parse("HEAD^{/}") == merge + with pytest.raises(ValueError): + repo.rev_parse("HEAD^{/}") def test_rev_parse_tree_and_index_paths(rev_parse_repo): @@ -114,6 +125,10 @@ def test_rev_parse_reflog_selectors(rev_parse_repo): assert repo.rev_parse("%s@{0}" % main.name) == merge assert repo.rev_parse("@{-1}") == side + SymbolicReference.create(repo, "refs/remotes/origin/%s" % main.name, merge) + main.set_tracking_branch(RemoteReference(repo, "refs/remotes/origin/%s" % main.name)) + assert repo.rev_parse("%s@{upstream}" % main.name) == merge + def test_rev_parse_commit_message_search(rev_parse_repo): repo = rev_parse_repo["repo"] @@ -132,6 +147,10 @@ def test_rev_parse_rejects_invalid_object_specs(rev_parse_repo): repo.rev_parse(":") with pytest.raises(ValueError): repo.rev_parse(":/") + with pytest.raises(ValueError): + repo.rev_parse(":/[") + with pytest.raises(ValueError): + repo.rev_parse("HEAD^{/[}") with pytest.raises(ValueError): repo.rev_parse("@{-0}") with pytest.raises(ValueError): From aee2fd5c13770954469e650f1df8f92f0183bc70 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 29 Apr 2026 08:30:21 +0800 Subject: [PATCH 10/45] bump version to 3.1.49 --- VERSION | 2 +- doc/source/changes.rst | 11 +++++++++++ git/ext/gitdb | 2 +- 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/VERSION b/VERSION index 94c78f538..8335f2d61 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -3.1.48 +3.1.49 diff --git a/doc/source/changes.rst b/doc/source/changes.rst index 4ac67d077..020673826 100644 --- a/doc/source/changes.rst +++ b/doc/source/changes.rst @@ -2,6 +2,17 @@ Changelog ========= +3.1.49 +====== + +Save setting of configuration values, +which cuold be used to inject other more configuration. + +Also more conforming `rev-parse` implementation. + +See the following for all changes. +https://github.com/gitpython-developers/GitPython/releases/tag/3.1.49 + 3.1.48 ====== diff --git a/git/ext/gitdb b/git/ext/gitdb index 5c1b3036a..335c0f661 160000 --- a/git/ext/gitdb +++ b/git/ext/gitdb @@ -1 +1 @@ -Subproject commit 5c1b3036a6e34782e0ab6ce85e5ae64fe777fdbe +Subproject commit 335c0f66173eecdc7b2597c2b6c3d1fde795df30 From b17f11315b3c3baf7c073234670ce58cc2bbf5ec Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 1 May 2026 16:12:17 +0000 Subject: [PATCH 11/45] Bump https://github.com/astral-sh/ruff-pre-commit Bumps the pre-commit group with 1 update: [https://github.com/astral-sh/ruff-pre-commit](https://github.com/astral-sh/ruff-pre-commit). Updates `https://github.com/astral-sh/ruff-pre-commit` from v0.15.8 to 0.15.12 - [Release notes](https://github.com/astral-sh/ruff-pre-commit/releases) - [Commits](https://github.com/astral-sh/ruff-pre-commit/compare/v0.15.8...v0.15.12) --- updated-dependencies: - dependency-name: https://github.com/astral-sh/ruff-pre-commit dependency-version: 0.15.12 dependency-type: direct:production dependency-group: pre-commit ... Signed-off-by: dependabot[bot] --- .pre-commit-config.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 617111e1d..f3ab67035 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -7,7 +7,7 @@ repos: exclude: ^test/fixtures/ - repo: https://github.com/astral-sh/ruff-pre-commit - rev: v0.15.8 + rev: v0.15.12 hooks: - id: ruff-check args: ["--fix"] From 714e2e16dc2a67567ee48f7bcffcb59b9ca12caa Mon Sep 17 00:00:00 2001 From: "GPT 5.5" Date: Sun, 3 May 2026 00:02:49 +0800 Subject: [PATCH 12/45] Xfail Windows symlink-capable index mutation test The Windows CI jobs for PR 2140 failed in test/test_index.py::TestIndex::test_index_mutation. The failing checkout path creates my_fake_symlink and Git for Windows 2.54 reports a symlink warning before GitPython raises GitCommandError. This is the same unsupported Windows symlink behavior that the test already marks as an expected failure when core.symlinks is true. Detect Windows hosts that can create symlinks directly and include GitCommandError in the expected failure types, so symlink-capable Windows runners do not fail this unrelated Dependabot PR. Co-authored-by: Sebastian Thiel --- test/test_index.py | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/test/test_index.py b/test/test_index.py index 33490f907..f8280450a 100644 --- a/test/test_index.py +++ b/test/test_index.py @@ -172,6 +172,19 @@ def _decode(stdout): _win_bash_status = WinBashStatus.check() +def _windows_supports_symlinks(): + if sys.platform != "win32": + return False + + with tempfile.TemporaryDirectory(prefix="gitpython-symlink-check-") as temp_dir: + link_path = osp.join(temp_dir, "link") + try: + os.symlink("missing-target", link_path) + except (NotImplementedError, OSError): + return False + return S_ISLNK(os.lstat(link_path)[ST_MODE]) + + def _make_hook(git_dir, name, content, make_exec=True): """A helper to create a hook""" hp = hook_path(name, git_dir) @@ -553,9 +566,9 @@ def _count_existing(self, repo, files): # END num existing helper @pytest.mark.xfail( - sys.platform == "win32" and Git().config("core.symlinks") == "true", + sys.platform == "win32" and (Git().config("core.symlinks") == "true" or _windows_supports_symlinks()), reason="Assumes symlinks are not created on Windows and opens a symlink to a nonexistent target.", - raises=FileNotFoundError, + raises=(FileNotFoundError, GitCommandError), ) @with_rw_repo("0.1.6") def test_index_mutation(self, rw_repo): From 4e8cd45685d33c8b6af2f70c77a341c4a15acf14 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sun, 3 May 2026 13:32:18 +0000 Subject: [PATCH 13/45] Bump git/ext/gitdb from `335c0f6` to `53c94d6` Bumps [git/ext/gitdb](https://github.com/gitpython-developers/gitdb) from `335c0f6` to `53c94d6`. - [Release notes](https://github.com/gitpython-developers/gitdb/releases) - [Commits](https://github.com/gitpython-developers/gitdb/compare/335c0f66173eecdc7b2597c2b6c3d1fde795df30...53c94d682b541595918cea6fc2e96bb900eb0e8c) --- updated-dependencies: - dependency-name: git/ext/gitdb dependency-version: 53c94d682b541595918cea6fc2e96bb900eb0e8c dependency-type: direct:production ... Signed-off-by: dependabot[bot] --- git/ext/gitdb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git/ext/gitdb b/git/ext/gitdb index 335c0f661..53c94d682 160000 --- a/git/ext/gitdb +++ b/git/ext/gitdb @@ -1 +1 @@ -Subproject commit 335c0f66173eecdc7b2597c2b6c3d1fde795df30 +Subproject commit 53c94d682b541595918cea6fc2e96bb900eb0e8c From 54538428f79b0c91ba52cda5229856a6edf7ac06 Mon Sep 17 00:00:00 2001 From: "GPT 5.5" Date: Tue, 5 May 2026 21:34:42 +0800 Subject: [PATCH 14/45] Validate config key section names before writing GitConfigParser already rejected CR, LF, and NUL in config values before writing, but section and option names could still reach configparser. Because GitPython writes section headers itself, a newline-bearing section name could split the output into additional headers. Reject CR, LF, and NUL in section and option names on write paths that create or set config keys: add_section(), set(), set_value(), add_value(), and rename_section() destinations. This matches Git config key validation behavior; Git source commit 94f057755b7941b321fd11fec1b2e3ca5313a4e0 reports invalid keys containing newlines from config.c, and local git 2.50.1 rejects newline-bearing config keys. Add a regression test covering unsafe section and option names while preserving safe writes. Co-authored-by: Sebastian Thiel --- git/config.py | 17 ++++++++++++++++- test/test_config.py | 31 +++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/git/config.py b/git/config.py index 97ae054e5..82747eadd 100644 --- a/git/config.py +++ b/git/config.py @@ -72,6 +72,9 @@ See: https://git-scm.com/docs/git-config#_conditional_includes """ +UNSAFE_CONFIG_CHARS_RE = re.compile(r"[\r\n\x00]") +"""Characters that cannot be safely written in config names or values.""" + class MetaParserBuilder(abc.ABCMeta): # noqa: B024 """Utility class wrapping base-class methods into decorators that assure read-only @@ -778,6 +781,7 @@ def _assure_writable(self, method_name: str) -> None: def add_section(self, section: "cp._SectionName") -> None: """Assures added options will stay in order.""" + self._assure_config_name_safe(section, "section") return super().add_section(section) @property @@ -884,10 +888,14 @@ def _value_to_string(self, value: Union[str, bytes, int, float, bool]) -> str: def _value_to_string_safe(self, value: Union[str, bytes, int, float, bool]) -> str: value_str = self._value_to_string(value) - if re.search(r"[\r\n\x00]", value_str): + if UNSAFE_CONFIG_CHARS_RE.search(value_str): raise ValueError("Git config values must not contain CR, LF, or NUL") return value_str + def _assure_config_name_safe(self, name: "cp._SectionName", label: str) -> None: + if isinstance(name, str) and UNSAFE_CONFIG_CHARS_RE.search(name): + raise ValueError("Git config %s names must not contain CR, LF, or NUL" % label) + @needs_values @set_dirty_and_flush_changes def set( @@ -896,6 +904,8 @@ def set( option: str, value: Union[str, bytes, int, float, bool, None] = None, ) -> None: + self._assure_config_name_safe(section, "section") + self._assure_config_name_safe(option, "option") if value is not None: value = self._value_to_string_safe(value) return super().set(section, option, value) @@ -920,6 +930,8 @@ def set_value(self, section: str, option: str, value: Union[str, bytes, int, flo :return: This instance """ + self._assure_config_name_safe(section, "section") + self._assure_config_name_safe(option, "option") value_str = self._value_to_string_safe(value) if not self.has_section(section): self.add_section(section) @@ -948,6 +960,8 @@ def add_value(self, section: str, option: str, value: Union[str, bytes, int, flo :return: This instance """ + self._assure_config_name_safe(section, "section") + self._assure_config_name_safe(option, "option") value_str = self._value_to_string_safe(value) if not self.has_section(section): self.add_section(section) @@ -968,6 +982,7 @@ def rename_section(self, section: str, new_name: str) -> "GitConfigParser": """ if not self.has_section(section): raise ValueError("Source section '%s' doesn't exist" % section) + self._assure_config_name_safe(new_name, "section") if self.has_section(new_name): raise ValueError("Destination section '%s' already exists" % new_name) diff --git a/test/test_config.py b/test/test_config.py index a9dcdb087..3ddaf0a4b 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -163,6 +163,37 @@ def test_set_value_rejects_config_injection(self, rw_dir): self.assertFalse(git_config.has_section("user")) self.assertFalse(git_config.has_section("core")) + @with_rw_directory + def test_set_value_rejects_unsafe_section_and_option_names(self, rw_dir): + config_path = osp.join(rw_dir, "config") + bad_keys = ("user]\n[core", "user]\r[core", "user]\x00[core") + + with GitConfigParser(config_path, read_only=False) as git_config: + git_config.add_section("user") + for bad_key in bad_keys: + with pytest.raises(ValueError, match="CR, LF, or NUL"): + git_config.add_section(bad_key) + with pytest.raises(ValueError, match="CR, LF, or NUL"): + git_config.set(bad_key, "hooksPath", "/tmp/hooks") + with pytest.raises(ValueError, match="CR, LF, or NUL"): + git_config.set("user", bad_key, "/tmp/hooks") + with pytest.raises(ValueError, match="CR, LF, or NUL"): + git_config.set_value(bad_key, "hooksPath", "/tmp/hooks") + with pytest.raises(ValueError, match="CR, LF, or NUL"): + git_config.set_value("user", bad_key, "/tmp/hooks") + with pytest.raises(ValueError, match="CR, LF, or NUL"): + git_config.add_value(bad_key, "hooksPath", "/tmp/hooks") + with pytest.raises(ValueError, match="CR, LF, or NUL"): + git_config.add_value("user", bad_key, "/tmp/hooks") + with pytest.raises(ValueError, match="CR, LF, or NUL"): + git_config.rename_section("user", bad_key) + + git_config.set_value("user", "name", "safe") + + with GitConfigParser(config_path, read_only=True) as git_config: + self.assertEqual(git_config.get_value("user", "name"), "safe") + self.assertFalse(git_config.has_section("core")) + @with_rw_directory def test_set_and_add_value_reject_unsafe_value_characters(self, rw_dir): config_path = osp.join(rw_dir, "config") From 5a294a6fc7ed5dc0946d4b576257bf926178f269 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 6 May 2026 12:00:26 +0800 Subject: [PATCH 15/45] bump version to 3.1.50 --- VERSION | 2 +- doc/source/changes.rst | 10 +++++++++- git/ext/gitdb | 2 +- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/VERSION b/VERSION index 8335f2d61..0bc461141 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -3.1.49 +3.1.50 diff --git a/doc/source/changes.rst b/doc/source/changes.rst index 020673826..b5152b3c5 100644 --- a/doc/source/changes.rst +++ b/doc/source/changes.rst @@ -2,11 +2,19 @@ Changelog ========= +3.1.50 +====== + +Save setting of configuration values, this time sections as well, as follow-up to 3.1.49. + +See the following for all changes. +https://github.com/gitpython-developers/GitPython/releases/tag/3.1.50 + 3.1.49 ====== Save setting of configuration values, -which cuold be used to inject other more configuration. +which could be used to inject other more configuration. Also more conforming `rev-parse` implementation. diff --git a/git/ext/gitdb b/git/ext/gitdb index 53c94d682..335c0f661 160000 --- a/git/ext/gitdb +++ b/git/ext/gitdb @@ -1 +1 @@ -Subproject commit 53c94d682b541595918cea6fc2e96bb900eb0e8c +Subproject commit 335c0f66173eecdc7b2597c2b6c3d1fde795df30 From 4941c314fc8c38ed6df037c29d4809f7a7fd9af9 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 6 May 2026 14:52:31 +0800 Subject: [PATCH 16/45] Add AI-disclusure and quality requirements to the contribution guidelines. Co-authored-by: GPT 5.5 --- CONTRIBUTING.md | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 8536d7f73..d8a29f55d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -9,6 +9,35 @@ The following is a short step-by-step rundown of what one typically would do to - Feel free to add yourself to AUTHORS file. - Create a pull request. +## Quality expectations + +Contributions must be made with care and meet the quality bar of the surrounding code. +That means a change should not leave GitPython worse than it was before: it should be +readable, maintainable, tested where practical, documented and consistent with the +existing style and behavior. +A contribution that works only narrowly but lowers the quality of the +codebase may be declined, and the pull request closed without warning. + +## AI-assisted contributions + +If AI edits files for you, disclose it in the pull request description and commit +metadata. Prefer making the agent identity part of the commit, for example by using +an AI author such as `$agent $version ` or a co-author via +a `Co-authored-by: ` trailer. + +Agents operating through a person's GitHub account must identify themselves. For +example, comments posted by an agent should say so directly with phrases like +`AI agent on behalf of : ...`. + +Fully AI-generated comments on pull requests or issues must also be disclosed. +Undisclosed AI-generated comments may lead to the pull request or issue being closed. + +AI-assisted proofreading or wording polish does not need disclosure, but it is still +courteous to mention it when the AI materially influenced the final text. + +Automated or "full-auto" AI contributions without a human responsible for reviewing +and standing behind the work may be closed. + ## Fuzzing Test Specific Documentation For details related to contributing to the fuzzing test suite and OSS-Fuzz integration, please From 92ff6df86b82e94254874b52b05dee9ffe38716b Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Wed, 6 May 2026 09:58:33 -0400 Subject: [PATCH 17/45] Separate quality paragraphs and adjust decline wording Split the quality-expectations section into two paragraphs (the warning about low-quality contributions being declined was visually merged with the preceding paragraph). Replace "and the pull request closed without warning" with a note that maintainers may not always be able to provide detailed feedback, which conveys the same practical reality. Co-authored-by: Claude Opus 4.6 --- CONTRIBUTING.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index d8a29f55d..60e34a651 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -15,8 +15,10 @@ Contributions must be made with care and meet the quality bar of the surrounding That means a change should not leave GitPython worse than it was before: it should be readable, maintainable, tested where practical, documented and consistent with the existing style and behavior. + A contribution that works only narrowly but lowers the quality of the -codebase may be declined, and the pull request closed without warning. +codebase may be declined. The maintainers may not always be able to provide +detailed feedback. ## AI-assisted contributions From d172e541455679ebdfb55df1411836298cb47b8d Mon Sep 17 00:00:00 2001 From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Date: Thu, 7 May 2026 03:20:20 -0700 Subject: [PATCH 18/45] docs(cmd): clarify Git.execute() string vs list command argument Closes #2016 Users routinely hit GitCommandNotFound by passing a single string with spaces to repo.git.execute(...). With shell=False (default) subprocess treats the entire string as the executable name and fails. Document the recommended list form, the string-as-single-executable behavior, and the two ways to coerce a string into argv tokens (shlex.split or shell=True). --- git/cmd.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/git/cmd.py b/git/cmd.py index 78a9f4c78..1ecff9318 100644 --- a/git/cmd.py +++ b/git/cmd.py @@ -1119,9 +1119,16 @@ def execute( information (stdout). :param command: - The command argument list to execute. - It should be a sequence of program arguments, or a string. The - program to execute is the first item in the args sequence or string. + The command to execute. A sequence of program arguments is the + recommended form when `shell` is ``False`` (the default), e.g. + ``["git", "log", "-n", "1"]``. + + A string is accepted, but with `shell` set to ``False`` it is passed + as a single executable name to :class:`subprocess.Popen`. For example, + ``"git log -n 1"`` looks for an executable literally named + ``git log -n 1`` and will fail with :class:`GitCommandNotFound`. To + split a command string into argv tokens, pass ``shlex.split(...)`` as + a sequence or set `shell` to ``True`` (see the warning below). :param istream: Standard input filehandle passed to :class:`subprocess.Popen`. From 010a7bbfd237d2ab0627c20a542933abf978b690 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Fri, 8 May 2026 16:34:31 -0400 Subject: [PATCH 19/45] Rewrite Git.execute() command parameter docstring per #2146 #2146 identifies two factual problems in #2144's `:param command:` rewrite, both of which this fixes: 1. The claim that with `shell=False` a string "is passed as a single executable name to `subprocess.Popen`" is accurate on Unix-like systems, but on Windows `subprocess.Popen` forwards the string to `CreateProcessW` and Windows command-line parsing produces argv. So multi-word strings happen to work on Windows -- which makes the docstring misleading for Windows readers and unhelpful for anyone whose actual problem is portability. 2. The blanket recommendation to use `shlex.split` read as a general string-to-argv splitter. `shlex.split` parses POSIX shell syntax on all systems, and the resulting tokens are still unsafe for anything but fixed, fully trusted strings -- in particular, strings built by interpolating values can still inject extra arguments via embedded whitespace or quoting. Restructure `:param command:` into four paragraphs (description, platform-specific string handling, `shell=True` / `Git.USE_SHELL` warning, qualified `shlex.split` note) and cross-reference `USE_SHELL` for the long-form security discussion rather than reproducing it. Add a related paragraph to `:param shell:` noting `shlex.split`'s narrow value as a transitional tool when migrating existing string-command `shell=True` calls on Unix-like systems, with extreme care, and cross-referencing `:param command:` for the risks. This fixes #2146. Only documentation is changed. Co-authored-by: Claude Opus 4.7 (1M context) --- git/cmd.py | 37 +++++++++++++++++++++++++++---------- 1 file changed, 27 insertions(+), 10 deletions(-) diff --git a/git/cmd.py b/git/cmd.py index 7f2564d45..861009099 100644 --- a/git/cmd.py +++ b/git/cmd.py @@ -1131,16 +1131,28 @@ def execute( information (stdout). :param command: - The command to execute. A sequence of program arguments is the - recommended form when `shell` is ``False`` (the default), e.g. - ``["git", "log", "-n", "1"]``. - - A string is accepted, but with `shell` set to ``False`` it is passed - as a single executable name to :class:`subprocess.Popen`. For example, - ``"git log -n 1"`` looks for an executable literally named - ``git log -n 1`` and will fail with :class:`GitCommandNotFound`. To - split a command string into argv tokens, pass ``shlex.split(...)`` as - a sequence or set `shell` to ``True`` (see the warning below). + The command to execute. A sequence of program arguments is recommended. + A string is also accepted, but its meaning is strongly platform-dependent. + + By default, a shell is not used. On Unix-like systems, a string is the whole + program name (so ``"git log -n 1"`` raises :class:`GitCommandNotFound`). On + Windows, the program parses the arguments itself, so multi-word strings can + work but are not portable. + + Avoid ``shell=True`` (and :attr:`Git.USE_SHELL`): this runs the command in + a shell, which is generally unsafe. The shell interprets metacharacters + such as ``;``, ``|``, ``&``, ``$(...)``, ``$VAR``, ``%VAR%``, and ``^`` + (depending on the platform) as syntax. Any untrusted text in the command + can then execute arbitrary OS commands. See :attr:`Git.USE_SHELL`. + + Producing a sequence automatically by :func:`shlex.split` and passing it + as the command is far safer than ``shell=True``. But :func:`shlex.split` + parses POSIX shell syntax on all systems, and the result is still unsafe + for anything but *fixed, fully trusted* strings. Do not use it on strings + built by interpolating values: whitespace or quoting in an untrusted value + can still inject arguments. For input derived in any way from untrusted + data, build the argument sequence yourself, while ensuring each argument + is fully sanitized. :param istream: Standard input filehandle passed to :class:`subprocess.Popen`. @@ -1208,6 +1220,11 @@ def execute( needed (nor useful) to work around any known operating system specific issues. + On Unix-like systems, when migrating away from passing string commands with + ``shell=True``, :func:`shlex.split` may serve as a transitional step in rare + cases, but this should be undertaken with extreme care. See the `command` + parameter above on the risks. + :param env: A dictionary of environment variables to be passed to :class:`subprocess.Popen`. From 38d62c4b60b5cd2ed743c00f2cac1875c76449e0 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sat, 9 May 2026 21:19:10 -0400 Subject: [PATCH 20/45] Word `shell` mention of `shlex.split` more carefully To avert the mistake of not removing `shell=True` when using it. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 (1M context) --- git/cmd.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/git/cmd.py b/git/cmd.py index 861009099..98bb62775 100644 --- a/git/cmd.py +++ b/git/cmd.py @@ -1222,8 +1222,8 @@ def execute( On Unix-like systems, when migrating away from passing string commands with ``shell=True``, :func:`shlex.split` may serve as a transitional step in rare - cases, but this should be undertaken with extreme care. See the `command` - parameter above on the risks. + cases, with extreme care. (Drop ``shell=True`` and pass the resulting + sequence as the command.) See the `command` parameter above on the risks. :param env: A dictionary of environment variables to be passed to From 0c1e40982438b46e2e47dc833bccdced779a47db Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sat, 9 May 2026 21:49:47 -0400 Subject: [PATCH 21/45] Document init script behavior when multiple remotes have master When `master` is locally absent and more than one remote has it, `git checkout master --` fails by default even if all remotes agree, and the script falls back to creating `master` at `HEAD`. The reflog populated by the subsequent resets then traces `HEAD`'s history rather than a remote `master`'s. This is harmless, because `master` is reset to `__testing_point__` either way, but unintuitive. Add a comment so a reader of the script does not have to discover this from a confusing run. This fixes #2145. Co-Authored-By: Claude Opus 4.7 (1M context) --- init-tests-after-clone.sh | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/init-tests-after-clone.sh b/init-tests-after-clone.sh index bfada01b0..a88f983fc 100755 --- a/init-tests-after-clone.sh +++ b/init-tests-after-clone.sh @@ -40,6 +40,11 @@ fi git tag __testing_point__ # The tests need a branch called master. +# +# If master is locally absent but more than one remote has it, checkout fails +# by default even if all remotes agree, and we fall back to creating it at +# HEAD. The reflog we populate below then traces HEAD's history rather than +# a remote master's, but master is reset to __testing_point__ either way. git checkout master -- || git checkout -b master # The tests need a reflog history on the master branch. From 465f1d587a13795b35a4b72ad7fafbbe9c7731ac Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sun, 10 May 2026 13:32:12 +0000 Subject: [PATCH 22/45] Bump git/ext/gitdb from `335c0f6` to `0a019a2` Bumps [git/ext/gitdb](https://github.com/gitpython-developers/gitdb) from `335c0f6` to `0a019a2`. - [Release notes](https://github.com/gitpython-developers/gitdb/releases) - [Commits](https://github.com/gitpython-developers/gitdb/compare/335c0f66173eecdc7b2597c2b6c3d1fde795df30...0a019a2e2bd73158cf8b637ad78b5d4b8f15e42e) --- updated-dependencies: - dependency-name: git/ext/gitdb dependency-version: 0a019a2e2bd73158cf8b637ad78b5d4b8f15e42e dependency-type: direct:production ... Signed-off-by: dependabot[bot] --- git/ext/gitdb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git/ext/gitdb b/git/ext/gitdb index 335c0f661..0a019a2e2 160000 --- a/git/ext/gitdb +++ b/git/ext/gitdb @@ -1 +1 @@ -Subproject commit 335c0f66173eecdc7b2597c2b6c3d1fde795df30 +Subproject commit 0a019a2e2bd73158cf8b637ad78b5d4b8f15e42e From c334167a41f63fe6a16090d894d3bb26332bee9b Mon Sep 17 00:00:00 2001 From: "E-Love (Eric Loveland)" Date: Thu, 14 May 2026 21:57:51 -0400 Subject: [PATCH 23/45] fix(worktree): align with Git's gitdir resolution Git only normalizes relative references in `gitdir`[1], so do the same. [1]: https://github.com/git/git/blob/v2.54.0/setup.c#L1012-L1025 --- 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 7579e326f..08707fc5c 100644 --- a/git/repo/base.py +++ b/git/repo/base.py @@ -295,7 +295,7 @@ def __init__( sm_gitpath = find_worktree_git_dir(dotgit) if sm_gitpath is not None: - git_dir = expand_path(sm_gitpath, expand_vars) + git_dir = osp.normpath(sm_gitpath) self._working_tree_dir = curpath break From e94ccaff11ca7f4388e920bcc2ff617dd9e2a827 Mon Sep 17 00:00:00 2001 From: "E-Love (Eric Loveland)" Date: Tue, 12 May 2026 15:49:14 -0400 Subject: [PATCH 24/45] support relative worktree paths (git 2.48+ worktree.useRelativePaths) git 2.48 introduced the `worktree.useRelativePaths` config option, which causes `git worktree add` to write a relative `gitdir` into the worktree's `.git` file. Resolve the relative `gitdir` against the worktree directory before normalizing it. Absolute paths are unaffected because `os.path.join` ignores the prefix when joined with an absolute path. --- git/repo/base.py | 3 ++- test/test_repo.py | 17 +++++++++++++++-- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/git/repo/base.py b/git/repo/base.py index 08707fc5c..2d3cf24f0 100644 --- a/git/repo/base.py +++ b/git/repo/base.py @@ -295,7 +295,8 @@ def __init__( sm_gitpath = find_worktree_git_dir(dotgit) if sm_gitpath is not None: - git_dir = osp.normpath(sm_gitpath) + # worktrees can use relative paths as of Git 2.48, so we join to curpath + git_dir = osp.normpath(osp.join(curpath, sm_gitpath)) self._working_tree_dir = curpath break diff --git a/test/test_repo.py b/test/test_repo.py index fae3dc0b9..13bff52e9 100644 --- a/test/test_repo.py +++ b/test/test_repo.py @@ -1094,7 +1094,7 @@ def test_is_valid_object(self): self.assertFalse(repo.is_valid_object(tag_sha, "commit")) @with_rw_directory - def test_git_work_tree_dotgit(self, rw_dir): + def test_git_work_tree_dotgit(self, rw_dir, use_relative_paths=False): """Check that we find .git as a worktree file and find the worktree based on it.""" git = Git(rw_dir) @@ -1106,7 +1106,11 @@ def test_git_work_tree_dotgit(self, rw_dir): worktree_path = join_path_native(rw_dir, "worktree_repo") if Git.is_cygwin(): worktree_path = cygpath(worktree_path) - rw_master.git.worktree("add", worktree_path, branch.name) + wt_add_kwargs = {"insert_kwargs_after": "add"} + # relative worktree paths introduced in git 2.48.0 + if use_relative_paths and git.version_info[:3] >= (2, 48, 0): + wt_add_kwargs["relative_paths"] = True + rw_master.git.worktree("add", worktree_path, branch.name, **wt_add_kwargs) # This ensures that we can read the repo's gitdir correctly. repo = Repo(worktree_path) @@ -1124,6 +1128,15 @@ def test_git_work_tree_dotgit(self, rw_dir): self.assertIsInstance(repo.heads["aaaaaaaa"], Head) + def test_git_work_tree_dotgit_relative(self): + """Check that we find .git as a worktree file containing a relative path + and find the worktree based on it.""" + if Git().version_info[:3] < (2, 48, 0): + pytest.skip("relative worktree feature unsupported, needs git 2.48.0 or later") + # this class inherits from TestCase so we can't use pytest.mark.parametrize on + # test_git_work_tree_dotgit; delegate instead + self.test_git_work_tree_dotgit(use_relative_paths=True) + @with_rw_directory def test_git_work_tree_env(self, rw_dir): """Check that we yield to GIT_WORK_TREE.""" From 29c98613fd7f286786cf450ffa8a2e314f4317e7 Mon Sep 17 00:00:00 2001 From: "E-Love (Eric Loveland)" Date: Tue, 12 May 2026 16:16:44 -0400 Subject: [PATCH 25/45] doc(_call_process): correct example --- git/cmd.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git/cmd.py b/git/cmd.py index 7f2564d45..637225f0b 100644 --- a/git/cmd.py +++ b/git/cmd.py @@ -1595,7 +1595,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 03baced04b00f4505201e38b0507fb8d071e29ca Mon Sep 17 00:00:00 2001 From: "E-Love (Eric Loveland)" Date: Fri, 15 May 2026 11:51:11 -0400 Subject: [PATCH 26/45] Add xfail_if_raises context manager to defer xfail condition evaluation The @pytest.mark.xfail decorator with raises=... evaluates its condition during test collection, which can trigger external calls (e.g., Git().config()) that have side effects. Introduce xfail_if_raises as a context manager that delays this evaluation until test execution time, and apply it to test_index_mutation. --- test/lib/helper.py | 27 ++ test/test_index.py | 721 +++++++++++++++++++++++---------------------- 2 files changed, 388 insertions(+), 360 deletions(-) diff --git a/test/lib/helper.py b/test/lib/helper.py index 6a8b714e6..2fc015dfa 100644 --- a/test/lib/helper.py +++ b/test/lib/helper.py @@ -18,6 +18,7 @@ "skipIf", "GIT_REPO", "GIT_DAEMON_PORT", + "xfail_if_raises", ] import contextlib @@ -35,8 +36,10 @@ import time import unittest import venv +from typing import Union, Type, Tuple import gitdb +import pytest from git.util import rmtree, cwd @@ -465,3 +468,27 @@ def _executable(self, basename): if osp.isfile(path) or osp.islink(path): return path raise RuntimeError(f"no regular file or symlink {path!r}") + + +@contextlib.contextmanager +def xfail_if_raises( + condition: bool, + *, + raises: Union[Type[BaseException], Tuple[Type[BaseException], ...]], + reason: str = "", + strict: bool = False, +): + """Approximates the behavior of @pytest.mark.xfail(..., raises=...) as a context + manager that can be used within a test, such as when the condition is complex or has + side effects + + One difference is it will not report XPASS if the test passes, but setting `strict` + simulates it by raising an exception""" + try: + yield + except raises: + if condition: + pytest.xfail(reason) + raise + if strict and condition: + pytest.fail("[XPASS(strict)] " + reason) diff --git a/test/test_index.py b/test/test_index.py index f8280450a..cb45d3e90 100644 --- a/test/test_index.py +++ b/test/test_index.py @@ -38,6 +38,7 @@ 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, PathLikeMock +from test.lib.helper import xfail_if_raises HOOKS_SHEBANG = "#!/usr/bin/env sh\n" @@ -565,369 +566,369 @@ def _count_existing(self, repo, files): # END num existing helper - @pytest.mark.xfail( - sys.platform == "win32" and (Git().config("core.symlinks") == "true" or _windows_supports_symlinks()), - reason="Assumes symlinks are not created on Windows and opens a symlink to a nonexistent target.", - raises=(FileNotFoundError, GitCommandError), - ) @with_rw_repo("0.1.6") def test_index_mutation(self, rw_repo): - index = rw_repo.index - num_entries = len(index.entries) - cur_head = rw_repo.head - - uname = "Thomas Müller" - umail = "sd@company.com" - with rw_repo.config_writer() as writer: - writer.set_value("user", "name", uname) - writer.set_value("user", "email", umail) - self.assertEqual(writer.get_value("user", "name"), uname) - - # Remove all of the files, provide a wild mix of paths, BaseIndexEntries, - # IndexEntries. - def mixed_iterator(): - count = 0 - for entry in index.entries.values(): - type_id = count % 5 - 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 == 3: # path mock in a blob - yield Blob(rw_repo, entry.binsha, entry.mode, entry.path) - 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 == 6: # IndexEntry - yield entry - else: - raise AssertionError("Invalid Type") - count += 1 - # END for each entry - - # END mixed iterator - deleted_files = index.remove(mixed_iterator(), working_tree=False) - assert deleted_files - self.assertEqual(self._count_existing(rw_repo, deleted_files), len(deleted_files)) - self.assertEqual(len(index.entries), 0) - - # Reset the index to undo our changes. - index.reset() - self.assertEqual(len(index.entries), num_entries) - - # Remove with working copy. - deleted_files = index.remove(mixed_iterator(), working_tree=True) - assert deleted_files - self.assertEqual(self._count_existing(rw_repo, deleted_files), 0) - - # Reset everything. - index.reset(working_tree=True) - self.assertEqual(self._count_existing(rw_repo, deleted_files), len(deleted_files)) - - # Invalid type. - self.assertRaises(TypeError, index.remove, [1]) - - # Absolute path. - deleted_files = index.remove([osp.join(rw_repo.working_tree_dir, "lib")], r=True) - assert len(deleted_files) > 1 - self.assertRaises(ValueError, index.remove, ["/doesnt/exists"]) - - # TEST COMMITTING - # Commit changed index. - cur_commit = cur_head.commit - commit_message = "commit default head by Frèderic Çaufl€" - - new_commit = index.commit(commit_message, head=False) - assert cur_commit != new_commit - self.assertEqual(new_commit.author.name, uname) - self.assertEqual(new_commit.author.email, umail) - self.assertEqual(new_commit.committer.name, uname) - self.assertEqual(new_commit.committer.email, umail) - self.assertEqual(new_commit.message, commit_message) - self.assertEqual(new_commit.parents[0], cur_commit) - self.assertEqual(len(new_commit.parents), 1) - self.assertEqual(cur_head.commit, cur_commit) - - # Commit with other actor. - cur_commit = cur_head.commit - - my_author = Actor("Frèderic Çaufl€", "author@example.com") - my_committer = Actor("Committing Frèderic Çaufl€", "committer@example.com") - commit_actor = index.commit(commit_message, author=my_author, committer=my_committer) - assert cur_commit != commit_actor - self.assertEqual(commit_actor.author.name, "Frèderic Çaufl€") - self.assertEqual(commit_actor.author.email, "author@example.com") - self.assertEqual(commit_actor.committer.name, "Committing Frèderic Çaufl€") - self.assertEqual(commit_actor.committer.email, "committer@example.com") - self.assertEqual(commit_actor.message, commit_message) - self.assertEqual(commit_actor.parents[0], cur_commit) - self.assertEqual(len(new_commit.parents), 1) - self.assertEqual(cur_head.commit, commit_actor) - self.assertEqual(cur_head.log()[-1].actor, my_committer) - - # Commit with author_date and commit_date. - cur_commit = cur_head.commit - commit_message = "commit with dates by Avinash Sajjanshetty" - - new_commit = index.commit( - commit_message, - author_date="2006-04-07T22:13:13", - commit_date="2005-04-07T22:13:13", - ) - assert cur_commit != new_commit - print(new_commit.authored_date, new_commit.committed_date) - self.assertEqual(new_commit.message, commit_message) - self.assertEqual(new_commit.authored_date, 1144447993) - self.assertEqual(new_commit.committed_date, 1112911993) - - # Same index, no parents. - commit_message = "index without parents" - commit_no_parents = index.commit(commit_message, parent_commits=[], head=True) - self.assertEqual(commit_no_parents.message, commit_message) - self.assertEqual(len(commit_no_parents.parents), 0) - self.assertEqual(cur_head.commit, commit_no_parents) - - # same index, multiple parents. - commit_message = "Index with multiple parents\n commit with another line" - commit_multi_parent = index.commit(commit_message, parent_commits=(commit_no_parents, new_commit)) - self.assertEqual(commit_multi_parent.message, commit_message) - self.assertEqual(len(commit_multi_parent.parents), 2) - self.assertEqual(commit_multi_parent.parents[0], commit_no_parents) - self.assertEqual(commit_multi_parent.parents[1], new_commit) - self.assertEqual(cur_head.commit, commit_multi_parent) - - # Re-add all files in lib. - # Get the lib folder back on disk, but get an index without it. - index.reset(new_commit.parents[0], working_tree=True).reset(new_commit, working_tree=False) - lib_file_path = osp.join("lib", "git", "__init__.py") - assert (lib_file_path, 0) not in index.entries - assert osp.isfile(osp.join(rw_repo.working_tree_dir, lib_file_path)) - - # Directory. - entries = index.add(["lib"], fprogress=self._fprogress_add) - self._assert_entries(entries) - self._assert_fprogress(entries) - assert len(entries) > 1 - - # Glob. - entries = index.reset(new_commit).add([osp.join("lib", "git", "*.py")], fprogress=self._fprogress_add) - self._assert_entries(entries) - self._assert_fprogress(entries) - self.assertEqual(len(entries), 14) - - # Same file. - entries = index.reset(new_commit).add( - [osp.join(rw_repo.working_tree_dir, "lib", "git", "head.py")] * 2, - fprogress=self._fprogress_add, - ) - self._assert_entries(entries) - self.assertEqual(entries[0].mode & 0o644, 0o644) - # Would fail, test is too primitive to handle this case. - # self._assert_fprogress(entries) - self._reset_progress() - self.assertEqual(len(entries), 2) - - # Missing path. - self.assertRaises(OSError, index.reset(new_commit).add, ["doesnt/exist/must/raise"]) - - # Blob from older revision overrides current index revision. - old_blob = new_commit.parents[0].tree.blobs[0] - entries = index.reset(new_commit).add([old_blob], fprogress=self._fprogress_add) - self._assert_entries(entries) - self._assert_fprogress(entries) - self.assertEqual(index.entries[(old_blob.path, 0)].hexsha, old_blob.hexsha) - self.assertEqual(len(entries), 1) - - # Mode 0 not allowed. - null_hex_sha = Diff.NULL_HEX_SHA - null_bin_sha = b"\0" * 20 - self.assertRaises( - ValueError, - index.reset(new_commit).add, - [BaseIndexEntry((0, null_bin_sha, 0, "doesntmatter"))], - ) - - # Add new file. - new_file_relapath = "my_new_file" - self._make_file(new_file_relapath, "hello world", rw_repo) - entries = index.reset(new_commit).add( - [BaseIndexEntry((0o10644, null_bin_sha, 0, new_file_relapath))], - fprogress=self._fprogress_add, - ) - self._assert_entries(entries) - self._assert_fprogress(entries) - self.assertEqual(len(entries), 1) - self.assertNotEqual(entries[0].hexsha, null_hex_sha) - - # Add symlink. - if sys.platform != "win32": - for target in ("/etc/nonexisting", "/etc/passwd", "/etc"): - basename = "my_real_symlink" - - link_file = osp.join(rw_repo.working_tree_dir, basename) - os.symlink(target, link_file) - entries = index.reset(new_commit).add([link_file], fprogress=self._fprogress_add) - self._assert_entries(entries) - self._assert_fprogress(entries) - self.assertEqual(len(entries), 1) - self.assertTrue(S_ISLNK(entries[0].mode)) - self.assertTrue(S_ISLNK(index.entries[index.entry_key("my_real_symlink", 0)].mode)) - - # We expect only the target to be written. - self.assertEqual( - index.repo.odb.stream(entries[0].binsha).read().decode("ascii"), - target, - ) - - os.remove(link_file) - # END for each target - # END real symlink test - - # Add fake symlink and assure it checks out as a symlink. - fake_symlink_relapath = "my_fake_symlink" - link_target = "/etc/that" - fake_symlink_path = self._make_file(fake_symlink_relapath, link_target, rw_repo) - fake_entry = BaseIndexEntry((0o120000, null_bin_sha, 0, fake_symlink_relapath)) - entries = index.reset(new_commit).add([fake_entry], fprogress=self._fprogress_add) - self._assert_entries(entries) - self._assert_fprogress(entries) - assert entries[0].hexsha != null_hex_sha - self.assertEqual(len(entries), 1) - self.assertTrue(S_ISLNK(entries[0].mode)) - - # Check that this also works with an alternate method. - full_index_entry = IndexEntry.from_base(BaseIndexEntry((0o120000, entries[0].binsha, 0, entries[0].path))) - entry_key = index.entry_key(full_index_entry) - index.reset(new_commit) - - assert entry_key not in index.entries - index.entries[entry_key] = full_index_entry - index.write() - index.update() # Force reread of entries. - new_entry = index.entries[entry_key] - assert S_ISLNK(new_entry.mode) + with xfail_if_raises( + sys.platform == "win32" and (Git().config("core.symlinks") == "true" or _windows_supports_symlinks()), + raises=(FileNotFoundError, GitCommandError), + reason="Assumes symlinks are not created on Windows and opens a symlink to a nonexistent target.", + ): + index = rw_repo.index + num_entries = len(index.entries) + cur_head = rw_repo.head + + uname = "Thomas Müller" + umail = "sd@company.com" + with rw_repo.config_writer() as writer: + writer.set_value("user", "name", uname) + writer.set_value("user", "email", umail) + self.assertEqual(writer.get_value("user", "name"), uname) + + # Remove all of the files, provide a wild mix of paths, BaseIndexEntries, + # IndexEntries. + def mixed_iterator(): + count = 0 + for entry in index.entries.values(): + type_id = count % 5 + 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 == 3: # path mock in a blob + yield Blob(rw_repo, entry.binsha, entry.mode, entry.path) + 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 == 6: # IndexEntry + yield entry + else: + raise AssertionError("Invalid Type") + count += 1 + # END for each entry + + # END mixed iterator + deleted_files = index.remove(mixed_iterator(), working_tree=False) + assert deleted_files + self.assertEqual(self._count_existing(rw_repo, deleted_files), len(deleted_files)) + self.assertEqual(len(index.entries), 0) + + # Reset the index to undo our changes. + index.reset() + self.assertEqual(len(index.entries), num_entries) + + # Remove with working copy. + deleted_files = index.remove(mixed_iterator(), working_tree=True) + assert deleted_files + self.assertEqual(self._count_existing(rw_repo, deleted_files), 0) + + # Reset everything. + index.reset(working_tree=True) + self.assertEqual(self._count_existing(rw_repo, deleted_files), len(deleted_files)) + + # Invalid type. + self.assertRaises(TypeError, index.remove, [1]) + + # Absolute path. + deleted_files = index.remove([osp.join(rw_repo.working_tree_dir, "lib")], r=True) + assert len(deleted_files) > 1 + self.assertRaises(ValueError, index.remove, ["/doesnt/exists"]) + + # TEST COMMITTING + # Commit changed index. + cur_commit = cur_head.commit + commit_message = "commit default head by Frèderic Çaufl€" + + new_commit = index.commit(commit_message, head=False) + assert cur_commit != new_commit + self.assertEqual(new_commit.author.name, uname) + self.assertEqual(new_commit.author.email, umail) + self.assertEqual(new_commit.committer.name, uname) + self.assertEqual(new_commit.committer.email, umail) + self.assertEqual(new_commit.message, commit_message) + self.assertEqual(new_commit.parents[0], cur_commit) + self.assertEqual(len(new_commit.parents), 1) + self.assertEqual(cur_head.commit, cur_commit) + + # Commit with other actor. + cur_commit = cur_head.commit + + my_author = Actor("Frèderic Çaufl€", "author@example.com") + my_committer = Actor("Committing Frèderic Çaufl€", "committer@example.com") + commit_actor = index.commit(commit_message, author=my_author, committer=my_committer) + assert cur_commit != commit_actor + self.assertEqual(commit_actor.author.name, "Frèderic Çaufl€") + self.assertEqual(commit_actor.author.email, "author@example.com") + self.assertEqual(commit_actor.committer.name, "Committing Frèderic Çaufl€") + self.assertEqual(commit_actor.committer.email, "committer@example.com") + self.assertEqual(commit_actor.message, commit_message) + self.assertEqual(commit_actor.parents[0], cur_commit) + self.assertEqual(len(new_commit.parents), 1) + self.assertEqual(cur_head.commit, commit_actor) + self.assertEqual(cur_head.log()[-1].actor, my_committer) + + # Commit with author_date and commit_date. + cur_commit = cur_head.commit + commit_message = "commit with dates by Avinash Sajjanshetty" + + new_commit = index.commit( + commit_message, + author_date="2006-04-07T22:13:13", + commit_date="2005-04-07T22:13:13", + ) + assert cur_commit != new_commit + print(new_commit.authored_date, new_commit.committed_date) + self.assertEqual(new_commit.message, commit_message) + self.assertEqual(new_commit.authored_date, 1144447993) + self.assertEqual(new_commit.committed_date, 1112911993) + + # Same index, no parents. + commit_message = "index without parents" + commit_no_parents = index.commit(commit_message, parent_commits=[], head=True) + self.assertEqual(commit_no_parents.message, commit_message) + self.assertEqual(len(commit_no_parents.parents), 0) + self.assertEqual(cur_head.commit, commit_no_parents) + + # same index, multiple parents. + commit_message = "Index with multiple parents\n commit with another line" + commit_multi_parent = index.commit(commit_message, parent_commits=(commit_no_parents, new_commit)) + self.assertEqual(commit_multi_parent.message, commit_message) + self.assertEqual(len(commit_multi_parent.parents), 2) + self.assertEqual(commit_multi_parent.parents[0], commit_no_parents) + self.assertEqual(commit_multi_parent.parents[1], new_commit) + self.assertEqual(cur_head.commit, commit_multi_parent) + + # Re-add all files in lib. + # Get the lib folder back on disk, but get an index without it. + index.reset(new_commit.parents[0], working_tree=True).reset(new_commit, working_tree=False) + lib_file_path = osp.join("lib", "git", "__init__.py") + assert (lib_file_path, 0) not in index.entries + assert osp.isfile(osp.join(rw_repo.working_tree_dir, lib_file_path)) + + # Directory. + entries = index.add(["lib"], fprogress=self._fprogress_add) + self._assert_entries(entries) + self._assert_fprogress(entries) + assert len(entries) > 1 + + # Glob. + entries = index.reset(new_commit).add([osp.join("lib", "git", "*.py")], fprogress=self._fprogress_add) + self._assert_entries(entries) + self._assert_fprogress(entries) + self.assertEqual(len(entries), 14) + + # Same file. + entries = index.reset(new_commit).add( + [osp.join(rw_repo.working_tree_dir, "lib", "git", "head.py")] * 2, + fprogress=self._fprogress_add, + ) + self._assert_entries(entries) + self.assertEqual(entries[0].mode & 0o644, 0o644) + # Would fail, test is too primitive to handle this case. + # self._assert_fprogress(entries) + self._reset_progress() + self.assertEqual(len(entries), 2) + + # Missing path. + self.assertRaises(OSError, index.reset(new_commit).add, ["doesnt/exist/must/raise"]) + + # Blob from older revision overrides current index revision. + old_blob = new_commit.parents[0].tree.blobs[0] + entries = index.reset(new_commit).add([old_blob], fprogress=self._fprogress_add) + self._assert_entries(entries) + self._assert_fprogress(entries) + self.assertEqual(index.entries[(old_blob.path, 0)].hexsha, old_blob.hexsha) + self.assertEqual(len(entries), 1) + + # Mode 0 not allowed. + null_hex_sha = Diff.NULL_HEX_SHA + null_bin_sha = b"\0" * 20 + self.assertRaises( + ValueError, + index.reset(new_commit).add, + [BaseIndexEntry((0, null_bin_sha, 0, "doesntmatter"))], + ) - # A tree created from this should contain the symlink. - tree = index.write_tree() - assert fake_symlink_relapath in tree - index.write() # Flush our changes for the checkout. - - # Check out the fake link, should be a link then. - assert not S_ISLNK(os.stat(fake_symlink_path)[ST_MODE]) - os.remove(fake_symlink_path) - index.checkout(fake_symlink_path) - - # On Windows, we currently assume we will never get symlinks. - if sys.platform == "win32": - # Symlinks should contain the link as text (which is what a - # symlink actually is). - with open(fake_symlink_path, "rt") as fd: - self.assertEqual(fd.read(), link_target) - else: - self.assertTrue(S_ISLNK(os.lstat(fake_symlink_path)[ST_MODE])) - - # TEST RENAMING - def assert_mv_rval(rval): - for source, dest in rval: - assert not osp.exists(source) and osp.exists(dest) - # END for each renamed item - - # END move assertion utility - - self.assertRaises(ValueError, index.move, ["just_one_path"]) - # Try to move a file onto an existing file. - files = ["AUTHORS", "LICENSE"] - self.assertRaises(GitCommandError, index.move, files) - - # Again, with force. - assert_mv_rval(index.move(files, f=True)) - - # Move files into a directory - dry run. - paths = ["LICENSE", "VERSION", "doc"] - rval = index.move(paths, dry_run=True) - self.assertEqual(len(rval), 2) - assert osp.exists(paths[0]) - - # Again, no dry run. - rval = index.move(paths) - assert_mv_rval(rval) - - # Move dir into dir. - rval = index.move(["doc", "test"]) - assert_mv_rval(rval) - - # TEST PATH REWRITING - ###################### - count = [0] - - def rewriter(entry): - rval = str(count[0]) - count[0] += 1 - return rval - - # END rewriter - - def make_paths(): - """Help out the test by yielding two existing paths and one new path.""" - yield "CHANGES" - yield "ez_setup.py" - yield index.entries[index.entry_key("README", 0)] - yield index.entries[index.entry_key(".gitignore", 0)] - - for fid in range(3): - fname = "newfile%i" % fid - with open(fname, "wb") as fd: - fd.write(b"abcd") - yield Blob(rw_repo, Blob.NULL_BIN_SHA, 0o100644, fname) - # END for each new file - - # END path producer - paths = list(make_paths()) - self._assert_entries(index.add(paths, path_rewriter=rewriter)) - - for filenum in range(len(paths)): - assert index.entry_key(str(filenum), 0) in index.entries - - # TEST RESET ON PATHS - ###################### - arela = "aa" - brela = "bb" - afile = self._make_file(arela, "adata", rw_repo) - bfile = self._make_file(brela, "bdata", rw_repo) - akey = index.entry_key(arela, 0) - bkey = index.entry_key(brela, 0) - keys = (akey, bkey) - absfiles = (afile, bfile) - files = (arela, brela) - - for fkey in keys: - assert fkey not in index.entries - - index.add(files, write=True) - nc = index.commit("2 files committed", head=False) - - for fkey in keys: - assert fkey in index.entries - - # Just the index. - index.reset(paths=(arela, afile)) - assert akey not in index.entries - assert bkey in index.entries - - # Now with working tree - files on disk as well as entries must be recreated. - rw_repo.head.commit = nc - for absfile in absfiles: - os.remove(absfile) - - index.reset(working_tree=True, paths=files) - - for fkey in keys: - assert fkey in index.entries - for absfile in absfiles: - assert osp.isfile(absfile) + # Add new file. + new_file_relapath = "my_new_file" + self._make_file(new_file_relapath, "hello world", rw_repo) + entries = index.reset(new_commit).add( + [BaseIndexEntry((0o10644, null_bin_sha, 0, new_file_relapath))], + fprogress=self._fprogress_add, + ) + self._assert_entries(entries) + self._assert_fprogress(entries) + self.assertEqual(len(entries), 1) + self.assertNotEqual(entries[0].hexsha, null_hex_sha) + + # Add symlink. + if sys.platform != "win32": + for target in ("/etc/nonexisting", "/etc/passwd", "/etc"): + basename = "my_real_symlink" + + link_file = osp.join(rw_repo.working_tree_dir, basename) + os.symlink(target, link_file) + entries = index.reset(new_commit).add([link_file], fprogress=self._fprogress_add) + self._assert_entries(entries) + self._assert_fprogress(entries) + self.assertEqual(len(entries), 1) + self.assertTrue(S_ISLNK(entries[0].mode)) + self.assertTrue(S_ISLNK(index.entries[index.entry_key("my_real_symlink", 0)].mode)) + + # We expect only the target to be written. + self.assertEqual( + index.repo.odb.stream(entries[0].binsha).read().decode("ascii"), + target, + ) + + os.remove(link_file) + # END for each target + # END real symlink test + + # Add fake symlink and assure it checks out as a symlink. + fake_symlink_relapath = "my_fake_symlink" + link_target = "/etc/that" + fake_symlink_path = self._make_file(fake_symlink_relapath, link_target, rw_repo) + fake_entry = BaseIndexEntry((0o120000, null_bin_sha, 0, fake_symlink_relapath)) + entries = index.reset(new_commit).add([fake_entry], fprogress=self._fprogress_add) + self._assert_entries(entries) + self._assert_fprogress(entries) + assert entries[0].hexsha != null_hex_sha + self.assertEqual(len(entries), 1) + self.assertTrue(S_ISLNK(entries[0].mode)) + + # Check that this also works with an alternate method. + full_index_entry = IndexEntry.from_base(BaseIndexEntry((0o120000, entries[0].binsha, 0, entries[0].path))) + entry_key = index.entry_key(full_index_entry) + index.reset(new_commit) + + assert entry_key not in index.entries + index.entries[entry_key] = full_index_entry + index.write() + index.update() # Force reread of entries. + new_entry = index.entries[entry_key] + assert S_ISLNK(new_entry.mode) + + # A tree created from this should contain the symlink. + tree = index.write_tree() + assert fake_symlink_relapath in tree + index.write() # Flush our changes for the checkout. + + # Check out the fake link, should be a link then. + assert not S_ISLNK(os.stat(fake_symlink_path)[ST_MODE]) + os.remove(fake_symlink_path) + index.checkout(fake_symlink_path) + + # On Windows, we currently assume we will never get symlinks. + if sys.platform == "win32": + # Symlinks should contain the link as text (which is what a + # symlink actually is). + with open(fake_symlink_path, "rt") as fd: + self.assertEqual(fd.read(), link_target) + else: + self.assertTrue(S_ISLNK(os.lstat(fake_symlink_path)[ST_MODE])) + + # TEST RENAMING + def assert_mv_rval(rval): + for source, dest in rval: + assert not osp.exists(source) and osp.exists(dest) + # END for each renamed item + + # END move assertion utility + + self.assertRaises(ValueError, index.move, ["just_one_path"]) + # Try to move a file onto an existing file. + files = ["AUTHORS", "LICENSE"] + self.assertRaises(GitCommandError, index.move, files) + + # Again, with force. + assert_mv_rval(index.move(files, f=True)) + + # Move files into a directory - dry run. + paths = ["LICENSE", "VERSION", "doc"] + rval = index.move(paths, dry_run=True) + self.assertEqual(len(rval), 2) + assert osp.exists(paths[0]) + + # Again, no dry run. + rval = index.move(paths) + assert_mv_rval(rval) + + # Move dir into dir. + rval = index.move(["doc", "test"]) + assert_mv_rval(rval) + + # TEST PATH REWRITING + ###################### + count = [0] + + def rewriter(entry): + rval = str(count[0]) + count[0] += 1 + return rval + + # END rewriter + + def make_paths(): + """Help out the test by yielding two existing paths and one new path.""" + yield "CHANGES" + yield "ez_setup.py" + yield index.entries[index.entry_key("README", 0)] + yield index.entries[index.entry_key(".gitignore", 0)] + + for fid in range(3): + fname = "newfile%i" % fid + with open(fname, "wb") as fd: + fd.write(b"abcd") + yield Blob(rw_repo, Blob.NULL_BIN_SHA, 0o100644, fname) + # END for each new file + + # END path producer + paths = list(make_paths()) + self._assert_entries(index.add(paths, path_rewriter=rewriter)) + + for filenum in range(len(paths)): + assert index.entry_key(str(filenum), 0) in index.entries + + # TEST RESET ON PATHS + ###################### + arela = "aa" + brela = "bb" + afile = self._make_file(arela, "adata", rw_repo) + bfile = self._make_file(brela, "bdata", rw_repo) + akey = index.entry_key(arela, 0) + bkey = index.entry_key(brela, 0) + keys = (akey, bkey) + absfiles = (afile, bfile) + files = (arela, brela) + + for fkey in keys: + assert fkey not in index.entries + + index.add(files, write=True) + nc = index.commit("2 files committed", head=False) + + for fkey in keys: + assert fkey in index.entries + + # Just the index. + index.reset(paths=(arela, afile)) + assert akey not in index.entries + assert bkey in index.entries + + # Now with working tree - files on disk as well as entries must be recreated. + rw_repo.head.commit = nc + for absfile in absfiles: + os.remove(absfile) + + index.reset(working_tree=True, paths=files) + + for fkey in keys: + assert fkey in index.entries + for absfile in absfiles: + assert osp.isfile(absfile) @with_rw_repo("HEAD") def test_compare_write_tree(self, rw_repo): From fa0d9bc17928e7c11dc11c875c1047587f5354c3 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Thu, 7 May 2026 11:35:08 -0400 Subject: [PATCH 27/45] Add a test that fixture directories are usable by git Many tests rely on git accepting their fixture directories: the GitPython repository itself, and the gitdb and smmap submodules used by the test suite as direct and nested submodule fixtures. When git rejects one for "dubious ownership" (typically because a CI workflow's `safe.directory` list is missing an entry), three submodule-related tests fail in opaque ways. The exact failure type depends on which side of a race wins in the flush to the cached `git cat-file --batch-check` subprocess: - When Python wins, `ValueError` reaches the test directly. - When git wins, `BrokenPipeError` is caught internally as `IOError` and `iter_items` returns early. Each test then fails on the empty-or-short result in its own way: - `test_docs::Tutorials::test_submodules` indexes the empty `sm.children()` list with `[0]` and raises `IndexError`. - `test_repo::TestRepo::test_submodules` and `test_submodule::TestSubmodule::test_root_module` see a length-1 (not length-2) submodule list from their recursive traversals and fail their length assertion with `AssertionError`. This commit adds `test/test_fixture_health.py`, which runs `git rev-parse --show-toplevel` in each fixture directory and asserts both that git is willing to operate there and that it reports the directory as its own toplevel. The failure message names `safe.directory` and ownership as the likely causes, so a misconfigured environment is recognizable directly from the test output rather than needing to be pieced together from a cascade of failing tests not conceptually related to safe directory protections. This adds the test only. Further replication and a fix are forthcoming. On the Cygwin CI workflow now, the test fails for `[gitdb]` and passes for `[repo_root]` and `[smmap]`. The asymmetry between the two submodules relates to NTFS ownership inherited from `actions/checkout`'s clone of the main tree. Co-authored-by: Claude Opus 4.7 (1M context) --- test/test_fixture_health.py | 91 +++++++++++++++++++++++++++++++++++++ 1 file changed, 91 insertions(+) create mode 100644 test/test_fixture_health.py diff --git a/test/test_fixture_health.py b/test/test_fixture_health.py new file mode 100644 index 000000000..b2e8cfa6b --- /dev/null +++ b/test/test_fixture_health.py @@ -0,0 +1,91 @@ +# This module is part of GitPython and is released under the +# 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/ + +"""Verify that fixture directories are usable by git. + +If a directory the test suite relies on is rejected by git for "dubious +ownership" -- because the directory's owner doesn't match the running user +and there is no ``safe.directory`` entry overriding the check -- three +submodule-related tests fail in confusing ways. The checks here name the +root cause clearly so a misconfigured environment is recognizable from the +test output. + +The rejection is most often a CI-workflow problem (the workflow's +``safe.directory`` list doesn't cover the path); on a developer's own +clone, it usually reflects an ownership mismatch (sudo clone, restored +backup, container mount, networked filesystem) rather than a config gap. + +These tests do not exercise GitPython's production code. They verify the +conditions under which production code is exercised are valid. + +A check is skipped, rather than failed, if a fixture directory is missing or +has no ``.git`` marker, since that condition is more naturally diagnosed as +"``init-tests-after-clone.sh`` hasn't been run" than as a problem with +``safe.directory``. +""" + +import subprocess +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).resolve().parent.parent + +# Directories git must trust for the test suite to operate normally. The +# current set is the GitPython working tree plus the working trees of its +# gitdb submodule and the smmap submodule nested inside gitdb. New entries +# should be added here whenever the test suite gains a dependency on git +# accepting another directory. +FIXTURE_DIRS = [ + pytest.param(REPO_ROOT, id="repo_root"), + pytest.param(REPO_ROOT / "git" / "ext" / "gitdb", id="gitdb"), + pytest.param( + REPO_ROOT / "git" / "ext" / "gitdb" / "gitdb" / "ext" / "smmap", + id="smmap", + ), +] + + +@pytest.mark.parametrize("fixture_dir", FIXTURE_DIRS) +def test_fixture_dir_is_trusted_by_git(fixture_dir: Path) -> None: + """git accepts ``fixture_dir`` as its own repository owned by a trusted user. + + Run ``git -C rev-parse --show-toplevel`` and assert it + succeeds and reports ``fixture_dir`` itself as the toplevel. Failure + typically means the directory's on-disk ownership doesn't match the + running user and the CI workflow's ``safe.directory`` list is missing + an entry that would override the check. + """ + if not fixture_dir.exists(): + pytest.skip(f"{fixture_dir} not present (run `git submodule update --init --recursive` from the repo root)") + if not (fixture_dir / ".git").exists(): + pytest.skip( + f"{fixture_dir} has no .git marker " + "(submodule not initialized; run " + "`git submodule update --init --recursive` from the repo root)" + ) + try: + result = subprocess.run( + ["git", "-C", str(fixture_dir), "rev-parse", "--show-toplevel"], + capture_output=True, + text=True, + check=False, + ) + except FileNotFoundError: + pytest.skip("git is not installed or not on PATH") + assert result.returncode == 0, ( + f"git refuses to operate in {fixture_dir}.\n" + f"stderr: {result.stderr.strip()}\n" + "The directory's owner doesn't match the running user and no " + "`safe.directory` entry overrides the check. On CI, the " + "workflow's `safe.directory` list typically needs an entry for " + "this path. Locally, this is unexpected and usually indicates " + "an ownership problem worth investigating." + ) + reported = Path(result.stdout.strip()) + assert reported.samefile(fixture_dir), ( + f"git reports the toplevel as {reported}, " + f"not as {fixture_dir} itself. " + "This usually means the directory is not an initialized git " + "repository (its `.git` marker may be stale or pointing elsewhere)." + ) From d5fb280613feb71a335ae371db1c32d3de09d594 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Thu, 7 May 2026 18:55:39 -0400 Subject: [PATCH 28/45] Add a test that required submodules are initialized This adds the new test, `test_required_submodule_is_initialized`, to `test_fixture_health.py`. For the gitdb and smmap submodules, the test asserts that the working tree directory exists and contains a `.git` marker (file or directory). The failure message reminds the developer to run `git submodule update --init --recursive`. This is a regression test for a different contract than the preexisting `test_fixture_dir_is_trusted_by_git` in `test_fixture_health.py`. That test verifies git's willingness to operate in each fixture directory (the `safe.directory` / dubious-ownership contract). This one verifies that the directories are populated at all. When the gitdb and smmap submodules aren't populated, the rest of the suite fails in ways that don't name the cause: a cascade of failures across `test_docs.py`, `test_repo.py`, and `test_submodule.py` -- tests that need submodule content but don't set out to check for it. That cascade was the failure mode of #1713 on the Arch Linux build, where `init-tests-after-clone.sh` had stopped running `git submodule update` on CI. Wherever the submodules are actually missing, this test reports the missing path directly instead. The test skips when the source tree is not a git clone (no `.git` at `REPO_ROOT`). This is to accommodate setups that run pytest from an extracted release tarball and prepare submodules in a separately-pointed tree via `GIT_PYTHON_TEST_GIT_REPO_BASE` (e.g. OpenIndiana's package build). In such setups, `git submodule update` cannot operate on the source tree, and the submodule paths checked here would never be populated there, but the test suite still works because `rorepo` points elsewhere. Co-authored-by: Claude Opus 4.7 (1M context) --- test/test_fixture_health.py | 76 ++++++++++++++++++++++++++++--------- 1 file changed, 58 insertions(+), 18 deletions(-) diff --git a/test/test_fixture_health.py b/test/test_fixture_health.py index b2e8cfa6b..b18d5e8f9 100644 --- a/test/test_fixture_health.py +++ b/test/test_fixture_health.py @@ -3,25 +3,15 @@ """Verify that fixture directories are usable by git. -If a directory the test suite relies on is rejected by git for "dubious -ownership" -- because the directory's owner doesn't match the running user -and there is no ``safe.directory`` entry overriding the check -- three -submodule-related tests fail in confusing ways. The checks here name the -root cause clearly so a misconfigured environment is recognizable from the -test output. +If a fixture directory is missing, isn't an initialized git repository, +or is rejected by git for "dubious ownership", dependent tests +elsewhere in the suite fail in opaque ways. The checks here name the +preconditions directly so a misconfigured environment is recognizable +from the test output rather than from a cascade of unrelated-seeming +failures. -The rejection is most often a CI-workflow problem (the workflow's -``safe.directory`` list doesn't cover the path); on a developer's own -clone, it usually reflects an ownership mismatch (sudo clone, restored -backup, container mount, networked filesystem) rather than a config gap. - -These tests do not exercise GitPython's production code. They verify the -conditions under which production code is exercised are valid. - -A check is skipped, rather than failed, if a fixture directory is missing or -has no ``.git`` marker, since that condition is more naturally diagnosed as -"``init-tests-after-clone.sh`` hasn't been run" than as a problem with -``safe.directory``. +These tests do not exercise GitPython's production code. They verify +the conditions under which production code is exercised are valid. """ import subprocess @@ -45,6 +35,19 @@ ), ] +# Submodule working trees that must be present and initialized for the +# test suite to operate normally: gitdb at `git/ext/gitdb`, and smmap +# nested inside gitdb at `git/ext/gitdb/gitdb/ext/smmap`. The paths +# below are anchored at REPO_ROOT (the GitPython source tree), not at +# any rorepo redirection target. +SUBMODULE_DIRS = [ + pytest.param(REPO_ROOT / "git" / "ext" / "gitdb", id="gitdb"), + pytest.param( + REPO_ROOT / "git" / "ext" / "gitdb" / "gitdb" / "ext" / "smmap", + id="smmap", + ), +] + @pytest.mark.parametrize("fixture_dir", FIXTURE_DIRS) def test_fixture_dir_is_trusted_by_git(fixture_dir: Path) -> None: @@ -89,3 +92,40 @@ def test_fixture_dir_is_trusted_by_git(fixture_dir: Path) -> None: "This usually means the directory is not an initialized git " "repository (its `.git` marker may be stale or pointing elsewhere)." ) + + +@pytest.mark.parametrize("submodule_dir", SUBMODULE_DIRS) +def test_required_submodule_is_initialized(submodule_dir: Path) -> None: + """The submodule's working tree is present and initialized. + + Failure means the source tree is a git clone but the submodule's + working tree hasn't been populated. Skipped when the source tree + itself isn't a git clone (e.g. an extracted release tarball), since + ``git submodule update`` cannot operate there; setups that handle + submodules in a separately-prepared tree (via + ``GIT_PYTHON_TEST_GIT_REPO_BASE``) are exempted from this check. + """ + if not (REPO_ROOT / ".git").exists(): + pytest.skip( + "Source tree is not a git clone (no .git in REPO_ROOT); submodules " + "cannot be initialized via `git submodule update` here. Setups " + "that prepare submodules in a separately-pointed tree (via " + "GIT_PYTHON_TEST_GIT_REPO_BASE) are exempted from this check." + ) + # The assertion messages below recommend `git submodule update --init + # --recursive` rather than `init-tests-after-clone.sh`, even though the + # latter is the documented entry point for first-time test setup. Two + # reasons: the script performs `git reset --hard` operations that can + # destroy local work, and #1713 showed the script itself can carry + # submodule-init regressions, in which case recommending it would lead + # developers in a circle. The direct git command is a safe minimal fix + # for this test's specific failure mode and bypasses any such regression. + assert submodule_dir.is_dir(), ( + f"Submodule working tree missing: {submodule_dir}.\n" + "Run `git submodule update --init --recursive` from the repo root." + ) + assert (submodule_dir / ".git").exists(), ( + f"Submodule directory exists but has no .git marker: {submodule_dir}.\n" + "The submodule hasn't been initialized. " + "Run `git submodule update --init --recursive` from the repo root." + ) From ce68322cc3b3fea4077b3a1800283d00fe4dcb3e Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Wed, 6 May 2026 14:26:57 -0400 Subject: [PATCH 29/45] Clearly reproduce Cygwin safe.directory submodule bug Remove the Cygwin xfail decorations from `test_submodules` in `test_docs.py` and `test_repo.py`, and from `test_root_module` in `test_submodule.py`, so the tests surface the underlying failure directly. Also temporarily add 256 `reproduce-safe-dir` matrix jobs to `.github/workflows/cygwin-test.yml`, each running these three tests under the current `safe.directory` configuration. All or most of these `reproduce-safe-dir` jobs shall be removed before the associated bugfix is integrated. The existing test job's `env`, `defaults`, and setup steps gain YAML anchors so the new job can reference them without duplication. The anchors will be removed when the `reproduce-safe-dir` jobs are. Root cause ---------- The CI workflow adds the main repo and its `.git` directory to `safe.directory` but not the gitdb or smmap submodule working trees. Git matches `safe.directory` by specific paths (or wildcards, but we are not using wildcards). When GitPython opens a submodule as a `Repo` and runs `git cat-file --batch-check` against it, git rejects the repository for dubious ownership and exits. The race -------- GitPython caches the cat-file process. When git has exited, the next call to `__get_object_header` hits one of two outcomes depending on whether Python's `cmd.stdin.flush()` runs before or after the kernel marks the read end of the pipe as closed. (This is the same mechanism as the long-standing #427, in which SIGINT kills the cat-file process and the next `flush()` raises `BrokenPipeError`.) 1. If Python wins the race, `flush()` succeeds (data goes into the kernel buffer); the subsequent `stdout.readline()` returns `b""` (EOF); `_parse_object_header` raises a `ValueError` whose message names the rejected directory. 2. If git wins, `flush()` raises `BrokenPipeError`, a subclass of `OSError` (a.k.a. `IOError`). In both paths, the exception travels from `Object.new_from_sha` (outside `name_to_object`'s `except ValueError`, which covers only the `dereference_recursive` call), up through `repo.commit("HEAD")`, into `iter_items` in `git/objects/submodule/base.py`. That function's `except (IOError, BadName)` clause catches `BrokenPipeError` but not `ValueError`. So path (1) propagates `ValueError` all the way to the test, while path (2) ends with `iter_items` returning early and the test seeing an empty submodule list. Per-test outcomes ----------------- What the test sees in path (2) is determined by what the test does with the empty list: - `test_docs::test_submodules` does `sm.children()[0].name`, so the `[0]` on the empty list raises `IndexError`. - `test_repo::test_submodules` does `assertGreaterEqual(len(list(self.rorepo.iter_submodules())), 2)`. The recursive traversal yields gitdb (its `iter_items` on the *main* repo succeeds, because the main repo IS in `safe.directory`) but not smmap, so length 1 fails the assertion. - `test_submodule::test_root_module` does `assert len(rsmsp) >= 2` on a similar traversal result, failing. The race itself is non-deterministic, but the mapping from race outcome to exception type is deterministic per test. So each test has exactly two possible failure types, one per side of the race: - `test_docs`: `ValueError` -> Python wins, OR `IndexError` -> git wins. - `test_repo`: `ValueError` -> Python wins, OR `AssertionError` -> git wins. - `test_submodule`: `ValueError` -> Python wins, OR `AssertionError` -> git wins. In particular, `test_docs` never produces `AssertionError`, and `test_repo` and `test_submodule` never produce `IndexError`. Empirical confirmation ---------------------- In 1024 `reproduce-safe-dir` jobs and 4 buggy-config "test (fast)" job runs, 100% of 3084 target-test failures match the per-test prediction. `ValueError` accounts for ~99.0%, while the race-win exception types from the list above account for the others. `reproduce-safe-dir` runs: https://github.com/EliahKagan/GitPython/actions/runs/25836741324 https://github.com/EliahKagan/GitPython/actions/runs/25836329241 https://github.com/EliahKagan/GitPython/actions/runs/25836334196 https://github.com/EliahKagan/GitPython/actions/runs/25836339345 Run on the fix commit (256 jobs, 768 test outcomes, all PASSED): https://github.com/EliahKagan/GitPython/actions/runs/25836344886 The race condition, from a "test (fast)" job in a PR #2143 CI run: https://github.com/gitpython-developers/GitPython/actions/runs/25440735020/attempts/1 Co-authored-by: Claude Opus 4.7 (1M context) --- .github/workflows/cygwin-test.yml | 65 +++++++++++++++++++++++++------ test/test_docs.py | 8 ---- test/test_repo.py | 5 --- test/test_submodule.py | 5 --- 4 files changed, 53 insertions(+), 30 deletions(-) diff --git a/.github/workflows/cygwin-test.yml b/.github/workflows/cygwin-test.yml index 327e1f10c..e62106a18 100644 --- a/.github/workflows/cygwin-test.yml +++ b/.github/workflows/cygwin-test.yml @@ -20,46 +20,53 @@ jobs: runs-on: windows-latest - env: + env: &cygwin-env CHERE_INVOKING: "1" CYGWIN_NOWINPATH: "1" - defaults: + defaults: &cygwin-defaults run: shell: C:\cygwin\bin\bash.exe --login --norc -eo pipefail -o igncr "{0}" steps: - - name: Force LF line endings + - &force-lf + name: Force LF line endings run: | git config --global core.autocrlf false # Affects the non-Cygwin git. shell: pwsh # Do this outside Cygwin, to affect actions/checkout. - - uses: actions/checkout@v6 + - &checkout + uses: actions/checkout@v6 with: fetch-depth: 0 - - name: Install Cygwin + - &install-cygwin + name: Install Cygwin uses: cygwin/cygwin-install-action@v6 with: packages: git python39 python-pip-wheel python-setuptools-wheel python-wheel-wheel add-to-path: false # No need to change $PATH outside the Cygwin environment. - - name: Arrange for verbose output + - &verbose-output + name: Arrange for verbose output run: | # Arrange for verbose output but without shell environment setup details. echo 'set -x' >~/.bash_profile - - name: Special configuration for Cygwin git + - &safe-directory + name: Special configuration for Cygwin git run: | git config --global --add safe.directory "$(pwd)" git config --global --add safe.directory "$(pwd)/.git" git config --global core.autocrlf false - - name: Prepare this repo for tests + - &prepare-repo + name: Prepare this repo for tests run: | ./init-tests-after-clone.sh - - name: Set git user identity and command aliases for the tests + - &git-identity + name: Set git user identity and command aliases for the tests run: | git config --global user.email "travis@ci.com" git config --global user.name "Travis Runner" @@ -67,16 +74,19 @@ jobs: # and cause subsequent tests to fail cat test/fixtures/.gitconfig >> ~/.gitconfig - - name: Set up virtual environment + - &setup-venv + name: Set up virtual environment run: | python3.9 -m venv .venv echo 'BASH_ENV=.venv/bin/activate' >>"$GITHUB_ENV" - - name: Update PyPA packages + - &update-pypa + name: Update PyPA packages run: | python -m pip install -U pip 'setuptools; python_version<"3.12"' wheel - - name: Install project and test dependencies + - &install-deps + name: Install project and test dependencies run: | pip install '.[test]' @@ -91,3 +101,34 @@ jobs: - name: Test with pytest (${{ matrix.additional-pytest-args }}) run: | pytest --color=yes -p no:sugar --instafail -vv ${{ matrix.additional-pytest-args }} + + reproduce-safe-dir: + strategy: + matrix: + run: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63, 64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 80, 81, 82, 83, 84, 85, 86, 87, 88, 89, 90, 91, 92, 93, 94, 95, 96, 97, 98, 99, 100, 101, 102, 103, 104, 105, 106, 107, 108, 109, 110, 111, 112, 113, 114, 115, 116, 117, 118, 119, 120, 121, 122, 123, 124, 125, 126, 127, 128, 129, 130, 131, 132, 133, 134, 135, 136, 137, 138, 139, 140, 141, 142, 143, 144, 145, 146, 147, 148, 149, 150, 151, 152, 153, 154, 155, 156, 157, 158, 159, 160, 161, 162, 163, 164, 165, 166, 167, 168, 169, 170, 171, 172, 173, 174, 175, 176, 177, 178, 179, 180, 181, 182, 183, 184, 185, 186, 187, 188, 189, 190, 191, 192, 193, 194, 195, 196, 197, 198, 199, 200, 201, 202, 203, 204, 205, 206, 207, 208, 209, 210, 211, 212, 213, 214, 215, 216, 217, 218, 219, 220, 221, 222, 223, 224, 225, 226, 227, 228, 229, 230, 231, 232, 233, 234, 235, 236, 237, 238, 239, 240, 241, 242, 243, 244, 245, 246, 247, 248, 249, 250, 251, 252, 253, 254, 255, 256] + fail-fast: false + + runs-on: windows-latest + + env: *cygwin-env + + defaults: *cygwin-defaults + + steps: + - *force-lf + - *checkout + - *install-cygwin + - *verbose-output + - *safe-directory + - *prepare-repo + - *git-identity + - *setup-venv + - *update-pypa + - *install-deps + + - name: Run submodule tests + run: | + python -m pytest -vv \ + test/test_docs.py::Tutorials::test_submodules \ + test/test_repo.py::TestRepo::test_submodules \ + test/test_submodule.py::TestSubmodule::test_root_module diff --git a/test/test_docs.py b/test/test_docs.py index cc0bbf26a..c3426a807 100644 --- a/test/test_docs.py +++ b/test/test_docs.py @@ -6,9 +6,6 @@ import gc import os import os.path -import sys - -import pytest from test.lib import TestBase from test.lib.helper import with_rw_directory @@ -478,11 +475,6 @@ def test_references_and_objects(self, rw_dir): repo.git.clear_cache() - @pytest.mark.xfail( - sys.platform == "cygwin", - reason="Cygwin GitPython can't find SHA for submodule", - raises=ValueError, - ) def test_submodules(self): # [1-test_submodules] repo = self.rorepo diff --git a/test/test_repo.py b/test/test_repo.py index 13bff52e9..d2dd1ea5d 100644 --- a/test/test_repo.py +++ b/test/test_repo.py @@ -877,11 +877,6 @@ def test_repo_odbtype(self): target_type = GitCmdObjectDB self.assertIsInstance(self.rorepo.odb, target_type) - @pytest.mark.xfail( - sys.platform == "cygwin", - reason="Cygwin GitPython can't find submodule SHA", - raises=ValueError, - ) def test_submodules(self): self.assertEqual(len(self.rorepo.submodules), 1) # non-recursive self.assertGreaterEqual(len(list(self.rorepo.iter_submodules())), 2) diff --git a/test/test_submodule.py b/test/test_submodule.py index 63bb007de..0373e26f8 100644 --- a/test/test_submodule.py +++ b/test/test_submodule.py @@ -480,11 +480,6 @@ def test_base_rw(self, rwrepo): def test_base_bare(self, rwrepo): self._do_base_tests(rwrepo) - @pytest.mark.xfail( - sys.platform == "cygwin", - reason="Cygwin GitPython can't find submodule SHA", - raises=ValueError, - ) @pytest.mark.xfail( HIDE_WINDOWS_KNOWN_ERRORS, reason=( From 76f0160681f551b5199edd6dfd005012e65ad6d3 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sun, 10 May 2026 14:33:14 -0400 Subject: [PATCH 30/45] Add a diagnostic job demonstrating the TokenOwner mechanism Temporarily add a `diag-token` job to the `cygwin-test` workflow that creates a directory through four code paths and reports its NTFS Owner. The idea is to empirically establish the cause of the Owner asymmetry: a Cygwin or Cygwin-derived runtime (`cygwin1.dll` for Cygwin git; `msys-2.0.dll` loaded by Git for Windows's bundled MSYS2 `sh.exe`) rewrites the process token's `TokenOwner` field at DLL initialization, and `CreateProcessW` propagates that mutation to every descendant. The four tests are: | Test | Sequence | Predicted Owner | | ---- | ----------------------------------------------- | ------------------------ | | A | PowerShell `New-Item` | `BUILTIN\Administrators` | | B | Cygwin `mkdir` | `runneradmin` (197108) | | C | Cygwin bash -> Git for Windows `git init` | `runneradmin` (197108) | | D | PowerShell -> Git Bash -> PowerShell `New-Item` | `runneradmin` (197108) | Test A is the Win32 baseline. The kernel-default `TokenOwner` for `runneradmin`'s full administrative token is `BUILTIN\Administrators`, because `runneradmin` is the built-in local Administrator (RID 500) and `FilterAdministratorToken=0` exempts it from UAC token filtering. Test B is the Cygwin baseline. Cygwin's `cygheap_user::init` calls `NtSetInformationToken(hProcToken, TokenOwner, &effec_cygsid, ...)` at DLL init, and the resulting Owner reflects the rewritten token. Test C is the load-bearing case. The child is a native Windows program that loads no Cygwin runtime of its own. It produces a user-owned `.git`, showing that the rewrite performed by the parent Cygwin bash propagates through `CreateProcessW`'s token duplication to a native Windows descendant. Test D strengthens the case: the first and last links are native Windows programs (a fresh PowerShell using its built-in `New-Item` cmdlet). Only the middle link is a Cygwin-family runtime (Git for Windows's MSYS2 `bash.exe`, linked against `msys-2.0.dll`). The directory is still user-owned, showing that the mechanism does not depend on git, nor on shell dispatch via `git submodule`, nor on any property of the final-link binary, but only on whether *any* process in the ancestry has loaded a Cygwin-family runtime. This `diag-token` job, like the `reproduce-safe-dir` matrix, is temporary and should be removed before this work is integrated. Co-authored-by: Claude Opus 4.7 (1M context) --- .github/workflows/cygwin-test.yml | 67 +++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/.github/workflows/cygwin-test.yml b/.github/workflows/cygwin-test.yml index e62106a18..9cc90804e 100644 --- a/.github/workflows/cygwin-test.yml +++ b/.github/workflows/cygwin-test.yml @@ -102,6 +102,73 @@ jobs: run: | pytest --color=yes -p no:sugar --instafail -vv ${{ matrix.additional-pytest-args }} + diag-token: + runs-on: windows-latest + + env: *cygwin-env + + defaults: *cygwin-defaults + + steps: + - *force-lf + - *checkout + - *install-cygwin + + - name: PowerShell-side token state and file-creation tests + shell: pwsh + run: | + $repo = "D:\a\GitPython\GitPython" + Write-Host "==================== whoami /all (PowerShell) ====================" + whoami /all + Write-Host "" + Write-Host "==================== Test A: PowerShell New-Item directory ====================" + $td = "$repo\test-pwsh-mkdir" + New-Item -ItemType Directory -Path $td -Force | Out-Null + $acl = Get-Acl -LiteralPath $td + Write-Host "Owner of $td : $($acl.Owner)" + Remove-Item $td -Force + Write-Host "" + Write-Host "==================== Test D: PowerShell -> Git Bash -> PowerShell New-Item ====================" + $td4 = "$repo\test-pwsh-bash-pwsh-mkdir" + $scriptPath = "$repo\inner-mkdir.ps1" + "New-Item -ItemType Directory -Path '$td4' -Force | Out-Null" | + Set-Content -Path $scriptPath -Encoding utf8 + $env:MSYS2_ARG_CONV_EXCL = '*' + try { + & "C:\Program Files\Git\bin\bash.exe" -c "powershell.exe -NoProfile -ExecutionPolicy Bypass -File '$scriptPath'" 2>&1 | Out-Null + } finally { + Remove-Item Env:MSYS2_ARG_CONV_EXCL -ErrorAction SilentlyContinue + } + if (Test-Path -LiteralPath $td4) { + $acl = Get-Acl -LiteralPath $td4 + Write-Host "Owner of $td4 (PowerShell -> bash -> PowerShell New-Item) : $($acl.Owner)" + Remove-Item $td4 -Force + } else { + Write-Host "Owner of $td4 (PowerShell -> bash -> PowerShell New-Item) : (directory not created)" + } + Remove-Item $scriptPath -Force -ErrorAction SilentlyContinue + + - name: Cygwin-side token state and file-creation tests + run: | + set +e + echo "==================== id (Cygwin) ====================" + id + echo + echo "==================== Test B: Cygwin mkdir ====================" + td="$(pwd)/test-cygwin-mkdir" + mkdir "$td" + echo "Owner: $(stat -c '%U(%u)' "$td")" + rmdir "$td" + echo + echo "==================== Test C: Cygwin-spawned Git for Windows git init ====================" + td3="$(pwd)/test-cygwin-spawns-wingit" + mkdir "$td3" + ( cd "$td3" && /cygdrive/c/Program\ Files/Git/bin/git.exe init -q ) + echo "Owner of $td3 (Cygwin-mkdir) : $(stat -c '%U(%u)' "$td3")" + echo "Owner of $td3/.git (Cygwin->Win git init): $(stat -c '%U(%u)' "$td3/.git")" + rm -rf "$td3" + true + reproduce-safe-dir: strategy: matrix: From f368f17d161854cf05d871ef8f48a22ab194bc63 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sun, 10 May 2026 14:34:47 -0400 Subject: [PATCH 31/45] Show file ownership and safe.directory entries on every test job In each workflow that runs the test suite, add a step to the `test` job that prints the ownership (NTFS Owner / POSIX uid+gid) of key paths: the workspace, its `.git`, gitdb and smmap submodule worktrees and gitfiles, and the runner `HOME` and `~/.gitconfig`. Also print the full list of `safe.directory` entries at that point. GitPython's tests are intentionally coupled to the layout and state of the GitPython repository they run against. The ownership and trust config that gate whether git will operate on a path are part of that state. Surfacing them in every test job gives diagnostics to read alongside any failure that turns out to be a CI setup problem. The step is added to: - `cygwin-test.yml`'s `test` job, with a YAML anchor (`&ownership-display`) so the temporary `reproduce-safe-dir` matrix job can reuse it via `*ownership-display`. - `pythonpackage.yml`'s `test` job (Linux / macOS / native Windows). - `alpine-test.yml`'s `test` job. It runs after `init-tests-after-clone.sh` has populated the submodules and after `safe.directory` has been configured (in workflows that configure it), so the values reported are what the tests will see. Co-authored-by: Claude Opus 4.7 (1M context) --- .github/workflows/alpine-test.yml | 20 ++++++++++ .github/workflows/cygwin-test.yml | 60 +++++++++++++++++++++++++++++ .github/workflows/pythonpackage.yml | 20 ++++++++++ 3 files changed, 100 insertions(+) diff --git a/.github/workflows/alpine-test.yml b/.github/workflows/alpine-test.yml index b7de7482e..bb297efab 100644 --- a/.github/workflows/alpine-test.yml +++ b/.github/workflows/alpine-test.yml @@ -61,6 +61,26 @@ jobs: . .venv/bin/activate pip install '.[test]' + - name: Show file ownership and safe.directory entries + run: | + echo "==================== File ownership ====================" + for p in \ + "$(pwd)" \ + "$(pwd)/.git" \ + "$(pwd)/git/ext/gitdb" \ + "$(pwd)/git/ext/gitdb/.git" \ + "$(pwd)/git/ext/gitdb/gitdb/ext/smmap" \ + "$(pwd)/git/ext/gitdb/gitdb/ext/smmap/.git" \ + "${HOME:-}" \ + "${HOME:-}/.gitconfig"; do + if [ -n "$p" ]; then + ls -ld -- "$p" 2>/dev/null || echo "(missing: $p)" + fi + done + echo + echo "==================== safe.directory entries ====================" + git config --global --get-all safe.directory 2>&1 || echo "(none)" + - name: Show version and platform information run: | . .venv/bin/activate diff --git a/.github/workflows/cygwin-test.yml b/.github/workflows/cygwin-test.yml index 9cc90804e..6af9d098d 100644 --- a/.github/workflows/cygwin-test.yml +++ b/.github/workflows/cygwin-test.yml @@ -90,6 +90,65 @@ jobs: run: | pip install '.[test]' + - &ownership-display + name: Show file ownership and safe.directory entries + run: | + echo "==================== File ownership (Cygwin view, ls -ld) ====================" + for p in \ + "$(pwd)" \ + "$(pwd)/.git" \ + "$(pwd)/git/ext/gitdb" \ + "$(pwd)/git/ext/gitdb/.git" \ + "$(pwd)/.git/modules/gitdb" \ + "$(pwd)/git/ext/gitdb/gitdb/ext/smmap" \ + "$(pwd)/git/ext/gitdb/gitdb/ext/smmap/.git" \ + "$(pwd)/.git/modules/gitdb/modules/smmap" \ + "${HOME:-}" \ + "${HOME:-}/.gitconfig"; do + if [ -n "$p" ]; then + ls -ld -- "$p" 2>/dev/null || echo "(missing: $p)" + fi + done + echo + echo "==================== File ownership (Win32 view, Get-Acl) ====================" + # Cygwin's ls -ld and stat go through Cygwin's SID-to-uid mapping + # (well-known SIDs by their RID, machine-local accounts by 0x30000+RID). + # The mapping is deterministic, but going via Win32 Get-Acl gives the + # NTAccount form of the NTFS Owner SID directly, with no Cygwin layer + # in between -- useful for confirming that what Cygwin reports as + # "Administrators" really is the BUILTIN\Administrators SID (S-1-5-32-544) + # rather than some local-machine account that Cygwin happens to map to + # the same uid. The workflow sets CYGWIN_NOWINPATH=1, so Windows paths + # are not on Cygwin's $PATH; invoke powershell.exe by absolute path. + ps_exe=/cygdrive/c/Windows/System32/WindowsPowerShell/v1.0/powershell.exe + if [ -x "$ps_exe" ]; then + for p in \ + "$(pwd)" \ + "$(pwd)/.git" \ + "$(pwd)/git/ext/gitdb" \ + "$(pwd)/git/ext/gitdb/.git" \ + "$(pwd)/.git/modules/gitdb" \ + "$(pwd)/git/ext/gitdb/gitdb/ext/smmap" \ + "$(pwd)/git/ext/gitdb/gitdb/ext/smmap/.git" \ + "$(pwd)/.git/modules/gitdb/modules/smmap" \ + "${HOME:-}/.gitconfig"; do + if [ -n "$p" ] && [ -e "$p" ]; then + wp=$(cygpath -w "$p") + # Escape single-quotes for PowerShell single-quoted string literal: ' -> '' + wp_escaped=${wp//\'/\'\'} + owner=$("$ps_exe" -NoProfile -NonInteractive -Command \ + "try { (Get-Acl -LiteralPath '${wp_escaped}').Owner } catch { 'ERROR: ' + \$_.Exception.Message }" \ + 2>/dev/null | tr -d '\r') + printf " %-44s %s\n" "$owner" "$wp" + fi + done + else + echo "($ps_exe not found -- skipping Win32 view)" + fi + echo + echo "==================== safe.directory entries ====================" + git config --global --get-all safe.directory 2>&1 || echo "(none)" + - name: Show version and platform information run: | uname -a @@ -192,6 +251,7 @@ jobs: - *setup-venv - *update-pypa - *install-deps + - *ownership-display - name: Run submodule tests run: | diff --git a/.github/workflows/pythonpackage.yml b/.github/workflows/pythonpackage.yml index 874e18a8f..3d0dfede0 100644 --- a/.github/workflows/pythonpackage.yml +++ b/.github/workflows/pythonpackage.yml @@ -87,6 +87,26 @@ jobs: run: | pip install '.[test]' + - name: Show file ownership and safe.directory entries + run: | + echo "==================== File ownership ====================" + for p in \ + "$(pwd)" \ + "$(pwd)/.git" \ + "$(pwd)/git/ext/gitdb" \ + "$(pwd)/git/ext/gitdb/.git" \ + "$(pwd)/git/ext/gitdb/gitdb/ext/smmap" \ + "$(pwd)/git/ext/gitdb/gitdb/ext/smmap/.git" \ + "${HOME:-}" \ + "${HOME:-}/.gitconfig"; do + if [ -n "$p" ]; then + ls -ld -- "$p" 2>/dev/null || echo "(missing: $p)" + fi + done + echo + echo "==================== safe.directory entries ====================" + git config --global --get-all safe.directory 2>&1 || echo "(none)" + - name: Show version and platform information run: | uname -a From 14cdc52fe97bbee17974ddb687b87b3c09af608c Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sun, 17 May 2026 17:25:51 -0400 Subject: [PATCH 32/45] Restructure CI diagnostic steps Cleanups on the ownership-display step from the previous commit: - Split into per-shell steps (bash POSIX `ls -ld`, pwsh NTFS `Get-Acl`, bash safe.directory `git config`), removing the bash-driven PowerShell subprocess with `cygpath -w` and quote-escaping. Use `pwsh`, not `powershell.exe`. Gate pythonpackage's two views by `matrix.os-type` (Git Bash's `ls -ld` on Windows reports a uniform `runneradmin 197121` for every path, ignoring NTFS Owner -- MSYS2's SID-to-uid mapping doesn't have Cygwin's fidelity). - Trim the decorative `$HOME` entry from POSIX path lists: it isn't part of any git trust decision -- only `~/.gitconfig` is. Use `${HOME:?HOME is not set}/.gitconfig` for the remaining entry so an unset HOME fails loudly. - Move the pwsh path list into a `$paths = @(...)` variable. Unix shells keep the inline `for p in WORD WORD ...` form: alpine's `sh` (busybox ash) doesn't support arrays, and the others shouldn't differ from it unnecessarily. - Drop `2>&1` from the safe.directory step. Drop the `|| echo "(none)"` fallback on cygwin (entries are explicitly configured; bare command fails on regression). Keep it on pythonpackage and alpine, where `actions/checkout`'s safe.directory add lives under a throwaway HOME override and doesn't persist, so there legitimately are no entries to display. Co-authored-by: Claude Opus 4.7 (1M context) --- .github/workflows/alpine-test.yml | 35 +++++---- .github/workflows/cygwin-test.yml | 112 ++++++++++++++-------------- .github/workflows/pythonpackage.yml | 70 +++++++++++++---- 3 files changed, 129 insertions(+), 88 deletions(-) diff --git a/.github/workflows/alpine-test.yml b/.github/workflows/alpine-test.yml index bb297efab..4183f0e0d 100644 --- a/.github/workflows/alpine-test.yml +++ b/.github/workflows/alpine-test.yml @@ -61,25 +61,28 @@ jobs: . .venv/bin/activate pip install '.[test]' - - name: Show file ownership and safe.directory entries + - name: Show POSIX file ownership run: | - echo "==================== File ownership ====================" for p in \ - "$(pwd)" \ - "$(pwd)/.git" \ - "$(pwd)/git/ext/gitdb" \ - "$(pwd)/git/ext/gitdb/.git" \ - "$(pwd)/git/ext/gitdb/gitdb/ext/smmap" \ - "$(pwd)/git/ext/gitdb/gitdb/ext/smmap/.git" \ - "${HOME:-}" \ - "${HOME:-}/.gitconfig"; do - if [ -n "$p" ]; then - ls -ld -- "$p" 2>/dev/null || echo "(missing: $p)" - fi + "$(pwd)" \ + "$(pwd)/.git" \ + "$(pwd)/git/ext/gitdb" \ + "$(pwd)/git/ext/gitdb/.git" \ + "$(pwd)/git/ext/gitdb/gitdb/ext/smmap" \ + "$(pwd)/git/ext/gitdb/gitdb/ext/smmap/.git" \ + "${HOME:?HOME is not set}/.gitconfig" + do + ls -ld -- "$p" 2>/dev/null || echo "(missing: $p)" done - echo - echo "==================== safe.directory entries ====================" - git config --global --get-all safe.directory 2>&1 || echo "(none)" + + - name: Show safe.directory entries + # `actions/checkout`'s safe.directory add is only durable for the + # checkout itself (it writes under a throwaway HOME override and + # then discards it), so by the time this step runs the runner + # user's `~/.gitconfig` has no entries -- and the Alpine container + # chowns the workspace to runner:docker to match the test user, so + # git accepts the ownership without one. Expected: `(none)`. + run: git config --global --get-all safe.directory || echo "(none)" - name: Show version and platform information run: | diff --git a/.github/workflows/cygwin-test.yml b/.github/workflows/cygwin-test.yml index 6af9d098d..16b88851e 100644 --- a/.github/workflows/cygwin-test.yml +++ b/.github/workflows/cygwin-test.yml @@ -90,64 +90,62 @@ jobs: run: | pip install '.[test]' - - &ownership-display - name: Show file ownership and safe.directory entries + - &ownership-posix-display + name: Show POSIX file ownership + # Cygwin's `ls -ld` reports the NTFS Owner SID via Cygwin's SID-to-uid + # mapping (well-known SIDs by their RID, machine-local accounts by + # 0x30000+RID). That mapping is what Cygwin git's + # `is_path_owned_by_current_user` reduces to, so this is the view that + # determines whether `safe.directory` is consulted. run: | - echo "==================== File ownership (Cygwin view, ls -ld) ====================" for p in \ - "$(pwd)" \ - "$(pwd)/.git" \ - "$(pwd)/git/ext/gitdb" \ - "$(pwd)/git/ext/gitdb/.git" \ - "$(pwd)/.git/modules/gitdb" \ - "$(pwd)/git/ext/gitdb/gitdb/ext/smmap" \ - "$(pwd)/git/ext/gitdb/gitdb/ext/smmap/.git" \ - "$(pwd)/.git/modules/gitdb/modules/smmap" \ - "${HOME:-}" \ - "${HOME:-}/.gitconfig"; do - if [ -n "$p" ]; then - ls -ld -- "$p" 2>/dev/null || echo "(missing: $p)" - fi + "$(pwd)" \ + "$(pwd)/.git" \ + "$(pwd)/git/ext/gitdb" \ + "$(pwd)/git/ext/gitdb/.git" \ + "$(pwd)/.git/modules/gitdb" \ + "$(pwd)/git/ext/gitdb/gitdb/ext/smmap" \ + "$(pwd)/git/ext/gitdb/gitdb/ext/smmap/.git" \ + "$(pwd)/.git/modules/gitdb/modules/smmap" \ + "${HOME:?HOME is not set}/.gitconfig" + do + ls -ld -- "$p" 2>/dev/null || echo "(missing: $p)" done - echo - echo "==================== File ownership (Win32 view, Get-Acl) ====================" - # Cygwin's ls -ld and stat go through Cygwin's SID-to-uid mapping - # (well-known SIDs by their RID, machine-local accounts by 0x30000+RID). - # The mapping is deterministic, but going via Win32 Get-Acl gives the - # NTAccount form of the NTFS Owner SID directly, with no Cygwin layer - # in between -- useful for confirming that what Cygwin reports as - # "Administrators" really is the BUILTIN\Administrators SID (S-1-5-32-544) - # rather than some local-machine account that Cygwin happens to map to - # the same uid. The workflow sets CYGWIN_NOWINPATH=1, so Windows paths - # are not on Cygwin's $PATH; invoke powershell.exe by absolute path. - ps_exe=/cygdrive/c/Windows/System32/WindowsPowerShell/v1.0/powershell.exe - if [ -x "$ps_exe" ]; then - for p in \ - "$(pwd)" \ - "$(pwd)/.git" \ - "$(pwd)/git/ext/gitdb" \ - "$(pwd)/git/ext/gitdb/.git" \ - "$(pwd)/.git/modules/gitdb" \ - "$(pwd)/git/ext/gitdb/gitdb/ext/smmap" \ - "$(pwd)/git/ext/gitdb/gitdb/ext/smmap/.git" \ - "$(pwd)/.git/modules/gitdb/modules/smmap" \ - "${HOME:-}/.gitconfig"; do - if [ -n "$p" ] && [ -e "$p" ]; then - wp=$(cygpath -w "$p") - # Escape single-quotes for PowerShell single-quoted string literal: ' -> '' - wp_escaped=${wp//\'/\'\'} - owner=$("$ps_exe" -NoProfile -NonInteractive -Command \ - "try { (Get-Acl -LiteralPath '${wp_escaped}').Owner } catch { 'ERROR: ' + \$_.Exception.Message }" \ - 2>/dev/null | tr -d '\r') - printf " %-44s %s\n" "$owner" "$wp" - fi - done - else - echo "($ps_exe not found -- skipping Win32 view)" - fi - echo - echo "==================== safe.directory entries ====================" - git config --global --get-all safe.directory 2>&1 || echo "(none)" + + - &ownership-ntfs-display + name: Show NTFS file ownership + # Authoritative NTFS Owner via Get-Acl, with no Cygwin SID-to-uid layer + # in between -- useful for confirming what the Cygwin view reports as + # "Administrators" is the BUILTIN\Administrators SID (S-1-5-32-544). + shell: pwsh + run: | + $paths = @( + "$pwd", + "$pwd\.git", + "$pwd\git\ext\gitdb", + "$pwd\git\ext\gitdb\.git", + "$pwd\.git\modules\gitdb", + "$pwd\git\ext\gitdb\gitdb\ext\smmap", + "$pwd\git\ext\gitdb\gitdb\ext\smmap\.git", + "$pwd\.git\modules\gitdb\modules\smmap", + "$env:USERPROFILE\.gitconfig" + ) + foreach ($p in $paths) { + if (Test-Path -LiteralPath $p) { + try { + $owner = (Get-Acl -LiteralPath $p).Owner + } catch { + $owner = "ERROR: $($_.Exception.Message)" + } + "{0,-44} {1}" -f $owner, $p + } else { + "(missing: $p)" + } + } + + - &safe-directory-display + name: Show safe.directory entries + run: git config --global --get-all safe.directory - name: Show version and platform information run: | @@ -251,7 +249,9 @@ jobs: - *setup-venv - *update-pypa - *install-deps - - *ownership-display + - *ownership-posix-display + - *ownership-ntfs-display + - *safe-directory-display - name: Run submodule tests run: | diff --git a/.github/workflows/pythonpackage.yml b/.github/workflows/pythonpackage.yml index 3d0dfede0..cffafc59a 100644 --- a/.github/workflows/pythonpackage.yml +++ b/.github/workflows/pythonpackage.yml @@ -87,25 +87,63 @@ jobs: run: | pip install '.[test]' - - name: Show file ownership and safe.directory entries + - name: Show POSIX file ownership + # Linux and macOS only. On Windows, Git Bash's `ls -ld` reports a + # uniform uid+gid for every path regardless of NTFS Owner (MSYS2's + # SID-to-uid mapping doesn't have Cygwin's fidelity), so it would + # not be informative here. The NTFS Owner check below covers Windows. + if: matrix.os-type != 'windows' run: | - echo "==================== File ownership ====================" for p in \ - "$(pwd)" \ - "$(pwd)/.git" \ - "$(pwd)/git/ext/gitdb" \ - "$(pwd)/git/ext/gitdb/.git" \ - "$(pwd)/git/ext/gitdb/gitdb/ext/smmap" \ - "$(pwd)/git/ext/gitdb/gitdb/ext/smmap/.git" \ - "${HOME:-}" \ - "${HOME:-}/.gitconfig"; do - if [ -n "$p" ]; then - ls -ld -- "$p" 2>/dev/null || echo "(missing: $p)" - fi + "$(pwd)" \ + "$(pwd)/.git" \ + "$(pwd)/git/ext/gitdb" \ + "$(pwd)/git/ext/gitdb/.git" \ + "$(pwd)/git/ext/gitdb/gitdb/ext/smmap" \ + "$(pwd)/git/ext/gitdb/gitdb/ext/smmap/.git" \ + "${HOME:?HOME is not set}/.gitconfig" + do + ls -ld -- "$p" 2>/dev/null || echo "(missing: $p)" done - echo - echo "==================== safe.directory entries ====================" - git config --global --get-all safe.directory 2>&1 || echo "(none)" + + - name: Show NTFS file ownership + # Windows only. Reads NTFS Owner directly via Get-Acl, which is the + # authoritative view for Windows-side ownership questions; the POSIX + # view via Git Bash's MSYS2 layer is not a reliable proxy here. + if: matrix.os-type == 'windows' + shell: pwsh + run: | + $paths = @( + "$pwd", + "$pwd\.git", + "$pwd\git\ext\gitdb", + "$pwd\git\ext\gitdb\.git", + "$pwd\git\ext\gitdb\gitdb\ext\smmap", + "$pwd\git\ext\gitdb\gitdb\ext\smmap\.git", + "$env:USERPROFILE\.gitconfig" + ) + foreach ($p in $paths) { + if (Test-Path -LiteralPath $p) { + try { + $owner = (Get-Acl -LiteralPath $p).Owner + } catch { + $owner = "ERROR: $($_.Exception.Message)" + } + "{0,-44} {1}" -f $owner, $p + } else { + "(missing: $p)" + } + } + + - name: Show safe.directory entries + # `actions/checkout`'s safe.directory add is only durable for the + # checkout itself (it writes under a throwaway HOME override and + # then discards it), so by the time this step runs the runner + # user's `~/.gitconfig` has no entries -- and git accepts the + # workspace's ownership anyway: Git for Windows via its + # Admins-group exemption on the windows matrix; on Linux/macOS + # the workspace is owned by the test user. Expected: `(none)`. + run: git config --global --get-all safe.directory || echo "(none)" - name: Show version and platform information run: | From 8ef4c21859d61683ac93f679790c23ef1cf4b17d Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sun, 10 May 2026 14:35:14 -0400 Subject: [PATCH 33/45] Add `submodules: recursive` temporarily, for testing Add `submodules: recursive` to the `actions/checkout` step in every workflow that runs the test suite (`cygwin-test.yml`, `pythonpackage.yml`, `alpine-test.yml`). This is *temporary* and will be reverted after the bugfix, with the explicit intent of demonstrating that the bugfix works regardless of which mechanism populates the submodules. The standing decision is to NOT use `submodules: recursive` in CI. `init-tests-after-clone.sh` is the documented setup mechanism that downstream packagers (Arch Linux and others) rely on, and keeping it as the sole submodule source on upstream CI is meant to catch regressions like #1713 before they reach distros. See #1715 for the full rationale. The CI run on this commit is expected to show: - Cygwin (`test-cygwin`): the `safe.directory` bug still triggers, with the same `ValueError`/`IndexError`/`AssertionError` pattern as the previous commits. The bug is independent of which process clones the submodules; the gitdb worktree directory itself is created Admin-owned by the outer `git clone`'s checkout phase before any submodule init runs, regardless of which mechanism populates the submodule contents afterward. - Linux / macOS / native Windows (`Python package`) and Alpine Linux (`test-alpine`): tests pass as before. These platforms are unaffected by the bug. Each workflow's checkout step carries an inline comment pointing at #1715 so the temporary nature of the change is legible at a glance. Co-authored-by: Claude Opus 4.7 (1M context) --- .github/workflows/alpine-test.yml | 3 +++ .github/workflows/cygwin-test.yml | 3 +++ .github/workflows/pythonpackage.yml | 3 +++ 3 files changed, 9 insertions(+) diff --git a/.github/workflows/alpine-test.yml b/.github/workflows/alpine-test.yml index 4183f0e0d..259a6d9f2 100644 --- a/.github/workflows/alpine-test.yml +++ b/.github/workflows/alpine-test.yml @@ -29,6 +29,9 @@ jobs: - uses: actions/checkout@v6 with: fetch-depth: 0 + # Temporary, for testing. The standing decision is to NOT pre-clone + # submodules on CI; see https://github.com/gitpython-developers/GitPython/pull/1715 + submodules: recursive - name: Set workspace ownership run: | diff --git a/.github/workflows/cygwin-test.yml b/.github/workflows/cygwin-test.yml index 16b88851e..f5e1309e9 100644 --- a/.github/workflows/cygwin-test.yml +++ b/.github/workflows/cygwin-test.yml @@ -39,6 +39,9 @@ jobs: uses: actions/checkout@v6 with: fetch-depth: 0 + # Temporary, for testing. The standing decision is to NOT pre-clone + # submodules on CI; see https://github.com/gitpython-developers/GitPython/pull/1715 + submodules: recursive - &install-cygwin name: Install Cygwin diff --git a/.github/workflows/pythonpackage.yml b/.github/workflows/pythonpackage.yml index cffafc59a..919a32721 100644 --- a/.github/workflows/pythonpackage.yml +++ b/.github/workflows/pythonpackage.yml @@ -53,6 +53,9 @@ jobs: - uses: actions/checkout@v6 with: fetch-depth: 0 + # Temporary, for testing. The standing decision is to NOT pre-clone + # submodules on CI; see https://github.com/gitpython-developers/GitPython/pull/1715 + submodules: recursive - name: Set up Python ${{ matrix.python-version }} uses: actions/setup-python@v6 From b409946ca7e7c36782b47b54dd5b1f70280e8eab Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Wed, 6 May 2026 23:06:57 -0400 Subject: [PATCH 34/45] Cover submodule working trees in safe.directory on Cygwin Add the gitdb and smmap submodule working tree paths to the `safe.directory` config in the Cygwin CI workflow. Without this, when GitPython opens a submodule as a `Repo` and runs `git cat-file --batch-check` against it, git rejects the repository for dubious ownership. The user-visible failure modes (`ValueError`, `IndexError`, `AssertionError`) all trace back to this rejection. Why `[gitdb]` failed and `[smmap]` passed ----------------------------------------- The trust check in `test_fixture_health.py` failed for `[gitdb]` but passed for `[smmap]`, even though neither submodule's working tree was in the workflow's `safe.directory` list before this commit. The asymmetry comes down to which Owner SID NTFS records on each path, and which paths git's ownership check requires to be owned. There are six paths git's check might consider for the two submodules: gitdb's `.git` gitfile, worktree `git/ext/gitdb`, and gitdir `/.git/modules/gitdb`; smmap's `.git` gitfile, worktree `git/ext/gitdb/gitdb/ext/smmap`, and gitdir `/.git/modules/gitdb/modules/smmap`. Of these, only one had NTFS Owner = `BUILTIN\Administrators` (Cygwin uid 544): gitdb's worktree. The other five had Owner = `runneradmin`, whose Cygwin uid (197108) was the value `geteuid()` returned in the test process. The same Owner pattern held both with and without `submodules: recursive` in `actions/checkout`. The single `Administrators`-owned path was gitdb's worktree. All other paths were `runneradmin`-owned, including the ones that Git for Windows's recursive submodule clone had just produced when `submodules: recursive` was set. The differentiator is not which git binary clones the submodules, but that `git/ext/gitdb` is created by the outer `git clone`'s checkout phase. When `git checkout` materializes a tree entry of mode 160000, it calls `mkdir(path, 0777)` to create an empty submodule directory (see `entry.c::write_entry`, case `S_IFGITLINK` [1] [2]). On Windows GHA runners, jobs run as `runneradmin`. This is the built-in local Administrator account (its Cygwin uid 197108 = 196608 + 500 matches Cygwin's mapping [3] for machine-local accounts: 0x30000 plus the SID suffix 500, the well-known suffix of that account's SID). That account is exempt from UAC token filtering by default (Admin Approval Mode for the built-in Administrator account is disabled [4]), so its processes hold the full administrative access token. `CreateProcessW` propagates the parent's primary token unchanged through `actions/checkout`'s process tree. Inside that tree, the outer `git clone`'s `mkdir(path, 0777)` produces directories whose NTFS Owner is `BUILTIN\Administrators` -- as observed on every workspace directory the outer clone materialized, including the `git/ext/gitdb` placeholder. Subsequent submodule-update operations -- both Git for Windows if `actions/checkout` does a recursive clone, and Cygwin git if it happens later due to `init-tests-after-clone.sh` -- produce paths that NTFS records as `runneradmin`-owned. Both flows go through a process whose primary token's `TokenOwner` field has been rewritten from `BUILTIN\Administrators` to the user SID by a Cygwin or Cygwin-derived runtime at DLL initialization. The rewrite propagates to every descendant via `CreateProcessW`'s primary-token inheritance [5], so every `mkdir` issued after that point produces a directory owned by the user. - Cygwin git triggers the rewrite directly. `cygheap_user::init` in `cygwin1.dll` calls `NtSetInformationToken(hProcToken, TokenOwner, &effec_cygsid, ...)` at process startup [6]. - Git for Windows triggers it indirectly. `git submodule` is not a builtin (only `submodule--helper` is) [7]. So it falls through to `execv_dashed_external` and runs `git-submodule.sh`, a shell script whose shebang is resolved at runtime to `sh.exe` in the Git for Windows "Git Bash" environment. That `sh.exe` is an MSYS2 binary linked against `msys-2.0.dll`, a Cygwin fork that performs the same `TokenOwner` rewrite. From there, every `git.exe` the script spawns inherits the user-SID `TokenOwner` and produces user-owned directories. Cygwin's `lstat().st_uid` reports the actual NTFS Owner SID mapped through Cygwin's SID-to-uid table. `is_path_owned_by_current_user` reduces to `lstat(p).st_uid == geteuid()` on Cygwin (no Administrators group exemption). `ensure_valid_ownership` returns 1 (accepting the repository) without consulting `safe.directory` when the gitfile, worktree, and gitdir ALL pass that owned-by-current-user check. Otherwise it falls through to comparing the worktree's `real_pathdup` against each configured `safe.directory` entry. For gitdb the three Owners were `runneradmin` (gitfile), **`Administrators` (worktree)**, and `runneradmin` (gitdir), so the all-paths-owned check failed on the worktree. The workflow's `safe.directory` before this commit contained only `$(pwd)` and `$(pwd)/.git`, neither of which exact-matches `git/ext/gitdb`, so the `safe.directory` comparison also failed, and `ensure_valid_ownership` returned 0 -- git rejected the repository. For smmap the three Owners were all `runneradmin`, so the all-paths-owned check passed. Cygwin's `chown` cannot rewrite the gitdb worktree's Owner SID from `Administrators` to `runneradmin`: it returns "Permission denied". Adding both submodule worktree paths to `safe.directory` is the correct fix and is robust against shifts in what paths inherit which Owner. Why `actions/checkout`'s own `safe.directory` does not help ----------------------------------------------------------- `actions/checkout`'s `set-safe-directory: true` default writes the main repository path to `safe.directory` in a temporary config it points its own spawned git child at by overriding `HOME` for that child process. That `HOME` override applies only to git invocations the action itself spawns; subsequent steps' processes inherit the runner user's real `HOME` (e.g., `C:\Users\runneradmin` on the Windows runner) and read its actual `~/.gitconfig`, which never received the entry. So no git in a later step, whether Git for Windows or Cygwin git, sees it. That's why the `cygwin-test` workflow sets Cygwin git's `safe.directory` itself. This commit extends that to cover the gitdb and smmap working trees. The distinction between Cygwin git and Git for Windows is also why the bug affected the Cygwin jobs and no other Windows jobs. `compat/mingw.c` defines `is_path_owned_by_current_sid` [8], which accepts `BUILTIN\Administrators`-owned paths when the current user is a member of `Administrators`. Cygwin git compiles against the POSIX path (`is_path_owned_by_current_uid` in `git-compat-util.h` [9]) without that leniency. So the same `Administrators`-owned `git/ext/gitdb` that Cygwin git rejects is silently accepted by Git for Windows, and the main CI workflow's `windows-latest` jobs never trip the trust check. Verification ------------ The `reproduce-safe-dir` matrix on the previous commits produces failures for all three affected tests; this commit's CI run shows those tests passing instead. The Owner-SID claim above is verified by the `diag-token` job introduced for this purpose. That job creates a directory through four code paths (PowerShell-only, Cygwin-only, Cygwin-bash spawning Win32 `git init`, and a PowerShell -> Cygwin-bash -> PowerShell sandwich) and reports the NTFS Owner of each. The observed Owners match the predicted values in every case, including the load-bearing Cygwin -> Win32 propagation case (test C) and the sandwich case (test D) showing that the determinant is whether some process in the ancestry has loaded a Cygwin-family runtime, not the identity of the file-creating binary. The commit immediately preceding this one temporarily sets `submodules: recursive` on `actions/checkout` in every workflow that runs the test suite. Its CI run shows the bug still triggering on Cygwin (the gitdb worktree directory itself is created by the outer `git clone`'s checkout phase, before any submodule init runs, regardless of which mechanism subsequently populates the submodule contents). A subsequent commit will revert that change; its CI run shows this fix continues to hold without `submodules: recursive`, confirming the fix is independent of submodule source. [1]: https://github.com/git/git/blob/v2.51.0/entry.c#L397 [2]: https://github.com/git-for-windows/git/blob/v2.54.0.windows.1/entry.c#L397 [3]: https://cygwin.com/cygwin-ug-net/ntsec.html [4]: https://learn.microsoft.com/en-us/windows/security/application-security/application-control/user-account-control/settings-and-configuration [5]: https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessw [6]: https://sourceware.org/cgit/newlib-cygwin/tree/winsup/cygwin/uinfo.cc?h=cygwin-3.6.9#n82 [7]: https://github.com/git-for-windows/git/blob/v2.54.0.windows.1/git.c#L661 [8]: https://github.com/git-for-windows/git/blob/v2.54.0.windows.1/compat/mingw.c#L3931 [9]: https://github.com/git/git/blob/v2.51.0/git-compat-util.h#L346 Co-authored-by: Claude Opus 4.7 (1M context) --- .github/workflows/cygwin-test.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/cygwin-test.yml b/.github/workflows/cygwin-test.yml index f5e1309e9..32b353d2e 100644 --- a/.github/workflows/cygwin-test.yml +++ b/.github/workflows/cygwin-test.yml @@ -61,6 +61,8 @@ jobs: run: | git config --global --add safe.directory "$(pwd)" git config --global --add safe.directory "$(pwd)/.git" + git config --global --add safe.directory "$(pwd)/git/ext/gitdb" + git config --global --add safe.directory "$(pwd)/git/ext/gitdb/gitdb/ext/smmap" git config --global core.autocrlf false - &prepare-repo From 650eaafebd44cbe879d60eb8389eacc5db323c63 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sun, 10 May 2026 14:35:57 -0400 Subject: [PATCH 35/45] Remove `submodules: recursive` (testing complete) Revert the temporary addition of `submodules: recursive` to `actions/checkout` in `cygwin-test.yml`, `pythonpackage.yml`, and `alpine-test.yml`. The `safe.directory` fix has been verified to work regardless of which mechanism populates the submodules. Returning the workflows to their pre-test posture restores the standing arrangement: `init-tests-after-clone.sh` as the sole submodule source on upstream CI. See #1715 for the rationale. Co-authored-by: Claude Opus 4.7 (1M context) --- .github/workflows/alpine-test.yml | 3 --- .github/workflows/cygwin-test.yml | 3 --- .github/workflows/pythonpackage.yml | 3 --- 3 files changed, 9 deletions(-) diff --git a/.github/workflows/alpine-test.yml b/.github/workflows/alpine-test.yml index 259a6d9f2..4183f0e0d 100644 --- a/.github/workflows/alpine-test.yml +++ b/.github/workflows/alpine-test.yml @@ -29,9 +29,6 @@ jobs: - uses: actions/checkout@v6 with: fetch-depth: 0 - # Temporary, for testing. The standing decision is to NOT pre-clone - # submodules on CI; see https://github.com/gitpython-developers/GitPython/pull/1715 - submodules: recursive - name: Set workspace ownership run: | diff --git a/.github/workflows/cygwin-test.yml b/.github/workflows/cygwin-test.yml index 32b353d2e..caed9814b 100644 --- a/.github/workflows/cygwin-test.yml +++ b/.github/workflows/cygwin-test.yml @@ -39,9 +39,6 @@ jobs: uses: actions/checkout@v6 with: fetch-depth: 0 - # Temporary, for testing. The standing decision is to NOT pre-clone - # submodules on CI; see https://github.com/gitpython-developers/GitPython/pull/1715 - submodules: recursive - &install-cygwin name: Install Cygwin diff --git a/.github/workflows/pythonpackage.yml b/.github/workflows/pythonpackage.yml index 919a32721..cffafc59a 100644 --- a/.github/workflows/pythonpackage.yml +++ b/.github/workflows/pythonpackage.yml @@ -53,9 +53,6 @@ jobs: - uses: actions/checkout@v6 with: fetch-depth: 0 - # Temporary, for testing. The standing decision is to NOT pre-clone - # submodules on CI; see https://github.com/gitpython-developers/GitPython/pull/1715 - submodules: recursive - name: Set up Python ${{ matrix.python-version }} uses: actions/setup-python@v6 From de3a950d57c8057fdd36ead97a390a48180892ee Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sun, 10 May 2026 14:36:45 -0400 Subject: [PATCH 36/45] Remove temporary test jobs (testing complete) Remove three things from the `cygwin-test` workflow that were added to demonstrate the `safe.directory` bug and its fix: - The `reproduce-safe-dir` matrix (256 jobs running three submodule tests). Added to give a high-confidence reproduction of the failure pattern across runner-instance variation. - The `diag-token` job. Added to empirically establish the TokenOwner rewrite mechanism behind the gitdb-worktree Owner asymmetry. - The YAML anchors that only those temporary jobs needed (`&force-lf`, `&checkout`, `&install-cygwin`, `&verbose-output`, `&safe-directory`, `&prepare-repo`, `&git-identity`, `&setup-venv`, `&update-pypa`, `&install-deps`, `&ownership-posix-display`, `&ownership-ntfs-display`, `&safe-directory-display`, `&cygwin-env`, `&cygwin-defaults`). The `test` job still has all those steps; it just no longer needs to anchor them for reuse. What stays: the `test` job (the actual Cygwin test suite), the fixture-health and required-submodule checks, and the always-on file-ownership / `safe.directory` diagnostic steps (kept across all test workflows). Co-authored-by: Claude Opus 4.7 (1M context) --- .github/workflows/cygwin-test.yml | 144 ++++-------------------------- 1 file changed, 15 insertions(+), 129 deletions(-) diff --git a/.github/workflows/cygwin-test.yml b/.github/workflows/cygwin-test.yml index caed9814b..c12ccb3cf 100644 --- a/.github/workflows/cygwin-test.yml +++ b/.github/workflows/cygwin-test.yml @@ -20,41 +20,36 @@ jobs: runs-on: windows-latest - env: &cygwin-env + env: CHERE_INVOKING: "1" CYGWIN_NOWINPATH: "1" - defaults: &cygwin-defaults + defaults: run: shell: C:\cygwin\bin\bash.exe --login --norc -eo pipefail -o igncr "{0}" steps: - - &force-lf - name: Force LF line endings + - name: Force LF line endings run: | git config --global core.autocrlf false # Affects the non-Cygwin git. shell: pwsh # Do this outside Cygwin, to affect actions/checkout. - - &checkout - uses: actions/checkout@v6 + - uses: actions/checkout@v6 with: fetch-depth: 0 - - &install-cygwin - name: Install Cygwin + - name: Install Cygwin uses: cygwin/cygwin-install-action@v6 with: packages: git python39 python-pip-wheel python-setuptools-wheel python-wheel-wheel add-to-path: false # No need to change $PATH outside the Cygwin environment. - - &verbose-output - name: Arrange for verbose output + - name: Arrange for verbose output run: | # Arrange for verbose output but without shell environment setup details. echo 'set -x' >~/.bash_profile - - &safe-directory - name: Special configuration for Cygwin git + - name: Special configuration for Cygwin git run: | git config --global --add safe.directory "$(pwd)" git config --global --add safe.directory "$(pwd)/.git" @@ -62,13 +57,11 @@ jobs: git config --global --add safe.directory "$(pwd)/git/ext/gitdb/gitdb/ext/smmap" git config --global core.autocrlf false - - &prepare-repo - name: Prepare this repo for tests + - name: Prepare this repo for tests run: | ./init-tests-after-clone.sh - - &git-identity - name: Set git user identity and command aliases for the tests + - name: Set git user identity and command aliases for the tests run: | git config --global user.email "travis@ci.com" git config --global user.name "Travis Runner" @@ -76,24 +69,20 @@ jobs: # and cause subsequent tests to fail cat test/fixtures/.gitconfig >> ~/.gitconfig - - &setup-venv - name: Set up virtual environment + - name: Set up virtual environment run: | python3.9 -m venv .venv echo 'BASH_ENV=.venv/bin/activate' >>"$GITHUB_ENV" - - &update-pypa - name: Update PyPA packages + - name: Update PyPA packages run: | python -m pip install -U pip 'setuptools; python_version<"3.12"' wheel - - &install-deps - name: Install project and test dependencies + - name: Install project and test dependencies run: | pip install '.[test]' - - &ownership-posix-display - name: Show POSIX file ownership + - name: Show POSIX file ownership # Cygwin's `ls -ld` reports the NTFS Owner SID via Cygwin's SID-to-uid # mapping (well-known SIDs by their RID, machine-local accounts by # 0x30000+RID). That mapping is what Cygwin git's @@ -114,8 +103,7 @@ jobs: ls -ld -- "$p" 2>/dev/null || echo "(missing: $p)" done - - &ownership-ntfs-display - name: Show NTFS file ownership + - name: Show NTFS file ownership # Authoritative NTFS Owner via Get-Acl, with no Cygwin SID-to-uid layer # in between -- useful for confirming what the Cygwin view reports as # "Administrators" is the BUILTIN\Administrators SID (S-1-5-32-544). @@ -145,8 +133,7 @@ jobs: } } - - &safe-directory-display - name: Show safe.directory entries + - name: Show safe.directory entries run: git config --global --get-all safe.directory - name: Show version and platform information @@ -160,104 +147,3 @@ jobs: - name: Test with pytest (${{ matrix.additional-pytest-args }}) run: | pytest --color=yes -p no:sugar --instafail -vv ${{ matrix.additional-pytest-args }} - - diag-token: - runs-on: windows-latest - - env: *cygwin-env - - defaults: *cygwin-defaults - - steps: - - *force-lf - - *checkout - - *install-cygwin - - - name: PowerShell-side token state and file-creation tests - shell: pwsh - run: | - $repo = "D:\a\GitPython\GitPython" - Write-Host "==================== whoami /all (PowerShell) ====================" - whoami /all - Write-Host "" - Write-Host "==================== Test A: PowerShell New-Item directory ====================" - $td = "$repo\test-pwsh-mkdir" - New-Item -ItemType Directory -Path $td -Force | Out-Null - $acl = Get-Acl -LiteralPath $td - Write-Host "Owner of $td : $($acl.Owner)" - Remove-Item $td -Force - Write-Host "" - Write-Host "==================== Test D: PowerShell -> Git Bash -> PowerShell New-Item ====================" - $td4 = "$repo\test-pwsh-bash-pwsh-mkdir" - $scriptPath = "$repo\inner-mkdir.ps1" - "New-Item -ItemType Directory -Path '$td4' -Force | Out-Null" | - Set-Content -Path $scriptPath -Encoding utf8 - $env:MSYS2_ARG_CONV_EXCL = '*' - try { - & "C:\Program Files\Git\bin\bash.exe" -c "powershell.exe -NoProfile -ExecutionPolicy Bypass -File '$scriptPath'" 2>&1 | Out-Null - } finally { - Remove-Item Env:MSYS2_ARG_CONV_EXCL -ErrorAction SilentlyContinue - } - if (Test-Path -LiteralPath $td4) { - $acl = Get-Acl -LiteralPath $td4 - Write-Host "Owner of $td4 (PowerShell -> bash -> PowerShell New-Item) : $($acl.Owner)" - Remove-Item $td4 -Force - } else { - Write-Host "Owner of $td4 (PowerShell -> bash -> PowerShell New-Item) : (directory not created)" - } - Remove-Item $scriptPath -Force -ErrorAction SilentlyContinue - - - name: Cygwin-side token state and file-creation tests - run: | - set +e - echo "==================== id (Cygwin) ====================" - id - echo - echo "==================== Test B: Cygwin mkdir ====================" - td="$(pwd)/test-cygwin-mkdir" - mkdir "$td" - echo "Owner: $(stat -c '%U(%u)' "$td")" - rmdir "$td" - echo - echo "==================== Test C: Cygwin-spawned Git for Windows git init ====================" - td3="$(pwd)/test-cygwin-spawns-wingit" - mkdir "$td3" - ( cd "$td3" && /cygdrive/c/Program\ Files/Git/bin/git.exe init -q ) - echo "Owner of $td3 (Cygwin-mkdir) : $(stat -c '%U(%u)' "$td3")" - echo "Owner of $td3/.git (Cygwin->Win git init): $(stat -c '%U(%u)' "$td3/.git")" - rm -rf "$td3" - true - - reproduce-safe-dir: - strategy: - matrix: - run: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63, 64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 80, 81, 82, 83, 84, 85, 86, 87, 88, 89, 90, 91, 92, 93, 94, 95, 96, 97, 98, 99, 100, 101, 102, 103, 104, 105, 106, 107, 108, 109, 110, 111, 112, 113, 114, 115, 116, 117, 118, 119, 120, 121, 122, 123, 124, 125, 126, 127, 128, 129, 130, 131, 132, 133, 134, 135, 136, 137, 138, 139, 140, 141, 142, 143, 144, 145, 146, 147, 148, 149, 150, 151, 152, 153, 154, 155, 156, 157, 158, 159, 160, 161, 162, 163, 164, 165, 166, 167, 168, 169, 170, 171, 172, 173, 174, 175, 176, 177, 178, 179, 180, 181, 182, 183, 184, 185, 186, 187, 188, 189, 190, 191, 192, 193, 194, 195, 196, 197, 198, 199, 200, 201, 202, 203, 204, 205, 206, 207, 208, 209, 210, 211, 212, 213, 214, 215, 216, 217, 218, 219, 220, 221, 222, 223, 224, 225, 226, 227, 228, 229, 230, 231, 232, 233, 234, 235, 236, 237, 238, 239, 240, 241, 242, 243, 244, 245, 246, 247, 248, 249, 250, 251, 252, 253, 254, 255, 256] - fail-fast: false - - runs-on: windows-latest - - env: *cygwin-env - - defaults: *cygwin-defaults - - steps: - - *force-lf - - *checkout - - *install-cygwin - - *verbose-output - - *safe-directory - - *prepare-repo - - *git-identity - - *setup-venv - - *update-pypa - - *install-deps - - *ownership-posix-display - - *ownership-ntfs-display - - *safe-directory-display - - - name: Run submodule tests - run: | - python -m pytest -vv \ - test/test_docs.py::Tutorials::test_submodules \ - test/test_repo.py::TestRepo::test_submodules \ - test/test_submodule.py::TestSubmodule::test_root_module From 4dd89aff2896f63f8d7e61ef3736b1b60e386d16 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sun, 17 May 2026 15:49:25 -0400 Subject: [PATCH 37/45] Match test_root_module's deep-traversal assertion to gitdb's structure gitdb's `async` submodule was removed back in 2014 (gitpython-developers/gitdb@bf942a9); only smmap remains. The leading "gitdb / async" comment and the `assert len(rsmsp) >= 2` check (loosened back in 2011 from `== 2` in 4a8bdce7 when smmap was added to gitdb alongside async) are both stale. Replace with an exact list-equality check on the expected paths in traversal order. That order is also what later code in this function assumes via positional indexing `rsmsp[0]`, `rsmsp[1]`. Co-authored-by: Claude Opus 4.7 (1M context) --- test/test_submodule.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/test_submodule.py b/test/test_submodule.py index 0373e26f8..778d22e3f 100644 --- a/test/test_submodule.py +++ b/test/test_submodule.py @@ -508,9 +508,9 @@ def test_root_module(self, rwrepo): with rm.config_writer(): pass - # Deep traversal gitdb / async. + # Deep traversal yields gitdb and its nested smmap. rsmsp = [sm.path for sm in rm.traverse()] - assert len(rsmsp) >= 2 # gitdb and async [and smmap], async being a child of gitdb. + assert rsmsp == ["git/ext/gitdb", "gitdb/ext/smmap"] # Cannot set the parent commit as root module's path didn't exist. self.assertRaises(ValueError, rm.set_parent_commit, "HEAD") From 38956826cbf8a4ce8c345285a9d8fc78ef17aa21 Mon Sep 17 00:00:00 2001 From: Deepak kudi Date: Thu, 21 May 2026 13:09:53 +0530 Subject: [PATCH 38/45] Support index diffs against empty tree Assisted-by: OpenAI GPT-5 --- git/index/base.py | 43 ++++++++++++++++++++++++++++++++++++++++--- test/test_index.py | 30 +++++++++++++++++++++++++++++- 2 files changed, 69 insertions(+), 4 deletions(-) diff --git a/git/index/base.py b/git/index/base.py index 2276343f2..93d4cda52 100644 --- a/git/index/base.py +++ b/git/index/base.py @@ -1480,12 +1480,11 @@ def reset( return self - # FIXME: This is documented to accept the same parameters as Diffable.diff, but this - # does not handle NULL_TREE for `other`. (The suppressed mypy error is about this.) def diff( self, - other: Union[ # type: ignore[override] + other: Union[ Literal[git_diff.DiffConstants.INDEX], + Literal[git_diff.DiffConstants.NULL_TREE], "Tree", "Commit", str, @@ -1512,6 +1511,44 @@ def diff( if other is self.INDEX: return git_diff.DiffIndex() + if other is git_diff.NULL_TREE: + args: List[Union[PathLike, str]] = [ + "--cached", + "4b825dc642cb6eb9a060e54bf8d69288fbee4904", + "--abbrev=40", + "--full-index", + ] + + if not any(x in kwargs for x in ("find_renames", "no_renames", "M")): + args.append("-M") + + if create_patch: + args.append("-p") + args.append("--no-ext-diff") + else: + args.append("--raw") + args.append("-z") + + args.append("--no-color") + + if paths is not None and not isinstance(paths, (tuple, list)): + paths = [paths] + + if paths: + args.append("--") + args.extend(paths) + + kwargs["as_process"] = True + proc = self.repo.git.diff(*args, **kwargs) + + diff_method = ( + git_diff.Diff._index_from_patch_format if create_patch else git_diff.Diff._index_from_raw_format + ) + index = diff_method(self.repo, proc) + + proc.wait() + return index + # Index against anything but None is a reverse diff with the respective item. # Handle existing -R flags properly. # Transform strings to the object so that we can call diff on it. diff --git a/test/test_index.py b/test/test_index.py index cb45d3e90..327860d72 100644 --- a/test/test_index.py +++ b/test/test_index.py @@ -23,7 +23,7 @@ import ddt import pytest -from git import BlobFilter, Diff, Git, IndexFile, Object, Repo, Tree +from git import BlobFilter, Diff, Git, IndexFile, NULL_TREE, Object, Repo, Tree from git.exc import ( CheckoutError, GitCommandError, @@ -555,6 +555,34 @@ def test_index_file_diffing(self, rw_repo): rval = index.checkout("lib") assert len(list(rval)) > 1 + @with_rw_directory + def test_index_file_diff_null_tree_with_initial_index(self, rw_dir): + repo = Repo.init(rw_dir) + filename = ".gitkeep" + file_path = osp.join(repo.working_tree_dir, filename) + with open(file_path, "w") as fp: + fp.write("# Initial file\n") + + index = repo.index + index.add([filename]) + index.write() + + index = IndexFile(repo) + assert not index.diff(None) + + diff = index.diff(NULL_TREE) + self.assertEqual(len(diff), 1) + self.assertEqual(diff[0].change_type, "A") + assert diff[0].new_file + self.assertEqual(diff[0].b_path, filename) + + self.assertEqual(len(index.diff(NULL_TREE, paths=filename)), 1) + self.assertEqual(len(index.diff(NULL_TREE, paths="missing")), 0) + + patch = index.diff(NULL_TREE, create_patch=True) + self.assertEqual(len(patch), 1) + self.assertIn(b"+# Initial file", patch[0].diff) + def _count_existing(self, repo, files): """Return count of files that actually exist in the repository directory.""" existing = 0 From fe4b66dc621cf6d79da8cb95d20f775c8a373a93 Mon Sep 17 00:00:00 2001 From: Puneet Dixit <236133619+puneetdixit200@users.noreply.github.com> Date: Sat, 23 May 2026 20:34:42 +0530 Subject: [PATCH 39/45] fix: use shared empty tree sha for index diffs Assisted-by: ChatGPT --- git/diff.py | 5 ++++- git/index/base.py | 4 ++-- test/test_index.py | 4 +++- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/git/diff.py b/git/diff.py index 23cb5675e..b10f3f106 100644 --- a/git/diff.py +++ b/git/diff.py @@ -3,7 +3,7 @@ # This module is part of GitPython and is released under the # 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/ -__all__ = ["DiffConstants", "NULL_TREE", "INDEX", "Diffable", "DiffIndex", "Diff"] +__all__ = ["DiffConstants", "NULL_TREE", "NULL_TREE_SHA", "INDEX", "Diffable", "DiffIndex", "Diff"] import enum import re @@ -84,6 +84,9 @@ class DiffConstants(enum.Enum): :const:`git.NULL_TREE` and :const:`Diffable.NULL_TREE`. """ +NULL_TREE_SHA = "4b825dc642cb6eb9a060e54bf8d69288fbee4904" +"""SHA of Git's canonical empty tree object.""" + INDEX: Literal[DiffConstants.INDEX] = DiffConstants.INDEX """Stand-in indicating you want to diff against the index. diff --git a/git/index/base.py b/git/index/base.py index 93d4cda52..f03b452dc 100644 --- a/git/index/base.py +++ b/git/index/base.py @@ -1511,10 +1511,10 @@ def diff( if other is self.INDEX: return git_diff.DiffIndex() - if other is git_diff.NULL_TREE: + if other == git_diff.NULL_TREE or other == git_diff.NULL_TREE_SHA: args: List[Union[PathLike, str]] = [ "--cached", - "4b825dc642cb6eb9a060e54bf8d69288fbee4904", + git_diff.NULL_TREE_SHA, "--abbrev=40", "--full-index", ] diff --git a/test/test_index.py b/test/test_index.py index 327860d72..4a32dd5dc 100644 --- a/test/test_index.py +++ b/test/test_index.py @@ -24,6 +24,7 @@ import pytest from git import BlobFilter, Diff, Git, IndexFile, NULL_TREE, Object, Repo, Tree +from git.diff import NULL_TREE_SHA from git.exc import ( CheckoutError, GitCommandError, @@ -568,7 +569,7 @@ def test_index_file_diff_null_tree_with_initial_index(self, rw_dir): index.write() index = IndexFile(repo) - assert not index.diff(None) + self.assertEqual(len(index.diff(None)), 0) diff = index.diff(NULL_TREE) self.assertEqual(len(diff), 1) @@ -577,6 +578,7 @@ def test_index_file_diff_null_tree_with_initial_index(self, rw_dir): self.assertEqual(diff[0].b_path, filename) self.assertEqual(len(index.diff(NULL_TREE, paths=filename)), 1) + self.assertEqual(len(index.diff(NULL_TREE_SHA, paths=filename)), 1) self.assertEqual(len(index.diff(NULL_TREE, paths="missing")), 0) patch = index.diff(NULL_TREE, create_patch=True) From da07a64985290b76f6df4495faea8a569ca29288 Mon Sep 17 00:00:00 2001 From: Puneet Dixit <236133619+puneetdixit200@users.noreply.github.com> Date: Sun, 24 May 2026 00:00:40 +0530 Subject: [PATCH 40/45] test: allow alternate bad fetch errors Co-authored-by: OpenAI Codex --- test/test_remote.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/test/test_remote.py b/test/test_remote.py index 1c627127a..2230c8df4 100644 --- a/test/test_remote.py +++ b/test/test_remote.py @@ -687,7 +687,12 @@ def test_multiple_urls(self, rw_repo): def test_fetch_error(self): rem = self.rorepo.remote("origin") - with self.assertRaisesRegex(GitCommandError, "[Cc]ouldn't find remote ref __BAD_REF__"): + msg = ( + r"[Cc]ouldn't find remote ref __BAD_REF__|" + r"could not read Username|" + r"expected flush after ref listing" + ) + with self.assertRaisesRegex(GitCommandError, msg): rem.fetch("__BAD_REF__") @with_rw_repo("0.1.6", bare=False) From 59cc3bb1f5d43900f94f1c5044766e76b89f6445 Mon Sep 17 00:00:00 2001 From: Puneet Dixit <236133619+puneetdixit200@users.noreply.github.com> Date: Sun, 24 May 2026 00:13:13 +0530 Subject: [PATCH 41/45] fix: preserve diff process stderr Co-authored-by: OpenAI Codex --- git/diff.py | 18 +++++++++++++++--- test/test_index.py | 4 ++++ 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/git/diff.py b/git/diff.py index b10f3f106..5af53e556 100644 --- a/git/diff.py +++ b/git/diff.py @@ -602,7 +602,14 @@ def _index_from_patch_format(cls, repo: "Repo", proc: Union["Popen", "Git.AutoIn # FIXME: Here SLURPING raw, need to re-phrase header-regexes linewise. text_list: List[bytes] = [] - handle_process_output(proc, text_list.append, None, finalize_process, decode_streams=False) + stderr_list: List[bytes] = [] + + def finalize_process_with_stderr(proc: Union["Popen", "Git.AutoInterrupt"]) -> None: + finalize_process(proc, stderr=b"".join(stderr_list)) + + handle_process_output( + proc, text_list.append, stderr_list.append, finalize_process_with_stderr, decode_streams=False + ) # For now, we have to bake the stream. text = b"".join(text_list) @@ -768,11 +775,16 @@ def _index_from_raw_format(cls, repo: "Repo", proc: "Popen") -> "DiffIndex[Diff] # :100644 100644 687099101... 37c5e30c8... M .gitignore index: "DiffIndex" = DiffIndex() + stderr_list: List[bytes] = [] + + def finalize_process_with_stderr(proc: Union["Popen", "Git.AutoInterrupt"]) -> None: + finalize_process(proc, stderr=b"".join(stderr_list)) + handle_process_output( proc, lambda byt: cls._handle_diff_line(byt, repo, index), - None, - finalize_process, + stderr_list.append, + finalize_process_with_stderr, decode_streams=False, ) diff --git a/test/test_index.py b/test/test_index.py index 4a32dd5dc..3be750dbb 100644 --- a/test/test_index.py +++ b/test/test_index.py @@ -585,6 +585,10 @@ def test_index_file_diff_null_tree_with_initial_index(self, rw_dir): self.assertEqual(len(patch), 1) self.assertIn(b"+# Initial file", patch[0].diff) + with self.assertRaises(GitCommandError) as exc_info: + index.diff(NULL_TREE, bogus_option=True) + self.assertIn("usage: git diff", exc_info.exception.stderr) + def _count_existing(self, repo, files): """Return count of files that actually exist in the repository directory.""" existing = 0 From 4de94bc0e3ecc65e40a16cf19dd934ac3b413023 Mon Sep 17 00:00:00 2001 From: Puneet Dixit <236133619+puneetdixit200@users.noreply.github.com> Date: Sun, 24 May 2026 00:21:37 +0530 Subject: [PATCH 42/45] test: accept stderr in string process adapter Match the AutoInterrupt wait signature so diff parser tests can pass preserved stderr through finalize_process. Assisted-by: OpenAI GPT-5 --- test/lib/helper.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/lib/helper.py b/test/lib/helper.py index 2fc015dfa..1c110e103 100644 --- a/test/lib/helper.py +++ b/test/lib/helper.py @@ -90,7 +90,7 @@ def __init__(self, input_string): self.stdout = io.BytesIO(input_string) self.stderr = io.BytesIO() - def wait(self): + def wait(self, stderr=None): return 0 poll = wait From e3a75d0bf38616fbbf3029d540965506003531d8 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sat, 23 May 2026 13:36:58 -0400 Subject: [PATCH 43/45] Cut xtrace noise from POSIX-ownership diagnostic steps The "Show POSIX file ownership" step in each test workflow looped over a hard-coded path list with one `ls -ld` per iteration. Bash's xtrace -- active throughout (from `~/.bash_profile` on Cygwin and from the `-x` flag in GHA's default shell line on Ubuntu / macOS / Alpine) -- reprints the entire `for` expression's expanded word list at the start of every iteration. For nine paths that's nine long traces of the same word list, drowning out the `ls -ld` output we want to read. Collapse the loop into a single multi-arg `ls -ld --`: xtrace prints the expanded command line once, `ls` produces one line per existing path and a `ls: cannot access '': No such file or directory` line per missing path. `2>&1` merges those missing-path messages into the log stream alongside the existing-path output; `|| true` keeps the step from failing under `set -e` when any path is missing. The format of missing-path reporting changes from `(missing: )` to `ls: cannot access '': No such file or directory`. Both convey the same information; the new form is slightly more verbose per missing path but eliminates the per-iteration trace reprint that dominated the original output. Cosmetic-only; no change to the diagnostic information surfaced. Flagged on PR #2154 as a follow-up: https://github.com/gitpython-developers/GitPython/pull/2154#pullrequestreview-4307636857 Co-authored-by: Claude Opus 4.7 (1M context) --- .github/workflows/alpine-test.yml | 8 +++----- .github/workflows/cygwin-test.yml | 8 +++----- .github/workflows/pythonpackage.yml | 8 +++----- 3 files changed, 9 insertions(+), 15 deletions(-) diff --git a/.github/workflows/alpine-test.yml b/.github/workflows/alpine-test.yml index 4183f0e0d..5c999e487 100644 --- a/.github/workflows/alpine-test.yml +++ b/.github/workflows/alpine-test.yml @@ -63,17 +63,15 @@ jobs: - name: Show POSIX file ownership run: | - for p in \ + ls -ld -- \ "$(pwd)" \ "$(pwd)/.git" \ "$(pwd)/git/ext/gitdb" \ "$(pwd)/git/ext/gitdb/.git" \ "$(pwd)/git/ext/gitdb/gitdb/ext/smmap" \ "$(pwd)/git/ext/gitdb/gitdb/ext/smmap/.git" \ - "${HOME:?HOME is not set}/.gitconfig" - do - ls -ld -- "$p" 2>/dev/null || echo "(missing: $p)" - done + "${HOME:?HOME is not set}/.gitconfig" \ + 2>&1 || true - name: Show safe.directory entries # `actions/checkout`'s safe.directory add is only durable for the diff --git a/.github/workflows/cygwin-test.yml b/.github/workflows/cygwin-test.yml index c12ccb3cf..17ba4bc82 100644 --- a/.github/workflows/cygwin-test.yml +++ b/.github/workflows/cygwin-test.yml @@ -89,7 +89,7 @@ jobs: # `is_path_owned_by_current_user` reduces to, so this is the view that # determines whether `safe.directory` is consulted. run: | - for p in \ + ls -ld -- \ "$(pwd)" \ "$(pwd)/.git" \ "$(pwd)/git/ext/gitdb" \ @@ -98,10 +98,8 @@ jobs: "$(pwd)/git/ext/gitdb/gitdb/ext/smmap" \ "$(pwd)/git/ext/gitdb/gitdb/ext/smmap/.git" \ "$(pwd)/.git/modules/gitdb/modules/smmap" \ - "${HOME:?HOME is not set}/.gitconfig" - do - ls -ld -- "$p" 2>/dev/null || echo "(missing: $p)" - done + "${HOME:?HOME is not set}/.gitconfig" \ + 2>&1 || true - name: Show NTFS file ownership # Authoritative NTFS Owner via Get-Acl, with no Cygwin SID-to-uid layer diff --git a/.github/workflows/pythonpackage.yml b/.github/workflows/pythonpackage.yml index cffafc59a..6746b92c6 100644 --- a/.github/workflows/pythonpackage.yml +++ b/.github/workflows/pythonpackage.yml @@ -94,17 +94,15 @@ jobs: # not be informative here. The NTFS Owner check below covers Windows. if: matrix.os-type != 'windows' run: | - for p in \ + ls -ld -- \ "$(pwd)" \ "$(pwd)/.git" \ "$(pwd)/git/ext/gitdb" \ "$(pwd)/git/ext/gitdb/.git" \ "$(pwd)/git/ext/gitdb/gitdb/ext/smmap" \ "$(pwd)/git/ext/gitdb/gitdb/ext/smmap/.git" \ - "${HOME:?HOME is not set}/.gitconfig" - do - ls -ld -- "$p" 2>/dev/null || echo "(missing: $p)" - done + "${HOME:?HOME is not set}/.gitconfig" \ + 2>&1 || true - name: Show NTFS file ownership # Windows only. Reads NTFS Owner directly via Get-Acl, which is the From 5d8d15675011dde9e874a94339d1994808ffbd92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20L=C3=B6sch?= Date: Wed, 27 May 2026 16:52:19 +0200 Subject: [PATCH 44/45] refactor: seperate out Progress type --- git/remote.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/git/remote.py b/git/remote.py index 20e42b412..18d4829af 100644 --- a/git/remote.py +++ b/git/remote.py @@ -517,6 +517,9 @@ def iter_items(cls, repo: "Repo", *args: Any, **kwargs: Any) -> NoReturn: # -> raise NotImplementedError +Progress = Union[RemoteProgress, "UpdateProgress", Callable[..., RemoteProgress], None] + + class Remote(LazyMixin, IterableObj): """Provides easy read and write access to a git remote. @@ -872,7 +875,7 @@ def update(self, **kwargs: Any) -> "Remote": def _get_fetch_info_from_stderr( self, proc: "Git.AutoInterrupt", - progress: Union[Callable[..., Any], RemoteProgress, None], + progress: Progress, kill_after_timeout: Union[None, float] = None, ) -> IterableList["FetchInfo"]: progress = to_progress_instance(progress) @@ -1000,7 +1003,7 @@ def _assert_refspec(self) -> None: def fetch( self, refspec: Union[str, List[str], None] = None, - progress: Union[RemoteProgress, None, "UpdateProgress"] = None, + progress: Progress = None, verbose: bool = True, kill_after_timeout: Union[None, float] = None, allow_unsafe_protocols: bool = False, @@ -1081,7 +1084,7 @@ def fetch( def pull( self, refspec: Union[str, List[str], None] = None, - progress: Union[RemoteProgress, "UpdateProgress", None] = None, + progress: Progress = None, kill_after_timeout: Union[None, float] = None, allow_unsafe_protocols: bool = False, allow_unsafe_options: bool = False, @@ -1135,7 +1138,7 @@ def pull( def push( self, refspec: Union[str, List[str], None] = None, - progress: Union[RemoteProgress, "UpdateProgress", Callable[..., RemoteProgress], None] = None, + progress: Progress = None, kill_after_timeout: Union[None, float] = None, allow_unsafe_protocols: bool = False, allow_unsafe_options: bool = False, From 9d2d01ccef2f79c840828afe04698254dff354c1 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 2 Jun 2026 16:49:49 +0000 Subject: [PATCH 45/45] Bump https://github.com/astral-sh/ruff-pre-commit Bumps the pre-commit group with 1 update: [https://github.com/astral-sh/ruff-pre-commit](https://github.com/astral-sh/ruff-pre-commit). Updates `https://github.com/astral-sh/ruff-pre-commit` from v0.15.12 to 0.15.15 - [Release notes](https://github.com/astral-sh/ruff-pre-commit/releases) - [Commits](https://github.com/astral-sh/ruff-pre-commit/compare/v0.15.12...v0.15.15) --- updated-dependencies: - dependency-name: https://github.com/astral-sh/ruff-pre-commit dependency-version: 0.15.15 dependency-type: direct:production dependency-group: pre-commit ... Signed-off-by: dependabot[bot] --- .pre-commit-config.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index f3ab67035..9cc97962d 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -7,7 +7,7 @@ repos: exclude: ^test/fixtures/ - repo: https://github.com/astral-sh/ruff-pre-commit - rev: v0.15.12 + rev: v0.15.15 hooks: - id: ruff-check args: ["--fix"]