Skip to content

Commit a01707f

Browse files
EliahKaganclaude
andcommitted
Add jobs to reproduce Cygwin safe.directory submodule issue
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. Add 256 reproduce-safe-dir matrix jobs to cygwin-test.yml, each running these three tests under the current safe.directory configuration. All or most of these reproduce-safe-dir jobs must be removed before this work is integrated. The existing test job's env, defaults, and setup steps gain YAML anchors so the new job can reference them without duplication. Root cause hypothesis --------------------- The CI's safe.directory list (set in the "Special configuration for Cygwin git" step) covers the main repo and its .git directory but not the gitdb submodule's working tree at git/ext/gitdb. Git matches safe.directory exactly, not by prefix, so when GitPython spawns "git cat-file --batch-check" in the gitdb submodule (via Repo(submodule_path).odb.info(sha)), git rejects the repository for dubious ownership and exits. Three observed failure modes, all from the same root cause ---------------------------------------------------------- The git subprocess exits soon after starting. What Python sees depends on a race between Python writing to the subprocess's stdin and the subprocess exiting and closing its stdout pipe. 1. ValueError ("SHA is empty, possible dubious ownership..."): Python's write/flush completes before git has finished exiting. The buffered write succeeds, then stdout.readline() returns b"" (EOF). _parse_object_header(b"") in git/cmd.py raises this ValueError. The error message names the rejected directory and even suggests the safe.directory fix. This propagates uncaught from Object.new_from_sha through name_to_object (line 229 of git/repo/fun.py is outside the try/except loop) through repo.commit("HEAD") to iter_items, where the "except (IOError, BadName)" clause does not catch ValueError. 2. IndexError ("list index out of range"): Git exits before Python's write/flush runs. cmd.stdin.write or cmd.stdin.flush raises BrokenPipeError, a subclass of OSError (IOError). This time iter_items's "except (IOError, BadName)" catches it, returning an empty iterator. children() therefore returns an empty list, and "[0]" in test_submodules raises IndexError. 3. AssertionError ("1 not greater than or equal to 2"): Same BrokenPipeError-caught-as-IOError mechanism as case 2, but manifesting in test_repo.py::test_submodules, which does "assertGreaterEqual(len(list(self.rorepo.iter_submodules())), 2)". The recursive traversal finds gitdb (via the main repo, which is in safe.directory) but cannot enumerate gitdb's children, so only one submodule is yielded. Evidence -------- Recent main-branch CI runs show all three xfailed tests consistently matching their raises=ValueError xfail, e.g. job 74546730688 in run 25415738988. Verified across the five most recent main runs. PR gitpython-developers#2143 attempt 1 (job 74630986491, run 25440735020 attempt 1) had test_docs.py FAILED with IndexError while test_repo.py XFAILed with ValueError in the same job. PR gitpython-developers#2143 attempt 2 (job 74633063805) had both XFAIL with ValueError. Same commit, same workflow, same runner image -- a flaky race. The 3-job trial of these reproduce-safe-dir jobs (run 25454533092, commit baf3526) produced 8 ValueError and 1 IndexError out of 9 test runs. The 30-job run (run 25454836713) produced 90 ValueError and 0 IndexError in the reproduce jobs, but the same run's test (fast) job exhibited the AssertionError variant in test_repo.py::test_submodules. That AssertionError is the third manifestation of the BrokenPipeError race. Plan ---- 1. (this commit) Reproduce the failure under the current safe.directory configuration, with xfails removed so failures surface directly. The 256-job matrix is intended to characterize the rate of each variant. 2. Apply the fix: extend the safe.directory configuration to cover the gitdb submodule's working tree (and smmap's, recursively). Verify that the tests now pass. 3. Remove the reproduce-safe-dir jobs and the YAML anchors that only the reproduce job needed. The existing test job retains the permanent fix. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent c7648c0 commit a01707f

4 files changed

Lines changed: 53 additions & 30 deletions

File tree

.github/workflows/cygwin-test.yml

Lines changed: 53 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -20,63 +20,73 @@ jobs:
2020

2121
runs-on: windows-latest
2222

23-
env:
23+
env: &cygwin-env
2424
CHERE_INVOKING: "1"
2525
CYGWIN_NOWINPATH: "1"
2626

27-
defaults:
27+
defaults: &cygwin-defaults
2828
run:
2929
shell: C:\cygwin\bin\bash.exe --login --norc -eo pipefail -o igncr "{0}"
3030

3131
steps:
32-
- name: Force LF line endings
32+
- &force-lf
33+
name: Force LF line endings
3334
run: |
3435
git config --global core.autocrlf false # Affects the non-Cygwin git.
3536
shell: pwsh # Do this outside Cygwin, to affect actions/checkout.
3637

37-
- uses: actions/checkout@v6
38+
- &checkout
39+
uses: actions/checkout@v6
3840
with:
3941
fetch-depth: 0
4042

