Repository navigation
Conversation
Compare the path's tree entries to determine whether the commit modified the path compared to its parents. Only if the path is missing in a parent is it useful to run rename detection since it doesn't consider modified files as targets. Similarly, an unchanged path can reuse the existing blob.
rene-langer
added a commit
to rene-langer/browsitory
that referenced
this pull request
Nov 12, 2026
* Tests: blame owners per line, compared with git blame (#839) Blame is to move off libgit2's `git_blame_file`, which diffs both commits' whole trees at every commit of history (#839). These tests write out the owner of every line and, when the git CLI is available, compare with `git blame --porcelain`: edits, insertions and deletions, a rename with an edit, a renamed folder, a merge with a resolution line, a merge that took one side, a re-add after a delete, CRLF lines without a final newline, and an empty file. They pass on libgit2's blame. The last one removes an unrelated folder's objects from the object database. Blame should read only the file's path, but libgit2's full tree diff at the commit that changed the file can't read the folder, finds no origin in the parent and gives that commit every line. Red run: $ cargo test -p git-core --test blame test blame_reads_only_the_files_path_not_the_whole_tree ... FAILED thread 'blame_reads_only_the_files_path_not_the_whole_tree' panicked at crates/git-core/tests/blame.rs:413:5: assertion `left == right` failed left: [("d84d8b8fe494d003bcf35d6dc8b1e0050c735033", "a"), ("d84d8b8fe494d003bcf35d6dc8b1e0050c735033", "B")] right: [("59e91e794b77ec6a19b6c4aef10ec8a46b309063", "a"), ("d84d8b8fe494d003bcf35d6dc8b1e0050c735033", "B")] test result: FAILED. 12 passed; 1 failed Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A4tHCJjnzi8pBrcnnZcgkk * Blame on top of the file's path walk instead of libgit2's blame (#839) libgit2's git_blame_file reads and parses both commits' root trees about three times per commit and runs a pathspec tree diff (with a config snapshot) just to learn that the file didn't change, then diffs the two commits' whole trees wherever it did. On a 130k-commit repository that was about a minute. The upstream fix, libgit2/libgit2#7472, is unmerged and in no release. blame::blame_file now assigns lines itself, the way git and libgit2 do: - lines still to explain sit with a suspect (commit, path, line numbers there), taken in topological order; - a parent with the identical file takes all of them (the first such parent); - otherwise each parent that has the file (same path, or the rename source libgit2's rename detection finds), skipping one with the same version as an earlier parent, takes the lines a zero-context diff leaves unchanged; - the rest belong to the commit. The walk compares the path's tree entries top-down (#838's `parent_path_entries`), so it reads about one root-tree entry per commit and diffs blobs only where the file changed. BlameLine and the wire format are unchanged; libgit2's blame is no longer called. Not yet: the diff doesn't trim the versions' common tail first as xdiff's blame does (`trim_common_tail` is an identity stub here), which decides where some ambiguous changes land; the next commits test and add it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A4tHCJjnzi8pBrcnnZcgkk * Tests: blame trims the versions' common tail before diffing, as xdiff's blame does (#839) git's and libgit2's blame drop the 1 KiB blocks both versions end with before diffing, then give back the last block up to its first newline (`trim_common_tail`). It decides where some ambiguous changes land: without it, blame gave 34 of 43,537 lines in 87 files of this repository to another commit than libgit2 and git (pairs of equal lines swapped). These tests pin the trimming against the identity stub. Red run: $ cargo test -p git-core --lib blame::tests test blame::tests::a_common_tail_under_a_block_is_kept ... ok test blame::tests::a_common_block_without_a_newline_is_trimmed_whole ... FAILED test blame::tests::whole_common_blocks_are_trimmed_back_to_the_first_newline_in_them ... FAILED test blame::tests::a_tail_that_differs_in_its_last_block_is_kept ... ok thread 'blame::tests::a_common_block_without_a_newline_is_trimmed_whole' panicked at crates/git-core/src/blame.rs:353:9: assertion `left == right` failed left: ([65, 10, 120, 120, ... (the whole 1026 bytes)], [66, 10, 120, ...]) right: ([65, 10], [66, 10]) thread 'blame::tests::whole_common_blocks_are_trimmed_back_to_the_first_newline_in_them' panicked at crates/git-core/src/blame.rs:331:9: assertion `left == right` failed left: [65, 10, 108, 105, 110, 101, 10, ... (the whole 2052 bytes)] right: [65, 10, 108, 105, 110, 101, 10] test result: FAILED. 2 passed; 2 failed Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A4tHCJjnzi8pBrcnnZcgkk * Test change: a common tail block without a newline is given back whole (#839) The previous commit expected xdiff's trim_common_tail to drop a common 1 KiB block that has no newline. It doesn't: it gives the trimmed blocks back up to their first newline (`while (recovered < trimmed) if (ap[recovered++] == '\n') break;`), so with no newline the whole block comes back and nothing is trimmed. The expectation was wrong, not the port; the test now asserts xdiff's behaviour. It passes on the identity stub too, as it should, and the other trimming test still fails on it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A4tHCJjnzi8pBrcnnZcgkk * Blame trims the versions' common tail before diffing, as xdiff's blame does (#839) A port of xdiff's trim_common_tail, which git's and libgit2's blame apply before each diff. With it, blame gives every line of this repository's 1,188 tracked source files (383,778 lines) the same commit as libgit2's blame, except the 40 lines of one file where libgit2 differs from `git blame` and this agrees with git (see the PR). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A4tHCJjnzi8pBrcnnZcgkk * docs: blame and the history walks in ARCHITECTURE.md (#839) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A4tHCJjnzi8pBrcnnZcgkk * Tests: blame's walk order and lines reaching one commit under two paths (#839) The mutants run on #865 missed two mutants that change results, now caught: - blame.rs:130 `Sort::TOPOLOGICAL | Sort::TIME` -> `&` (no sorting): a commit reached through a short and a long path was examined before the long path's lines arrived. `a_commit_reached_by_paths_of_different_lengths_gets_the_lines_of_both` fails on it: panicked at crates/git-core/tests/blame.rs:435:5 (assertion `left == right` failed) - blame.rs:231 `s.path == path` -> `!=` in add_suspect: lines reaching one commit under two paths (two renames merged) were merged into the other path's suspect. `lines_reaching_one_commit_under_two_paths_keep_their_own_paths` fails on it: panicked at crates/git-core/tests/blame.rs:493:5 (assertion `left == right` failed) Both tests compare with `git blame` too, and pass on the current code. The third missed mutant, `|` -> `^` at blame.rs:130, is equivalent: the two flags share no bits, so `^` is `|`. The timed-out `+=` -> `*=` in trim_common_tail (blame.rs:280) never ends the loop (trimmed stays 0), which the run counted as a timeout, not a pass. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A4tHCJjnzi8pBrcnnZcgkk --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This branch has not been deployed
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.
This applies various tricks in
git_blame__get_originto avoid diffing trees until its absolutely necessary. By avoiding unnecessary calls togit_diff_find_similar, the performance is vastly improved in scenarios never requiring or benefiting from rename detection. If the path is untouched, or only modified (instead of added), rename detection will not change the result since modified files are not considered targets. Similarly, by avoiding creating diffs, far less data is required unpacked. This thus minimises costly hashing and decompressing.On my machine
lg2 blame CMakeLists.txt(in this repo) runs in ~100ms instead of ~3.1sec. Since that is a file that has never moved, this is the best case scenario improvement. If I run blame onsrc/libgit2/blame.c, which is a file that has moved in the past, there's only a modest ~1.9s to ~1.4s improvement.Relates to #3027