Skip to content

Commit 273ebf6

Browse files
committed
line-log: fix crash when combined with pickaxe options
queue_diffs() calls diffcore_std() to detect renames so that line-level history can follow files across renames. When pickaxe options are present on the command line (-G and -S to filter by text pattern, --find-object to filter by object identity), diffcore_std() also runs diffcore_pickaxe(), which may discard diff pairs that are relevant for rename detection. Losing those pairs breaks rename following. Before a2bb801 (line-log: avoid unnecessary full tree diffs, 2019-08-21), diffcore_std() was only invoked when a rename was already suspected, so the pickaxe interference was unlikely in practice. That commit restructured queue_diffs() to gate both diffcore_std() and the surrounding filter_diffs_for_paths() calls behind diff_might_be_rename(). When pickaxe breaks rename following at one commit, a later commit may produce a deletion pair that bypasses this gate entirely, reaching process_diff_filepair() with an invalid filespec and triggering an assertion failure. Fix this by calling diffcore_rename() directly instead of diffcore_std(). The line-log machinery only needs rename detection from this call site; the other stages run by diffcore_std() (pickaxe, order, break/rewrite) are unnecessary here. Note that this only fixes the crash. The -G, -S, and --find-object options still have no effect on -L output because line-log uses its own commit-filtering logic that bypasses the normal pickaxe pipeline. Add tests that verify the crash is fixed and mark the silent-ignore behavior as known breakage for all three options. Reported-by: Matthew Hughes <matthewhughes934@gmail.com> Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
1 parent 67ad421 commit 273ebf6

File tree

2 files changed

+56
-1
lines changed

2 files changed

+56
-1
lines changed

line-log.c

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -865,7 +865,13 @@ static void queue_diffs(struct line_log_data *range,
865865
diff_tree_oid(parent_tree_oid, tree_oid, "", opt);
866866

867867
filter_diffs_for_paths(range, 1);
868-
diffcore_std(opt);
868+
/*
869+
* Call diffcore_rename() directly, as only rename
870+
* detection is needed. diffcore_std() would also run
871+
* pickaxe, which may discard pairs needed for rename
872+
* detection and break rename following.
873+
*/
874+
diffcore_rename(opt);
869875
filter_diffs_for_paths(range, 0);
870876
}
871877
move_diff_queue(queue, &diff_queued_diff);

t/t4211-line-log.sh

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -367,4 +367,53 @@ test_expect_success 'show line-log with graph' '
367367
test_cmp expect actual
368368
'
369369

370+
test_expect_success 'setup for -L with -G/-S/--find-object and a merge with rename' '
371+
git checkout --orphan pickaxe-rename &&
372+
git reset --hard &&
373+
374+
echo content >file &&
375+
git add file &&
376+
git commit -m "add file" &&
377+
378+
git checkout -b pickaxe-rename-side &&
379+
git mv file renamed-file &&
380+
git commit -m "rename file" &&
381+
382+
git checkout pickaxe-rename &&
383+
git commit --allow-empty -m "diverge" &&
384+
git merge --no-edit pickaxe-rename-side &&
385+
386+
git mv renamed-file file &&
387+
git commit -m "rename back"
388+
'
389+
390+
test_expect_success '-L -G does not crash with merge and rename' '
391+
git log --format="%s" --no-patch -L 1,1:file -G "." >actual
392+
'
393+
394+
test_expect_success '-L -S does not crash with merge and rename' '
395+
git log --format="%s" --no-patch -L 1,1:file -S content >actual
396+
'
397+
398+
test_expect_success '-L --find-object does not crash with merge and rename' '
399+
git log --format="%s" --no-patch -L 1,1:file \
400+
--find-object=$(git rev-parse HEAD:file) >actual
401+
'
402+
403+
test_expect_failure '-L -G should filter commits by pattern' '
404+
git log --format="%s" --no-patch -L 1,1:file -G "nomatch" >actual &&
405+
test_must_be_empty actual
406+
'
407+
408+
test_expect_failure '-L -S should filter commits by pattern' '
409+
git log --format="%s" --no-patch -L 1,1:file -S "nomatch" >actual &&
410+
test_must_be_empty actual
411+
'
412+
413+
test_expect_failure '-L --find-object should filter commits by object' '
414+
git log --format="%s" --no-patch -L 1,1:file \
415+
--find-object=$ZERO_OID >actual &&
416+
test_must_be_empty actual
417+
'
418+
370419
test_done

0 commit comments

Comments
 (0)