{"thread":{"id":"66390","subject":"[BUG] revision: premature free-and-null causes “unknown option `(null)`”","startedAt":"2026-09-25T00:31:27Z","lastAt":"2026-09-29T07:43:39Z","messageCount":9,"participants":["Kristoffer Haugsbakk","Jeff King","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"553255","messageId":"74796901-ffb1-4cf3-bd63-7294328f70bc@app.fastmail.com","threadId":"66390","inReplyTo":null,"subject":"[BUG] revision: premature free-and-null causes “unknown option `(null)`”","fromName":"Kristoffer Haugsbakk","fromEmail":"kristofferhaugsbakk@fastmail.com","sentAt":"2026-09-25T00:28:57Z","receivedAt":"2026-09-25T00:31:27Z","isPatch":false,"body":"(the subject is my preliminary speculation)\n\n     Thank you for filling out a Git bug report!\n     Please answer the following questions to help us understand your\n     issue.\n\n     What did you do before the bug happened? (Steps to reproduce your\n     issue)\n\n```\ngit shortlog -n --not-an-option master\n```\n\nNote that you need a real option like `-n` before the\n`--not-an-option`. Or else it will work correctly.\n\n    What did you expect to happen? (Expected behavior)\n\nThis error:\n\n```\nerror: unknown option `--not-an-option'\n[usage printout]\n```\n\n    What happened instead? (Actual behavior)\n\nThis error:\n\n```\nerror: unknown option `(null)'\n[usage printout]\n```\n\n    What's different between what you expected and what actually\n    happened?\n\nI expected it to print the option in quotes. Instead it printed `(null)`\nwhich I think is the placeholder for when the `%s` arg is `NULL`.\n\n    Anything else you want to add:\n\nI have tested and reproduced on:\n\n• master: 0f8e75ab (Revert \"Merge branch\n  'en/no-amend-during-conflicts'\", 2026-09-23)\n• seen: e844e042 (Merge branch 'je/doc-merge-conflicts' into seen,\n  2026-09-24)\n• next: d58861e6 (Revert \"Merge branch 'gg/http-ssl-verify-status' into\n  next\", 2026-09-23)\n\nI have bisected this to cd439487 (revision: manage memory ownership of\nargv in setup_revisions(), 2025-09-19).\n\nNote that the recent topic jk/rev-info-argv-to-free fixes issues caused\nby commit cd439487, but that topic does not change how this option\nhandling behaves; the topic is part of `master` now which I tested. I\nalso tested on top of the topic and got the same `(null)' result.\n\n    Please review the rest of the bug report below.\n    You can delete any lines you don't wish to share.\n\n[System Info]\ngit version:\ngit version 2.55.0.793.gc667de3f2c5\ncpu: x86_64\nbuilt from commit: c667de3f2c5e43830a8dfaa79a27d5d1f106f0ab\nsizeof-long: 8\nsizeof-size_t: 8\nshell-path: /bin/sh\nrust: enabled\nfeature: fsmonitor--daemon\ngettext: enabled\nlibcurl: 7.81.0\nOpenSSL: OpenSSL 3.0.2 15 Mar 2022\nzlib: 1.2.11\nSHA-1: SHA1_DC\nSHA-256: SHA256_BLK\ndefault-ref-format: files\ndefault-hash: sha1\nuname: Linux 6.8.0-138-generic #138~22.04.1-Ubuntu SMP PREEMPT_DYNAMIC Fri Aug  7 13:43:15 UTC  x86_64\ncompiler info: gnuc: 11.4\nlibc info: glibc: 2.35\n$SHELL (typically, interactive shell): /bin/bash\n\n\n[Enabled Hooks]\ncommit-msg\npost-applypatch\npost-checkout\npost-commit\nsendemail-validate\n\n-- \n  Kristoffer Haugsbakk\n  kristofferhaugsbakk@fastmail.com\n"},{"id":"553267","messageId":"20260925082636.GA1493716@coredump.intra.peff.net","threadId":"66390","inReplyTo":"74796901-ffb1-4cf3-bd63-7294328f70bc@app.fastmail.com","subject":"Re: [BUG] revision: premature free-and-null causes “unknown option `(null)`”","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-09-25T08:26:36Z","receivedAt":"2026-09-25T08:26:38Z","isPatch":false,"body":"On Fri, Sep 25, 2026 at 02:28:57AM +0200, Kristoffer Haugsbakk wrote:\n\n> git shortlog -n --not-an-option master\n> [...]\n>\n> I expected it to print the option in quotes. Instead it printed `(null)`\n> which I think is the placeholder for when the `%s` arg is `NULL`.\n> [...]\n> \n> I have bisected this to cd439487 (revision: manage memory ownership of\n> argv in setup_revisions(), 2025-09-19).\n\nYep, definitely my fault. I don't have time to do a full write-up now,\nbut the most direct solution is:\n\ndiff --git a/revision.c b/revision.c\nindex ee1df92d1d..501a4ba36e 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2768,13 +2768,13 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n void parse_revision_opt(struct rev_info *revs, struct parse_opt_ctx_t *ctx,\n \t\t\tconst struct option *options,\n \t\t\tconst char * const usagestr[])\n {\n \tint n = handle_revision_opt(revs, ctx->argc, ctx->argv,\n \t\t\t\t    &ctx->cpidx, ctx->out, NULL);\n \tif (n <= 0) {\n-\t\terror(\"unknown option `%s'\", ctx->argv[0]);\n+\t\terror(\"unknown option `%s'\", ctx->out[ctx->cpidx - 1]);\n \t\tusage_with_options(usagestr, options);\n \t}\n \tctx->argv += n;\n \tctx->argc -= n;\n }\n\nBut I think instead doing this:\n\ndiff --git a/revision.c b/revision.c\nindex ee1df92d1d..7b858d54c1 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2340,7 +2340,8 @@ static void overwrite_argv(int *argc, const char **argv,\n \tif (*value != argv[*argc]) {\n \t\tmark_argv_for_free(opt, revs, argv[*argc]);\n \t\targv[*argc] = *value;\n-\t\t*value = NULL;\n+\t\tif (opt && opt->free_removed_argv_elements)\n+\t\t\t*value = NULL;\n \t}\n \t(*argc)++;\n }\n\nwill restore some of the hidden assumptions made by pre-cd439487 code.\nSo it would fix this case, along with any other lurkers.\n\nI know that's probably quite opaque. ;) I'll fully explain what's going\non in a follow-up tomorrow, but I wanted to post the solution quickly so\nnobody else wasted time digging.\n\nThanks for a clear report.\n\n-Peff\n"},{"id":"553322","messageId":"20260925203359.GA1506705@coredump.intra.peff.net","threadId":"66390","inReplyTo":"20260925082636.GA1493716@coredump.intra.peff.net","subject":"[PATCH 0/2] some parse_revision_opt() bugfixes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-09-25T20:33:59Z","receivedAt":"2026-09-25T20:34:01Z","isPatch":true,"body":"On Fri, Sep 25, 2026 at 04:26:36AM -0400, Jeff King wrote:\n\n> > I have bisected this to cd439487 (revision: manage memory ownership of\n> > argv in setup_revisions(), 2025-09-19).\n> \n> Yep, definitely my fault. I don't have time to do a full write-up now,\n> but the most direct solution is:\n> [...]\n> But I think instead doing this:\n\nI ended up reversing my decision there, for reasons that are explained\nin the second commit. But the good news is that doing so also revealed a\nrelated bug that predates even cd439487.\n\nSo here are the fixes. I apologize in advance for the length of the\nsecond commit message. At least I feel good about my decision to go to\nbed before writing it. ;)\n\n  [1/2]: revision: avoid reporting known options as unknown on error\n  [2/2]: revision: handle argv movement in parse_revision_opt()\n\n revision.c          |  7 +++++--\n t/t4201-shortlog.sh | 11 +++++++++++\n 2 files changed, 16 insertions(+), 2 deletions(-)\n\n-Peff\n"},{"id":"553323","messageId":"20260925203548.GA1544493@coredump.intra.peff.net","threadId":"66390","inReplyTo":"20260925203359.GA1506705@coredump.intra.peff.net","subject":"[PATCH 1/2] revision: avoid reporting known options as unknown on error","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-09-25T20:35:48Z","receivedAt":"2026-09-25T20:35:50Z","isPatch":true,"body":"In parse_revision_opt(), we report \"unknown option\" when\nhandle_revision_opt() returns a negative value or zero. But these are\ntwo different conditions: a negative value indicates a malformed option\n(like a missing argument) and zero indicates an unknown option.\n\nSo malformed options produce nonsense like this:\n\n  $ git shortlog --default\n  error: bad --default argument\n  error: unknown option `--default'\n  usage: [...]\n\nWe should treat a negative return as a logic error which has already\nbeen reported by handle_revision_opt(), and just show the regular usage\nmessage.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nThere's obviously a way to write this that makes the diff a little\nshorter and doesn't repeat the usage_with_options() line. I think the\nif/else cascade makes the mental model more clear, though.\n\n revision.c          | 5 ++++-\n t/t4201-shortlog.sh | 6 ++++++\n 2 files changed, 10 insertions(+), 1 deletion(-)\n\ndiff --git a/revision.c b/revision.c\nindex ee1df92d1d..f958d8c301 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2771,7 +2771,10 @@ void parse_revision_opt(struct rev_info *revs, struct parse_opt_ctx_t *ctx,\n {\n \tint n = handle_revision_opt(revs, ctx->argc, ctx->argv,\n \t\t\t\t    &ctx->cpidx, ctx->out, NULL);\n-\tif (n <= 0) {\n+\tif (n < 0) {\n+\t\t/* handle_revision_opt() has already reported the error. */\n+\t\tusage_with_options(usagestr, options);\n+\t} else if (!n) {\n \t\terror(\"unknown option `%s'\", ctx->argv[0]);\n \t\tusage_with_options(usagestr, options);\n \t}\ndiff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh\nindex 023fbff546..4ba7f5aec6 100755\n--- a/t/t4201-shortlog.sh\n+++ b/t/t4201-shortlog.sh\n@@ -436,4 +436,10 @@ test_expect_success 'stdin with multiple groups reports error' '\n \ttest_must_fail git shortlog --group=author --group=committer <log\n '\n \n+test_expect_success 'invalid revision options are not reported as unknown' '\n+\ttest_must_fail git shortlog --default 2>err &&\n+\ttest_grep \"bad --default argument\" err &&\n+\ttest_grep ! \"unknown option\" err\n+'\n+\n test_done\n-- \n2.56.0.rc2.289.g137cf50cac\n\n"},{"id":"553324","messageId":"20260925203958.GB1544493@coredump.intra.peff.net","threadId":"66390","inReplyTo":"20260925203359.GA1506705@coredump.intra.peff.net","subject":"[PATCH 2/2] revision: handle argv movement in parse_revision_opt()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-09-25T20:39:58Z","receivedAt":"2026-09-25T20:40:00Z","isPatch":true,"body":"The argument parser used by setup_revisions() modifies the argv array\nthat is passed to it, consolidating non-options and unknown options at\nthe start of the array. This led to problems with memory leaks when argv\npointed to allocated strings. We addressed that in cd43948798 (revision:\nmanage memory ownership of argv in setup_revisions(), 2025-09-19). Now\ninstead of copying strings to the earlier part of argv, we actually move\nthem, setting the original location to NULL (so that we know we have\nexactly one pointer to the string).\n\nThis works fine for setup_revisions() itself, but the underlying\nhandle_revision_opt() has another entry point: parse_revision_opt().\nThis lets a parse-options user parse a single revision option, but the\nmovement introduced by cd43948798 confuses its error code path. If we\nsee an unknown option, then handle_revision_opt() will move it out of\nthe way (to the \"unknown options\" section) and return an error. But\nparse_revision_opt() then tries to access the original argv location,\nwhich has now been set to NULL, and you get:\n\n  $ git shortlog -n --no-such-option\n  error: unknown option `(null)'\n\nWhereas prior to cd43948798, it would have been a leftover copy of the\npointer (that may or may not eventually get written over, but was valid\nfor this immediate message). And you get what you'd expect:\n\n  $ git shortlog -n --no-such-option\n  error: unknown option `--no-such-option'\n\nMaking things even more confusing, it only happens if there's another\noption before the unknown one! That's because with just:\n\n  $ git shortlog --no-such-option\n\nwe \"consolidate\" to the exact same spot, and no movement occurs at all.\n\nNote that we use shortlog in these examples because it is one of only\ntwo commands that use the parse_revision_opt() interface (the other is\nblame).\n\nThere are a few options for fixing this. One is that we can observe that\nthe \"move\" semantics introduced by cd43948798 only matter when the argv\nstrings are allocated on the heap, in which case the caller passes in\nthe free_removed_argv_elements flag to tell us. But we never use that\nflag with parse_revision_opt(). So we could do something like this:\n\n  diff --git a/revision.c b/revision.c\n  index ee1df92d1d..7b858d54c1 100644\n  --- a/revision.c\n  +++ b/revision.c\n  @@ -2340,7 +2340,8 @@ static void overwrite_argv(int *argc, const char **argv,\n   \tif (*value != argv[*argc]) {\n   \t\tmark_argv_for_free(opt, revs, argv[*argc]);\n   \t\targv[*argc] = *value;\n  -\t\t*value = NULL;\n  +\t\tif (opt && opt->free_removed_argv_elements)\n  +\t\t\t*value = NULL;\n   \t}\n   \t(*argc)++;\n   }\n\nto restore the pre-cd43948798 semantics when heap-allocated strings are\nnot in use. We'd just keep the extra pointer in the original location,\nbut nobody cares because they're not going to free anything anyway.\nThat's enough to fix this case, and could fix any other theoretical\ncases we haven't noticed. The downside is that it's an accident waiting\nto happen if we ever do teach parse_revision_opt() to handle allocated\nargv strings.\n\nBut are there other theoretical cases? I don't think so. The code paths\ntouched by cd43948798 are either in setup_revisions() itself (which also\nlearned how to handle this movement) or in handle_revision_opt(), the\nlow-level static helper. It has only two callers: setup_revisions()\nitself, and parse_revision_opt() in which we see the current breakage.\nSo fixing parse_revision_opt() should cover all of our bases, and keep\nthe code ready for a potential future change to handle allocated\nstrings.\n\nThe fix is just to tell parse_revision_opt() to look for the unknown\noption in the consolidated destination rather than the original\nlocation.  We might write to that consolidated location for other\nreasons (like moving pseudo-revision options like \"--all\"), but there is\nonly one code path that returns the 0 for an unknown option, and it\nalways moves the option before doing so. So the \"end\" of that\nconsolidated area will always have our unknown option.\n\nThis patch implements that solution and demonstrates the breakage and\nfix using shortlog.\n\nReported-by: Kristoffer Haugsbakk <kristofferhaugsbakk@fastmail.com>\nSigned-off-by: Jeff King <peff@peff.net>\n---\nObviously another possible fix is for parse_revision_opt() to record the\nstring before passing it along, and use that for its error message. That\nseemed clunkier to me.\n\n revision.c          | 2 +-\n t/t4201-shortlog.sh | 5 +++++\n 2 files changed, 6 insertions(+), 1 deletion(-)\n\ndiff --git a/revision.c b/revision.c\nindex f958d8c301..a83e499047 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2775,7 +2775,7 @@ void parse_revision_opt(struct rev_info *revs, struct parse_opt_ctx_t *ctx,\n \t\t/* handle_revision_opt() has already reported the error. */\n \t\tusage_with_options(usagestr, options);\n \t} else if (!n) {\n-\t\terror(\"unknown option `%s'\", ctx->argv[0]);\n+\t\terror(\"unknown option `%s'\", ctx->out[ctx->cpidx - 1]);\n \t\tusage_with_options(usagestr, options);\n \t}\n \tctx->argv += n;\ndiff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh\nindex 4ba7f5aec6..10c43e6e75 100755\n--- a/t/t4201-shortlog.sh\n+++ b/t/t4201-shortlog.sh\n@@ -442,4 +442,9 @@ test_expect_success 'invalid revision options are not reported as unknown' '\n \ttest_grep ! \"unknown option\" err\n '\n \n+test_expect_success 'unknown revision options are reported correctly' '\n+\ttest_must_fail git shortlog -n --no-such-option 2>err &&\n+\ttest_grep \"unknown option .*--no-such-option\" err\n+'\n+\n test_done\n-- \n2.56.0.rc2.289.g137cf50cac\n"},{"id":"553336","messageId":"xmqqbj9lt1gn.fsf@gitster.g","threadId":"66390","inReplyTo":"20260925203359.GA1506705@coredump.intra.peff.net","subject":"Re: [PATCH 0/2] some parse_revision_opt() bugfixes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-09-25T22:23:20Z","receivedAt":"2026-09-25T22:23:23Z","isPatch":true,"body":"Jeff King <peff@peff.net> writes:\n\n> On Fri, Sep 25, 2026 at 04:26:36AM -0400, Jeff King wrote:\n>\n>> > I have bisected this to cd439487 (revision: manage memory ownership of\n>> > argv in setup_revisions(), 2025-09-19).\n>> \n>> Yep, definitely my fault. I don't have time to do a full write-up now,\n>> but the most direct solution is:\n>> [...]\n>> But I think instead doing this:\n>\n> I ended up reversing my decision there, for reasons that are explained\n> in the second commit. But the good news is that doing so also revealed a\n> related bug that predates even cd439487.\n>\n> So here are the fixes. I apologize in advance for the length of the\n> second commit message. At least I feel good about my decision to go to\n> bed before writing it. ;)\n>\n>   [1/2]: revision: avoid reporting known options as unknown on error\n>   [2/2]: revision: handle argv movement in parse_revision_opt()\n>\n>  revision.c          |  7 +++++--\n>  t/t4201-shortlog.sh | 11 +++++++++++\n>  2 files changed, 16 insertions(+), 2 deletions(-)\n\nBoth patches make sense.\n\n: git; rungit v2.52.0 shortlog -n --no-such-option 2>&1 | head -n1\nerror: unknown option `(null)'\n: git; rungit v2.51.0 shortlog -n --no-such-option 2>&1 | head -n1\nerror: unknown option `--no-such-option'\n: git; ./git shortlog -n --no-such-option 2>&1 | head -n1\nerror: unknown option `--no-such-option'\n\n"},{"id":"553341","messageId":"add1abaa-5d51-43dc-9907-d6d3851004f5@app.fastmail.com","threadId":"66390","inReplyTo":"20260925203958.GB1544493@coredump.intra.peff.net","subject":"Re: [PATCH 2/2] revision: handle argv movement in parse_revision_opt()","fromName":"Kristoffer Haugsbakk","fromEmail":"kristofferhaugsbakk@fastmail.com","sentAt":"2026-09-26T09:00:05Z","receivedAt":"2026-09-26T09:00:29Z","isPatch":true,"body":"On Fri, Sep 25, 2026, at 22:39, Jeff King wrote:\n> The argument parser used by setup_revisions() modifies the argv array\n> that is passed to it, consolidating non-options and unknown options at\n> the start of the array. This led to problems with memory leaks when argv\n> pointed to allocated strings. We addressed that in cd43948798 (revision:\n> manage memory ownership of argv in setup_revisions(), 2025-09-19). Now\n> instead of copying strings to the earlier part of argv, we actually move\n> them, setting the original location to NULL (so that we know we have\n> exactly one pointer to the string).\n>\n>[snip]\n>\n> Reported-by: Kristoffer Haugsbakk <kristofferhaugsbakk@fastmail.com>\n\nI personally prefer the email that I use for commits:\n\n<code@khaugsbakk.name>\n\n(Which has always been the case. But I didn’t want to disrupt the\nprocess previously.)\n\nI ought to send in a `.mailmap` change with my canonical email address.\n\n>[snip]\n> +test_expect_success 'unknown revision options are reported correctly' '\n> +\ttest_must_fail git shortlog -n --no-such-option 2>err &&\n> +\ttest_grep \"unknown option .*--no-such-option\" err\n> +'\n\nJust thinking this through. This is a regression test indirectly related\nto git-shortlog(1). So the test does not name `shortlog`, so that’s good.\nThe subtlety of the previously discussed:\n\n    Making things even more confusing, it only happens if there's\n    another option before the unknown one!\n\nis not obvious from the test description, but one can surmise that it is\nneeded since it’s there in the first place.\n\nFor the next readers of this test suite that come along, it might seem\nstrange that this specific sequence is tested for, and on a shortlog\ntest suite. But it seems normal in this project to add tests that, in\nthe context of the file alone, might not be obvious why they are there\n(because they are regression tests for very specific bugs). I could\nimagine some system where regression tests are marked with some\nidentifier that however indirectly links back to whatever triggered the\nfix. But for one, this would be a new system/convention and wouldn’t\nmake sense to use on just one test. And second, this would just make it\nmore directly accessible; it is still directly accessible for people who\nknow how to query git(1). Well, maybe more indirectly as time goes on if\nthe test is changed and you use the “pickaxe” technique.\n\nThis is all to say that this test makes sense as it is written now.\n\n> +\n>  test_done\n> --\n> 2.56.0.rc2.289.g137cf50cac\n\nThanks for fixing. :)\n"},{"id":"553397","messageId":"20260928032553.GA493672@coredump.intra.peff.net","threadId":"66390","inReplyTo":"add1abaa-5d51-43dc-9907-d6d3851004f5@app.fastmail.com","subject":"Re: [PATCH 2/2] revision: handle argv movement in parse_revision_opt()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-09-28T03:25:53Z","receivedAt":"2026-09-28T03:26:00Z","isPatch":true,"body":"On Sat, Sep 26, 2026 at 11:00:05AM +0200, Kristoffer Haugsbakk wrote:\n\n> > Reported-by: Kristoffer Haugsbakk <kristofferhaugsbakk@fastmail.com>\n> \n> I personally prefer the email that I use for commits:\n> \n> <code@khaugsbakk.name>\n> \n> (Which has always been the case. But I didn’t want to disrupt the\n> process previously.)\n\nOK. I pulled it from your From header, of course. :)\n\n> I ought to send in a `.mailmap` change with my canonical email address.\n\nWe don't mailmap trailers, though. I have a patch to let you do so with\n%(trailers:mailmap), but you'd still see the original most of the time\n(since git-log, etc, just dump the raw contents and expect the trailers\nto be readable).\n\n> > +test_expect_success 'unknown revision options are reported correctly' '\n> > +\ttest_must_fail git shortlog -n --no-such-option 2>err &&\n> > +\ttest_grep \"unknown option .*--no-such-option\" err\n> > +'\n> \n> Just thinking this through. This is a regression test indirectly related\n> to git-shortlog(1). So the test does not name `shortlog`, so that’s good.\n> The subtlety of the previously discussed:\n> \n>     Making things even more confusing, it only happens if there's\n>     another option before the unknown one!\n> \n> is not obvious from the test description, but one can surmise that it is\n> needed since it’s there in the first place.\n\nYeah. I sort of assume that anybody wondering about the details of a\nline of code in this project will be able to dig around with blame or\npickaxe. Perhaps a comment could help, but I think anything beyond \"it\nis important that there are two options here\" would end up re-hashing\nthe whole explanation in the commit message.\n\n> For the next readers of this test suite that come along, it might seem\n> strange that this specific sequence is tested for, and on a shortlog\n> test suite. But it seems normal in this project to add tests that, in\n> the context of the file alone, might not be obvious why they are there\n> (because they are regression tests for very specific bugs). I could\n> imagine some system where regression tests are marked with some\n> identifier that however indirectly links back to whatever triggered the\n> fix. But for one, this would be a new system/convention and wouldn’t\n> make sense to use on just one test. And second, this would just make it\n> more directly accessible; it is still directly accessible for people who\n> know how to query git(1). Well, maybe more indirectly as time goes on if\n> the test is changed and you use the “pickaxe” technique.\n\nYeah, exactly. Both patches are really bugs in the revision /\nparse-options integration function that just happens to be triggerable\nby shortlog. Possibly something like t0040 would make sense, but it\nfeels weird to be sticking a shortlog invocation there. I dunno. Again,\nI sort of rely on people to find the relevant commits.\n\n> This is all to say that this test makes sense as it is written now.\n\nThanks!\n\n-Peff\n"},{"id":"553556","messageId":"7ffbbf0b-4954-4ee8-863c-4ca89ae662c6@app.fastmail.com","threadId":"66390","inReplyTo":"20260928032553.GA493672@coredump.intra.peff.net","subject":"Re: [PATCH 2/2] revision: handle argv movement in parse_revision_opt()","fromName":"Kristoffer Haugsbakk","fromEmail":"kristofferhaugsbakk@fastmail.com","sentAt":"2026-09-29T07:43:15Z","receivedAt":"2026-09-29T07:43:39Z","isPatch":true,"body":" On Mon, Sep 28, 2026, at 05:25, Jeff King wrote:\n> On Sat, Sep 26, 2026 at 11:00:05AM +0200, Kristoffer Haugsbakk wrote:\n>> I ought to send in a `.mailmap` change with my canonical email address.\n>\n> We don't mailmap trailers, though. [...]\n\nYes, that’s the reason. There’s no dynamic remapping after the fact. But\nwith a mailmap entry you can ask git-check-mailmap(1) if you have the\ncorrect entry while writing or normalizing the commit.\n\nI had use for that once when I was collecting `Reported-by`. One report\nwas from three years prior. But in the meantime, this person had changed\ntheir address. But they had recorded it in the `.mailmap`.\n\nBut for such a normalization to be workable at all I would have to try\nto add something like `--evaluate-all-cmds` to git-interpret-\ntrailers(1). Because `trailer.<key-alias>.cmd` will not evaluate trailer\nvalues if it is just reading in a file without any `--trailer` options\n(as well).\n\n***\n\nWhich is an itch that no one else has to care about.\n"}]}