Repository navigation
fix(util): tolerate deleted temporary environment variables - #2340
Merged
Byron merged 2 commits intoNov 5, 2026
Merged
Conversation
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
Contributor
There was a problem hiding this comment.
🟢 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
1 task
Byron
added a commit
that referenced
this pull request
Nov 5, 2026
<!-- 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 <codex@openai.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
patch_env()raisesKeyErroron 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:
GIT_PYTHON_BACKEND=cliandpython -m pytest --noconftest -o addopts='' test/test_patch_env.py -q --tb=short.git diff --checkpassed.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.