From fa0d9bc17928e7c11dc11c875c1047587f5354c3 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Thu, 7 May 2026 11:35:08 -0400 Subject: [PATCH 01/11] 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 02/11] 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 03/11] 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 04/11] 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 05/11] 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 06/11] 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 07/11] 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 08/11] 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 09/11] 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 10/11] 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 11/11] 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")