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: 18+ 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
2026-03-31 21:49 ` [PATCH v2 0/4] line-log: route -L output through the standard diff pipeline Junio C Hamano
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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.