[PATCH v2 1/2] line-log: fix crash when combined with pickaxe options

["Michael Montalbo via GitGitGadget" <gitgitgadget@gmail.com>]

From: Michael Montalbo <mmontalbo@gmail.com>

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 a2bb801f6a (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>
---
 line-log.c          |  8 +++++++-
 t/t4211-line-log.sh | 49 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+), 1 deletion(-)

diff --git a/line-log.c b/line-log.c
index 8bd422148d..8a404f5c22 100644
--- a/line-log.c
+++ b/line-log.c
@@ -865,7 +865,13 @@ static void queue_diffs(struct line_log_data *range,
 		diff_tree_oid(parent_tree_oid, tree_oid, "", opt);
 
 		filter_diffs_for_paths(range, 1);
-		diffcore_std(opt);
+		/*
+		 * Call diffcore_rename() directly, as only rename
+		 * detection is needed.  diffcore_std() would also run
+		 * pickaxe, which may discard pairs needed for rename
+		 * detection and break rename following.
+		 */
+		diffcore_rename(opt);
 		filter_diffs_for_paths(range, 0);
 	}
 	move_diff_queue(queue, &diff_queued_diff);
diff --git a/t/t4211-line-log.sh b/t/t4211-line-log.sh
index 0a7c3ca42f..7acc38f72d 100755
--- a/t/t4211-line-log.sh
+++ b/t/t4211-line-log.sh
@@ -367,4 +367,53 @@ test_expect_success 'show line-log with graph' '
 	test_cmp expect actual
 '
 
+test_expect_success 'setup for -L with -G/-S/--find-object and a merge with rename' '
+	git checkout --orphan pickaxe-rename &&
+	git reset --hard &&
+
+	echo content >file &&
+	git add file &&
+	git commit -m "add file" &&
+
+	git checkout -b pickaxe-rename-side &&
+	git mv file renamed-file &&
+	git commit -m "rename file" &&
+
+	git checkout pickaxe-rename &&
+	git commit --allow-empty -m "diverge" &&
+	git merge --no-edit pickaxe-rename-side &&
+
+	git mv renamed-file file &&
+	git commit -m "rename back"
+'
+
+test_expect_success '-L -G does not crash with merge and rename' '
+	git log --format="%s" --no-patch -L 1,1:file -G "." >actual
+'
+
+test_expect_success '-L -S does not crash with merge and rename' '
+	git log --format="%s" --no-patch -L 1,1:file -S content >actual
+'
+
+test_expect_success '-L --find-object does not crash with merge and rename' '
+	git log --format="%s" --no-patch -L 1,1:file \
+		--find-object=$(git rev-parse HEAD:file) >actual
+'
+
+test_expect_failure '-L -G should filter commits by pattern' '
+	git log --format="%s" --no-patch -L 1,1:file -G "nomatch" >actual &&
+	test_must_be_empty actual
+'
+
+test_expect_failure '-L -S should filter commits by pattern' '
+	git log --format="%s" --no-patch -L 1,1:file -S "nomatch" >actual &&
+	test_must_be_empty actual
+'
+
+test_expect_failure '-L --find-object should filter commits by object' '
+	git log --format="%s" --no-patch -L 1,1:file \
+		--find-object=$ZERO_OID >actual &&
+	test_must_be_empty actual
+'
+
 test_done
-- 
gitgitgadget