From 9429be69c85442e744ef697bd79bad3fb4236e0a Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Thu, 2 Jan 2025 04:21:49 -0500 Subject: [PATCH 1/7] Special-case Python 3.7 for OSes we can install it on This is analogous to the 3.7-related CI change in gitdb that was part of https://github.com/gitpython-developers/gitdb/pull/114, as to part of https://github.com/gitpython-developers/smmap/pull/58. Since some tests are not yet passing on 3.13, this does not add 3.13 to CI, nor to the documentation of supported versions in `setup.py`. Note that the list there is not enforced; GitPython can already be installed on Python 3.13 and probably *mostly* works. (See https://github.com/gitpython-developers/GitPython/pull/1955 for details on other changes that should be made to fully support running GitPython on Python 3.13.) --- .github/workflows/pythonpackage.yml | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/.github/workflows/pythonpackage.yml b/.github/workflows/pythonpackage.yml index 747db62f0..deebe9e11 100644 --- a/.github/workflows/pythonpackage.yml +++ b/.github/workflows/pythonpackage.yml @@ -11,15 +11,19 @@ permissions: jobs: build: strategy: - fail-fast: false matrix: - os: ["ubuntu-22.04", "macos-latest", "windows-latest"] - python-version: ["3.7", "3.8", "3.9", "3.10", "3.11", "3.12"] - exclude: - - os: "macos-latest" - python-version: "3.7" + os: [ubuntu-latest, macos-latest, windows-latest] + python-version: ["3.8", "3.9", "3.10", "3.11", "3.12"] include: - experimental: false + - os: ubuntu-22.04 + python-version: "3.7" + experimental: false + - os: windows-latest + python-version: "3.7" + experimental: false + + fail-fast: false runs-on: ${{ matrix.os }} From affab8eda6a716363bc36c703de305676afc4ae3 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 9 Dec 2024 13:14:20 +0000 Subject: [PATCH 2/7] Bump Vampire/setup-wsl from 3.1.1 to 4.0.0 Bumps [Vampire/setup-wsl](https://github.com/vampire/setup-wsl) from 3.1.1 to 4.0.0. - [Release notes](https://github.com/vampire/setup-wsl/releases) - [Commits](https://github.com/vampire/setup-wsl/compare/v3.1.1...v4.0.0) --- updated-dependencies: - dependency-name: Vampire/setup-wsl dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] --- .github/workflows/pythonpackage.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/pythonpackage.yml b/.github/workflows/pythonpackage.yml index deebe9e11..b8e6391a1 100644 --- a/.github/workflows/pythonpackage.yml +++ b/.github/workflows/pythonpackage.yml @@ -44,7 +44,7 @@ jobs: - name: Set up WSL (Windows) if: startsWith(matrix.os, 'windows') - uses: Vampire/setup-wsl@v3.1.1 + uses: Vampire/setup-wsl@v4.0.0 with: distribution: Alpine additional-packages: bash From 01e40a7b06c4ba3d0cf937ba0974949392446b51 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Thu, 2 Jan 2025 04:38:44 -0500 Subject: [PATCH 3/7] Do everything in the venv in the Alpine test --- .github/workflows/alpine-test.yml | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/.github/workflows/alpine-test.yml b/.github/workflows/alpine-test.yml index 2c1eed391..8c00b1086 100644 --- a/.github/workflows/alpine-test.yml +++ b/.github/workflows/alpine-test.yml @@ -16,10 +16,10 @@ jobs: steps: - name: Prepare Alpine Linux run: | - apk add sudo git git-daemon python3 py3-pip + apk add sudo git git-daemon python3 py3-pip py3-setuptools py3-virtualenv py3-wheel echo 'Defaults env_keep += "CI GITHUB_* RUNNER_*"' >/etc/sudoers.d/ci_env addgroup -g 127 docker - adduser -D -u 1001 runner + adduser -D -u 1001 runner # TODO: Check if this still works on GHA as intended. adduser runner docker shell: sh -exo pipefail {0} # Run this as root, not the "runner" user. @@ -50,17 +50,14 @@ jobs: . .venv/bin/activate printf '%s=%s\n' 'PATH' "$PATH" 'VIRTUAL_ENV' "$VIRTUAL_ENV" >>"$GITHUB_ENV" - - name: Update PyPA packages - run: | - # Get the latest pip, wheel, and prior to Python 3.12, setuptools. - python -m pip install -U pip $(pip freeze --all | grep -ow ^setuptools) wheel - - name: Install project and test dependencies run: | + . .venv/bin/activate pip install ".[test]" - name: Show version and platform information run: | + . .venv/bin/activate uname -a command -v git python git version @@ -69,4 +66,5 @@ jobs: - name: Test with pytest run: | + . .venv/bin/activate pytest --color=yes -p no:sugar --instafail -vv From 0300de986ef78a4e7a5562638592f5e91bfd8fa7 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Thu, 2 Jan 2025 05:24:47 -0500 Subject: [PATCH 4/7] Instrument `handle_process_output` test To try to find the bug that causes it to fail on Cygwin on CI, but not on other systems on CI, and not locally on Cygwin. It looks like there's an extra line being read from stderr when the test fails, so let's examine the lines themselves. --- test/test_git.py | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/test/test_git.py b/test/test_git.py index 94e68ecf0..3f9687dfe 100644 --- a/test/test_git.py +++ b/test/test_git.py @@ -762,14 +762,17 @@ def test_environment(self, rw_dir): def test_handle_process_output(self): from git.cmd import handle_process_output, safer_popen - line_count = 5002 - count = [None, 0, 0] + expected_line_count = 5002 + line_counts = [None, 0, 0] + lines = [None, [], []] - def counter_stdout(line): - count[1] += 1 + def stdout_handler(line): + line_counts[1] += 1 + lines[1].append(line) - def counter_stderr(line): - count[2] += 1 + def stderr_handler(line): + line_counts[2] += 1 + lines[2].append(line) cmdline = [ sys.executable, @@ -784,10 +787,10 @@ def counter_stderr(line): shell=False, ) - handle_process_output(proc, counter_stdout, counter_stderr, finalize_process) + handle_process_output(proc, stdout_handler, stderr_handler, finalize_process) - self.assertEqual(count[1], line_count) - self.assertEqual(count[2], line_count) + self.assertEqual(line_counts[1], expected_line_count, repr(lines[1])) + self.assertEqual(line_counts[2], expected_line_count, repr(lines[2])) def test_execute_kwargs_set_agrees_with_method(self): parameter_names = inspect.signature(cmd.Git.execute).parameters.keys() From a0f8425c94992bdf3fdde9cbf7c3c7f9dc12e52c Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Thu, 2 Jan 2025 05:28:31 -0500 Subject: [PATCH 5/7] Slightly simplify and clarify instrumented code --- test/test_git.py | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/test/test_git.py b/test/test_git.py index 3f9687dfe..274511f8d 100644 --- a/test/test_git.py +++ b/test/test_git.py @@ -763,16 +763,13 @@ def test_handle_process_output(self): from git.cmd import handle_process_output, safer_popen expected_line_count = 5002 - line_counts = [None, 0, 0] - lines = [None, [], []] + actual_lines = [None, [], []] def stdout_handler(line): - line_counts[1] += 1 - lines[1].append(line) + actual_lines[1].append(line) def stderr_handler(line): - line_counts[2] += 1 - lines[2].append(line) + actual_lines[2].append(line) cmdline = [ sys.executable, @@ -789,8 +786,8 @@ def stderr_handler(line): handle_process_output(proc, stdout_handler, stderr_handler, finalize_process) - self.assertEqual(line_counts[1], expected_line_count, repr(lines[1])) - self.assertEqual(line_counts[2], expected_line_count, repr(lines[2])) + self.assertEqual(len(actual_lines[1]), expected_line_count, repr(actual_lines[1])) + self.assertEqual(len(actual_lines[2]), expected_line_count, repr(actual_lines[2])) def test_execute_kwargs_set_agrees_with_method(self): parameter_names = inspect.signature(cmd.Git.execute).parameters.keys() From 4aad37a303ca51a594700976b04e17f0f835d61d Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Thu, 2 Jan 2025 06:00:23 -0500 Subject: [PATCH 6/7] Improve environment isolation in Cygwin and Alpine Linux on CI The main change here is to the Cygwin test workflow, which now runs the tests in a venv (a Python virtual environment), to avoid mixing up our intended dependencies with other files installed by Python related packages on the system. This should either fix the problem where `test_handle_process_output` reports an extra line in stderr for the `cat_file.py` subprocess on CI or Cygwin, or at least make it somewhat easier to continue investigating the problem. I think this change is also valuable for its own sake. The connection to the `test_handle_process_output` failure is that the extra line in stderr appears at the beginning and is an error message about a missing import needed for subprocess code coverage. With the recently added more detailed error reporting, it shows: self.assertEqual(line_counts[1], expected_line_count, repr(lines[1])) > self.assertEqual(line_counts[2], expected_line_count, repr(lines[2])) E AssertionError: 5003 != 5002 : ['pytest-cov: Failed to setup subprocess coverage. Environ: {\'COV_CORE_SOURCE\': \'git\', \'COV_CORE_CONFIG\': \':\', \'COV_CORE_DATAFILE\': \'/cygdrive/d/a/GitPython/GitPython/.coverage\'} Exception: ModuleNotFoundError("No module named \'iniconfig\'")\n', 'From github.com:jantman/gitpython_issue_301\n', ' = [up to date] master -> origin/master\n', ' = [up to date] testcommit1 -> origin/testcommit1\n', ' = [up to date] testcommit10 -> origin/testcommit10\n', ' = [up to date] testcommit100 -> origin/testcommit100\n', ' = [up to date] testcommit1000 -> origin/testcommit1000\n', ' = [up to date] testcommit1001 -> origin/testcommit1001\n', ' = [up to date] testcommit1002 -> origin/testcommit1002\n', ' = [up to date] testcommit1003 -> origin/testcommit1003\n', ' = [up to date] testcommit1004 -> origin/testcommit1004\n', ' = [up to date] testcommit1005 -> origin/testcommit1005\n', ' = [up to date] testcommit test/test_git.py:793: AssertionError This could possibly be worked around by attempting to install a package to provide `iniconfig`, by configuring `pytest-cov` in a way that does not require it, or by temporarily disabling code coverage reports on Cygwin. Before exploring those or other options, this change seeks to prepare a more isolated environment in which different package versions are more likely to work properly together. In addition to that change to the Cygwin workflow, this also changes the way the Alpine Linux test workflow is made to use a venv, using the technique that is used here, and undoing some changes in the Alpine Linux workflow that should not be necessary. The reason for making that change together with this Cygwin change is that if either does not work as expected, it might shed light on what is going wrong with the other. Although the same technique is used on Cygwin and in Alpine Linux, it looks a little different, because the environment variable on Cygwin is `BASH_ENV` (since script steps are run in `bash`), while the environment variable is `ENV` (since script steps are run in `busybox sh`) and this must also be allowed to pass through `sudo` (since `sh`, which is `busybox sh` on this system, is being invoked through `sudo`). --- .github/workflows/alpine-test.yml | 15 ++++++++------- .github/workflows/cygwin-test.yml | 6 +++--- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/.github/workflows/alpine-test.yml b/.github/workflows/alpine-test.yml index 8c00b1086..ca141cf03 100644 --- a/.github/workflows/alpine-test.yml +++ b/.github/workflows/alpine-test.yml @@ -16,8 +16,8 @@ jobs: steps: - name: Prepare Alpine Linux run: | - apk add sudo git git-daemon python3 py3-pip py3-setuptools py3-virtualenv py3-wheel - echo 'Defaults env_keep += "CI GITHUB_* RUNNER_*"' >/etc/sudoers.d/ci_env + apk add sudo git git-daemon python3 py3-pip py3-virtualenv + echo 'Defaults env_keep += "CI ENV GITHUB_* RUNNER_*"' >/etc/sudoers.d/ci_env addgroup -g 127 docker adduser -D -u 1001 runner # TODO: Check if this still works on GHA as intended. adduser runner docker @@ -47,17 +47,19 @@ jobs: - name: Set up virtualenv run: | python -m venv .venv - . .venv/bin/activate - printf '%s=%s\n' 'PATH' "$PATH" 'VIRTUAL_ENV' "$VIRTUAL_ENV" >>"$GITHUB_ENV" + echo 'ENV=.venv/bin/activate' >> "$GITHUB_ENV" # ENV (not BASH_ENV) for BusyBox sh. + + - name: Update PyPA packages + run: | + # Get the latest pip, wheel, and prior to Python 3.12, setuptools. + python -m pip install -U pip $(pip freeze --all | grep -ow ^setuptools) wheel - name: Install project and test dependencies run: | - . .venv/bin/activate pip install ".[test]" - name: Show version and platform information run: | - . .venv/bin/activate uname -a command -v git python git version @@ -66,5 +68,4 @@ jobs: - name: Test with pytest run: | - . .venv/bin/activate pytest --color=yes -p no:sugar --instafail -vv diff --git a/.github/workflows/cygwin-test.yml b/.github/workflows/cygwin-test.yml index bde4ea659..ebe50240d 100644 --- a/.github/workflows/cygwin-test.yml +++ b/.github/workflows/cygwin-test.yml @@ -55,10 +55,10 @@ jobs: # and cause subsequent tests to fail cat test/fixtures/.gitconfig >> ~/.gitconfig - - name: Ensure the "pip" command is available + - name: Set up virtualenv run: | - # This is used unless, and before, an updated pip is installed. - ln -s pip3 /usr/bin/pip + python -m venv .venv + echo 'BASH_ENV=.venv/bin/activate' >>"$GITHUB_ENV" - name: Update PyPA packages run: | From 39cd608b762256663b862224bcb46bdb2fc18817 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Thu, 2 Jan 2025 06:28:48 -0500 Subject: [PATCH 7/7] Put back explicit venv activation in Alpine Linux `busybox sh` does not appear to read commands from a file whose path is given as the value of `$ENV`, in this situation. I think I may have misunderstood that; the documentation does not say much about it and maybe, in Almquist-style shells, it is only read by interactive shells? I am not sure. This removes everything about `ENV` and activates the venv in each step where the venv should be used. The good news is that the technique did work fully in Cygwin: not only did `BASH_ENV` work (which was not much in doubt), but using a virtual environment for all steps that run test code on Cygwin fixed the problem and allowed all tests to pass. That seems to have been the reason I didn't reproduce the problem locally: on my local system I always use a venv in Cygwin since I would otherwise not have adequate isolation. Thus, this commit changes only the Alpine workflow and not the Cygwin workflow. --- .github/workflows/alpine-test.yml | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/.github/workflows/alpine-test.yml b/.github/workflows/alpine-test.yml index ca141cf03..6dc62f596 100644 --- a/.github/workflows/alpine-test.yml +++ b/.github/workflows/alpine-test.yml @@ -17,7 +17,7 @@ jobs: - name: Prepare Alpine Linux run: | apk add sudo git git-daemon python3 py3-pip py3-virtualenv - echo 'Defaults env_keep += "CI ENV GITHUB_* RUNNER_*"' >/etc/sudoers.d/ci_env + echo 'Defaults env_keep += "CI GITHUB_* RUNNER_*"' >/etc/sudoers.d/ci_env addgroup -g 127 docker adduser -D -u 1001 runner # TODO: Check if this still works on GHA as intended. adduser runner docker @@ -47,19 +47,21 @@ jobs: - name: Set up virtualenv run: | python -m venv .venv - echo 'ENV=.venv/bin/activate' >> "$GITHUB_ENV" # ENV (not BASH_ENV) for BusyBox sh. - name: Update PyPA packages run: | # Get the latest pip, wheel, and prior to Python 3.12, setuptools. + . .venv/bin/activate python -m pip install -U pip $(pip freeze --all | grep -ow ^setuptools) wheel - name: Install project and test dependencies run: | + . .venv/bin/activate pip install ".[test]" - name: Show version and platform information run: | + . .venv/bin/activate uname -a command -v git python git version @@ -68,4 +70,5 @@ jobs: - name: Test with pytest run: | + . .venv/bin/activate pytest --color=yes -p no:sugar --instafail -vv