41-
- name: Install Cygwin
43+
- &install-cygwin
44+
name: Install Cygwin
4245
uses: cygwin/cygwin-install-action@v6
4346
with:
4447
packages: git python39 python-pip-wheel python-setuptools-wheel python-wheel-wheel
4548
add-to-path: false # No need to change $PATH outside the Cygwin environment.
4649

47-
- name: Arrange for verbose output
50+
- &verbose-output
51+
name: Arrange for verbose output
4852
run: |
4953
# Arrange for verbose output but without shell environment setup details.
5054
echo 'set -x' >~/.bash_profile
5155
52-
- name: Special configuration for Cygwin git
56+
- &safe-directory
57+
name: Special configuration for Cygwin git
5358
run: |
5459
git config --global --add safe.directory "$(pwd)"
5560
git config --global --add safe.directory "$(pwd)/.git"
5661
git config --global core.autocrlf false
5762
58-
- name: Prepare this repo for tests
63+
- &prepare-repo
64+
name: Prepare this repo for tests
5965
run: |
6066
./init-tests-after-clone.sh
6167
62-
- name: Set git user identity and command aliases for the tests
68+
- &git-identity
69+
name: Set git user identity and command aliases for the tests
6370
run: |
6471
git config --global user.email "travis@ci.com"
6572
git config --global user.name "Travis Runner"
6673
# If we rewrite the user's config by accident, we will mess it up
6774
# and cause subsequent tests to fail
6875
cat test/fixtures/.gitconfig >> ~/.gitconfig
6976
70-
- name: Set up virtual environment
77+
- &setup-venv
78+
name: Set up virtual environment
7179
run: |
7280
python3.9 -m venv .venv
7381
echo 'BASH_ENV=.venv/bin/activate' >>"$GITHUB_ENV"
7482
75-
- name: Update PyPA packages
83+
- &update-pypa
84+
name: Update PyPA packages
7685
run: |
7786
python -m pip install -U pip 'setuptools; python_version<"3.12"' wheel
7887
79-
- name: Install project and test dependencies
88+
- &install-deps
89+
name: Install project and test dependencies
8090
run: |
8191
pip install '.[test]'
8292
@@ -91,3 +101,34 @@ jobs:
91101
- name: Test with pytest (${{ matrix.additional-pytest-args }})
92102
run: |
93103
pytest --color=yes -p no:sugar --instafail -vv ${{ matrix.additional-pytest-args }}
104+
105+
reproduce-safe-dir:
106+
strategy:
107+
matrix:
108+
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]
109+
fail-fast: false
110+
111+
runs-on: windows-latest
112+
113+
env: *cygwin-env
114+
115+
defaults: *cygwin-defaults
116+
117+
steps:
118+
- *force-lf
119+
- *checkout
120+
- *install-cygwin
121+
- *verbose-output
122+
- *safe-directory
123+
- *prepare-repo
124+
- *git-identity
125+
- *setup-venv
126+
- *update-pypa
127+
- *install-deps
128+
129+
- name: Run submodule tests
130+
run: |
131+
python -m pytest -vv \
132+
test/test_docs.py::Tutorials::test_submodules \
133+
test/test_repo.py::TestRepo::test_submodules \
134+
test/test_submodule.py::TestSubmodule::test_root_module

test/test_docs.py

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,6 @@
66
import gc
77
import os
88
import os.path
9-
import sys
10-
11-
import pytest
129

1310
from test.lib import TestBase
1411
from test.lib.helper import with_rw_directory
@@ -478,11 +475,6 @@ def test_references_and_objects(self, rw_dir):
478475

479476
repo.git.clear_cache()
480477

481-
@pytest.mark.xfail(
482-
sys.platform == "cygwin",
483-
reason="Cygwin GitPython can't find SHA for submodule",
484-
raises=ValueError,
485-
)
486478
def test_submodules(self):
487479
# [1-test_submodules]
488480
repo = self.rorepo

test/test_repo.py

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -877,11 +877,6 @@ def test_repo_odbtype(self):
877877
target_type = GitCmdObjectDB
878878
self.assertIsInstance(self.rorepo.odb, target_type)
879879

880-
@pytest.mark.xfail(
881-
sys.platform == "cygwin",
882-
reason="Cygwin GitPython can't find submodule SHA",
883-
raises=ValueError,
884-
)
885880
def test_submodules(self):
886881
self.assertEqual(len(self.rorepo.submodules), 1) # non-recursive
887882
self.assertGreaterEqual(len(list(self.rorepo.iter_submodules())), 2)

test/test_submodule.py

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -480,11 +480,6 @@ def test_base_rw(self, rwrepo):
480480
def test_base_bare(self, rwrepo):
481481
self._do_base_tests(rwrepo)
482482

483-
@pytest.mark.xfail(
484-
sys.platform == "cygwin",
485-
reason="Cygwin GitPython can't find submodule SHA",
486-
raises=ValueError,
487-
)
488483
@pytest.mark.xfail(
489484
HIDE_WINDOWS_KNOWN_ERRORS,
490485
reason=(

0 commit comments

Comments
 (0)