{"thread":{"id":"65135","subject":"[PATCH 0/2] line-log: fix -L with pickaxe options","startedAt":"2026-03-04T19:11:27Z","lastAt":"2026-03-04T22:36:42Z","messageCount":10,"participants":["Michael Montalbo via GitGitGadget","Junio C Hamano","Michael Montalbo"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"537825","messageId":"pull.2061.git.1772651484.gitgitgadget@gmail.com","threadId":"65135","inReplyTo":null,"subject":"[PATCH 0/2] line-log: fix -L with pickaxe options","fromName":"Michael Montalbo via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-03-04T19:11:22Z","receivedAt":"2026-03-04T19:11:27Z","isPatch":true,"sender":{"key":"mmontalbo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/498667?v=4"},"body":"This series fixes a crash in git log -L when combined with pickaxe options\n(-G, -S, or --find-object) on a history involving renames, reported in [1].\n\nThe crash bisects to a2bb801f6a (line-log: avoid unnecessary full tree\ndiffs, 2019-08-21), which made the diffcore_std() call in queue_diffs()\nunconditional. Before that commit, the same combination silently truncated\nhistory at rename boundaries rather than crashing. The root cause is that\ndiffcore_std() runs diffcore_pickaxe(), which may discard diff pairs needed\nfor rename detection.\n\nPatch 1 fixes the crash by calling diffcore_rename() directly instead of\ndiffcore_std(), and adds tests including known-breakage markers showing that\nthe pickaxe options are silently ignored by -L.\n\nPatch 2 explicitly rejects the unsupported combination with die(), replacing\nthe known-breakage tests with rejection tests.\n\n[1] https://lore.kernel.org/git/aac-QdjY1ohAqgw_@desktop/\n\nMichael Montalbo (2):\n  line-log: fix crash when combined with pickaxe options\n  log: reject pickaxe options when combined with -L\n\n builtin/log.c       |  4 ++++\n line-log.c          |  8 +++++++-\n t/t4211-line-log.sh | 15 +++++++++++++++\n 3 files changed, 26 insertions(+), 1 deletion(-)\n\n\nbase-commit: 67ad42147a7acc2af6074753ebd03d904476118f\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-2061%2Fmmontalbo%2Ffix-line-log-G-crash-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2061/mmontalbo/fix-line-log-G-crash-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/2061\n-- \ngitgitgadget\n"},{"id":"537826","messageId":"6e97d88993dbab4070ac0aa999f70564368f47b1.1772651484.git.gitgitgadget@gmail.com","threadId":"65135","inReplyTo":"pull.2061.git.1772651484.gitgitgadget@gmail.com","subject":"[PATCH 1/2] line-log: fix crash when combined with pickaxe options","fromName":"Michael Montalbo via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-03-04T19:11:23Z","receivedAt":"2026-03-04T19:11:29Z","isPatch":true,"sender":{"key":"mmontalbo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/498667?v=4"},"body":"From: Michael Montalbo <mmontalbo@gmail.com>\n\nqueue_diffs() calls diffcore_std() to detect renames so that line-level\nhistory can follow files across renames.  When pickaxe options are\npresent on the command line (-G and -S to filter by text pattern,\n--find-object to filter by object identity), diffcore_std() also runs\ndiffcore_pickaxe(), which may discard diff pairs that are relevant for\nrename detection.  Losing those pairs breaks rename following.\n\nBefore a2bb801f6a (line-log: avoid unnecessary full tree diffs,\n2019-08-21), diffcore_std() was only invoked when a rename was already\nsuspected, so the pickaxe interference was unlikely in practice.  That\ncommit made the diffcore_std() call unconditional, and with\nfilter_diffs_for_paths() now framing that call, a queue pruned by\npickaxe violates filter_diffs_for_paths()'s expectation that diff\npairs correspond to tracked paths, triggering an assertion failure.\n\nFix this by calling diffcore_rename() directly instead of\ndiffcore_std().  The line-log machinery only needs rename detection\nfrom this call site; the other stages run by diffcore_std() (pickaxe,\norder, break/rewrite) are unnecessary here.\n\nNote that this only fixes the crash.  The -G, -S, and --find-object\noptions still have no effect on -L output because line-log uses its\nown commit-filtering logic that bypasses the normal pickaxe pipeline.\nAdd tests that verify the crash is fixed and mark the silent-ignore\nbehavior as known breakage for all three options.\n\nReported-by: Matthew Hughes <matthewhughes934@gmail.com>\nSigned-off-by: Michael Montalbo <mmontalbo@gmail.com>\n---\n line-log.c          |  8 +++++++-\n t/t4211-line-log.sh | 49 +++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 56 insertions(+), 1 deletion(-)\n\ndiff --git a/line-log.c b/line-log.c\nindex 8bd422148d..8a404f5c22 100644\n--- a/line-log.c\n+++ b/line-log.c\n@@ -865,7 +865,13 @@ static void queue_diffs(struct line_log_data *range,\n \t\tdiff_tree_oid(parent_tree_oid, tree_oid, \"\", opt);\n \n \t\tfilter_diffs_for_paths(range, 1);\n-\t\tdiffcore_std(opt);\n+\t\t/*\n+\t\t * Call diffcore_rename() directly, as only rename\n+\t\t * detection is needed.  diffcore_std() would also run\n+\t\t * pickaxe, which may discard pairs needed for rename\n+\t\t * detection and break rename following.\n+\t\t */\n+\t\tdiffcore_rename(opt);\n \t\tfilter_diffs_for_paths(range, 0);\n \t}\n \tmove_diff_queue(queue, &diff_queued_diff);\ndiff --git a/t/t4211-line-log.sh b/t/t4211-line-log.sh\nindex 0a7c3ca42f..7acc38f72d 100755\n--- a/t/t4211-line-log.sh\n+++ b/t/t4211-line-log.sh\n@@ -367,4 +367,53 @@ test_expect_success 'show line-log with graph' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'setup for -L with -G/-S/--find-object and a merge with rename' '\n+\tgit checkout --orphan pickaxe-rename &&\n+\tgit reset --hard &&\n+\n+\techo content >file &&\n+\tgit add file &&\n+\tgit commit -m \"add file\" &&\n+\n+\tgit checkout -b pickaxe-rename-side &&\n+\tgit mv file renamed-file &&\n+\tgit commit -m \"rename file\" &&\n+\n+\tgit checkout pickaxe-rename &&\n+\tgit commit --allow-empty -m \"diverge\" &&\n+\tgit merge --no-edit pickaxe-rename-side &&\n+\n+\tgit mv renamed-file file &&\n+\tgit commit -m \"rename back\"\n+'\n+\n+test_expect_success '-L -G does not crash with merge and rename' '\n+\tgit log --format=\"%s\" --no-patch -L 1,1:file -G \".\" >actual\n+'\n+\n+test_expect_success '-L -S does not crash with merge and rename' '\n+\tgit log --format=\"%s\" --no-patch -L 1,1:file -S content >actual\n+'\n+\n+test_expect_success '-L --find-object does not crash with merge and rename' '\n+\tgit log --format=\"%s\" --no-patch -L 1,1:file \\\n+\t\t--find-object=$(git rev-parse HEAD:file) >actual\n+'\n+\n+test_expect_failure '-L -G should filter commits by pattern' '\n+\tgit log --format=\"%s\" --no-patch -L 1,1:file -G \"nomatch\" >actual &&\n+\ttest_must_be_empty actual\n+'\n+\n+test_expect_failure '-L -S should filter commits by pattern' '\n+\tgit log --format=\"%s\" --no-patch -L 1,1:file -S \"nomatch\" >actual &&\n+\ttest_must_be_empty actual\n+'\n+\n+test_expect_failure '-L --find-object should filter commits by object' '\n+\tgit log --format=\"%s\" --no-patch -L 1,1:file \\\n+\t\t--find-object=$ZERO_OID >actual &&\n+\ttest_must_be_empty actual\n+'\n+\n test_done\n-- \ngitgitgadget\n\n"},{"id":"537827","messageId":"ae5269af0b08d75bcad38b0263debe4e23e479a8.1772651484.git.gitgitgadget@gmail.com","threadId":"65135","inReplyTo":"pull.2061.git.1772651484.gitgitgadget@gmail.com","subject":"[PATCH 2/2] log: reject pickaxe options when combined with -L","fromName":"Michael Montalbo via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-03-04T19:11:24Z","receivedAt":"2026-03-04T19:11:30Z","isPatch":true,"sender":{"key":"mmontalbo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/498667?v=4"},"body":"From: Michael Montalbo <mmontalbo@gmail.com>\n\nThe previous commit fixed a crash when -G, -S, or --find-object was\nused together with -L and rename detection.  However, these options\nstill have no effect on -L output: line-log uses its own\ncommit-filtering logic in line_log_filter() and never consults the\npickaxe machinery.  Rather than silently ignoring these options, reject\nthe combination with a clear error message.\n\nThis replaces the known-breakage tests from the previous commit with\ntests that verify the rejection for all three options.  A future series\ncould teach line-log to honor these options and remove this restriction.\n\nSigned-off-by: Michael Montalbo <mmontalbo@gmail.com>\n---\n builtin/log.c       |  4 ++++\n t/t4211-line-log.sh | 52 ++++++++-------------------------------------\n 2 files changed, 13 insertions(+), 43 deletions(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 5c9a8ef363..44e2399d59 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -317,6 +317,10 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,\n \tif (rev->line_level_traverse && rev->prune_data.nr)\n \t\tdie(_(\"-L<range>:<file> cannot be used with pathspec\"));\n \n+\tif (rev->line_level_traverse &&\n+\t    (rev->diffopt.pickaxe_opts & DIFF_PICKAXE_KINDS_MASK))\n+\t\tdie(_(\"-L does not yet support -G, -S, or --find-object\"));\n+\n \tmemset(&w, 0, sizeof(w));\n \tuserformat_find_requirements(NULL, &w);\n \ndiff --git a/t/t4211-line-log.sh b/t/t4211-line-log.sh\nindex 7acc38f72d..8ebc73d2d9 100755\n--- a/t/t4211-line-log.sh\n+++ b/t/t4211-line-log.sh\n@@ -367,53 +367,19 @@ test_expect_success 'show line-log with graph' '\n \ttest_cmp expect actual\n '\n \n-test_expect_success 'setup for -L with -G/-S/--find-object and a merge with rename' '\n-\tgit checkout --orphan pickaxe-rename &&\n-\tgit reset --hard &&\n-\n-\techo content >file &&\n-\tgit add file &&\n-\tgit commit -m \"add file\" &&\n-\n-\tgit checkout -b pickaxe-rename-side &&\n-\tgit mv file renamed-file &&\n-\tgit commit -m \"rename file\" &&\n-\n-\tgit checkout pickaxe-rename &&\n-\tgit commit --allow-empty -m \"diverge\" &&\n-\tgit merge --no-edit pickaxe-rename-side &&\n-\n-\tgit mv renamed-file file &&\n-\tgit commit -m \"rename back\"\n-'\n-\n-test_expect_success '-L -G does not crash with merge and rename' '\n-\tgit log --format=\"%s\" --no-patch -L 1,1:file -G \".\" >actual\n-'\n-\n-test_expect_success '-L -S does not crash with merge and rename' '\n-\tgit log --format=\"%s\" --no-patch -L 1,1:file -S content >actual\n-'\n-\n-test_expect_success '-L --find-object does not crash with merge and rename' '\n-\tgit log --format=\"%s\" --no-patch -L 1,1:file \\\n-\t\t--find-object=$(git rev-parse HEAD:file) >actual\n-'\n-\n-test_expect_failure '-L -G should filter commits by pattern' '\n-\tgit log --format=\"%s\" --no-patch -L 1,1:file -G \"nomatch\" >actual &&\n-\ttest_must_be_empty actual\n+test_expect_success '-L with -G is rejected' '\n+\ttest_must_fail git log -L 1,1:a.c -G \"pattern\" 2>err &&\n+\ttest_grep \"does not yet support\" err\n '\n \n-test_expect_failure '-L -S should filter commits by pattern' '\n-\tgit log --format=\"%s\" --no-patch -L 1,1:file -S \"nomatch\" >actual &&\n-\ttest_must_be_empty actual\n+test_expect_success '-L with -S is rejected' '\n+\ttest_must_fail git log -L 1,1:a.c -S \"pattern\" 2>err &&\n+\ttest_grep \"does not yet support\" err\n '\n \n-test_expect_failure '-L --find-object should filter commits by object' '\n-\tgit log --format=\"%s\" --no-patch -L 1,1:file \\\n-\t\t--find-object=$ZERO_OID >actual &&\n-\ttest_must_be_empty actual\n+test_expect_success '-L with --find-object is rejected' '\n+\ttest_must_fail git log -L 1,1:a.c --find-object=HEAD 2>err &&\n+\ttest_grep \"does not yet support\" err\n '\n \n test_done\n-- \ngitgitgadget\n"},{"id":"537828","messageId":"pull.2061.v2.git.1772652091.gitgitgadget@gmail.com","threadId":"65135","inReplyTo":"pull.2061.git.1772651484.gitgitgadget@gmail.com","subject":"[PATCH v2 0/2] line-log: fix -L with pickaxe options","fromName":"Michael Montalbo via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-03-04T19:21:29Z","receivedAt":"2026-03-04T19:21:34Z","isPatch":true,"sender":{"key":"mmontalbo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/498667?v=4"},"body":"This series fixes a crash in git log -L when combined with pickaxe options\n(-G, -S, or --find-object) on a history involving renames, reported in [1].\n\nThe crash bisects to a2bb801f6a (line-log: avoid unnecessary full tree\ndiffs, 2019-08-21). Before that commit, the same combination silently\ntruncated history at rename boundaries rather than crashing. The root cause\nis that diffcore_std() runs diffcore_pickaxe(), which may discard diff pairs\nneeded for rename detection.\n\nPatch 1 fixes the crash by calling diffcore_rename() directly instead of\ndiffcore_std(), and adds tests including known-breakage markers showing that\nthe pickaxe options are silently ignored by -L.\n\nPatch 2 explicitly rejects the unsupported combination with die(), replacing\nthe known-breakage tests with rejection tests.\n\n[1] https://lore.kernel.org/git/aac-QdjY1ohAqgw_@desktop/\n\nMichael Montalbo (2):\n  line-log: fix crash when combined with pickaxe options\n  log: reject pickaxe options when combined with -L\n\n builtin/log.c       |  4 ++++\n line-log.c          |  8 +++++++-\n t/t4211-line-log.sh | 15 +++++++++++++++\n 3 files changed, 26 insertions(+), 1 deletion(-)\n\n\nbase-commit: 67ad42147a7acc2af6074753ebd03d904476118f\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-2061%2Fmmontalbo%2Ffix-line-log-G-crash-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2061/mmontalbo/fix-line-log-G-crash-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/2061\n\nRange-diff vs v1:\n\n 1:  6e97d88993 ! 1:  273ebf640d line-log: fix crash when combined with pickaxe options\n     @@ Commit message\n          Before a2bb801f6a (line-log: avoid unnecessary full tree diffs,\n          2019-08-21), diffcore_std() was only invoked when a rename was already\n          suspected, so the pickaxe interference was unlikely in practice.  That\n     -    commit made the diffcore_std() call unconditional, and with\n     -    filter_diffs_for_paths() now framing that call, a queue pruned by\n     -    pickaxe violates filter_diffs_for_paths()'s expectation that diff\n     -    pairs correspond to tracked paths, triggering an assertion failure.\n     +    commit restructured queue_diffs() to gate both diffcore_std() and the\n     +    surrounding filter_diffs_for_paths() calls behind\n     +    diff_might_be_rename().  When pickaxe breaks rename following at one\n     +    commit, a later commit may produce a deletion pair that bypasses this\n     +    gate entirely, reaching process_diff_filepair() with an invalid\n     +    filespec and triggering an assertion failure.\n      \n          Fix this by calling diffcore_rename() directly instead of\n          diffcore_std().  The line-log machinery only needs rename detection\n 2:  ae5269af0b = 2:  81cb521401 log: reject pickaxe options when combined with -L\n\n-- \ngitgitgadget\n"},{"id":"537829","messageId":"273ebf640da562d9a20daec530e82968232e99bf.1772652091.git.gitgitgadget@gmail.com","threadId":"65135","inReplyTo":"pull.2061.v2.git.1772652091.gitgitgadget@gmail.com","subject":"[PATCH v2 1/2] line-log: fix crash when combined with pickaxe options","fromName":"Michael Montalbo via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-03-04T19:21:30Z","receivedAt":"2026-03-04T19:21:35Z","isPatch":true,"sender":{"key":"mmontalbo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/498667?v=4"},"body":"From: Michael Montalbo <mmontalbo@gmail.com>\n\nqueue_diffs() calls diffcore_std() to detect renames so that line-level\nhistory can follow files across renames.  When pickaxe options are\npresent on the command line (-G and -S to filter by text pattern,\n--find-object to filter by object identity), diffcore_std() also runs\ndiffcore_pickaxe(), which may discard diff pairs that are relevant for\nrename detection.  Losing those pairs breaks rename following.\n\nBefore a2bb801f6a (line-log: avoid unnecessary full tree diffs,\n2019-08-21), diffcore_std() was only invoked when a rename was already\nsuspected, so the pickaxe interference was unlikely in practice.  That\ncommit restructured queue_diffs() to gate both diffcore_std() and the\nsurrounding filter_diffs_for_paths() calls behind\ndiff_might_be_rename().  When pickaxe breaks rename following at one\ncommit, a later commit may produce a deletion pair that bypasses this\ngate entirely, reaching process_diff_filepair() with an invalid\nfilespec and triggering an assertion failure.\n\nFix this by calling diffcore_rename() directly instead of\ndiffcore_std().  The line-log machinery only needs rename detection\nfrom this call site; the other stages run by diffcore_std() (pickaxe,\norder, break/rewrite) are unnecessary here.\n\nNote that this only fixes the crash.  The -G, -S, and --find-object\noptions still have no effect on -L output because line-log uses its\nown commit-filtering logic that bypasses the normal pickaxe pipeline.\nAdd tests that verify the crash is fixed and mark the silent-ignore\nbehavior as known breakage for all three options.\n\nReported-by: Matthew Hughes <matthewhughes934@gmail.com>\nSigned-off-by: Michael Montalbo <mmontalbo@gmail.com>\n---\n line-log.c          |  8 +++++++-\n t/t4211-line-log.sh | 49 +++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 56 insertions(+), 1 deletion(-)\n\ndiff --git a/line-log.c b/line-log.c\nindex 8bd422148d..8a404f5c22 100644\n--- a/line-log.c\n+++ b/line-log.c\n@@ -865,7 +865,13 @@ static void queue_diffs(struct line_log_data *range,\n \t\tdiff_tree_oid(parent_tree_oid, tree_oid, \"\", opt);\n \n \t\tfilter_diffs_for_paths(range, 1);\n-\t\tdiffcore_std(opt);\n+\t\t/*\n+\t\t * Call diffcore_rename() directly, as only rename\n+\t\t * detection is needed.  diffcore_std() would also run\n+\t\t * pickaxe, which may discard pairs needed for rename\n+\t\t * detection and break rename following.\n+\t\t */\n+\t\tdiffcore_rename(opt);\n \t\tfilter_diffs_for_paths(range, 0);\n \t}\n \tmove_diff_queue(queue, &diff_queued_diff);\ndiff --git a/t/t4211-line-log.sh b/t/t4211-line-log.sh\nindex 0a7c3ca42f..7acc38f72d 100755\n--- a/t/t4211-line-log.sh\n+++ b/t/t4211-line-log.sh\n@@ -367,4 +367,53 @@ test_expect_success 'show line-log with graph' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'setup for -L with -G/-S/--find-object and a merge with rename' '\n+\tgit checkout --orphan pickaxe-rename &&\n+\tgit reset --hard &&\n+\n+\techo content >file &&\n+\tgit add file &&\n+\tgit commit -m \"add file\" &&\n+\n+\tgit checkout -b pickaxe-rename-side &&\n+\tgit mv file renamed-file &&\n+\tgit commit -m \"rename file\" &&\n+\n+\tgit checkout pickaxe-rename &&\n+\tgit commit --allow-empty -m \"diverge\" &&\n+\tgit merge --no-edit pickaxe-rename-side &&\n+\n+\tgit mv renamed-file file &&\n+\tgit commit -m \"rename back\"\n+'\n+\n+test_expect_success '-L -G does not crash with merge and rename' '\n+\tgit log --format=\"%s\" --no-patch -L 1,1:file -G \".\" >actual\n+'\n+\n+test_expect_success '-L -S does not crash with merge and rename' '\n+\tgit log --format=\"%s\" --no-patch -L 1,1:file -S content >actual\n+'\n+\n+test_expect_success '-L --find-object does not crash with merge and rename' '\n+\tgit log --format=\"%s\" --no-patch -L 1,1:file \\\n+\t\t--find-object=$(git rev-parse HEAD:file) >actual\n+'\n+\n+test_expect_failure '-L -G should filter commits by pattern' '\n+\tgit log --format=\"%s\" --no-patch -L 1,1:file -G \"nomatch\" >actual &&\n+\ttest_must_be_empty actual\n+'\n+\n+test_expect_failure '-L -S should filter commits by pattern' '\n+\tgit log --format=\"%s\" --no-patch -L 1,1:file -S \"nomatch\" >actual &&\n+\ttest_must_be_empty actual\n+'\n+\n+test_expect_failure '-L --find-object should filter commits by object' '\n+\tgit log --format=\"%s\" --no-patch -L 1,1:file \\\n+\t\t--find-object=$ZERO_OID >actual &&\n+\ttest_must_be_empty actual\n+'\n+\n test_done\n-- \ngitgitgadget\n\n"},{"id":"537830","messageId":"81cb521401210bfbcd05f8201f75e93bccfba712.1772652091.git.gitgitgadget@gmail.com","threadId":"65135","inReplyTo":"pull.2061.v2.git.1772652091.gitgitgadget@gmail.com","subject":"[PATCH v2 2/2] log: reject pickaxe options when combined with -L","fromName":"Michael Montalbo via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-03-04T19:21:31Z","receivedAt":"2026-03-04T19:21:36Z","isPatch":true,"sender":{"key":"mmontalbo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/498667?v=4"},"body":"From: Michael Montalbo <mmontalbo@gmail.com>\n\nThe previous commit fixed a crash when -G, -S, or --find-object was\nused together with -L and rename detection.  However, these options\nstill have no effect on -L output: line-log uses its own\ncommit-filtering logic in line_log_filter() and never consults the\npickaxe machinery.  Rather than silently ignoring these options, reject\nthe combination with a clear error message.\n\nThis replaces the known-breakage tests from the previous commit with\ntests that verify the rejection for all three options.  A future series\ncould teach line-log to honor these options and remove this restriction.\n\nSigned-off-by: Michael Montalbo <mmontalbo@gmail.com>\n---\n builtin/log.c       |  4 ++++\n t/t4211-line-log.sh | 52 ++++++++-------------------------------------\n 2 files changed, 13 insertions(+), 43 deletions(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 5c9a8ef363..44e2399d59 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -317,6 +317,10 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,\n \tif (rev->line_level_traverse && rev->prune_data.nr)\n \t\tdie(_(\"-L<range>:<file> cannot be used with pathspec\"));\n \n+\tif (rev->line_level_traverse &&\n+\t    (rev->diffopt.pickaxe_opts & DIFF_PICKAXE_KINDS_MASK))\n+\t\tdie(_(\"-L does not yet support -G, -S, or --find-object\"));\n+\n \tmemset(&w, 0, sizeof(w));\n \tuserformat_find_requirements(NULL, &w);\n \ndiff --git a/t/t4211-line-log.sh b/t/t4211-line-log.sh\nindex 7acc38f72d..8ebc73d2d9 100755\n--- a/t/t4211-line-log.sh\n+++ b/t/t4211-line-log.sh\n@@ -367,53 +367,19 @@ test_expect_success 'show line-log with graph' '\n \ttest_cmp expect actual\n '\n \n-test_expect_success 'setup for -L with -G/-S/--find-object and a merge with rename' '\n-\tgit checkout --orphan pickaxe-rename &&\n-\tgit reset --hard &&\n-\n-\techo content >file &&\n-\tgit add file &&\n-\tgit commit -m \"add file\" &&\n-\n-\tgit checkout -b pickaxe-rename-side &&\n-\tgit mv file renamed-file &&\n-\tgit commit -m \"rename file\" &&\n-\n-\tgit checkout pickaxe-rename &&\n-\tgit commit --allow-empty -m \"diverge\" &&\n-\tgit merge --no-edit pickaxe-rename-side &&\n-\n-\tgit mv renamed-file file &&\n-\tgit commit -m \"rename back\"\n-'\n-\n-test_expect_success '-L -G does not crash with merge and rename' '\n-\tgit log --format=\"%s\" --no-patch -L 1,1:file -G \".\" >actual\n-'\n-\n-test_expect_success '-L -S does not crash with merge and rename' '\n-\tgit log --format=\"%s\" --no-patch -L 1,1:file -S content >actual\n-'\n-\n-test_expect_success '-L --find-object does not crash with merge and rename' '\n-\tgit log --format=\"%s\" --no-patch -L 1,1:file \\\n-\t\t--find-object=$(git rev-parse HEAD:file) >actual\n-'\n-\n-test_expect_failure '-L -G should filter commits by pattern' '\n-\tgit log --format=\"%s\" --no-patch -L 1,1:file -G \"nomatch\" >actual &&\n-\ttest_must_be_empty actual\n+test_expect_success '-L with -G is rejected' '\n+\ttest_must_fail git log -L 1,1:a.c -G \"pattern\" 2>err &&\n+\ttest_grep \"does not yet support\" err\n '\n \n-test_expect_failure '-L -S should filter commits by pattern' '\n-\tgit log --format=\"%s\" --no-patch -L 1,1:file -S \"nomatch\" >actual &&\n-\ttest_must_be_empty actual\n+test_expect_success '-L with -S is rejected' '\n+\ttest_must_fail git log -L 1,1:a.c -S \"pattern\" 2>err &&\n+\ttest_grep \"does not yet support\" err\n '\n \n-test_expect_failure '-L --find-object should filter commits by object' '\n-\tgit log --format=\"%s\" --no-patch -L 1,1:file \\\n-\t\t--find-object=$ZERO_OID >actual &&\n-\ttest_must_be_empty actual\n+test_expect_success '-L with --find-object is rejected' '\n+\ttest_must_fail git log -L 1,1:a.c --find-object=HEAD 2>err &&\n+\ttest_grep \"does not yet support\" err\n '\n \n test_done\n-- \ngitgitgadget\n"},{"id":"537833","messageId":"xmqqh5qv74a9.fsf@gitster.g","threadId":"65135","inReplyTo":"6e97d88993dbab4070ac0aa999f70564368f47b1.1772651484.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/2] line-log: fix crash when combined with pickaxe options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-04T20:01:18Z","receivedAt":"2026-03-04T20:01:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Michael Montalbo via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> From: Michael Montalbo <mmontalbo@gmail.com>\n>\n> queue_diffs() calls diffcore_std() to detect renames so that line-level\n> history can follow files across renames.  When pickaxe options are\n> present on the command line (-G and -S to filter by text pattern,\n> --find-object to filter by object identity), diffcore_std() also runs\n> diffcore_pickaxe(), which may discard diff pairs that are relevant for\n> rename detection.  Losing those pairs breaks rename following.\n\nShouldn't that be solved not by omitting the necessary call to\ndiffcore_std(), but by using the \"--pickaxe-all\" option?\n\n> Note that this only fixes the crash.  The -G, -S, and --find-object\n> options still have no effect on -L output because line-log uses its\n> own commit-filtering logic that bypasses the normal pickaxe pipeline.\n\nI do not know exactly what -L really wants to do, but from the look\nat a patch like this, it smells like it is abusing the diffcore\nmachinery.  If it wants to follow the rename history for individual\npaths, even if the end-user's top-level command line option included\npickaxe or other fancy diffcore options, should it be *reusing* the\ndiff_options struct, prepared from the end-user request?  Shouldn't\nit rather be using its own diffopt crafted for that rename tracking\npurpose, I have to wonder.\n\nThanks.\n"},{"id":"537839","messageId":"xmqq4imv71g8.fsf@gitster.g","threadId":"65135","inReplyTo":"81cb521401210bfbcd05f8201f75e93bccfba712.1772652091.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 2/2] log: reject pickaxe options when combined with -L","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-04T21:02:31Z","receivedAt":"2026-03-04T21:02:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Michael Montalbo via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> From: Michael Montalbo <mmontalbo@gmail.com>\n>\n> The previous commit fixed a crash when -G, -S, or --find-object was\n> used together with -L and rename detection.  However, these options\n> still have no effect on -L output: line-log uses its own\n> commit-filtering logic in line_log_filter() and never consults the\n> pickaxe machinery.  Rather than silently ignoring these options, reject\n> the combination with a clear error message.\n>\n> This replaces the known-breakage tests from the previous commit with\n> tests that verify the rejection for all three options.  A future series\n> could teach line-log to honor these options and remove this restriction.\n>\n> Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>\n> ---\n>  builtin/log.c       |  4 ++++\n>  t/t4211-line-log.sh | 52 ++++++++-------------------------------------\n>  2 files changed, 13 insertions(+), 43 deletions(-)\n>\n> diff --git a/builtin/log.c b/builtin/log.c\n> index 5c9a8ef363..44e2399d59 100644\n> --- a/builtin/log.c\n> +++ b/builtin/log.c\n> @@ -317,6 +317,10 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,\n>  \tif (rev->line_level_traverse && rev->prune_data.nr)\n>  \t\tdie(_(\"-L<range>:<file> cannot be used with pathspec\"));\n>  \n> +\tif (rev->line_level_traverse &&\n> +\t    (rev->diffopt.pickaxe_opts & DIFF_PICKAXE_KINDS_MASK))\n> +\t\tdie(_(\"-L does not yet support -G, -S, or --find-object\"));\n\nI do not think \"-L\" meant to work well with these features to begin\nwith, and I've never used -L with any other options (-L does not\neven work with --stat), so I personally do not mind this change.\n\nBut if this is in place, would we still need [1/2]?\n"},{"id":"537850","messageId":"CAC2QwmKh1DFXfDVKDv1xdj7-AqswgEPSDDXcgTn6dLLgQ9ALKw@mail.gmail.com","threadId":"65135","inReplyTo":"xmqqh5qv74a9.fsf@gitster.g","subject":"Re: [PATCH 1/2] line-log: fix crash when combined with pickaxe options","fromName":"Michael Montalbo","fromEmail":"mmontalbo@gmail.com","sentAt":"2026-03-04T22:33:15Z","receivedAt":"2026-03-04T22:33:28Z","isPatch":true,"sender":{"key":"mmontalbo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/498667?v=4"},"body":"On Wed, Mar 4, 2026 at 12:01 PM Junio C Hamano <gitster@pobox.com> wrote:\n>\n> \"Michael Montalbo via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n>\n> > From: Michael Montalbo <mmontalbo@gmail.com>\n> >\n> > queue_diffs() calls diffcore_std() to detect renames so that line-level\n> > history can follow files across renames.  When pickaxe options are\n> > present on the command line (-G and -S to filter by text pattern,\n> > --find-object to filter by object identity), diffcore_std() also runs\n> > diffcore_pickaxe(), which may discard diff pairs that are relevant for\n> > rename detection.  Losing those pairs breaks rename following.\n>\n> Shouldn't that be solved not by omitting the necessary call to\n> diffcore_std(), but by using the \"--pickaxe-all\" option?\n>\n\nI looked into --pickaxe-all but my understanding is that it\nonly preserves pairs when at least one pair matches the pattern.\nFor a pure rename commit with no content change, I believe\n-G \"pattern\" would find zero matches, and even with --pickaxe-all\nthe entire queue would still get discarded, losing the rename\npair. Just in case I tested this to confirm and it still hits\nthe same assertion failure. I could be wrong about my\nunderstanding of the intent though.\n\n> > Note that this only fixes the crash.  The -G, -S, and --find-object\n> > options still have no effect on -L output because line-log uses its\n> > own commit-filtering logic that bypasses the normal pickaxe pipeline.\n>\n> I do not know exactly what -L really wants to do, but from the look\n> at a patch like this, it smells like it is abusing the diffcore\n> machinery.  If it wants to follow the rename history for individual\n> paths, even if the end-user's top-level command line option included\n> pickaxe or other fancy diffcore options, should it be *reusing* the\n> diff_options struct, prepared from the end-user request?  Shouldn't\n> it rather be using its own diffopt crafted for that rename tracking\n> purpose, I have to wonder.\n>\n\nYes I think that makes more sense. I can update v3 to follow the\npattern in blame.c::find_rename(), building a private diff_options\ninside queue_diffs().\n\n> Thanks.\n\nThank you for the review.\n"},{"id":"537851","messageId":"CAC2Qwm+2pjMk=XFq0cU0Pt1tWkqDy_tOKMtP0jF6JArFX0jmOg@mail.gmail.com","threadId":"65135","inReplyTo":"xmqq4imv71g8.fsf@gitster.g","subject":"Re: [PATCH v2 2/2] log: reject pickaxe options when combined with -L","fromName":"Michael Montalbo","fromEmail":"mmontalbo@gmail.com","sentAt":"2026-03-04T22:36:30Z","receivedAt":"2026-03-04T22:36:42Z","isPatch":true,"sender":{"key":"mmontalbo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/498667?v=4"},"body":"On Wed, Mar 4, 2026 at 1:02 PM Junio C Hamano <gitster@pobox.com> wrote:\n>\n> \"Michael Montalbo via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n>\n> > From: Michael Montalbo <mmontalbo@gmail.com>\n> >\n> > The previous commit fixed a crash when -G, -S, or --find-object was\n> > used together with -L and rename detection.  However, these options\n> > still have no effect on -L output: line-log uses its own\n> > commit-filtering logic in line_log_filter() and never consults the\n> > pickaxe machinery.  Rather than silently ignoring these options, reject\n> > the combination with a clear error message.\n> >\n> > This replaces the known-breakage tests from the previous commit with\n> > tests that verify the rejection for all three options.  A future series\n> > could teach line-log to honor these options and remove this restriction.\n> >\n> > Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>\n> > ---\n> >  builtin/log.c       |  4 ++++\n> >  t/t4211-line-log.sh | 52 ++++++++-------------------------------------\n> >  2 files changed, 13 insertions(+), 43 deletions(-)\n> >\n> > diff --git a/builtin/log.c b/builtin/log.c\n> > index 5c9a8ef363..44e2399d59 100644\n> > --- a/builtin/log.c\n> > +++ b/builtin/log.c\n> > @@ -317,6 +317,10 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,\n> >       if (rev->line_level_traverse && rev->prune_data.nr)\n> >               die(_(\"-L<range>:<file> cannot be used with pathspec\"));\n> >\n> > +     if (rev->line_level_traverse &&\n> > +         (rev->diffopt.pickaxe_opts & DIFF_PICKAXE_KINDS_MASK))\n> > +             die(_(\"-L does not yet support -G, -S, or --find-object\"));\n>\n> I do not think \"-L\" meant to work well with these features to begin\n> with, and I've never used -L with any other options (-L does not\n> even work with --stat), so I personally do not mind this change.\n>\n> But if this is in place, would we still need [1/2]?\n\nI went back and forth on whether to keep [1/2]. My main reason\nfor keeping it was as future-proofing if someone removes the die()\nto implement support for these features working together.\n\nHowever, I can easily see the argument that whoever does that work\nwould likely rework queue_diffs() anyway and it's simpler to drop it.\nHappy to do so if it's not worth the churn.\n"}]}