Repository navigation
fix(repo): return actual historical paths from incremental blame - #2315
Merged
Merged
Conversation
<!-- agent --> `Repo.blame_incremental()` copied Git's `filename` field into `BlameEntry.orig_path` without interpreting its C-style quoting. After renaming `café.txt` to `current.txt`, callers received `"caf\303\251.txt"` instead of `café.txt`, so following an attribution back to its original tree entry failed. Quotes, backslashes and control characters had the same problem, even with `core.quotePath=false`. Decode quoted filenames with the existing `git.diff._unquote_path()` helper before constructing each entry. This shared point handles both newly encountered and cached commits, leaves unquoted names unchanged, and retains `safe_decode()`'s byte-preserving `surrogateescape` behavior. `Repo.blame(..., incremental=True)` receives the same correction. Blame continues to use Git's CLI on both backends; this is an output-parser fix, not a new Gix implementation. Add an independently written tree-based fixture and regression covering eight historical filenames with both `core.quotePath` settings. Renaming then editing the middle line produces two separated entries attributed to the same original commit, exercising both parser branches. Check the commit, original path and both line ranges through both public entry points. Tree construction avoids platform restrictions on checking out control-character and invalid-UTF-8 filenames. The regression failed in ten quoted-filename subcases on each backend before the fix and passes afterward. Repository and diff tests pass on both backends; Ruff lint/format, mypy and ShellCheck also pass. Git evidence: the comparison binary is `git version 2.54.0 (Apple Git-157)`. Reference checkout revision `6de20f6092dcf9bdb1c8efe03db4b70c82b423dd` documents the terminating `filename` field in `Documentation/git-blame.adoc`; its `builtin/blame.c:write_filename_info()` uses `write_name_quoted()`. The regression's real `git blame --incremental HEAD -- current.txt` output quotes the historical path in both entries for the original commit, matching that format. Independent solution to the problem reported and investigated by `Keerthana-64` in #2314. No implementation or tests were taken from that PR. Assisted-by: GPT 6.1 Sol Co-authored-by: GPT 6.1 Sol <codex@openai.com>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The localized fix reuses established decoding behavior and has focused regression coverage, with no unresolved findings.
No open issues 🎉
What changed in this PR
GitHub Copilot review: This PR fixes incremental blame so BlameEntry.orig_path returns the actual historical filename rather than Git’s quoted representation.
Changes:
- Reuses the existing byte-oriented path decoder before converting filenames to text.
- Adds regression coverage for special filenames, both quoting settings, repeated commit entries, and both incremental blame APIs.
- Keeps the existing CLI blame implementation for both backends.
| File | Description |
|---|---|
tests/fixtures/blame-paths.sh |
Creates rename histories containing special filename bytes. |
test/test_repo.py |
Verifies historical paths and attribution through both APIs. |
git/repo/base.py |
Unquotes historical filenames before yielding blame entries. |
Review effort: Balanced
The `test (windows, 3.15)` and `test (windows, 3.12, gix)` CI jobs fail the new historical-path regression for its combined control-character filename under both `core.quotePath` settings. Their assertion messages truncate the actual and expected entries and omit the full diff, leaving the precise filename mismatch unavailable in the job logs. Set `TestCase.maxDiff` to `None` in this regression so CI reports the complete values. This preserves every assertion and the implementation; it provides the evidence needed to distinguish fixture handling from a parser defect before changing behavior. The focused regression passes locally on the CLI and Gix backends with all sixteen filename/quoting subcases. Ruff lint and formatting checks and `git diff --check` pass.
Windows CI with Git `2.56.0.windows.2` fails the historical-path regression for its combined control-character filename under both `core.quotePath` settings. The attributed original commit is `e5ed22d659d5aa4ff5f7527069752d20f74fc2d9`, which a local Git reproduction matches exactly when removing the carriage return from the original path. Keeping that byte produces `73813b4d58d085503f9426c6e50ce5c201f715b6`. The failure therefore comes from fixture creation losing the byte, rather than incremental blame returning a different path from the stored tree. Represent special paths as Git C-quoted text and feed the original tree through `git mktree` instead of sending raw control bytes to `mktree -z`. Git decodes the escapes while constructing the tree. This keeps carriage returns out of the raw fixture input and retains coverage for every control character, invalid UTF-8, literal escapes, and both quoting settings. The parser implementation and expected results are unchanged. The focused regression passes locally on both backends with all sixteen subcases. Raw `git ls-tree -z HEAD~2` confirms the carriage return is present, and the original commit retains its expected ID. ShellCheck, `bash -n`, and `git diff --check` pass. Windows CI must verify the fixture on its native Git/Bash combination.
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.
Tasks
This section is for Byron only. Models continuing this PR must not add, remove, check, uncheck, rename, or reorder checkboxes here.
Everything below this line was generated by
GPT-6 (Codex).Created by Codex on behalf of Byron. Byron will review before this is ready to merge.
Supersedes: #2314. This is an independent solution to the underlying problem reported and investigated by @Keerthana-64.
Problem: After a file is renamed,
Repo.blame_incremental()can return a historical filename that does not exist in the attributed commit. Git quotes special filenames in its output, and GitPython currently exposes that quoted text asBlameEntry.orig_path. For example, renamingcafé.txttocurrent.txtyields"caf\303\251.txt"instead ofcafé.txt. Tools following blame entries for provenance, ownership or license attribution cannot look up the original file. Quotes, backslashes and control characters are affected too.Relationship to Git: Incremental blame identifies the original commit, path and line range for each group of lines. Its terminating
filenamefield uses Git's C-style quoting. This fixes GitPython's interpretation of that format soorig_pathnames the actual tree entry. Git 2.54.0 (Apple Git-157) demonstrates the behavior; Git's reference source and documentation confirm it.Solution: Reuse the existing byte-oriented path-unquoting helper once before yielding each entry, then use the existing
safe_decode(). This covers first and repeated entries for a commit, preserves invalid UTF-8 bytes and literal escape sequences, and leaves unquoted paths intact. The delegatedRepo.blame(..., incremental=True)API receives the same fix. Both backends retain their existing Git CLI blame path.Validation: Windows CI found and prompted a fixture correction; the corrected Windows run is pending. The independent regression failed in ten filename subcases on each backend before the fix and passes afterward. Repository and diff tests pass on both backends (184 passed, 2 skipped and 32 passing subtests each), along with Ruff lint/format, mypy and ShellCheck. The fixture covers eight filenames under both
core.quotePath=trueandfalse, including separated entries from one historical commit and both API entry points.Reproduction, Git evidence, and validation details
The new
tests/fixtures/blame-paths.shconstructs each original tree with C-quoted input togit mktree, renames its file tocurrent.txt, and edits only the middle of three lines. Using tree entries allows the test to represent control characters and arbitrary filename bytes on platforms that cannot check them out.The expected attribution is:
current.txtFor
café.txt, realgit blame --incremental HEAD -- current.txtemits these historical entries:The binary used for comparisons reports
git version 2.54.0 (Apple Git-157). Separately, the reference checkout at6de20f6092dcf9bdb1c8efe03db4b70c82b423dddocuments incremental output inDocumentation/git-blame.adoc:202and emits filenames withwrite_name_quoted()inbuiltin/blame.c:write_filename_info().Before the fix, the regression fails on both backends for six filename cases with quoting enabled and four with it disabled. Plain and space-containing filenames already pass, as do raw non-ASCII filenames with quoting disabled.
Validation commands, run with Python 3.12.14 and GixPython 0.3.0:
Windows CI with Git
2.56.0.windows.2showed that the original raw-control-byte fixture lost its carriage return. Its original commit ID,e5ed22d659d5aa4ff5f7527069752d20f74fc2d9, matches a local reproduction with that byte removed; retaining it yields73813b4d58d085503f9426c6e50ce5c201f715b6. Commitbb378dd7feeds special paths as Git C-quoted tree input so the control bytes remain escaped until Git constructs the tree. The focused regression passes locally on both backends with all sixteen subcases, and rawgit ls-tree -zconfirms the complete path. The production parser and expected results are unchanged.The Gix report records blame as
CLI: not converted, consistent with the existing GIX-15 compatibility limitation indoc/gix-backend.md. No native blame support or CLI reduction is claimed.An isolated
-k blameGix run exposes two existing mock tests that intercept a cold Git version probe. The broader repository/diff run passes those tests with the normal initialization order. The new real-Git regression passes in both selections.