GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — fix(util): tolerate deleted temporary environment variables by Abedyn · Pull Request #2340 · gitpython-developers/GitPython

Conversation

Abedyn commented Nov 4, 2026

Copy link

Copy Markdown

Contributor

patch_env() raises KeyError on exit if its variable was originally absent and the context body deletes it. If the body also raises an exception, the cleanup error masks that exception.

Use os.environ.pop(name, None) when restoring an originally absent variable. Add 18 regression cases covering absent, empty and populated original values; unchanged, replaced and deleted temporary values; and normal and exceptional exits.

Motivation: this was discovered while inspecting GitPython with a user seeking their first small Python contribution to Git tooling on Windows. The goal is to make the environment helper's restoration behavior consistent when a caller changes the temporary variable. Codex prepared the implementation, tests and this description.

Validation on Windows, Python 3.13.17 and Git 2.56.0.windows.4:

Before the fix: 2 failed, 16 passed.
After the fix: 18 passed with GIT_PYTHON_BACKEND=cli and python -m pytest --noconftest -o addopts='' test/test_patch_env.py -q --tb=short.
Ruff lint and formatting checks passed for both changed files; git diff --check passed.

The full suite and its conftest hooks were not run. Installing the test extra with binary-only GixPython failed because no compatible Windows wheel for GixPython 0.3.0 was available. The focused tests do not use repository fixtures or Gix.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          fix(util): tolerate deleted temporary environment variables

e52b7f8

When a `patch_env()` body deletes a variable that was initially absent,
cleanup raises `KeyError` and can mask an exception from the body.
Use `os.environ.pop(name, None)` to restore absence idempotently while
preserving restoration of existing values.

Add 18 regression cases for absent, empty and populated original values,
keep/replace/delete body mutations and normal/exceptional exits.
The focused CLI-backend tests changed from 2 failed and 16 passed to
18 passed. Ruff lint/format and `git diff --check` passed. The full suite
was not run because its required `GixPython` Windows wheel was unavailable.

Agent-Model: unknown

Byron requested a balanced review from Copilot

November 4, 2026 17:55

Copilot started reviewing on behalf of Byron

November 4, 2026 17:56

View session

Copilot AI reviewed

Nov 4, 2026

View reviewed changes

Copilot AI left a comment

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No active review issues are reported for the change that prevents environment cleanup from masking context-body exceptions.

No open issues 🎉

What changed in this PR

This PR makes GitPython’s patch_env() cleanup tolerate deletion of a temporary environment variable, preventing a cleanup KeyError from masking an exception raised inside the context.

Changes:

Use os.environ.pop(name, None) when restoring an originally absent variable.
Add 18 regression cases covering original values, temporary-variable mutations, and normal or exceptional exits.

Review by GitHub Copilot.

File	Description
`test/test_patch_env.py`	Tests environment restoration and preservation of context-body exceptions.
`git/util.py`	Makes cleanup safe when an originally absent variable has already been deleted.

_{Review effort: Balanced}

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          review

32b4d8e

Byron merged commit f6db6e7 into gitpython-developers:main

Nov 5, 2026

56 of 64 checks passed

Uh oh!

There was an error while loading. Please reload this page.

Byron mentioned this pull request

Nov 5, 2026

test: isolate repository fixtures and restore Windows reference coverage #2341

Merged

1 task

Byron added a commit that referenced this pull request

Nov 5, 2026


          test(refs): use POSIX paths for Git reference names

9bfb816

<!-- agent -->
Restore Windows CI coverage for `PathLike` revision arguments. The test
introduced in `6de90f1c` uses `Path("refs/heads/main")`, which emits
backslashes on Windows. Those are invalid Git reference names, so resolving
`SymbolicReference` fails during ref-name validation in all four
files/reftable and SHA-1/SHA-256 variants. The same failures were already
present in PR #2336, before the unrelated `patch_env()` change in PR #2340.

Use `PurePosixPath` for the test's Git reference names, which use forward
slashes on every platform. It still implements `os.PathLike`, preserving
coverage through `SymbolicReference` and direct and nested `merge_base()`
arguments alongside `PathLikeMock`. Keep native `Path` for filesystem
operations elsewhere in the module and preserve production validation.

Assisted-by: GPT 6.1 Sol
Co-authored-by: GPT 6.1 Sol <[email protected]>

Reviewers

Copilot code review Copilot

Copilot review effort

Applies to this pull request for everyone.Learn more about Copilot code review.

Copilot left review comments

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.