From: "Michael Montalbo via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Michael Montalbo <mmontalbo@gmail.com>,
Michael Montalbo <mmontalbo@gmail.com>
Subject: [PATCH 1/4] line-log: fix crash when combined with pickaxe options
Date: Sat, 07 Mar 2026 01:02:15 +0000 [thread overview]
Message-ID: <e7b8cc2c78b0e67420a46dae8fd444dfe925a6ec.1772845338.git.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.2065.git.1772845338.gitgitgadget@gmail.com>
From: Michael Montalbo <mmontalbo@gmail.com>
queue_diffs() passes the caller's diff_options, which may carry
user-specified pickaxe state, to diff_tree_oid() and diffcore_std()
when detecting renames for line-level history tracking. 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), this silently truncated history at rename boundaries.
That commit moved filter_diffs_for_paths() inside the rename-
detection block, so it only runs when diff_might_be_rename() returns
true. When pickaxe discards a rename pair, the rename goes
undetected, and a deletion pair at a subsequent commit passes
through uncleaned, reaching process_diff_filepair() with an invalid
filespec and triggering an assertion failure.
Fix this by building a private diff_options for the rename-detection
path inside queue_diffs(), following the same pattern used by blame's
find_rename(). This isolates the rename machinery from unrelated
user-specified options.
Reported-by: Matthew Hughes <matthewhughes934@gmail.com>
Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
---
line-log.c | 22 ++++++++++++++++----
t/t4211-line-log.sh | 49 +++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 67 insertions(+), 4 deletions(-)
diff --git a/line-log.c b/line-log.c
index eeaf68454e..9d12ece181 100644
--- a/line-log.c
+++ b/line-log.c
@@ -858,15 +858,29 @@ static void queue_diffs(struct line_log_data *range,
diff_queue_clear(&diff_queued_diff);
diff_tree_oid(parent_tree_oid, tree_oid, "", opt);
if (opt->detect_rename && diff_might_be_rename()) {
+ struct diff_options rename_opts;
+
+ /*
+ * Build a private diff_options for rename detection so
+ * that any user-specified options on the original opts
+ * (e.g. pickaxe) cannot discard diff pairs needed for
+ * rename tracking. Similar to blame's find_rename().
+ */
+ repo_diff_setup(opt->repo, &rename_opts);
+ rename_opts.flags.recursive = 1;
+ rename_opts.detect_rename = opt->detect_rename;
+ rename_opts.rename_score = opt->rename_score;
+ rename_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
+ diff_setup_done(&rename_opts);
+
/* must look at the full tree diff to detect renames */
- clear_pathspec(&opt->pathspec);
diff_queue_clear(&diff_queued_diff);
-
- diff_tree_oid(parent_tree_oid, tree_oid, "", opt);
+ diff_tree_oid(parent_tree_oid, tree_oid, "", &rename_opts);
filter_diffs_for_paths(range, 1);
- diffcore_std(opt);
+ diffcore_std(&rename_opts);
filter_diffs_for_paths(range, 0);
+ diff_free(&rename_opts);
}
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
next prev parent reply other threads:[~2026-03-07 1:02 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-07 1:02 [PATCH 0/4] line-log: route -L output through the standard diff pipeline Michael Montalbo via GitGitGadget
2026-03-07 1:02 ` Michael Montalbo via GitGitGadget [this message]
2026-03-07 1:02 ` [PATCH 2/4] " Michael Montalbo via GitGitGadget
2026-03-07 1:02 ` [PATCH 3/4] t4211: add tests for -L with standard diff options Michael Montalbo via GitGitGadget
2026-03-07 1:02 ` [PATCH 4/4] doc: note that -L supports patch formatting and pickaxe options Michael Montalbo via GitGitGadget
2026-03-11 8:41 ` Kristoffer Haugsbakk
2026-03-11 17:35 ` Michael Montalbo
2026-03-07 1:28 ` [PATCH 0/4] line-log: route -L output through the standard diff pipeline Junio C Hamano
2026-03-07 1:37 ` Michael Montalbo
2026-03-07 2:05 ` Junio C Hamano
2026-03-07 2:10 ` Michael Montalbo
2026-03-17 2:21 ` [PATCH v2 " Michael Montalbo via GitGitGadget
2026-03-17 2:21 ` [PATCH v2 1/4] line-log: fix crash when combined with pickaxe options Michael Montalbo via GitGitGadget
2026-03-17 2:21 ` [PATCH v2 2/4] line-log: route -L output through the standard diff pipeline Michael Montalbo via GitGitGadget
2026-03-17 20:52 ` Junio C Hamano
2026-03-17 2:21 ` [PATCH v2 3/4] t4211: add tests for -L with standard diff options Michael Montalbo via GitGitGadget
2026-03-17 2:21 ` [PATCH v2 4/4] doc: note that -L supports patch formatting and pickaxe options Michael Montalbo via GitGitGadget
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e7b8cc2c78b0e67420a46dae8fd444dfe925a6ec.1772845338.git.gitgitgadget@gmail.com \
--to=gitgitgadget@gmail.com \
--cc=git@vger.kernel.org \
--cc=mmontalbo@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox