{"thread":{"id":"64784","subject":"[PATCH] last-modified: verify revision argument is a commit-ish","startedAt":"2026-01-12T16:17:59Z","lastAt":"2026-02-06T15:55:39Z","messageCount":55,"participants":["Toon Claes","Junio C Hamano","Patrick Steinhardt","Kristoffer Haugsbakk"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"533654","messageId":"20260112-toon-last-modified-tree-v1-1-ecbc78341f76@iotcl.com","threadId":"64784","inReplyTo":null,"subject":"[PATCH] last-modified: verify revision argument is a commit-ish","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2026-01-12T16:17:41Z","receivedAt":"2026-01-12T16:17:59Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"Passing a tree OID to git-last-modified(1) would trigger BUG behavior.\n\n git last-modified HEAD^{tree}\n BUG: builtin/last-modified.c:456: paths remaining beyond boundary in last-modified\n\nFix this error by verifying the parsed revision is peels to a\ncommit-ish.\n\nWhile at it, also fix a memory leak in populate_paths_from_revs().\n\nReported-by: Gusted \nSigned-off-by: Toon Claes \n---\nRecently there was a bug reported[1] passing a tree OID triggers a BUG:\n\n $ git last-modified fb06ce04173d47aaaa498385621cba8b8dfd7584\n BUG: builtin/last-modified.c:456: paths remaining beyond boundary in last-modified\n [1] 690163 IOT instruction (core dumped) git last-modified\n\n `fb06ce04173d47aaaa498385621cba8b8dfd7584` is the tree commit id of web_src. I\n suppose this should've returned a nice error message or blank output.\n\nFix this bug by checking the revision argument.\n\n[1]: https://lore.kernel.org/git/03f96860-29fc-42a7-a220-c3ec65eb8516@codeberg.org/\n---\n builtin/last-modified.c | 15 +++++++++++----\n t/t8020-last-modified.sh | 5 +++++\n 2 files changed, 16 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/last-modified.c b/builtin/last-modified.c\nindex c80f0535f6..cac94e384d 100644\n--- a/builtin/last-modified.c\n+++ b/builtin/last-modified.c\n@@ -123,7 +123,7 @@ static void add_path_from_diff(struct diff_queue_struct *q,\n \n static int populate_paths_from_revs(struct last_modified *lm)\n {\n-\tint num_interesting = 0;\n+\tint num_interesting = 0, ret = 0;\n \tstruct diff_options diffopt;\n \n \t/*\n@@ -145,8 +145,15 @@ static int populate_paths_from_revs(struct last_modified *lm)\n \t\tif (obj->item->flags & UNINTERESTING)\n \t\t\tcontinue;\n \n-\t\tif (num_interesting++)\n-\t\t\treturn error(_(\"last-modified can only operate on one tree at a time\"));\n+\t\tif (num_interesting++) {\n+\t\t\tret = error(_(\"last-modified can only operate on one tree at a time\"));\n+\t\t\tbreak;\n+\t\t}\n+\n+\t\tif (!repo_peel_to_type(lm->rev.repo, obj->path, 0, obj->item, OBJ_COMMIT)) {\n+\t\t\tret = error(_(\"revision argument is not a commit-ish\"));\n+\t\t\tbreak;\n+\t\t}\n \n \t\tdiff_tree_oid(lm->rev.repo->hash_algo->empty_tree,\n \t\t\t &obj->item->oid, \"\", &diffopt);\n@@ -154,7 +161,7 @@ static int populate_paths_from_revs(struct last_modified *lm)\n \t}\n \tclear_pathspec(&diffopt.pathspec);\n \n-\treturn 0;\n+\treturn ret;\n }\n \n static void last_modified_emit(struct last_modified *lm,\ndiff --git a/t/t8020-last-modified.sh b/t/t8020-last-modified.sh\nindex 50f4312f71..d0d52add05 100755\n--- a/t/t8020-last-modified.sh\n+++ b/t/t8020-last-modified.sh\n@@ -235,4 +235,9 @@ test_expect_success 'last-modified complains about unknown arguments' '\n \tgrep \"unknown last-modified argument: --foo\" err\n '\n \n+test_expect_success 'last-modified expects commit-ish' '\n+\ttest_must_fail git last-modified HEAD^{tree} 2>err &&\n+\tgrep \"revision argument is not a commit-ish\" err\n+'\n+\n test_done\n\n---\nbase-commit: d529f3a197364881746f558e5652f0236131eb86\nchange-id: 20260112-toon-last-modified-tree-fdd96b2feaf7\n\n"},{"id":"533680","messageId":"xmqq344amvgq.fsf@gitster.g","threadId":"64784","inReplyTo":"20260112-toon-last-modified-tree-v1-1-ecbc78341f76@iotcl.com","subject":"Re: [PATCH] last-modified: verify revision argument is a commit-ish","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-01-12T20:23:17Z","receivedAt":"2026-01-12T20:23:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Toon Claes writes:\n\n> Passing a tree OID to git-last-modified(1) would trigger BUG behavior.\n>\n> git last-modified HEAD^{tree}\n> BUG: builtin/last-modified.c:456: paths remaining beyond boundary in last-modified\n\nOf course, the starting point last-modified needs to be something\nthat will yield series of commits when given to get_revision() ;-)\n\n> Fix this error by verifying the parsed revision is peels to a\n> commit-ish.\n\nOK.\n"},{"id":"533706","messageId":"aWXsP1GJ2YfrZyh0@pks.im","threadId":"64784","inReplyTo":"20260112-toon-last-modified-tree-v1-1-ecbc78341f76@iotcl.com","subject":"Re: [PATCH] last-modified: verify revision argument is a commit-ish","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-13T06:54:55Z","receivedAt":"2026-01-13T06:55:01Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Jan 12, 2026 at 05:17:41PM +0100, Toon Claes wrote:\n> Passing a tree OID to git-last-modified(1) would trigger BUG behavior.\n> \n> git last-modified HEAD^{tree}\n> BUG: builtin/last-modified.c:456: paths remaining beyond boundary in last-modified\n> \n> Fix this error by verifying the parsed revision is peels to a\n\ns/is peels/peels/\n\n> diff --git a/builtin/last-modified.c b/builtin/last-modified.c\n> index c80f0535f6..cac94e384d 100644\n> --- a/builtin/last-modified.c\n> +++ b/builtin/last-modified.c\n> @@ -145,8 +145,15 @@ static int populate_paths_from_revs(struct last_modified *lm)\n> \t\tif (obj->item->flags & UNINTERESTING)\n> \t\t\tcontinue;\n> \n> -\t\tif (num_interesting++)\n> -\t\t\treturn error(_(\"last-modified can only operate on one tree at a time\"));\n> +\t\tif (num_interesting++) {\n> +\t\t\tret = error(_(\"last-modified can only operate on one tree at a time\"));\n\nA preexisting issue, but isn't this error message a bit weird? Below we\nassert that we've got a commit, but here we say that we expect to work\non a tree.\n\n> +\t\t\tbreak;\n> +\t\t}\n> +\n> +\t\tif (!repo_peel_to_type(lm->rev.repo, obj->path, 0, obj->item, OBJ_COMMIT)) {\n> +\t\t\tret = error(_(\"revision argument is not a commit-ish\"));\n> +\t\t\tbreak;\n> +\t\t}\n\nI'd prefer a `goto out` in both error cases, but that's a matter of\nstyle and thus a subjective proposal. So no need to address this.\n\n> \t\tdiff_tree_oid(lm->rev.repo->hash_algo->empty_tree,\n> \t\t\t &obj->item->oid, \"\", &diffopt);\n> diff --git a/t/t8020-last-modified.sh b/t/t8020-last-modified.sh\n> index 50f4312f71..d0d52add05 100755\n> --- a/t/t8020-last-modified.sh\n> +++ b/t/t8020-last-modified.sh\n> @@ -235,4 +235,9 @@ test_expect_success 'last-modified complains about unknown arguments' '\n> \tgrep \"unknown last-modified argument: --foo\" err\n> '\n> \n> +test_expect_success 'last-modified expects commit-ish' '\n> +\ttest_must_fail git last-modified HEAD^{tree} 2>err &&\n> +\tgrep \"revision argument is not a commit-ish\" err\n> +'\n\nDo we have tests that verify that this works when passed for example an\nannotated tag?\n\nThanks!\n\nPatrick\n"},{"id":"533813","messageId":"20260114-toon-last-modified-tree-v2-0-ba3b1860898f@iotcl.com","threadId":"64784","inReplyTo":"20260112-toon-last-modified-tree-v1-1-ecbc78341f76@iotcl.com","subject":"[PATCH v2 0/3] Fix git-last-modified(1) bug triggered when passing a tree-ish","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2026-01-14T10:24:44Z","receivedAt":"2026-01-14T10:25:01Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"Recently there was a bug reported[1] passing a tree OID triggers a BUG:\n\n $ git last-modified fb06ce04173d47aaaa498385621cba8b8dfd7584\n BUG: builtin/last-modified.c:456: paths remaining beyond boundary in last-modified\n [1] 690163 IOT instruction (core dumped) git last-modified\n\n `fb06ce04173d47aaaa498385621cba8b8dfd7584` is the tree commit id of web_src. I\n suppose this should've returned a nice error message or blank output.\n\nFix this bug by checking the revision argument.\n\n[1]: https://lore.kernel.org/git/03f96860-29fc-42a7-a220-c3ec65eb8516@codeberg.org/\n\n---\nChanges in v2:\n- Prepend the change with a commit that modifies the error message\n when more than one revision is given.\n- Prepend another commit that removes double error message.\n- Add test to ensure the command works with annotated tags too.\n- Link to v1: https://patch.msgid.link/20260112-toon-last-modified-tree-v1-1-ecbc78341f76@iotcl.com\n\n---\nToon Claes (3):\n last-modified: rewrite error message when more than one revision given\n last-modified: remove double error message\n last-modified: verify revision argument is a commit-ish\n\n builtin/last-modified.c | 19 ++++++++++++++-----\n t/t8020-last-modified.sh | 15 ++++++++++++++-\n 2 files changed, 28 insertions(+), 6 deletions(-)\n\nRange-diff versus v1:\n\n1: ebd05211ab ! 1: 70baa9b4eb last-modified: verify revision argument is a commit-ish\n @@ Metadata\n Author: Toon Claes \n \n ## Commit message ##\n - last-modified: verify revision argument is a commit-ish\n + last-modified: rewrite error message when more than one revision given\n \n - Passing a tree OID to git-last-modified(1) would trigger BUG behavior.\n + When more than one revision is passed to the git-last-modified(1)\n + command, this error message was printed:\n \n - git last-modified HEAD^{tree}\n - BUG: builtin/last-modified.c:456: paths remaining beyond boundary in last-modified\n + error: last-modified can only operate on one tree at a time\n \n - Fix this error by verifying the parsed revision is peels to a\n - commit-ish.\n + Calling these a \"tree\" is technically not correct. git-last-modified(1)\n + expects revisions that peel to a commit.\n \n - While at it, also fix a memory leak in populate_paths_from_revs().\n + Rephrase the error message to:\n +\n + error: last-modified can only operate on one revision at a time\n +\n + While at it, also fix a memory leak that remained uncovered so far.\n \n - Reported-by: Gusted \n Signed-off-by: Toon Claes \n \n ## builtin/last-modified.c ##\n @@ builtin/last-modified.c: static int populate_paths_from_revs(struct last_modifie\n -\t\tif (num_interesting++)\n -\t\t\treturn error(_(\"last-modified can only operate on one tree at a time\"));\n +\t\tif (num_interesting++) {\n -+\t\t\tret = error(_(\"last-modified can only operate on one tree at a time\"));\n -+\t\t\tbreak;\n -+\t\t}\n -+\n -+\t\tif (!repo_peel_to_type(lm->rev.repo, obj->path, 0, obj->item, OBJ_COMMIT)) {\n -+\t\t\tret = error(_(\"revision argument is not a commit-ish\"));\n -+\t\t\tbreak;\n ++\t\t\tret = error(_(\"last-modified can only operate on one revision at a time\"));\n ++\t\t\tgoto out;\n +\t\t}\n \n \t\tdiff_tree_oid(lm->rev.repo->hash_algo->empty_tree,\n \t\t\t &obj->item->oid, \"\", &diffopt);\n -@@ builtin/last-modified.c: static int populate_paths_from_revs(struct last_modified *lm)\n + \t\tdiff_flush(&diffopt);\n \t}\n ++\n ++out:\n \tclear_pathspec(&diffopt.pathspec);\n \n -\treturn 0;\n @@ builtin/last-modified.c: static int populate_paths_from_revs(struct last_modifie\n static void last_modified_emit(struct last_modified *lm,\n \n ## t/t8020-last-modified.sh ##\n -@@ t/t8020-last-modified.sh: test_expect_success 'last-modified complains about unknown arguments' '\n - \tgrep \"unknown last-modified argument: --foo\" err\n +@@ t/t8020-last-modified.sh: test_expect_success 'setup' '\n + \ttest_commit 3 a/b/file\n + '\n + \n +-test_expect_success 'cannot run last-modified on two trees' '\n ++test_expect_success 'cannot run last-modified on two revision' '\n + \ttest_must_fail git last-modified HEAD HEAD~1\n '\n \n -+test_expect_success 'last-modified expects commit-ish' '\n -+\ttest_must_fail git last-modified HEAD^{tree} 2>err &&\n -+\tgrep \"revision argument is not a commit-ish\" err\n -+'\n -+\n - test_done\n-: ---------- > 2: 150c43580d last-modified: remove double error message\n-: ---------- > 3: ab89cb1ef3 last-modified: verify revision argument is a commit-ish\n\n\n---\nbase-commit: d529f3a197364881746f558e5652f0236131eb86\nchange-id: 20260112-toon-last-modified-tree-fdd96b2feaf7\n\n"},{"id":"533814","messageId":"20260114-toon-last-modified-tree-v2-1-ba3b1860898f@iotcl.com","threadId":"64784","inReplyTo":"20260114-toon-last-modified-tree-v2-0-ba3b1860898f@iotcl.com","subject":"[PATCH v2 1/3] last-modified: rewrite error message when more than one revision given","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2026-01-14T10:24:45Z","receivedAt":"2026-01-14T10:25:03Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"When more than one revision is passed to the git-last-modified(1)\ncommand, this error message was printed:\n\n error: last-modified can only operate on one tree at a time\n\nCalling these a \"tree\" is technically not correct. git-last-modified(1)\nexpects revisions that peel to a commit.\n\nRephrase the error message to:\n\n error: last-modified can only operate on one revision at a time\n\nWhile at it, also fix a memory leak that remained uncovered so far.\n\nSigned-off-by: Toon Claes \n---\n builtin/last-modified.c | 12 ++++++++----\n t/t8020-last-modified.sh | 2 +-\n 2 files changed, 9 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/last-modified.c b/builtin/last-modified.c\nindex c80f0535f6..06e3f79aec 100644\n--- a/builtin/last-modified.c\n+++ b/builtin/last-modified.c\n@@ -123,7 +123,7 @@ static void add_path_from_diff(struct diff_queue_struct *q,\n \n static int populate_paths_from_revs(struct last_modified *lm)\n {\n-\tint num_interesting = 0;\n+\tint num_interesting = 0, ret = 0;\n \tstruct diff_options diffopt;\n \n \t/*\n@@ -145,16 +145,20 @@ static int populate_paths_from_revs(struct last_modified *lm)\n \t\tif (obj->item->flags & UNINTERESTING)\n \t\t\tcontinue;\n \n-\t\tif (num_interesting++)\n-\t\t\treturn error(_(\"last-modified can only operate on one tree at a time\"));\n+\t\tif (num_interesting++) {\n+\t\t\tret = error(_(\"last-modified can only operate on one revision at a time\"));\n+\t\t\tgoto out;\n+\t\t}\n \n \t\tdiff_tree_oid(lm->rev.repo->hash_algo->empty_tree,\n \t\t\t &obj->item->oid, \"\", &diffopt);\n \t\tdiff_flush(&diffopt);\n \t}\n+\n+out:\n \tclear_pathspec(&diffopt.pathspec);\n \n-\treturn 0;\n+\treturn ret;\n }\n \n static void last_modified_emit(struct last_modified *lm,\ndiff --git a/t/t8020-last-modified.sh b/t/t8020-last-modified.sh\nindex 50f4312f71..1183ae667b 100755\n--- a/t/t8020-last-modified.sh\n+++ b/t/t8020-last-modified.sh\n@@ -12,7 +12,7 @@ test_expect_success 'setup' '\n \ttest_commit 3 a/b/file\n '\n \n-test_expect_success 'cannot run last-modified on two trees' '\n+test_expect_success 'cannot run last-modified on two revision' '\n \ttest_must_fail git last-modified HEAD HEAD~1\n '\n \n\n-- \n2.52.0\n\n"},{"id":"533816","messageId":"20260114-toon-last-modified-tree-v2-2-ba3b1860898f@iotcl.com","threadId":"64784","inReplyTo":"20260114-toon-last-modified-tree-v2-0-ba3b1860898f@iotcl.com","subject":"[PATCH v2 2/3] last-modified: remove double error message","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2026-01-14T10:24:46Z","receivedAt":"2026-01-14T10:25:05Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"When the user passes two revisions, they get the following output:\n\n $ git last-modified HEAD HEAD~\n error: last-modified can only operate on one revision at a time\n error: unable to setup last-modified\n\nThe error message about \"unable to setup\" is not very informative,\nremove it.\n\nSigned-off-by: Toon Claes \n---\n builtin/last-modified.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/last-modified.c b/builtin/last-modified.c\nindex 06e3f79aec..0df85be318 100644\n--- a/builtin/last-modified.c\n+++ b/builtin/last-modified.c\n@@ -495,7 +495,7 @@ static int last_modified_init(struct last_modified *lm, struct repository *r,\n \tlm->rev.bloom_filter_settings = get_bloom_filter_settings(lm->rev.repo);\n \n \tif (populate_paths_from_revs(lm) < 0)\n-\t\treturn error(_(\"unable to setup last-modified\"));\n+\t\treturn -1;\n \n \tCALLOC_ARRAY(lm->all_paths, hashmap_get_size(&lm->paths));\n \tlm->all_paths_nr = 0;\n\n-- \n2.52.0\n\n"},{"id":"533815","messageId":"20260114-toon-last-modified-tree-v2-3-ba3b1860898f@iotcl.com","threadId":"64784","inReplyTo":"20260114-toon-last-modified-tree-v2-0-ba3b1860898f@iotcl.com","subject":"[PATCH v2 3/3] last-modified: verify revision argument is a commit-ish","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2026-01-14T10:24:47Z","receivedAt":"2026-01-14T10:25:08Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"Passing a tree OID to git-last-modified(1) would trigger BUG behavior.\n\n git last-modified HEAD^{tree}\n BUG: builtin/last-modified.c:456: paths remaining beyond boundary in last-modified\n\nFix this error by verifying the parsed revision peels to a commit-ish.\n\nWhile at it, also fix a memory leak in populate_paths_from_revs().\n\nReported-by: Gusted \nSigned-off-by: Toon Claes \n---\n builtin/last-modified.c | 5 +++++\n t/t8020-last-modified.sh | 13 +++++++++++++\n 2 files changed, 18 insertions(+)\n\ndiff --git a/builtin/last-modified.c b/builtin/last-modified.c\nindex 0df85be318..5366cedd0f 100644\n--- a/builtin/last-modified.c\n+++ b/builtin/last-modified.c\n@@ -150,6 +150,11 @@ static int populate_paths_from_revs(struct last_modified *lm)\n \t\t\tgoto out;\n \t\t}\n \n+\t\tif (!repo_peel_to_type(lm->rev.repo, obj->path, 0, obj->item, OBJ_COMMIT)) {\n+\t\t\tret = error(_(\"revision argument is not a commit-ish\"));\n+\t\t\tgoto out;\n+\t\t}\n+\n \t\tdiff_tree_oid(lm->rev.repo->hash_algo->empty_tree,\n \t\t\t &obj->item->oid, \"\", &diffopt);\n \t\tdiff_flush(&diffopt);\ndiff --git a/t/t8020-last-modified.sh b/t/t8020-last-modified.sh\nindex 1183ae667b..22635de447 100755\n--- a/t/t8020-last-modified.sh\n+++ b/t/t8020-last-modified.sh\n@@ -8,6 +8,7 @@ test_expect_success 'setup' '\n \ttest_commit 1 file &&\n \tmkdir a &&\n \ttest_commit 2 a/file &&\n+\tgit tag -mA t2 2 &&\n \tmkdir a/b &&\n \ttest_commit 3 a/b/file\n '\n@@ -55,6 +56,13 @@ test_expect_success 'last-modified recursive' '\n \tEOF\n '\n \n+test_expect_success 'last-modified on annotated tag' '\n+\tcheck_last_modified t2 <<-\\EOF\n+\t2 a\n+\t1 file\n+\tEOF\n+'\n+\n test_expect_success 'last-modified recursive with show-trees' '\n \tcheck_last_modified -r -t <<-\\EOF\n \t3 a/b\n@@ -235,4 +243,9 @@ test_expect_success 'last-modified complains about unknown arguments' '\n \tgrep \"unknown last-modified argument: --foo\" err\n '\n \n+test_expect_success 'last-modified expects commit-ish' '\n+\ttest_must_fail git last-modified HEAD^{tree} 2>err &&\n+\tgrep \"revision argument is not a commit-ish\" err\n+'\n+\n test_done\n\n-- \n2.52.0\n\n"},{"id":"533819","messageId":"aWd2SLrBQRBC1e2S@pks.im","threadId":"64784","inReplyTo":"20260114-toon-last-modified-tree-v2-1-ba3b1860898f@iotcl.com","subject":"Re: [PATCH v2 1/3] last-modified: rewrite error message when more than one revision given","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-14T10:56:08Z","receivedAt":"2026-01-14T10:56:14Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Jan 14, 2026 at 11:24:45AM +0100, Toon Claes wrote:\n> diff --git a/builtin/last-modified.c b/builtin/last-modified.c\n> index c80f0535f6..06e3f79aec 100644\n> --- a/builtin/last-modified.c\n> +++ b/builtin/last-modified.c\n> @@ -145,16 +145,20 @@ static int populate_paths_from_revs(struct last_modified *lm)\n> \t\tif (obj->item->flags & UNINTERESTING)\n> \t\t\tcontinue;\n> \n> -\t\tif (num_interesting++)\n> -\t\t\treturn error(_(\"last-modified can only operate on one tree at a time\"));\n> +\t\tif (num_interesting++) {\n> +\t\t\tret = error(_(\"last-modified can only operate on one revision at a time\"));\n\nDo we maybe want to be a bit more specific and say committish instead of\nrevision?\n\n> diff --git a/t/t8020-last-modified.sh b/t/t8020-last-modified.sh\n> index 50f4312f71..1183ae667b 100755\n> --- a/t/t8020-last-modified.sh\n> +++ b/t/t8020-last-modified.sh\n> @@ -12,7 +12,7 @@ test_expect_success 'setup' '\n> \ttest_commit 3 a/b/file\n> '\n> \n> -test_expect_success 'cannot run last-modified on two trees' '\n> +test_expect_success 'cannot run last-modified on two revision' '\n\nNit: s/revision/revisions/\n\n> \ttest_must_fail git last-modified HEAD HEAD~1\n\nAnother tiny nit: I'm always a bit wary around tests that don't verify\nthe reason for failure. We might want to add:\n\n test_grep \"last-modified can only operate on one revision at a time\" err\n\nPatrick\n"},{"id":"533817","messageId":"aWd2T2mJSvE3XCZN@pks.im","threadId":"64784","inReplyTo":"20260114-toon-last-modified-tree-v2-2-ba3b1860898f@iotcl.com","subject":"Re: [PATCH v2 2/3] last-modified: remove double error message","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-14T10:56:15Z","receivedAt":"2026-01-14T10:56:20Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Jan 14, 2026 at 11:24:46AM +0100, Toon Claes wrote:\n> diff --git a/builtin/last-modified.c b/builtin/last-modified.c\n> index 06e3f79aec..0df85be318 100644\n> --- a/builtin/last-modified.c\n> +++ b/builtin/last-modified.c\n> @@ -495,7 +495,7 @@ static int last_modified_init(struct last_modified *lm, struct repository *r,\n> \tlm->rev.bloom_filter_settings = get_bloom_filter_settings(lm->rev.repo);\n> \n> \tif (populate_paths_from_revs(lm) < 0)\n> -\t\treturn error(_(\"unable to setup last-modified\"));\n> +\t\treturn -1;\n\nMakes sense. There's only one error condition in the function, and that\nerror condition already prints an error message.\n\nPatrick\n"},{"id":"533818","messageId":"aWd2VcX7gERTDK2t@pks.im","threadId":"64784","inReplyTo":"20260114-toon-last-modified-tree-v2-3-ba3b1860898f@iotcl.com","subject":"Re: [PATCH v2 3/3] last-modified: verify revision argument is a commit-ish","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-14T10:56:21Z","receivedAt":"2026-01-14T10:56:26Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Jan 14, 2026 at 11:24:47AM +0100, Toon Claes wrote:\n> Passing a tree OID to git-last-modified(1) would trigger BUG behavior.\n\nI guess passing a blob OID would cause the same. So maybe:\n\n Passing a non-committish revision to git-lsat-modified(1) triggers\n the following BUG:\n\n> git last-modified HEAD^{tree}\n> BUG: builtin/last-modified.c:456: paths remaining beyond boundary in last-modified\n> \n> Fix this error by verifying the parsed revision peels to a commit-ish.\n> \n> While at it, also fix a memory leak in populate_paths_from_revs().\n\nYou fixed this memory leak in a preceding commit now, so this remark is\nnot accurate anymore.\n\n> diff --git a/t/t8020-last-modified.sh b/t/t8020-last-modified.sh\n> index 1183ae667b..22635de447 100755\n> --- a/t/t8020-last-modified.sh\n> +++ b/t/t8020-last-modified.sh\n> @@ -55,6 +56,13 @@ test_expect_success 'last-modified recursive' '\n> \tEOF\n> '\n> \n> +test_expect_success 'last-modified on annotated tag' '\n> +\tcheck_last_modified t2 <<-\\EOF\n> +\t2 a\n> +\t1 file\n> +\tEOF\n> +'\n> +\n> test_expect_success 'last-modified recursive with show-trees' '\n> \tcheck_last_modified -r -t <<-\\EOF\n> \t3 a/b\n\nNice, thanks for adding this test.\n\n> @@ -235,4 +243,9 @@ test_expect_success 'last-modified complains about unknown arguments' '\n> \tgrep \"unknown last-modified argument: --foo\" err\n> '\n> \n> +test_expect_success 'last-modified expects commit-ish' '\n> +\ttest_must_fail git last-modified HEAD^{tree} 2>err &&\n> +\tgrep \"revision argument is not a commit-ish\" err\n> +'\n\nWe typically use `test_grep ()` so that we know what the actual contents\nare in case the assertion ever fails.\n\nPatrick\n"},{"id":"533944","messageId":"87jyxjjejz.fsf@iotcl.com","threadId":"64784","inReplyTo":"aWd2SLrBQRBC1e2S@pks.im","subject":"Re: [PATCH v2 1/3] last-modified: rewrite error message when more than one revision given","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2026-01-15T11:33:36Z","receivedAt":"2026-01-15T11:33:59Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"Patrick Steinhardt writes:\n\n> On Wed, Jan 14, 2026 at 11:24:45AM +0100, Toon Claes wrote:\n>> diff --git a/builtin/last-modified.c b/builtin/last-modified.c\n>> index c80f0535f6..06e3f79aec 100644\n>> --- a/builtin/last-modified.c\n>> +++ b/builtin/last-modified.c\n>> @@ -145,16 +145,20 @@ static int populate_paths_from_revs(struct last_modified *lm)\n>> \t\tif (obj->item->flags & UNINTERESTING)\n>> \t\t\tcontinue;\n>> \n>> -\t\tif (num_interesting++)\n>> -\t\t\treturn error(_(\"last-modified can only operate on one tree at a time\"));\n>> +\t\tif (num_interesting++) {\n>> +\t\t\tret = error(_(\"last-modified can only operate on one revision at a time\"));\n>\n> Do we maybe want to be a bit more specific and say committish instead of\n> revision?\n\nI was thinking about mentioning something like \"commit-ish\" instead, but\nI felt \"commit-ish\" isn't a commonly used term toward end-users. Looking\nat gitglossary(7), it says \"revision\" is a \"synonym for commit\". I'm\nhappy to change this message, but I'm not sure s/revision/commit-ish/ is\nthe best change for this.\n\n>> diff --git a/t/t8020-last-modified.sh b/t/t8020-last-modified.sh\n>> index 50f4312f71..1183ae667b 100755\n>> --- a/t/t8020-last-modified.sh\n>> +++ b/t/t8020-last-modified.sh\n>> @@ -12,7 +12,7 @@ test_expect_success 'setup' '\n>> \ttest_commit 3 a/b/file\n>> '\n>> \n>> -test_expect_success 'cannot run last-modified on two trees' '\n>> +test_expect_success 'cannot run last-modified on two revision' '\n>\n> Nit: s/revision/revisions/\n\nThanks.\n\n>> \ttest_must_fail git last-modified HEAD HEAD~1\n>\n> Another tiny nit: I'm always a bit wary around tests that don't verify\n> the reason for failure. We might want to add:\n>\n> test_grep \"last-modified can only operate on one revision at a time\" err\n\nOkay, I'll rework this one and the one adding in the other commit.\n\n-- \nCheers,\nToon\n"},{"id":"533945","messageId":"aWjVWC71rFeTsHLP@pks.im","threadId":"64784","inReplyTo":"87jyxjjejz.fsf@iotcl.com","subject":"Re: [PATCH v2 1/3] last-modified: rewrite error message when more than one revision given","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-15T11:54:00Z","receivedAt":"2026-01-15T11:54:06Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, Jan 15, 2026 at 12:33:36PM +0100, Toon Claes wrote:\n> Patrick Steinhardt writes:\n> \n> > On Wed, Jan 14, 2026 at 11:24:45AM +0100, Toon Claes wrote:\n> >> diff --git a/builtin/last-modified.c b/builtin/last-modified.c\n> >> index c80f0535f6..06e3f79aec 100644\n> >> --- a/builtin/last-modified.c\n> >> +++ b/builtin/last-modified.c\n> >> @@ -145,16 +145,20 @@ static int populate_paths_from_revs(struct last_modified *lm)\n> >> \t\tif (obj->item->flags & UNINTERESTING)\n> >> \t\t\tcontinue;\n> >> \n> >> -\t\tif (num_interesting++)\n> >> -\t\t\treturn error(_(\"last-modified can only operate on one tree at a time\"));\n> >> +\t\tif (num_interesting++) {\n> >> +\t\t\tret = error(_(\"last-modified can only operate on one revision at a time\"));\n> >\n> > Do we maybe want to be a bit more specific and say committish instead of\n> > revision?\n> \n> I was thinking about mentioning something like \"commit-ish\" instead, but\n> I felt \"commit-ish\" isn't a commonly used term toward end-users. Looking\n> at gitglossary(7), it says \"revision\" is a \"synonym for commit\". I'm\n> happy to change this message, but I'm not sure s/revision/commit-ish/ is\n> the best change for this.\n\ngitglossary(7) also defines commit-ish, but I guess you're right that\nrevision is the more common term. I'm a bit surprised that it's defined\nto be a synonym for a commit, but oh, well.\n\nPatrick\n"},{"id":"533960","messageId":"ca6cfd7a-3392-46ce-99ab-3d93c9ce90dd@app.fastmail.com","threadId":"64784","inReplyTo":"aWjVWC71rFeTsHLP@pks.im","subject":"Re: [PATCH v2 1/3] last-modified: rewrite error message when more than one revision given","fromName":"Kristoffer Haugsbakk","fromEmail":"kristofferhaugsbakk@fastmail.com","sentAt":"2026-01-15T14:34:50Z","receivedAt":"2026-01-15T14:35:11Z","isPatch":true,"sender":{"key":"kristofferhaugsbakk@fastmail.com","avatar":null},"body":"On Thu, Jan 15, 2026, at 12:54, Patrick Steinhardt wrote:\n> On Thu, Jan 15, 2026 at 12:33:36PM +0100, Toon Claes wrote:\n>>[snip]\n>> I was thinking about mentioning something like \"commit-ish\" instead, but\n>> I felt \"commit-ish\" isn't a commonly used term toward end-users. Looking\n>> at gitglossary(7), it says \"revision\" is a \"synonym for commit\". I'm\n>> happy to change this message, but I'm not sure s/revision/commit-ish/ is\n>> the best change for this.\n>\n> gitglossary(7) also defines commit-ish, but I guess you're right that\n> revision is the more common term. I'm a bit surprised that it's defined\n> to be a synonym for a commit, but oh, well.\n\nSurprised that “revision” is a synonym for commit? Why is that?\n"},{"id":"533961","messageId":"116c267a-35c7-4159-8f7a-62792de4644b@app.fastmail.com","threadId":"64784","inReplyTo":"87jyxjjejz.fsf@iotcl.com","subject":"Re: [PATCH v2 1/3] last-modified: rewrite error message when more than one revision given","fromName":"Kristoffer Haugsbakk","fromEmail":"kristofferhaugsbakk@fastmail.com","sentAt":"2026-01-15T14:34:57Z","receivedAt":"2026-01-15T14:35:18Z","isPatch":true,"sender":{"key":"kristofferhaugsbakk@fastmail.com","avatar":null},"body":"On Thu, Jan 15, 2026, at 12:33, Toon Claes wrote:\n> Patrick Steinhardt writes:\n>> On Wed, Jan 14, 2026 at 11:24:45AM +0100, Toon Claes wrote:\n>>>[snip]\n>>> -\t\tif (num_interesting++)\n>>> -\t\t\treturn error(_(\"last-modified can only operate on one tree at a time\"));\n>>> +\t\tif (num_interesting++) {\n>>> +\t\t\tret = error(_(\"last-modified can only operate on one revision at a time\"));\n>>\n>> Do we maybe want to be a bit more specific and say committish instead of\n>> revision?\n>\n> I was thinking about mentioning something like \"commit-ish\" instead, but\n> I felt \"commit-ish\" isn't a commonly used term toward end-users. Looking\n> at gitglossary(7), it says \"revision\" is a \"synonym for commit\". I'm\n> happy to change this message, but I'm not sure s/revision/commit-ish/ is\n> the best change for this.\n\nI just stumbled upon the “IDENTIFIER TERMINOLOGY” section of\ngit(1). “commit-ish” is there.\n\n>[snip]\n"},{"id":"533967","messageId":"bc0db8d5-3c5f-4dd4-9c35-ccb02a6bf5c9@app.fastmail.com","threadId":"64784","inReplyTo":"20260114-toon-last-modified-tree-v2-3-ba3b1860898f@iotcl.com","subject":"Re: [PATCH v2 3/3] last-modified: verify revision argument is a commit-ish","fromName":"Kristoffer Haugsbakk","fromEmail":"kristofferhaugsbakk@fastmail.com","sentAt":"2026-01-15T16:02:09Z","receivedAt":"2026-01-15T16:02:30Z","isPatch":true,"sender":{"key":"kristofferhaugsbakk@fastmail.com","avatar":null},"body":"On Wed, Jan 14, 2026, at 11:24, Toon Claes wrote:\n> Passing a tree OID to git-last-modified(1) would trigger BUG behavior.\n>\n> git last-modified HEAD^{tree}\n> BUG: builtin/last-modified.c:456: paths remaining beyond boundary\n> in last-modified\n>\n> Fix this error by verifying the parsed revision peels to a commit-ish.\n\nNitpick: “peels to commit-ish” = commit-ish so this is a bit\nredundant. Either just “commit-ish” or “peels to commit” would be\nenough.\n\ns/verifying the parsed revision peels to a commit-ish./verifying that the parsed revision is a commit-ish./\n\n>\n> While at it, also fix a memory leak in populate_paths_from_revs().\n\n(Whether or not this is a stale sentence (see Patricks’)) Why not a\nseparate commit for fixing a memory leak?\n\n>\n> Reported-by: Gusted \n> Signed-off-by: Toon Claes \n> ---\n>[snip]\n"},{"id":"533970","messageId":"xmqqcy3adeae.fsf@gitster.g","threadId":"64784","inReplyTo":"bc0db8d5-3c5f-4dd4-9c35-ccb02a6bf5c9@app.fastmail.com","subject":"Re: [PATCH v2 3/3] last-modified: verify revision argument is a commit-ish","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-01-15T16:35:53Z","receivedAt":"2026-01-15T16:35:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Kristoffer Haugsbakk\" writes:\n\n> On Wed, Jan 14, 2026, at 11:24, Toon Claes wrote:\n>> Passing a tree OID to git-last-modified(1) would trigger BUG behavior.\n>>\n>> git last-modified HEAD^{tree}\n>> BUG: builtin/last-modified.c:456: paths remaining beyond boundary\n>> in last-modified\n>>\n>> Fix this error by verifying the parsed revision peels to a commit-ish.\n>\n> Nitpick: “peels to commit-ish” = commit-ish so this is a bit\n> redundant. Either just “commit-ish” or “peels to commit” would be\n> enough.\n\nGreat eyes ;-)\n\n>\n> s/verifying the parsed revision peels to a commit-ish./verifying that the parsed revision is a commit-ish./\n>\n>>\n>> While at it, also fix a memory leak in populate_paths_from_revs().\n>\n> (Whether or not this is a stale sentence (see Patricks’)) Why not a\n> separate commit for fixing a memory leak?\n>\n>>\n>> Reported-by: Gusted \n>> Signed-off-by: Toon Claes \n>> ---\n>>[snip]\n"},{"id":"534021","messageId":"aWnkJ_GpgbXBPwFp@pks.im","threadId":"64784","inReplyTo":"ca6cfd7a-3392-46ce-99ab-3d93c9ce90dd@app.fastmail.com","subject":"Re: [PATCH v2 1/3] last-modified: rewrite error message when more than one revision given","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-16T07:09:27Z","receivedAt":"2026-01-16T07:09:33Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, Jan 15, 2026 at 03:34:50PM +0100, Kristoffer Haugsbakk wrote:\n> On Thu, Jan 15, 2026, at 12:54, Patrick Steinhardt wrote:\n> > On Thu, Jan 15, 2026 at 12:33:36PM +0100, Toon Claes wrote:\n> >>[snip]\n> >> I was thinking about mentioning something like \"commit-ish\" instead, but\n> >> I felt \"commit-ish\" isn't a commonly used term toward end-users. Looking\n> >> at gitglossary(7), it says \"revision\" is a \"synonym for commit\". I'm\n> >> happy to change this message, but I'm not sure s/revision/commit-ish/ is\n> >> the best change for this.\n> >\n> > gitglossary(7) also defines commit-ish, but I guess you're right that\n> > revision is the more common term. I'm a bit surprised that it's defined\n> > to be a synonym for a commit, but oh, well.\n> \n> Surprised that “revision” is a synonym for commit? Why is that?\n\nBecause in my mind a revision can resolve to any object type.\n\"^{}\" for example is a revision, but it can resolve for\nexample to a tree if you say \"HEAD^{tree}\". Still a revision, but\ndefinitely does not resolve to a commit.\n\nAlso, it's confusing to conflate the way to name a commit with a commit\nitself. \"HEAD~10\" is a revision, but taken by itself it's not a commit.\nIt's not even clear whether it resolves, so it feels sensible to me to\nkeep these two concepts separate from one another.\n\nPatrick\n"},{"id":"534030","messageId":"87cy39kae3.fsf@iotcl.com","threadId":"64784","inReplyTo":"aWnkJ_GpgbXBPwFp@pks.im","subject":"Re: [PATCH v2 1/3] last-modified: rewrite error message when more than one revision given","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2026-01-16T12:30:28Z","receivedAt":"2026-01-16T12:30:40Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"Patrick Steinhardt writes:\n\n>> Surprised that “revision” is a synonym for commit? Why is that?\n\nI must admit, I was surprised too.\n\n> Because in my mind a revision can resolve to any object type.\n> \"^{}\" for example is a revision, but it can resolve for\n> example to a tree if you say \"HEAD^{tree}\". Still a revision, but\n> definitely does not resolve to a commit.\n\nBesides the discussion if \"HEAD^{tree}\" is a revision or not. Passing in\ntwo revisions to git-last-modified(1) is not allowed. So I think the\nerror message I proposed makes sense.\n\n> Also, it's confusing to conflate the way to name a commit with a commit\n> itself. \"HEAD~10\" is a revision, but taken by itself it's not a commit.\n> It's not even clear whether it resolves, so it feels sensible to me to\n> keep these two concepts separate from one another.\n\n-- \nCheers,\nToon\n"},{"id":"534031","messageId":"20260116-toon-last-modified-tree-v3-0-e6ade4dc35ab@iotcl.com","threadId":"64784","inReplyTo":"20260114-toon-last-modified-tree-v2-0-ba3b1860898f@iotcl.com","subject":"[PATCH v3 0/4] Fix git-last-modified(1) bug triggered when passing a tree-ish","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2026-01-16T13:08:36Z","receivedAt":"2026-01-16T13:08:53Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"Recently there was a bug reported[1] passing a tree OID triggers a BUG:\n\n $ git last-modified fb06ce04173d47aaaa498385621cba8b8dfd7584\n BUG: builtin/last-modified.c:456: paths remaining beyond boundary in last-modified\n [1] 690163 IOT instruction (core dumped) git last-modified\n\n `fb06ce04173d47aaaa498385621cba8b8dfd7584` is the tree commit id of web_src. I\n suppose this should've returned a nice error message or blank output.\n\nFix this bug by checking the revision argument.\n\n[1]: https://lore.kernel.org/git/03f96860-29fc-42a7-a220-c3ec65eb8516@codeberg.org/\n\n---\nChanges in v3:\n- Split the change to plug the leak into a separate commit.\n- Small changes to commit messages.\n- Link to v2: https://patch.msgid.link/20260114-toon-last-modified-tree-v2-0-ba3b1860898f@iotcl.com\n\nChanges in v2:\n- Prepend the change with a commit that modifies the error message\n when more than one revision is given.\n- Prepend another commit that removes double error message.\n- Add test to ensure the command works with annotated tags too.\n- Link to v1: https://patch.msgid.link/20260112-toon-last-modified-tree-v1-1-ecbc78341f76@iotcl.com\n\n---\nToon Claes (4):\n last-modified: rewrite error message when more than one revision given\n last-modified: fix memory leak when more than one revision is given\n last-modified: remove double error message\n last-modified: verify revision argument is a commit-ish\n\n builtin/last-modified.c | 19 ++++++++++++++-----\n t/t8020-last-modified.sh | 15 ++++++++++++++-\n 2 files changed, 28 insertions(+), 6 deletions(-)\n\nRange-diff versus v2:\n\n-: ---------- > 1: 053c119ab1 last-modified: rewrite error message when more than one revision given\n1: 5c9936500b ! 2: 827b1500fe last-modified: rewrite error message when more than one revision given\n @@ Metadata\n Author: Toon Claes \n \n ## Commit message ##\n - last-modified: rewrite error message when more than one revision given\n + last-modified: fix memory leak when more than one revision is given\n \n - When more than one revision is passed to the git-last-modified(1)\n - command, this error message was printed:\n -\n - error: last-modified can only operate on one tree at a time\n -\n - Calling these a \"tree\" is technically not correct. git-last-modified(1)\n - expects revisions that peel to a commit.\n -\n - Rephrase the error message to:\n -\n - error: last-modified can only operate on one revision at a time\n -\n - While at it, also fix a memory leak that remained uncovered so far.\n + When more than one revision is given, the function\n + populate_paths_from_revs() leaks a `struct pathspec`. Plug it.\n \n Signed-off-by: Toon Claes \n \n @@ builtin/last-modified.c: static int populate_paths_from_revs(struct last_modifie\n \t\t\tcontinue;\n \n -\t\tif (num_interesting++)\n --\t\t\treturn error(_(\"last-modified can only operate on one tree at a time\"));\n +-\t\t\treturn error(_(\"last-modified can only operate on one revision at a time\"));\n +\t\tif (num_interesting++) {\n +\t\t\tret = error(_(\"last-modified can only operate on one revision at a time\"));\n +\t\t\tgoto out;\n @@ builtin/last-modified.c: static int populate_paths_from_revs(struct last_modifie\n }\n \n static void last_modified_emit(struct last_modified *lm,\n -\n - ## t/t8020-last-modified.sh ##\n -@@ t/t8020-last-modified.sh: test_expect_success 'setup' '\n - \ttest_commit 3 a/b/file\n - '\n - \n --test_expect_success 'cannot run last-modified on two trees' '\n -+test_expect_success 'cannot run last-modified on two revision' '\n - \ttest_must_fail git last-modified HEAD HEAD~1\n - '\n - \n2: 5c964488fd = 3: 7748574724 last-modified: remove double error message\n3: cb6ff40853 ! 4: 6846722750 last-modified: verify revision argument is a commit-ish\n @@ Commit message\n git last-modified HEAD^{tree}\n BUG: builtin/last-modified.c:456: paths remaining beyond boundary in last-modified\n \n - Fix this error by verifying the parsed revision peels to a commit-ish.\n -\n - While at it, also fix a memory leak in populate_paths_from_revs().\n + Fix this error by verifying the parsed revision is a commit-ish.\n \n Reported-by: Gusted \n Signed-off-by: Toon Claes \n\n\n---\nbase-commit: 7264e61d87e58b9d0f5e6424c47c11e9657dfb75\nchange-id: 20260112-toon-last-modified-tree-fdd96b2feaf7\n\n"},{"id":"534032","messageId":"20260116-toon-last-modified-tree-v3-1-e6ade4dc35ab@iotcl.com","threadId":"64784","inReplyTo":"20260116-toon-last-modified-tree-v3-0-e6ade4dc35ab@iotcl.com","subject":"[PATCH v3 1/4] last-modified: rewrite error message when more than one revision given","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2026-01-16T13:08:37Z","receivedAt":"2026-01-16T13:08:57Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"When more than one revision is passed to the git-last-modified(1)\ncommand, this error message was printed:\n\n error: last-modified can only operate on one tree at a time\n\nCalling these a \"tree\" is technically not correct. git-last-modified(1)\nexpects revisions that peel to a commit.\n\nRephrase the error message to:\n\n error: last-modified can only operate on one revision at a time\n\nSigned-off-by: Toon Claes \n---\n builtin/last-modified.c | 2 +-\n t/t8020-last-modified.sh | 2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/last-modified.c b/builtin/last-modified.c\nindex c80f0535f6..7d95244e3f 100644\n--- a/builtin/last-modified.c\n+++ b/builtin/last-modified.c\n@@ -146,7 +146,7 @@ static int populate_paths_from_revs(struct last_modified *lm)\n \t\t\tcontinue;\n \n \t\tif (num_interesting++)\n-\t\t\treturn error(_(\"last-modified can only operate on one tree at a time\"));\n+\t\t\treturn error(_(\"last-modified can only operate on one revision at a time\"));\n \n \t\tdiff_tree_oid(lm->rev.repo->hash_algo->empty_tree,\n \t\t\t &obj->item->oid, \"\", &diffopt);\ndiff --git a/t/t8020-last-modified.sh b/t/t8020-last-modified.sh\nindex 50f4312f71..1183ae667b 100755\n--- a/t/t8020-last-modified.sh\n+++ b/t/t8020-last-modified.sh\n@@ -12,7 +12,7 @@ test_expect_success 'setup' '\n \ttest_commit 3 a/b/file\n '\n \n-test_expect_success 'cannot run last-modified on two trees' '\n+test_expect_success 'cannot run last-modified on two revision' '\n \ttest_must_fail git last-modified HEAD HEAD~1\n '\n \n\n-- \n2.52.0\n\n"},{"id":"534033","messageId":"20260116-toon-last-modified-tree-v3-2-e6ade4dc35ab@iotcl.com","threadId":"64784","inReplyTo":"20260116-toon-last-modified-tree-v3-0-e6ade4dc35ab@iotcl.com","subject":"[PATCH v3 2/4] last-modified: fix memory leak when more than one revision is given","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2026-01-16T13:08:38Z","receivedAt":"2026-01-16T13:09:10Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"When more than one revision is given, the function\npopulate_paths_from_revs() leaks a `struct pathspec`. Plug it.\n\nSigned-off-by: Toon Claes \n---\n builtin/last-modified.c | 12 ++++++++----\n 1 file changed, 8 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/last-modified.c b/builtin/last-modified.c\nindex 7d95244e3f..06e3f79aec 100644\n--- a/builtin/last-modified.c\n+++ b/builtin/last-modified.c\n@@ -123,7 +123,7 @@ static void add_path_from_diff(struct diff_queue_struct *q,\n \n static int populate_paths_from_revs(struct last_modified *lm)\n {\n-\tint num_interesting = 0;\n+\tint num_interesting = 0, ret = 0;\n \tstruct diff_options diffopt;\n \n \t/*\n@@ -145,16 +145,20 @@ static int populate_paths_from_revs(struct last_modified *lm)\n \t\tif (obj->item->flags & UNINTERESTING)\n \t\t\tcontinue;\n \n-\t\tif (num_interesting++)\n-\t\t\treturn error(_(\"last-modified can only operate on one revision at a time\"));\n+\t\tif (num_interesting++) {\n+\t\t\tret = error(_(\"last-modified can only operate on one revision at a time\"));\n+\t\t\tgoto out;\n+\t\t}\n \n \t\tdiff_tree_oid(lm->rev.repo->hash_algo->empty_tree,\n \t\t\t &obj->item->oid, \"\", &diffopt);\n \t\tdiff_flush(&diffopt);\n \t}\n+\n+out:\n \tclear_pathspec(&diffopt.pathspec);\n \n-\treturn 0;\n+\treturn ret;\n }\n \n static void last_modified_emit(struct last_modified *lm,\n\n-- \n2.52.0\n\n"},{"id":"534034","messageId":"20260116-toon-last-modified-tree-v3-3-e6ade4dc35ab@iotcl.com","threadId":"64784","inReplyTo":"20260116-toon-last-modified-tree-v3-0-e6ade4dc35ab@iotcl.com","subject":"[PATCH v3 3/4] last-modified: remove double error message","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2026-01-16T13:08:39Z","receivedAt":"2026-01-16T13:09:14Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"When the user passes two revisions, they get the following output:\n\n $ git last-modified HEAD HEAD~\n error: last-modified can only operate on one revision at a time\n error: unable to setup last-modified\n\nThe error message about \"unable to setup\" is not very informative,\nremove it.\n\nSigned-off-by: Toon Claes \n---\n builtin/last-modified.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/last-modified.c b/builtin/last-modified.c\nindex 06e3f79aec..0df85be318 100644\n--- a/builtin/last-modified.c\n+++ b/builtin/last-modified.c\n@@ -495,7 +495,7 @@ static int last_modified_init(struct last_modified *lm, struct repository *r,\n \tlm->rev.bloom_filter_settings = get_bloom_filter_settings(lm->rev.repo);\n \n \tif (populate_paths_from_revs(lm) < 0)\n-\t\treturn error(_(\"unable to setup last-modified\"));\n+\t\treturn -1;\n \n \tCALLOC_ARRAY(lm->all_paths, hashmap_get_size(&lm->paths));\n \tlm->all_paths_nr = 0;\n\n-- \n2.52.0\n\n"},{"id":"534035","messageId":"20260116-toon-last-modified-tree-v3-4-e6ade4dc35ab@iotcl.com","threadId":"64784","inReplyTo":"20260116-toon-last-modified-tree-v3-0-e6ade4dc35ab@iotcl.com","subject":"[PATCH v3 4/4] last-modified: verify revision argument is a commit-ish","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2026-01-16T13:08:40Z","receivedAt":"2026-01-16T13:09:18Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"Passing a tree OID to git-last-modified(1) would trigger BUG behavior.\n\n git last-modified HEAD^{tree}\n BUG: builtin/last-modified.c:456: paths remaining beyond boundary in last-modified\n\nFix this error by verifying the parsed revision is a commit-ish.\n\nReported-by: Gusted \nSigned-off-by: Toon Claes \n---\n builtin/last-modified.c | 5 +++++\n t/t8020-last-modified.sh | 13 +++++++++++++\n 2 files changed, 18 insertions(+)\n\ndiff --git a/builtin/last-modified.c b/builtin/last-modified.c\nindex 0df85be318..5366cedd0f 100644\n--- a/builtin/last-modified.c\n+++ b/builtin/last-modified.c\n@@ -150,6 +150,11 @@ static int populate_paths_from_revs(struct last_modified *lm)\n \t\t\tgoto out;\n \t\t}\n \n+\t\tif (!repo_peel_to_type(lm->rev.repo, obj->path, 0, obj->item, OBJ_COMMIT)) {\n+\t\t\tret = error(_(\"revision argument is not a commit-ish\"));\n+\t\t\tgoto out;\n+\t\t}\n+\n \t\tdiff_tree_oid(lm->rev.repo->hash_algo->empty_tree,\n \t\t\t &obj->item->oid, \"\", &diffopt);\n \t\tdiff_flush(&diffopt);\ndiff --git a/t/t8020-last-modified.sh b/t/t8020-last-modified.sh\nindex 1183ae667b..22635de447 100755\n--- a/t/t8020-last-modified.sh\n+++ b/t/t8020-last-modified.sh\n@@ -8,6 +8,7 @@ test_expect_success 'setup' '\n \ttest_commit 1 file &&\n \tmkdir a &&\n \ttest_commit 2 a/file &&\n+\tgit tag -mA t2 2 &&\n \tmkdir a/b &&\n \ttest_commit 3 a/b/file\n '\n@@ -55,6 +56,13 @@ test_expect_success 'last-modified recursive' '\n \tEOF\n '\n \n+test_expect_success 'last-modified on annotated tag' '\n+\tcheck_last_modified t2 <<-\\EOF\n+\t2 a\n+\t1 file\n+\tEOF\n+'\n+\n test_expect_success 'last-modified recursive with show-trees' '\n \tcheck_last_modified -r -t <<-\\EOF\n \t3 a/b\n@@ -235,4 +243,9 @@ test_expect_success 'last-modified complains about unknown arguments' '\n \tgrep \"unknown last-modified argument: --foo\" err\n '\n \n+test_expect_success 'last-modified expects commit-ish' '\n+\ttest_must_fail git last-modified HEAD^{tree} 2>err &&\n+\tgrep \"revision argument is not a commit-ish\" err\n+'\n+\n test_done\n\n-- \n2.52.0\n\n"},{"id":"534036","messageId":"87a4ydk8hg.fsf@iotcl.com","threadId":"64784","inReplyTo":"bc0db8d5-3c5f-4dd4-9c35-ccb02a6bf5c9@app.fastmail.com","subject":"Re: [PATCH v2 3/3] last-modified: verify revision argument is a commit-ish","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2026-01-16T13:11:39Z","receivedAt":"2026-01-16T13:11:51Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"\"Kristoffer Haugsbakk\" writes:\n\n> On Wed, Jan 14, 2026, at 11:24, Toon Claes wrote:\n>> Passing a tree OID to git-last-modified(1) would trigger BUG behavior.\n>>\n>> git last-modified HEAD^{tree}\n>> BUG: builtin/last-modified.c:456: paths remaining beyond boundary\n>> in last-modified\n>>\n>> Fix this error by verifying the parsed revision peels to a commit-ish.\n>\n> Nitpick: “peels to commit-ish” = commit-ish so this is a bit\n> redundant. Either just “commit-ish” or “peels to commit” would be\n> enough.\n>\n> s/verifying the parsed revision peels to a commit-ish./verifying that\n> the parsed revision is a commit-ish./\n\nThanks! I've addressed this in v3.\n\n>> While at it, also fix a memory leak in populate_paths_from_revs().\n>\n> (Whether or not this is a stale sentence (see Patricks’)) Why not a\n> separate commit for fixing a memory leak?\n\nYeah, it's better to do that separately, so I've done so now.\n\n-- \nCheers,\nToon\n"},{"id":"534060","messageId":"xmqq3445a370.fsf@gitster.g","threadId":"64784","inReplyTo":"aWnkJ_GpgbXBPwFp@pks.im","subject":"Re: [PATCH v2 1/3] last-modified: rewrite error message when more than one revision given","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-01-16T17:16:03Z","receivedAt":"2026-01-16T17:16:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n>> Surprised that “revision” is a synonym for commit? Why is that?\n>\n> Because in my mind a revision can resolve to any object type.\n\nYup, in the early days of this mailing list (like in 2005 ;-), the\nword \"revision\" was used more or less interchangeably with \"object\nname\", but \"a revision\" was much more likely to refer to a commit\nthan \"an object name\". The name of the file that implements one of\nthe more core-ish part of the system is \"revision.c\" and talks about\n\"revision traversal\", which is mostly about following parent pointers\nin commit DAG, but also follows into trees starting from commits.\n\n> Also, it's confusing to conflate the way to name a commit with a commit\n> itself. \"HEAD~10\" is a revision, but taken by itself it's not a commit.\n\nI do not know about this. If HEAD~10 does not resolve to anything,\nit would not be a commit and it would not be a revision, either.\n\n> It's not even clear whether it resolves, so it feels sensible to me to\n> keep these two concepts separate from one another.\n>\n> Patrick\n"},{"id":"534064","messageId":"xmqqpl798nw0.fsf@gitster.g","threadId":"64784","inReplyTo":"20260116-toon-last-modified-tree-v3-1-e6ade4dc35ab@iotcl.com","subject":"Re: [PATCH v3 1/4] last-modified: rewrite error message when more than one revision given","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-01-16T17:31:59Z","receivedAt":"2026-01-16T17:32:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Toon Claes writes:\n\n> -test_expect_success 'cannot run last-modified on two trees' '\n> +test_expect_success 'cannot run last-modified on two revision' '\n\nNot \"revisions\"?\n\n> \ttest_must_fail git last-modified HEAD HEAD~1\n> '\n"},{"id":"534068","messageId":"xmqqldhx8lja.fsf@gitster.g","threadId":"64784","inReplyTo":"20260116-toon-last-modified-tree-v3-3-e6ade4dc35ab@iotcl.com","subject":"Re: [PATCH v3 3/4] last-modified: remove double error message","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-01-16T18:22:49Z","receivedAt":"2026-01-16T18:22:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Toon Claes writes:\n\n> When the user passes two revisions, they get the following output:\n>\n> $ git last-modified HEAD HEAD~\n> error: last-modified can only operate on one revision at a time\n> error: unable to setup last-modified\n>\n> The error message about \"unable to setup\" is not very informative,\n> remove it.\n>\n> Signed-off-by: Toon Claes \n> ---\n> builtin/last-modified.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/builtin/last-modified.c b/builtin/last-modified.c\n> index 06e3f79aec..0df85be318 100644\n> --- a/builtin/last-modified.c\n> +++ b/builtin/last-modified.c\n> @@ -495,7 +495,7 @@ static int last_modified_init(struct last_modified *lm, struct repository *r,\n> \tlm->rev.bloom_filter_settings = get_bloom_filter_settings(lm->rev.repo);\n> \n> \tif (populate_paths_from_revs(lm) < 0)\n> -\t\treturn error(_(\"unable to setup last-modified\"));\n> +\t\treturn -1;\n\nIt makes perfect sense to keep the more detailed error that\nimmediately leads to a more correct action and take out the more\ngeneric \"we failed\" one. Nice.\n"},{"id":"534069","messageId":"xmqqh5sl8lg3.fsf@gitster.g","threadId":"64784","inReplyTo":"20260116-toon-last-modified-tree-v3-4-e6ade4dc35ab@iotcl.com","subject":"Re: [PATCH v3 4/4] last-modified: verify revision argument is a commit-ish","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-01-16T18:24:44Z","receivedAt":"2026-01-16T18:24:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Toon Claes writes:\n\n> Passing a tree OID to git-last-modified(1) would trigger BUG behavior.\n>\n> git last-modified HEAD^{tree}\n> BUG: builtin/last-modified.c:456: paths remaining beyond boundary in last-modified\n>\n> Fix this error by verifying the parsed revision is a commit-ish.\n>\n> Reported-by: Gusted \n> Signed-off-by: Toon Claes \n> ---\n> builtin/last-modified.c | 5 +++++\n> t/t8020-last-modified.sh | 13 +++++++++++++\n> 2 files changed, 18 insertions(+)\n>\n> diff --git a/builtin/last-modified.c b/builtin/last-modified.c\n> index 0df85be318..5366cedd0f 100644\n> --- a/builtin/last-modified.c\n> +++ b/builtin/last-modified.c\n> @@ -150,6 +150,11 @@ static int populate_paths_from_revs(struct last_modified *lm)\n> \t\t\tgoto out;\n> \t\t}\n> \n> +\t\tif (!repo_peel_to_type(lm->rev.repo, obj->path, 0, obj->item, OBJ_COMMIT)) {\n> +\t\t\tret = error(_(\"revision argument is not a commit-ish\"));\n> +\t\t\tgoto out;\n> +\t\t}\n\nThis will do, at least for now, but I tend to prefer to say what the\nuser gave us when we expected a commit, e.g., \"commit expected, got\na tree\".\n\n"},{"id":"534171","messageId":"aW3V7-RV9wCEw1vY@pks.im","threadId":"64784","inReplyTo":"xmqq3445a370.fsf@gitster.g","subject":"Re: [PATCH v2 1/3] last-modified: rewrite error message when more than one revision given","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-19T06:57:51Z","receivedAt":"2026-01-19T06:57:57Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Jan 16, 2026 at 09:16:03AM -0800, Junio C Hamano wrote:\n> Patrick Steinhardt writes:\n> \n> >> Surprised that “revision” is a synonym for commit? Why is that?\n> >\n> > Because in my mind a revision can resolve to any object type.\n> \n> Yup, in the early days of this mailing list (like in 2005 ;-), the\n> word \"revision\" was used more or less interchangeably with \"object\n> name\", but \"a revision\" was much more likely to refer to a commit\n> than \"an object name\". \n\nIt's probably still much more likely that a revision refers to a commit\nrather than anything else.\n\n> The name of the file that implements one of the more core-ish part of\n> the system is \"revision.c\" and talks about \"revision traversal\", which\n> is mostly about following parent pointers in commit DAG, but also\n> follows into trees starting from commits.\n\nThis discussion makes me wonder whether we should maybe update how we\ndefine a \"revision\" in our glossary. One could take gitrevisions(1) as a\nstarting point:\n\n A revision typically, but not necessarily, names a commit object. It\n uses what is called an extended SHA-1 syntax.\n\nWe should probably get rid of \"SHA-1\" though. So maybe:\n\n A revision is used to refer to a specific object, typically a\n commit, using extended object name syntax. Refer to\n gitlink:gitrevisions[7] for more information.\n\n> > Also, it's confusing to conflate the way to name a commit with a commit\n> > itself. \"HEAD~10\" is a revision, but taken by itself it's not a commit.\n> \n> I do not know about this. If HEAD~10 does not resolve to anything,\n> it would not be a commit and it would not be a revision, either.\n\nI guess things are getting philosophical here :) I rather see it like a\npointer: a pointer is still a pointer even if it doesn't point to\nanything.\n\nPatrick\n"},{"id":"534542","messageId":"20260123-toon-last-modified-tree-v4-0-86bf97bad4e2@iotcl.com","threadId":"64784","inReplyTo":"20260116-toon-last-modified-tree-v3-0-e6ade4dc35ab@iotcl.com","subject":"[PATCH v4 0/4] Fix git-last-modified(1) bug triggered when passing a tree-ish","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2026-01-23T14:33:00Z","receivedAt":"2026-01-23T14:33:23Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"Recently there was a bug reported[1] passing a tree OID triggers a BUG:\n\n $ git last-modified fb06ce04173d47aaaa498385621cba8b8dfd7584\n BUG: builtin/last-modified.c:456: paths remaining beyond boundary in last-modified\n [1] 690163 IOT instruction (core dumped) git last-modified\n\n `fb06ce04173d47aaaa498385621cba8b8dfd7584` is the tree commit id of web_src. I\n suppose this should've returned a nice error message or blank output.\n\nFix this bug by checking the revision argument.\n\n[1]: https://lore.kernel.org/git/03f96860-29fc-42a7-a220-c3ec65eb8516@codeberg.org/\n\n---\nChanges in v4:\n- Extend the error message when revision is not a tree\n- Extend the test to verify the error message when more than one\n revision is given.\n- Link to v3: https://patch.msgid.link/20260116-toon-last-modified-tree-v3-0-e6ade4dc35ab@iotcl.com\n\nChanges in v3:\n- Split the change to plug the leak into a separate commit.\n- Small changes to commit messages.\n- Link to v2: https://patch.msgid.link/20260114-toon-last-modified-tree-v2-0-ba3b1860898f@iotcl.com\n\nChanges in v2:\n- Prepend the change with a commit that modifies the error message\n when more than one revision is given.\n- Prepend another commit that removes double error message.\n- Add test to ensure the command works with annotated tags too.\n- Link to v1: https://patch.msgid.link/20260112-toon-last-modified-tree-v1-1-ecbc78341f76@iotcl.com\n\n---\nToon Claes (4):\n last-modified: rewrite error message when more than one revision given\n last-modified: fix memory leak when more than one revision is given\n last-modified: remove double error message\n last-modified: verify revision argument is a commit-ish\n\n builtin/last-modified.c | 19 ++++++++++++++-----\n t/t8020-last-modified.sh | 24 +++++++++++++++++++-----\n 2 files changed, 33 insertions(+), 10 deletions(-)\n\nRange-diff versus v3:\n\n1: 8786f5d6a4 ! 1: 1bd4bb7cb8 last-modified: rewrite error message when more than one revision given\n @@ Commit message\n \n error: last-modified can only operate on one revision at a time\n \n + While at it, ensure modify the test to ensure the correct error message\n + is printed.\n +\n Signed-off-by: Toon Claes \n \n ## builtin/last-modified.c ##\n @@ t/t8020-last-modified.sh: test_expect_success 'setup' '\n '\n \n -test_expect_success 'cannot run last-modified on two trees' '\n +-\ttest_must_fail git last-modified HEAD HEAD~1\n +-'\n +-\n + check_last_modified() {\n + \tlocal indir= &&\n + \twhile test $# != 0\n +@@ t/t8020-last-modified.sh: test_expect_success 'last-modified merge undoes changes' '\n + \tEOF\n + '\n + \n +test_expect_success 'cannot run last-modified on two revision' '\n - \ttest_must_fail git last-modified HEAD HEAD~1\n ++\ttest_must_fail git last-modified HEAD HEAD~1 2>err &&\n ++\ttest_grep \"last-modified can only operate on one revision at a time\" err\n ++'\n ++\n + test_expect_success 'last-modified complains about unknown arguments' '\n + \ttest_must_fail git last-modified --foo 2>err &&\n +-\tgrep \"unknown last-modified argument: --foo\" err\n ++\ttest_grep \"unknown last-modified argument: --foo\" err\n '\n \n + test_done\n2: 78ec93a9e2 = 2: ed1bd102a2 last-modified: fix memory leak when more than one revision is given\n3: 05b4424289 = 3: 7a6e587da1 last-modified: remove double error message\n4: 00e29cd6a1 ! 4: fee3aa92a9 last-modified: verify revision argument is a commit-ish\n @@ builtin/last-modified.c: static int populate_paths_from_revs(struct last_modifie\n \t\t}\n \n +\t\tif (!repo_peel_to_type(lm->rev.repo, obj->path, 0, obj->item, OBJ_COMMIT)) {\n -+\t\t\tret = error(_(\"revision argument is not a commit-ish\"));\n ++\t\t\tret = error(_(\"revision argument '%s' is a %s, not a commit-ish\"), obj->name, type_name(obj->item->type));\n +\t\t\tgoto out;\n +\t\t}\n +\n @@ t/t8020-last-modified.sh: test_expect_success 'last-modified recursive' '\n \tcheck_last_modified -r -t <<-\\EOF\n \t3 a/b\n @@ t/t8020-last-modified.sh: test_expect_success 'last-modified complains about unknown arguments' '\n - \tgrep \"unknown last-modified argument: --foo\" err\n + \ttest_grep \"unknown last-modified argument: --foo\" err\n '\n \n +test_expect_success 'last-modified expects commit-ish' '\n +\ttest_must_fail git last-modified HEAD^{tree} 2>err &&\n -+\tgrep \"revision argument is not a commit-ish\" err\n ++\tgrep \"revision argument '\"'\"'HEAD^{tree}'\"'\"' is a tree, not a commit-ish\" err\n +'\n +\n test_done\n\n\n---\nbase-commit: 7264e61d87e58b9d0f5e6424c47c11e9657dfb75\nchange-id: 20260112-toon-last-modified-tree-fdd96b2feaf7\n\n"},{"id":"534543","messageId":"20260123-toon-last-modified-tree-v4-1-86bf97bad4e2@iotcl.com","threadId":"64784","inReplyTo":"20260123-toon-last-modified-tree-v4-0-86bf97bad4e2@iotcl.com","subject":"[PATCH v4 1/4] last-modified: rewrite error message when more than one revision given","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2026-01-23T14:33:01Z","receivedAt":"2026-01-23T14:33:25Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"When more than one revision is passed to the git-last-modified(1)\ncommand, this error message was printed:\n\n error: last-modified can only operate on one tree at a time\n\nCalling these a \"tree\" is technically not correct. git-last-modified(1)\nexpects revisions that peel to a commit.\n\nRephrase the error message to:\n\n error: last-modified can only operate on one revision at a time\n\nWhile at it, ensure modify the test to ensure the correct error message\nis printed.\n\nSigned-off-by: Toon Claes \n---\n builtin/last-modified.c | 2 +-\n t/t8020-last-modified.sh | 11 ++++++-----\n 2 files changed, 7 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/last-modified.c b/builtin/last-modified.c\nindex c80f0535f6..7d95244e3f 100644\n--- a/builtin/last-modified.c\n+++ b/builtin/last-modified.c\n@@ -146,7 +146,7 @@ static int populate_paths_from_revs(struct last_modified *lm)\n \t\t\tcontinue;\n \n \t\tif (num_interesting++)\n-\t\t\treturn error(_(\"last-modified can only operate on one tree at a time\"));\n+\t\t\treturn error(_(\"last-modified can only operate on one revision at a time\"));\n \n \t\tdiff_tree_oid(lm->rev.repo->hash_algo->empty_tree,\n \t\t\t &obj->item->oid, \"\", &diffopt);\ndiff --git a/t/t8020-last-modified.sh b/t/t8020-last-modified.sh\nindex 50f4312f71..91901eed58 100755\n--- a/t/t8020-last-modified.sh\n+++ b/t/t8020-last-modified.sh\n@@ -12,10 +12,6 @@ test_expect_success 'setup' '\n \ttest_commit 3 a/b/file\n '\n \n-test_expect_success 'cannot run last-modified on two trees' '\n-\ttest_must_fail git last-modified HEAD HEAD~1\n-'\n-\n check_last_modified() {\n \tlocal indir= &&\n \twhile test $# != 0\n@@ -230,9 +226,14 @@ test_expect_success 'last-modified merge undoes changes' '\n \tEOF\n '\n \n+test_expect_success 'cannot run last-modified on two revision' '\n+\ttest_must_fail git last-modified HEAD HEAD~1 2>err &&\n+\ttest_grep \"last-modified can only operate on one revision at a time\" err\n+'\n+\n test_expect_success 'last-modified complains about unknown arguments' '\n \ttest_must_fail git last-modified --foo 2>err &&\n-\tgrep \"unknown last-modified argument: --foo\" err\n+\ttest_grep \"unknown last-modified argument: --foo\" err\n '\n \n test_done\n\n-- \n2.52.0\n\n"},{"id":"534544","messageId":"20260123-toon-last-modified-tree-v4-2-86bf97bad4e2@iotcl.com","threadId":"64784","inReplyTo":"20260123-toon-last-modified-tree-v4-0-86bf97bad4e2@iotcl.com","subject":"[PATCH v4 2/4] last-modified: fix memory leak when more than one revision is given","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2026-01-23T14:33:02Z","receivedAt":"2026-01-23T14:33:29Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"When more than one revision is given, the function\npopulate_paths_from_revs() leaks a `struct pathspec`. Plug it.\n\nSigned-off-by: Toon Claes \n---\n builtin/last-modified.c | 12 ++++++++----\n 1 file changed, 8 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/last-modified.c b/builtin/last-modified.c\nindex 7d95244e3f..06e3f79aec 100644\n--- a/builtin/last-modified.c\n+++ b/builtin/last-modified.c\n@@ -123,7 +123,7 @@ static void add_path_from_diff(struct diff_queue_struct *q,\n \n static int populate_paths_from_revs(struct last_modified *lm)\n {\n-\tint num_interesting = 0;\n+\tint num_interesting = 0, ret = 0;\n \tstruct diff_options diffopt;\n \n \t/*\n@@ -145,16 +145,20 @@ static int populate_paths_from_revs(struct last_modified *lm)\n \t\tif (obj->item->flags & UNINTERESTING)\n \t\t\tcontinue;\n \n-\t\tif (num_interesting++)\n-\t\t\treturn error(_(\"last-modified can only operate on one revision at a time\"));\n+\t\tif (num_interesting++) {\n+\t\t\tret = error(_(\"last-modified can only operate on one revision at a time\"));\n+\t\t\tgoto out;\n+\t\t}\n \n \t\tdiff_tree_oid(lm->rev.repo->hash_algo->empty_tree,\n \t\t\t &obj->item->oid, \"\", &diffopt);\n \t\tdiff_flush(&diffopt);\n \t}\n+\n+out:\n \tclear_pathspec(&diffopt.pathspec);\n \n-\treturn 0;\n+\treturn ret;\n }\n \n static void last_modified_emit(struct last_modified *lm,\n\n-- \n2.52.0\n\n"},{"id":"534545","messageId":"20260123-toon-last-modified-tree-v4-3-86bf97bad4e2@iotcl.com","threadId":"64784","inReplyTo":"20260123-toon-last-modified-tree-v4-0-86bf97bad4e2@iotcl.com","subject":"[PATCH v4 3/4] last-modified: remove double error message","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2026-01-23T14:33:03Z","receivedAt":"2026-01-23T14:33:32Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"When the user passes two revisions, they get the following output:\n\n $ git last-modified HEAD HEAD~\n error: last-modified can only operate on one revision at a time\n error: unable to setup last-modified\n\nThe error message about \"unable to setup\" is not very informative,\nremove it.\n\nSigned-off-by: Toon Claes \n---\n builtin/last-modified.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/last-modified.c b/builtin/last-modified.c\nindex 06e3f79aec..0df85be318 100644\n--- a/builtin/last-modified.c\n+++ b/builtin/last-modified.c\n@@ -495,7 +495,7 @@ static int last_modified_init(struct last_modified *lm, struct repository *r,\n \tlm->rev.bloom_filter_settings = get_bloom_filter_settings(lm->rev.repo);\n \n \tif (populate_paths_from_revs(lm) < 0)\n-\t\treturn error(_(\"unable to setup last-modified\"));\n+\t\treturn -1;\n \n \tCALLOC_ARRAY(lm->all_paths, hashmap_get_size(&lm->paths));\n \tlm->all_paths_nr = 0;\n\n-- \n2.52.0\n\n"},{"id":"534546","messageId":"20260123-toon-last-modified-tree-v4-4-86bf97bad4e2@iotcl.com","threadId":"64784","inReplyTo":"20260123-toon-last-modified-tree-v4-0-86bf97bad4e2@iotcl.com","subject":"[PATCH v4 4/4] last-modified: verify revision argument is a commit-ish","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2026-01-23T14:33:04Z","receivedAt":"2026-01-23T14:33:36Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"Passing a tree OID to git-last-modified(1) would trigger BUG behavior.\n\n git last-modified HEAD^{tree}\n BUG: builtin/last-modified.c:456: paths remaining beyond boundary in last-modified\n\nFix this error by verifying the parsed revision is a commit-ish.\n\nReported-by: Gusted \nSigned-off-by: Toon Claes \n---\n builtin/last-modified.c | 5 +++++\n t/t8020-last-modified.sh | 13 +++++++++++++\n 2 files changed, 18 insertions(+)\n\ndiff --git a/builtin/last-modified.c b/builtin/last-modified.c\nindex 0df85be318..686cb258bb 100644\n--- a/builtin/last-modified.c\n+++ b/builtin/last-modified.c\n@@ -150,6 +150,11 @@ static int populate_paths_from_revs(struct last_modified *lm)\n \t\t\tgoto out;\n \t\t}\n \n+\t\tif (!repo_peel_to_type(lm->rev.repo, obj->path, 0, obj->item, OBJ_COMMIT)) {\n+\t\t\tret = error(_(\"revision argument '%s' is a %s, not a commit-ish\"), obj->name, type_name(obj->item->type));\n+\t\t\tgoto out;\n+\t\t}\n+\n \t\tdiff_tree_oid(lm->rev.repo->hash_algo->empty_tree,\n \t\t\t &obj->item->oid, \"\", &diffopt);\n \t\tdiff_flush(&diffopt);\ndiff --git a/t/t8020-last-modified.sh b/t/t8020-last-modified.sh\nindex 91901eed58..c88ec1854f 100755\n--- a/t/t8020-last-modified.sh\n+++ b/t/t8020-last-modified.sh\n@@ -8,6 +8,7 @@ test_expect_success 'setup' '\n \ttest_commit 1 file &&\n \tmkdir a &&\n \ttest_commit 2 a/file &&\n+\tgit tag -mA t2 2 &&\n \tmkdir a/b &&\n \ttest_commit 3 a/b/file\n '\n@@ -51,6 +52,13 @@ test_expect_success 'last-modified recursive' '\n \tEOF\n '\n \n+test_expect_success 'last-modified on annotated tag' '\n+\tcheck_last_modified t2 <<-\\EOF\n+\t2 a\n+\t1 file\n+\tEOF\n+'\n+\n test_expect_success 'last-modified recursive with show-trees' '\n \tcheck_last_modified -r -t <<-\\EOF\n \t3 a/b\n@@ -236,4 +244,9 @@ test_expect_success 'last-modified complains about unknown arguments' '\n \ttest_grep \"unknown last-modified argument: --foo\" err\n '\n \n+test_expect_success 'last-modified expects commit-ish' '\n+\ttest_must_fail git last-modified HEAD^{tree} 2>err &&\n+\tgrep \"revision argument '\"'\"'HEAD^{tree}'\"'\"' is a tree, not a commit-ish\" err\n+'\n+\n test_done\n\n-- \n2.52.0\n\n"},{"id":"534563","messageId":"xmqqpl70i7pj.fsf@gitster.g","threadId":"64784","inReplyTo":"20260123-toon-last-modified-tree-v4-1-86bf97bad4e2@iotcl.com","subject":"Re: [PATCH v4 1/4] last-modified: rewrite error message when more than one revision given","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-01-23T17:01:44Z","receivedAt":"2026-01-23T17:01:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Toon Claes writes:\n\n> When more than one revision is passed to the git-last-modified(1)\n> command, this error message was printed:\n>\n> error: last-modified can only operate on one tree at a time\n>\n> Calling these a \"tree\" is technically not correct. git-last-modified(1)\n> expects revisions that peel to a commit.\n>\n> Rephrase the error message to:\n>\n> error: last-modified can only operate on one revision at a time\n>\n> While at it, ensure modify the test to ensure the correct error message\n> is printed.\n\n\"ensure modify\" -> \"modify\".\n\n> \t\tif (num_interesting++)\n> -\t\t\treturn error(_(\"last-modified can only operate on one tree at a time\"));\n> +\t\t\treturn error(_(\"last-modified can only operate on one revision at a time\"));\n\nI recall we earlier had discussions on \"commit\" vs \"revision\", but\nwas the conclusion that 'revision' is the appropriate term here? I\nsomehow feel that it is sufficient to insist on \"commit\" (not\n\"commit-ish\"). The way you are allowed to give that commit might be\nmore lenient and you may be able to pass a tag that points at a\ncommit, but that does not change the fact that the \"last-modified\"\ncommand can only operate on one commit at a time, does it?\n\n> +test_expect_success 'cannot run last-modified on two revision' '\n\n\"two revision\" -> \"two revisions\".\n\n> +\ttest_must_fail git last-modified HEAD HEAD~1 2>err &&\n> +\ttest_grep \"last-modified can only operate on one revision at a time\" err\n> +'\n> +\n> test_expect_success 'last-modified complains about unknown arguments' '\n> \ttest_must_fail git last-modified --foo 2>err &&\n> -\tgrep \"unknown last-modified argument: --foo\" err\n> +\ttest_grep \"unknown last-modified argument: --foo\" err\n> '\n> \n> test_done\n"},{"id":"534564","messageId":"xmqqldhoi7lp.fsf@gitster.g","threadId":"64784","inReplyTo":"20260123-toon-last-modified-tree-v4-2-86bf97bad4e2@iotcl.com","subject":"Re: [PATCH v4 2/4] last-modified: fix memory leak when more than one revision is given","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-01-23T17:04:02Z","receivedAt":"2026-01-23T17:04:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Toon Claes writes:\n\n> When more than one revision is given, the function\n> populate_paths_from_revs() leaks a `struct pathspec`. Plug it.\n\nMakes sense.\n\n>\n> Signed-off-by: Toon Claes \n> ---\n> builtin/last-modified.c | 12 ++++++++----\n> 1 file changed, 8 insertions(+), 4 deletions(-)\n>\n> diff --git a/builtin/last-modified.c b/builtin/last-modified.c\n> index 7d95244e3f..06e3f79aec 100644\n> --- a/builtin/last-modified.c\n> +++ b/builtin/last-modified.c\n> @@ -123,7 +123,7 @@ static void add_path_from_diff(struct diff_queue_struct *q,\n> \n> static int populate_paths_from_revs(struct last_modified *lm)\n> {\n> -\tint num_interesting = 0;\n> +\tint num_interesting = 0, ret = 0;\n> \tstruct diff_options diffopt;\n> \n> \t/*\n> @@ -145,16 +145,20 @@ static int populate_paths_from_revs(struct last_modified *lm)\n> \t\tif (obj->item->flags & UNINTERESTING)\n> \t\t\tcontinue;\n> \n> -\t\tif (num_interesting++)\n> -\t\t\treturn error(_(\"last-modified can only operate on one revision at a time\"));\n> +\t\tif (num_interesting++) {\n> +\t\t\tret = error(_(\"last-modified can only operate on one revision at a time\"));\n> +\t\t\tgoto out;\n> +\t\t}\n> \n> \t\tdiff_tree_oid(lm->rev.repo->hash_algo->empty_tree,\n> \t\t\t &obj->item->oid, \"\", &diffopt);\n> \t\tdiff_flush(&diffopt);\n> \t}\n> +\n> +out:\n> \tclear_pathspec(&diffopt.pathspec);\n> \n> -\treturn 0;\n> +\treturn ret;\n> }\n> \n> static void last_modified_emit(struct last_modified *lm,\n"},{"id":"534565","messageId":"xmqqh5sci7go.fsf@gitster.g","threadId":"64784","inReplyTo":"20260123-toon-last-modified-tree-v4-3-86bf97bad4e2@iotcl.com","subject":"Re: [PATCH v4 3/4] last-modified: remove double error message","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-01-23T17:07:03Z","receivedAt":"2026-01-23T17:07:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Toon Claes writes:\n\n> When the user passes two revisions, they get the following output:\n>\n> $ git last-modified HEAD HEAD~\n> error: last-modified can only operate on one revision at a time\n> error: unable to setup last-modified\n>\n> The error message about \"unable to setup\" is not very informative,\n> remove it.\n\nWe can see that an error message is always given when\npopulate_paths_from_revs() returns negative, so this change is safe\n(i.e., if there is a code path in the function that returns negative\nwithout giving any message, this change will lead to a silent\nfailure in such a code path). Looking good.\n\nThanks.\n\n> Signed-off-by: Toon Claes \n> ---\n> builtin/last-modified.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/builtin/last-modified.c b/builtin/last-modified.c\n> index 06e3f79aec..0df85be318 100644\n> --- a/builtin/last-modified.c\n> +++ b/builtin/last-modified.c\n> @@ -495,7 +495,7 @@ static int last_modified_init(struct last_modified *lm, struct repository *r,\n> \tlm->rev.bloom_filter_settings = get_bloom_filter_settings(lm->rev.repo);\n> \n> \tif (populate_paths_from_revs(lm) < 0)\n> -\t\treturn error(_(\"unable to setup last-modified\"));\n> +\t\treturn -1;\n> \n> \tCALLOC_ARRAY(lm->all_paths, hashmap_get_size(&lm->paths));\n> \tlm->all_paths_nr = 0;\n"},{"id":"534566","messageId":"xmqqcy30i77p.fsf@gitster.g","threadId":"64784","inReplyTo":"20260123-toon-last-modified-tree-v4-4-86bf97bad4e2@iotcl.com","subject":"Re: [PATCH v4 4/4] last-modified: verify revision argument is a commit-ish","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-01-23T17:12:26Z","receivedAt":"2026-01-23T17:12:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Toon Claes writes:\n\n> Passing a tree OID to git-last-modified(1) would trigger BUG behavior.\n>\n> git last-modified HEAD^{tree}\n> BUG: builtin/last-modified.c:456: paths remaining beyond boundary in last-modified\n>\n> Fix this error by verifying the parsed revision is a commit-ish.\n\nIs it \"the parsed revision\", or what the command line argument given\nto the program? I am wondering if it is easier to understand to\nrephrase this to something like \"... by ensuring that the given\nrevision peels to a commit\".\n\n> +\t\tif (!repo_peel_to_type(lm->rev.repo, obj->path, 0, obj->item, OBJ_COMMIT)) {\n> +\t\t\tret = error(_(\"revision argument '%s' is a %s, not a commit-ish\"), obj->name, type_name(obj->item->type));\n> +\t\t\tgoto out;\n> +\t\t}\n> +\n> \t\tdiff_tree_oid(lm->rev.repo->hash_algo->empty_tree,\n> \t\t\t &obj->item->oid, \"\", &diffopt);\n> \t\tdiff_flush(&diffopt);\n\nIt is somewhat unsatisfying that the result of peeling is discarded\nand diff_tree_oid() is forced to peel it down to tree again, but I\ndo not think of a better way offhand.\n\n"},{"id":"534568","messageId":"xmqq4ioci6cg.fsf@gitster.g","threadId":"64784","inReplyTo":"20260123-toon-last-modified-tree-v4-4-86bf97bad4e2@iotcl.com","subject":"Re: [PATCH v4 4/4] last-modified: verify revision argument is a commit-ish","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-01-23T17:31:11Z","receivedAt":"2026-01-23T17:31:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Toon Claes writes:\n\n> +\t\tif (!repo_peel_to_type(lm->rev.repo, obj->path, 0, obj->item, OBJ_COMMIT)) {\n> +\t\t\tret = error(_(\"revision argument '%s' is a %s, not a commit-ish\"), obj->name, type_name(obj->item->type));\n> +\t\t\tgoto out;\n> +\t\t}\n> +\n\nI do not use l10n version, but I am not sure how localization should\nwork with this message.\n\n _(\"revision argument '%s' is a %s, not a commit-ish\")\n\nThere is no way type_name(obj->item->type) would be localized in\nthis code (after all, it is used in format_object_header() that is\nthe helper function to prepare the contents of the loose object\nfile). Yet, it is tempting to translate \"commit-ish\" for those\npreparing the .po files, which would lead to mixture of C-locale\n'tree' and end-user-locale _(\"commit-ish\").\n\nI am wondering if we want to avoid this mixture by forcing the\nC-locale for both, i.e.,\n\n error(_(\"revision argument '%s' is a %s, not a %s\"),\n\t obj->name, type_name(obj->item->type), 'commit-ish')\n\nWe can leave it as-is and polish the error messages later, of\ncourse, but I am curious what the best practice is.\n\nThanks.\n"},{"id":"534610","messageId":"71c081da-e5de-49fd-a5fe-b61503e19eab@app.fastmail.com","threadId":"64784","inReplyTo":"aWnkJ_GpgbXBPwFp@pks.im","subject":"Re: [PATCH v2 1/3] last-modified: rewrite error message when more than one revision given","fromName":"Kristoffer Haugsbakk","fromEmail":"kristofferhaugsbakk@fastmail.com","sentAt":"2026-01-25T11:26:56Z","receivedAt":"2026-01-25T11:27:18Z","isPatch":true,"sender":{"key":"kristofferhaugsbakk@fastmail.com","avatar":null},"body":"On Fri, Jan 16, 2026, at 08:09, Patrick Steinhardt wrote:\n> On Thu, Jan 15, 2026 at 03:34:50PM +0100, Kristoffer Haugsbakk wrote:\n>> On Thu, Jan 15, 2026, at 12:54, Patrick Steinhardt wrote:\n>> > On Thu, Jan 15, 2026 at 12:33:36PM +0100, Toon Claes wrote:\n>> >>[snip]\n>> >> I was thinking about mentioning something like \"commit-ish\" instead, but\n>> >> I felt \"commit-ish\" isn't a commonly used term toward end-users. Looking\n>> >> at gitglossary(7), it says \"revision\" is a \"synonym for commit\". I'm\n>> >> happy to change this message, but I'm not sure s/revision/commit-ish/ is\n>> >> the best change for this.\n>> >\n>> > gitglossary(7) also defines commit-ish, but I guess you're right that\n>> > revision is the more common term. I'm a bit surprised that it's defined\n>> > to be a synonym for a commit, but oh, well.\n>>\n>> Surprised that “revision” is a synonym for commit? Why is that?\n>\n> Because in my mind a revision can resolve to any object type.\n> \"^{}\" for example is a revision, but it can resolve for\n> example to a tree if you say \"HEAD^{tree}\". Still a revision, but\n> definitely does not resolve to a commit.\n>\n> Also, it's confusing to conflate the way to name a commit with a commit\n> itself. \"HEAD~10\" is a revision, but taken by itself it's not a commit.\n> It's not even clear whether it resolves, so it feels sensible to me to\n> keep these two concepts separate from one another.\n\nMaybe I didn’t read the context well enough. I always read “revision” as\na generic word like “commit”. Not as the gitrevisions(7) expressions.\n"},{"id":"534708","messageId":"20260127-toon-last-modified-tree-v5-0-38d18a0956d4@iotcl.com","threadId":"64784","inReplyTo":"20260123-toon-last-modified-tree-v4-0-86bf97bad4e2@iotcl.com","subject":"[PATCH v5 0/4] Fix git-last-modified(1) bug triggered when passing a tree-ish","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2026-01-27T13:26:16Z","receivedAt":"2026-01-27T13:26:30Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"Recently there was a bug reported[1] passing a tree OID triggers a BUG:\n\n $ git last-modified fb06ce04173d47aaaa498385621cba8b8dfd7584\n BUG: builtin/last-modified.c:456: paths remaining beyond boundary in last-modified\n [1] 690163 IOT instruction (core dumped) git last-modified\n\n `fb06ce04173d47aaaa498385621cba8b8dfd7584` is the tree commit id of web_src. I\n suppose this should've returned a nice error message or blank output.\n\nFix this bug by checking the revision argument.\n\n[1]: https://lore.kernel.org/git/03f96860-29fc-42a7-a220-c3ec65eb8516@codeberg.org/\n\n---\nChanges in v5:\n- Use 'commit' instead of 'revision'.\n- Small typo fixes.\n- Link to v4: https://patch.msgid.link/20260123-toon-last-modified-tree-v4-0-86bf97bad4e2@iotcl.com\n\nChanges in v4:\n- Extend the error message when revision is not a tree\n- Extend the test to verify the error message when more than one\n revision is given.\n- Link to v3: https://patch.msgid.link/20260116-toon-last-modified-tree-v3-0-e6ade4dc35ab@iotcl.com\n\nChanges in v3:\n- Split the change to plug the leak into a separate commit.\n- Small changes to commit messages.\n- Link to v2: https://patch.msgid.link/20260114-toon-last-modified-tree-v2-0-ba3b1860898f@iotcl.com\n\nChanges in v2:\n- Prepend the change with a commit that modifies the error message\n when more than one revision is given.\n- Prepend another commit that removes double error message.\n- Add test to ensure the command works with annotated tags too.\n- Link to v1: https://patch.msgid.link/20260112-toon-last-modified-tree-v1-1-ecbc78341f76@iotcl.com\n\n---\nToon Claes (4):\n last-modified: rewrite error message when more than one commit given\n last-modified: fix memory leak when more than one commit is given\n last-modified: remove double error message\n last-modified: verify revision argument is a commit-ish\n\n builtin/last-modified.c | 19 ++++++++++++++-----\n t/t8020-last-modified.sh | 24 +++++++++++++++++++-----\n 2 files changed, 33 insertions(+), 10 deletions(-)\n\nRange-diff versus v4:\n\n1: 497901f0ab ! 1: 2107d672e9 last-modified: rewrite error message when more than one revision given\n @@ Metadata\n Author: Toon Claes \n \n ## Commit message ##\n - last-modified: rewrite error message when more than one revision given\n + last-modified: rewrite error message when more than one commit given\n \n - When more than one revision is passed to the git-last-modified(1)\n - command, this error message was printed:\n + When more than one commit is passed to the git-last-modified(1) command,\n + this error message was printed:\n \n error: last-modified can only operate on one tree at a time\n \n @@ Commit message\n \n Rephrase the error message to:\n \n - error: last-modified can only operate on one revision at a time\n + error: last-modified can only operate on one commit at a time\n \n - While at it, ensure modify the test to ensure the correct error message\n - is printed.\n + While at it, modify the test to ensure the correct error message is\n + printed.\n \n Signed-off-by: Toon Claes \n \n @@ builtin/last-modified.c: static int populate_paths_from_revs(struct last_modifie\n \n \t\tif (num_interesting++)\n -\t\t\treturn error(_(\"last-modified can only operate on one tree at a time\"));\n -+\t\t\treturn error(_(\"last-modified can only operate on one revision at a time\"));\n ++\t\t\treturn error(_(\"last-modified can only operate on one commit at a time\"));\n \n \t\tdiff_tree_oid(lm->rev.repo->hash_algo->empty_tree,\n \t\t\t &obj->item->oid, \"\", &diffopt);\n @@ t/t8020-last-modified.sh: test_expect_success 'last-modified merge undoes change\n \tEOF\n '\n \n -+test_expect_success 'cannot run last-modified on two revision' '\n ++test_expect_success 'cannot run last-modified on two commits' '\n +\ttest_must_fail git last-modified HEAD HEAD~1 2>err &&\n -+\ttest_grep \"last-modified can only operate on one revision at a time\" err\n ++\ttest_grep \"last-modified can only operate on one commit at a time\" err\n +'\n +\n test_expect_success 'last-modified complains about unknown arguments' '\n2: 1316ca90b4 ! 2: dbabde6b54 last-modified: fix memory leak when more than one revision is given\n @@ Metadata\n Author: Toon Claes \n \n ## Commit message ##\n - last-modified: fix memory leak when more than one revision is given\n + last-modified: fix memory leak when more than one commit is given\n \n - When more than one revision is given, the function\n + When more than one commit is given, the function\n populate_paths_from_revs() leaks a `struct pathspec`. Plug it.\n \n Signed-off-by: Toon Claes \n @@ builtin/last-modified.c: static int populate_paths_from_revs(struct last_modifie\n \t\t\tcontinue;\n \n -\t\tif (num_interesting++)\n --\t\t\treturn error(_(\"last-modified can only operate on one revision at a time\"));\n +-\t\t\treturn error(_(\"last-modified can only operate on one commit at a time\"));\n +\t\tif (num_interesting++) {\n -+\t\t\tret = error(_(\"last-modified can only operate on one revision at a time\"));\n ++\t\t\tret = error(_(\"last-modified can only operate on one commit at a time\"));\n +\t\t\tgoto out;\n +\t\t}\n \n3: 151f0ff0ae = 3: 8f37827504 last-modified: remove double error message\n4: f2cf0dd371 ! 4: 9a810e12be last-modified: verify revision argument is a commit-ish\n @@ Metadata\n ## Commit message ##\n last-modified: verify revision argument is a commit-ish\n \n - Passing a tree OID to git-last-modified(1) would trigger BUG behavior.\n + Passing a non-committish revision to git-last-modified(1) triggers the\n + following BUG:\n \n git last-modified HEAD^{tree}\n BUG: builtin/last-modified.c:456: paths remaining beyond boundary in last-modified\n \n - Fix this error by verifying the parsed revision is a commit-ish.\n + Fix this error by ensuring that the given revision peels to a commit.\n \n Reported-by: Gusted \n Signed-off-by: Toon Claes \n\n\n---\nbase-commit: ab689ea7f91ab0858e85776f31102203d3ea7b83\nchange-id: 20260112-toon-last-modified-tree-fdd96b2feaf7\n\n"},{"id":"534709","messageId":"20260127-toon-last-modified-tree-v5-1-38d18a0956d4@iotcl.com","threadId":"64784","inReplyTo":"20260127-toon-last-modified-tree-v5-0-38d18a0956d4@iotcl.com","subject":"[PATCH v5 1/4] last-modified: rewrite error message when more than one commit given","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2026-01-27T13:26:17Z","receivedAt":"2026-01-27T13:26:34Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"When more than one commit is passed to the git-last-modified(1) command,\nthis error message was printed:\n\n error: last-modified can only operate on one tree at a time\n\nCalling these a \"tree\" is technically not correct. git-last-modified(1)\nexpects revisions that peel to a commit.\n\nRephrase the error message to:\n\n error: last-modified can only operate on one commit at a time\n\nWhile at it, modify the test to ensure the correct error message is\nprinted.\n\nSigned-off-by: Toon Claes \n---\n builtin/last-modified.c | 2 +-\n t/t8020-last-modified.sh | 11 ++++++-----\n 2 files changed, 7 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/last-modified.c b/builtin/last-modified.c\nindex c80f0535f6..1219f6802e 100644\n--- a/builtin/last-modified.c\n+++ b/builtin/last-modified.c\n@@ -146,7 +146,7 @@ static int populate_paths_from_revs(struct last_modified *lm)\n \t\t\tcontinue;\n \n \t\tif (num_interesting++)\n-\t\t\treturn error(_(\"last-modified can only operate on one tree at a time\"));\n+\t\t\treturn error(_(\"last-modified can only operate on one commit at a time\"));\n \n \t\tdiff_tree_oid(lm->rev.repo->hash_algo->empty_tree,\n \t\t\t &obj->item->oid, \"\", &diffopt);\ndiff --git a/t/t8020-last-modified.sh b/t/t8020-last-modified.sh\nindex 50f4312f71..d1aad12319 100755\n--- a/t/t8020-last-modified.sh\n+++ b/t/t8020-last-modified.sh\n@@ -12,10 +12,6 @@ test_expect_success 'setup' '\n \ttest_commit 3 a/b/file\n '\n \n-test_expect_success 'cannot run last-modified on two trees' '\n-\ttest_must_fail git last-modified HEAD HEAD~1\n-'\n-\n check_last_modified() {\n \tlocal indir= &&\n \twhile test $# != 0\n@@ -230,9 +226,14 @@ test_expect_success 'last-modified merge undoes changes' '\n \tEOF\n '\n \n+test_expect_success 'cannot run last-modified on two commits' '\n+\ttest_must_fail git last-modified HEAD HEAD~1 2>err &&\n+\ttest_grep \"last-modified can only operate on one commit at a time\" err\n+'\n+\n test_expect_success 'last-modified complains about unknown arguments' '\n \ttest_must_fail git last-modified --foo 2>err &&\n-\tgrep \"unknown last-modified argument: --foo\" err\n+\ttest_grep \"unknown last-modified argument: --foo\" err\n '\n \n test_done\n\n-- \n2.53.0.rc1.267.g6e3a78c723\n\n"},{"id":"534710","messageId":"20260127-toon-last-modified-tree-v5-2-38d18a0956d4@iotcl.com","threadId":"64784","inReplyTo":"20260127-toon-last-modified-tree-v5-0-38d18a0956d4@iotcl.com","subject":"[PATCH v5 2/4] last-modified: fix memory leak when more than one commit is given","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2026-01-27T13:26:18Z","receivedAt":"2026-01-27T13:26:37Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"When more than one commit is given, the function\npopulate_paths_from_revs() leaks a `struct pathspec`. Plug it.\n\nSigned-off-by: Toon Claes \n---\n builtin/last-modified.c | 12 ++++++++----\n 1 file changed, 8 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/last-modified.c b/builtin/last-modified.c\nindex 1219f6802e..31dea975a0 100644\n--- a/builtin/last-modified.c\n+++ b/builtin/last-modified.c\n@@ -123,7 +123,7 @@ static void add_path_from_diff(struct diff_queue_struct *q,\n \n static int populate_paths_from_revs(struct last_modified *lm)\n {\n-\tint num_interesting = 0;\n+\tint num_interesting = 0, ret = 0;\n \tstruct diff_options diffopt;\n \n \t/*\n@@ -145,16 +145,20 @@ static int populate_paths_from_revs(struct last_modified *lm)\n \t\tif (obj->item->flags & UNINTERESTING)\n \t\t\tcontinue;\n \n-\t\tif (num_interesting++)\n-\t\t\treturn error(_(\"last-modified can only operate on one commit at a time\"));\n+\t\tif (num_interesting++) {\n+\t\t\tret = error(_(\"last-modified can only operate on one commit at a time\"));\n+\t\t\tgoto out;\n+\t\t}\n \n \t\tdiff_tree_oid(lm->rev.repo->hash_algo->empty_tree,\n \t\t\t &obj->item->oid, \"\", &diffopt);\n \t\tdiff_flush(&diffopt);\n \t}\n+\n+out:\n \tclear_pathspec(&diffopt.pathspec);\n \n-\treturn 0;\n+\treturn ret;\n }\n \n static void last_modified_emit(struct last_modified *lm,\n\n-- \n2.53.0.rc1.267.g6e3a78c723\n\n"},{"id":"534711","messageId":"20260127-toon-last-modified-tree-v5-3-38d18a0956d4@iotcl.com","threadId":"64784","inReplyTo":"20260127-toon-last-modified-tree-v5-0-38d18a0956d4@iotcl.com","subject":"[PATCH v5 3/4] last-modified: remove double error message","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2026-01-27T13:26:19Z","receivedAt":"2026-01-27T13:26:41Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"When the user passes two revisions, they get the following output:\n\n $ git last-modified HEAD HEAD~\n error: last-modified can only operate on one revision at a time\n error: unable to setup last-modified\n\nThe error message about \"unable to setup\" is not very informative,\nremove it.\n\nSigned-off-by: Toon Claes \n---\n builtin/last-modified.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/last-modified.c b/builtin/last-modified.c\nindex 31dea975a0..e02ec8428b 100644\n--- a/builtin/last-modified.c\n+++ b/builtin/last-modified.c\n@@ -495,7 +495,7 @@ static int last_modified_init(struct last_modified *lm, struct repository *r,\n \tlm->rev.bloom_filter_settings = get_bloom_filter_settings(lm->rev.repo);\n \n \tif (populate_paths_from_revs(lm) < 0)\n-\t\treturn error(_(\"unable to setup last-modified\"));\n+\t\treturn -1;\n \n \tCALLOC_ARRAY(lm->all_paths, hashmap_get_size(&lm->paths));\n \tlm->all_paths_nr = 0;\n\n-- \n2.53.0.rc1.267.g6e3a78c723\n\n"},{"id":"534712","messageId":"20260127-toon-last-modified-tree-v5-4-38d18a0956d4@iotcl.com","threadId":"64784","inReplyTo":"20260127-toon-last-modified-tree-v5-0-38d18a0956d4@iotcl.com","subject":"[PATCH v5 4/4] last-modified: verify revision argument is a commit-ish","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2026-01-27T13:26:20Z","receivedAt":"2026-01-27T13:26:44Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"Passing a non-committish revision to git-last-modified(1) triggers the\nfollowing BUG:\n\n git last-modified HEAD^{tree}\n BUG: builtin/last-modified.c:456: paths remaining beyond boundary in last-modified\n\nFix this error by ensuring that the given revision peels to a commit.\n\nReported-by: Gusted \nSigned-off-by: Toon Claes \n---\n builtin/last-modified.c | 5 +++++\n t/t8020-last-modified.sh | 13 +++++++++++++\n 2 files changed, 18 insertions(+)\n\ndiff --git a/builtin/last-modified.c b/builtin/last-modified.c\nindex e02ec8428b..d0944673f0 100644\n--- a/builtin/last-modified.c\n+++ b/builtin/last-modified.c\n@@ -150,6 +150,11 @@ static int populate_paths_from_revs(struct last_modified *lm)\n \t\t\tgoto out;\n \t\t}\n \n+\t\tif (!repo_peel_to_type(lm->rev.repo, obj->path, 0, obj->item, OBJ_COMMIT)) {\n+\t\t\tret = error(_(\"revision argument '%s' is a %s, not a commit-ish\"), obj->name, type_name(obj->item->type));\n+\t\t\tgoto out;\n+\t\t}\n+\n \t\tdiff_tree_oid(lm->rev.repo->hash_algo->empty_tree,\n \t\t\t &obj->item->oid, \"\", &diffopt);\n \t\tdiff_flush(&diffopt);\ndiff --git a/t/t8020-last-modified.sh b/t/t8020-last-modified.sh\nindex d1aad12319..6024e8bd60 100755\n--- a/t/t8020-last-modified.sh\n+++ b/t/t8020-last-modified.sh\n@@ -8,6 +8,7 @@ test_expect_success 'setup' '\n \ttest_commit 1 file &&\n \tmkdir a &&\n \ttest_commit 2 a/file &&\n+\tgit tag -mA t2 2 &&\n \tmkdir a/b &&\n \ttest_commit 3 a/b/file\n '\n@@ -51,6 +52,13 @@ test_expect_success 'last-modified recursive' '\n \tEOF\n '\n \n+test_expect_success 'last-modified on annotated tag' '\n+\tcheck_last_modified t2 <<-\\EOF\n+\t2 a\n+\t1 file\n+\tEOF\n+'\n+\n test_expect_success 'last-modified recursive with show-trees' '\n \tcheck_last_modified -r -t <<-\\EOF\n \t3 a/b\n@@ -236,4 +244,9 @@ test_expect_success 'last-modified complains about unknown arguments' '\n \ttest_grep \"unknown last-modified argument: --foo\" err\n '\n \n+test_expect_success 'last-modified expects commit-ish' '\n+\ttest_must_fail git last-modified HEAD^{tree} 2>err &&\n+\tgrep \"revision argument '\"'\"'HEAD^{tree}'\"'\"' is a tree, not a commit-ish\" err\n+'\n+\n test_done\n\n-- \n2.53.0.rc1.267.g6e3a78c723\n\n"},{"id":"534735","messageId":"xmqqjyx2elbs.fsf@gitster.g","threadId":"64784","inReplyTo":"20260127-toon-last-modified-tree-v5-0-38d18a0956d4@iotcl.com","subject":"Re: [PATCH v5 0/4] Fix git-last-modified(1) bug triggered when passing a tree-ish","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-01-27T22:34:47Z","receivedAt":"2026-01-27T22:34:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Toon Claes writes:\n\n> Changes in v5:\n> - Use 'commit' instead of 'revision'.\n> - Small typo fixes.\n> - Link to v4: https://patch.msgid.link/20260123-toon-last-modified-tree-v4-0-86bf97bad4e2@iotcl.com\n\nLooking good. Queued. Thanks.\n"},{"id":"534814","messageId":"87v7gkqxb0.fsf@iotcl.com","threadId":"64784","inReplyTo":"xmqqjyx2elbs.fsf@gitster.g","subject":"Re: [PATCH v5 0/4] Fix git-last-modified(1) bug triggered when passing a tree-ish","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2026-01-29T14:59:47Z","receivedAt":"2026-01-29T15:00:11Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"Junio C Hamano writes:\n\n> Toon Claes writes:\n>\n>> Changes in v5:\n>> - Use 'commit' instead of 'revision'.\n>> - Small typo fixes.\n>> - Link to v4: https://patch.msgid.link/20260123-toon-last-modified-tree-v4-0-86bf97bad4e2@iotcl.com\n>\n> Looking good. Queued. Thanks.\n\nI noticed this causes a failure[1] with Meson on Windows. Better leave\nit out for now. One of the failures:\n\n expecting success of 8020.3 'last-modified recursive': \n \tcheck_last_modified -r <<-\\EOF\n \t3 a/b/file\n \t2 a/file\n \t1 file\n \tEOF\n\n ++ check_last_modified -r\n ++ local indir=\n ++ test 1 '!=' 0\n ++ case \"$1\" in\n ++ break\n ++ cat\n ++ git last-modified -r\n ++ git name-rev --annotate-stdin --name-only --tags\n ++ tr '\\t' ' '\n ++ test_cmp expect actual\n ++ test 2 -ne 2\n ++ eval 'GIT_DIR=/dev/null git diff --no-index --ignore-cr-at-eol --' '\"$@\"'\n +++ GIT_DIR=/dev/null\n +++ git diff --no-index --ignore-cr-at-eol -- expect actual\n diff --git a/expect b/actual\n index 65ac8be..d17615e 100644\n --- a/expect\n +++ b/actual\n @@ -1,3 +1,3 @@\n 3 a/b/file\n -2 a/file\n +t2^0 a/file\n 1 file\n error: last command exited with $?=1\n\n[1]: https://gitlab.com/gitlab-org/git/-/jobs/12899403428\n\n\n-- \nCheers,\nToon\n"},{"id":"534876","messageId":"20260130-toon-last-modified-tree-v6-0-db827e5df985@iotcl.com","threadId":"64784","inReplyTo":"20260127-toon-last-modified-tree-v5-0-38d18a0956d4@iotcl.com","subject":"[PATCH v6 0/4] Fix git-last-modified(1) bug triggered when passing a tree-ish","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2026-01-30T14:26:34Z","receivedAt":"2026-01-30T14:26:54Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"Recently there was a bug reported[1] passing a tree OID triggers a BUG:\n\n $ git last-modified fb06ce04173d47aaaa498385621cba8b8dfd7584\n BUG: builtin/last-modified.c:456: paths remaining beyond boundary in last-modified\n [1] 690163 IOT instruction (core dumped) git last-modified\n\n `fb06ce04173d47aaaa498385621cba8b8dfd7584` is the tree commit id of web_src. I\n suppose this should've returned a nice error message or blank output.\n\nFix this bug by checking the revision argument.\n\n[1]: https://lore.kernel.org/git/03f96860-29fc-42a7-a220-c3ec65eb8516@codeberg.org/\n\n---\nChanges in v6:\n- Fix CI failure with Meson on Windows.\n- Link to v5: https://patch.msgid.link/20260127-toon-last-modified-tree-v5-0-38d18a0956d4@iotcl.com\n\nChanges in v5:\n- Use 'commit' instead of 'revision'.\n- Small typo fixes.\n- Link to v4: https://patch.msgid.link/20260123-toon-last-modified-tree-v4-0-86bf97bad4e2@iotcl.com\n\nChanges in v4:\n- Extend the error message when revision is not a tree\n- Extend the test to verify the error message when more than one\n revision is given.\n- Link to v3: https://patch.msgid.link/20260116-toon-last-modified-tree-v3-0-e6ade4dc35ab@iotcl.com\n\nChanges in v3:\n- Split the change to plug the leak into a separate commit.\n- Small changes to commit messages.\n- Link to v2: https://patch.msgid.link/20260114-toon-last-modified-tree-v2-0-ba3b1860898f@iotcl.com\n\nChanges in v2:\n- Prepend the change with a commit that modifies the error message\n when more than one revision is given.\n- Prepend another commit that removes double error message.\n- Add test to ensure the command works with annotated tags too.\n- Link to v1: https://patch.msgid.link/20260112-toon-last-modified-tree-v1-1-ecbc78341f76@iotcl.com\n\n---\nToon Claes (4):\n last-modified: rewrite error message when more than one commit given\n last-modified: fix memory leak when more than one commit is given\n last-modified: remove double error message\n last-modified: verify revision argument is a commit-ish\n\n builtin/last-modified.c | 19 ++++++++++++++-----\n t/t8020-last-modified.sh | 26 ++++++++++++++++++++------\n 2 files changed, 34 insertions(+), 11 deletions(-)\n\nRange-diff versus v5:\n\n1: d9f9531e67 = 1: 455e9bc0ca last-modified: rewrite error message when more than one commit given\n2: 19ccaa2aae = 2: 2d51b5e381 last-modified: fix memory leak when more than one commit is given\n3: 4bff975458 = 3: 68b3acefd7 last-modified: remove double error message\n4: a5d4c74ad7 ! 4: d66ab6a033 last-modified: verify revision argument is a commit-ish\n @@ Commit message\n \n Fix this error by ensuring that the given revision peels to a commit.\n \n + This change also adds a test to verify git-last-modified(1) can operate\n + on an annotated tag. For this an annotated tag is added that points to\n + the second commit. But this causes ambiguous results when calling\n + git-name-rev(1) with `--tags`, because now two tags point to the same\n + commit. To remove this ambiguity, pass `--exclude=` to\n + git-name-rev(1) to exclude the new annotated tag.\n +\n Reported-by: Gusted \n Signed-off-by: Toon Claes \n \n @@ t/t8020-last-modified.sh: test_expect_success 'setup' '\n \tmkdir a/b &&\n \ttest_commit 3 a/b/file\n '\n +@@ t/t8020-last-modified.sh: check_last_modified() {\n + \n + \tcat >expect &&\n + \tgit ${indir:+-C \"$indir\"} last-modified \"$@\" >tmp.1 &&\n +-\tgit name-rev --annotate-stdin --name-only --tags \\\n ++\tgit name-rev --annotate-stdin --name-only --tags --exclude=t2 \\\n + \t\ttmp.2 &&\n + \ttr '\\t' ' ' actual &&\n + \ttest_cmp expect actual\n @@ t/t8020-last-modified.sh: test_expect_success 'last-modified recursive' '\n \tEOF\n '\n @@ t/t8020-last-modified.sh: test_expect_success 'last-modified complains about unk\n \n +test_expect_success 'last-modified expects commit-ish' '\n +\ttest_must_fail git last-modified HEAD^{tree} 2>err &&\n -+\tgrep \"revision argument '\"'\"'HEAD^{tree}'\"'\"' is a tree, not a commit-ish\" err\n ++\ttest_grep \"revision argument ${SQ}HEAD^{tree}${SQ} is a tree, not a commit-ish\" err\n +'\n +\n test_done\n\n\n---\nbase-commit: ea717645d199f6f1b66058886475db3e8c9330e9\nchange-id: 20260112-toon-last-modified-tree-fdd96b2feaf7\n\n"},{"id":"534877","messageId":"20260130-toon-last-modified-tree-v6-1-db827e5df985@iotcl.com","threadId":"64784","inReplyTo":"20260130-toon-last-modified-tree-v6-0-db827e5df985@iotcl.com","subject":"[PATCH v6 1/4] last-modified: rewrite error message when more than one commit given","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2026-01-30T14:26:35Z","receivedAt":"2026-01-30T14:26:58Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"When more than one commit is passed to the git-last-modified(1) command,\nthis error message was printed:\n\n error: last-modified can only operate on one tree at a time\n\nCalling these a \"tree\" is technically not correct. git-last-modified(1)\nexpects revisions that peel to a commit.\n\nRephrase the error message to:\n\n error: last-modified can only operate on one commit at a time\n\nWhile at it, modify the test to ensure the correct error message is\nprinted.\n\nSigned-off-by: Toon Claes \n---\n builtin/last-modified.c | 2 +-\n t/t8020-last-modified.sh | 11 ++++++-----\n 2 files changed, 7 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/last-modified.c b/builtin/last-modified.c\nindex c80f0535f6..1219f6802e 100644\n--- a/builtin/last-modified.c\n+++ b/builtin/last-modified.c\n@@ -146,7 +146,7 @@ static int populate_paths_from_revs(struct last_modified *lm)\n \t\t\tcontinue;\n \n \t\tif (num_interesting++)\n-\t\t\treturn error(_(\"last-modified can only operate on one tree at a time\"));\n+\t\t\treturn error(_(\"last-modified can only operate on one commit at a time\"));\n \n \t\tdiff_tree_oid(lm->rev.repo->hash_algo->empty_tree,\n \t\t\t &obj->item->oid, \"\", &diffopt);\ndiff --git a/t/t8020-last-modified.sh b/t/t8020-last-modified.sh\nindex 50f4312f71..d1aad12319 100755\n--- a/t/t8020-last-modified.sh\n+++ b/t/t8020-last-modified.sh\n@@ -12,10 +12,6 @@ test_expect_success 'setup' '\n \ttest_commit 3 a/b/file\n '\n \n-test_expect_success 'cannot run last-modified on two trees' '\n-\ttest_must_fail git last-modified HEAD HEAD~1\n-'\n-\n check_last_modified() {\n \tlocal indir= &&\n \twhile test $# != 0\n@@ -230,9 +226,14 @@ test_expect_success 'last-modified merge undoes changes' '\n \tEOF\n '\n \n+test_expect_success 'cannot run last-modified on two commits' '\n+\ttest_must_fail git last-modified HEAD HEAD~1 2>err &&\n+\ttest_grep \"last-modified can only operate on one commit at a time\" err\n+'\n+\n test_expect_success 'last-modified complains about unknown arguments' '\n \ttest_must_fail git last-modified --foo 2>err &&\n-\tgrep \"unknown last-modified argument: --foo\" err\n+\ttest_grep \"unknown last-modified argument: --foo\" err\n '\n \n test_done\n\n-- \n2.53.0.rc1.267.g6e3a78c723\n\n"},{"id":"534878","messageId":"20260130-toon-last-modified-tree-v6-2-db827e5df985@iotcl.com","threadId":"64784","inReplyTo":"20260130-toon-last-modified-tree-v6-0-db827e5df985@iotcl.com","subject":"[PATCH v6 2/4] last-modified: fix memory leak when more than one commit is given","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2026-01-30T14:26:36Z","receivedAt":"2026-01-30T14:27:04Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"When more than one commit is given, the function\npopulate_paths_from_revs() leaks a `struct pathspec`. Plug it.\n\nSigned-off-by: Toon Claes \n---\n builtin/last-modified.c | 12 ++++++++----\n 1 file changed, 8 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/last-modified.c b/builtin/last-modified.c\nindex 1219f6802e..31dea975a0 100644\n--- a/builtin/last-modified.c\n+++ b/builtin/last-modified.c\n@@ -123,7 +123,7 @@ static void add_path_from_diff(struct diff_queue_struct *q,\n \n static int populate_paths_from_revs(struct last_modified *lm)\n {\n-\tint num_interesting = 0;\n+\tint num_interesting = 0, ret = 0;\n \tstruct diff_options diffopt;\n \n \t/*\n@@ -145,16 +145,20 @@ static int populate_paths_from_revs(struct last_modified *lm)\n \t\tif (obj->item->flags & UNINTERESTING)\n \t\t\tcontinue;\n \n-\t\tif (num_interesting++)\n-\t\t\treturn error(_(\"last-modified can only operate on one commit at a time\"));\n+\t\tif (num_interesting++) {\n+\t\t\tret = error(_(\"last-modified can only operate on one commit at a time\"));\n+\t\t\tgoto out;\n+\t\t}\n \n \t\tdiff_tree_oid(lm->rev.repo->hash_algo->empty_tree,\n \t\t\t &obj->item->oid, \"\", &diffopt);\n \t\tdiff_flush(&diffopt);\n \t}\n+\n+out:\n \tclear_pathspec(&diffopt.pathspec);\n \n-\treturn 0;\n+\treturn ret;\n }\n \n static void last_modified_emit(struct last_modified *lm,\n\n-- \n2.53.0.rc1.267.g6e3a78c723\n\n"},{"id":"534879","messageId":"20260130-toon-last-modified-tree-v6-3-db827e5df985@iotcl.com","threadId":"64784","inReplyTo":"20260130-toon-last-modified-tree-v6-0-db827e5df985@iotcl.com","subject":"[PATCH v6 3/4] last-modified: remove double error message","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2026-01-30T14:26:37Z","receivedAt":"2026-01-30T14:27:09Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"When the user passes two revisions, they get the following output:\n\n $ git last-modified HEAD HEAD~\n error: last-modified can only operate on one revision at a time\n error: unable to setup last-modified\n\nThe error message about \"unable to setup\" is not very informative,\nremove it.\n\nSigned-off-by: Toon Claes \n---\n builtin/last-modified.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/last-modified.c b/builtin/last-modified.c\nindex 31dea975a0..e02ec8428b 100644\n--- a/builtin/last-modified.c\n+++ b/builtin/last-modified.c\n@@ -495,7 +495,7 @@ static int last_modified_init(struct last_modified *lm, struct repository *r,\n \tlm->rev.bloom_filter_settings = get_bloom_filter_settings(lm->rev.repo);\n \n \tif (populate_paths_from_revs(lm) < 0)\n-\t\treturn error(_(\"unable to setup last-modified\"));\n+\t\treturn -1;\n \n \tCALLOC_ARRAY(lm->all_paths, hashmap_get_size(&lm->paths));\n \tlm->all_paths_nr = 0;\n\n-- \n2.53.0.rc1.267.g6e3a78c723\n\n"},{"id":"534880","messageId":"20260130-toon-last-modified-tree-v6-4-db827e5df985@iotcl.com","threadId":"64784","inReplyTo":"20260130-toon-last-modified-tree-v6-0-db827e5df985@iotcl.com","subject":"[PATCH v6 4/4] last-modified: verify revision argument is a commit-ish","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2026-01-30T14:26:38Z","receivedAt":"2026-01-30T14:27:14Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"Passing a non-committish revision to git-last-modified(1) triggers the\nfollowing BUG:\n\n git last-modified HEAD^{tree}\n BUG: builtin/last-modified.c:456: paths remaining beyond boundary in last-modified\n\nFix this error by ensuring that the given revision peels to a commit.\n\nThis change also adds a test to verify git-last-modified(1) can operate\non an annotated tag. For this an annotated tag is added that points to\nthe second commit. But this causes ambiguous results when calling\ngit-name-rev(1) with `--tags`, because now two tags point to the same\ncommit. To remove this ambiguity, pass `--exclude=` to\ngit-name-rev(1) to exclude the new annotated tag.\n\nReported-by: Gusted \nSigned-off-by: Toon Claes \n---\n builtin/last-modified.c | 5 +++++\n t/t8020-last-modified.sh | 15 ++++++++++++++-\n 2 files changed, 19 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/last-modified.c b/builtin/last-modified.c\nindex e02ec8428b..d0944673f0 100644\n--- a/builtin/last-modified.c\n+++ b/builtin/last-modified.c\n@@ -150,6 +150,11 @@ static int populate_paths_from_revs(struct last_modified *lm)\n \t\t\tgoto out;\n \t\t}\n \n+\t\tif (!repo_peel_to_type(lm->rev.repo, obj->path, 0, obj->item, OBJ_COMMIT)) {\n+\t\t\tret = error(_(\"revision argument '%s' is a %s, not a commit-ish\"), obj->name, type_name(obj->item->type));\n+\t\t\tgoto out;\n+\t\t}\n+\n \t\tdiff_tree_oid(lm->rev.repo->hash_algo->empty_tree,\n \t\t\t &obj->item->oid, \"\", &diffopt);\n \t\tdiff_flush(&diffopt);\ndiff --git a/t/t8020-last-modified.sh b/t/t8020-last-modified.sh\nindex d1aad12319..ec5bdc6aa0 100755\n--- a/t/t8020-last-modified.sh\n+++ b/t/t8020-last-modified.sh\n@@ -8,6 +8,7 @@ test_expect_success 'setup' '\n \ttest_commit 1 file &&\n \tmkdir a &&\n \ttest_commit 2 a/file &&\n+\tgit tag -mA t2 2 &&\n \tmkdir a/b &&\n \ttest_commit 3 a/b/file\n '\n@@ -30,7 +31,7 @@ check_last_modified() {\n \n \tcat >expect &&\n \tgit ${indir:+-C \"$indir\"} last-modified \"$@\" >tmp.1 &&\n-\tgit name-rev --annotate-stdin --name-only --tags \\\n+\tgit name-rev --annotate-stdin --name-only --tags --exclude=t2 \\\n \t\ttmp.2 &&\n \ttr '\\t' ' ' actual &&\n \ttest_cmp expect actual\n@@ -51,6 +52,13 @@ test_expect_success 'last-modified recursive' '\n \tEOF\n '\n \n+test_expect_success 'last-modified on annotated tag' '\n+\tcheck_last_modified t2 <<-\\EOF\n+\t2 a\n+\t1 file\n+\tEOF\n+'\n+\n test_expect_success 'last-modified recursive with show-trees' '\n \tcheck_last_modified -r -t <<-\\EOF\n \t3 a/b\n@@ -236,4 +244,9 @@ test_expect_success 'last-modified complains about unknown arguments' '\n \ttest_grep \"unknown last-modified argument: --foo\" err\n '\n \n+test_expect_success 'last-modified expects commit-ish' '\n+\ttest_must_fail git last-modified HEAD^{tree} 2>err &&\n+\ttest_grep \"revision argument ${SQ}HEAD^{tree}${SQ} is a tree, not a commit-ish\" err\n+'\n+\n test_done\n\n-- \n2.53.0.rc1.267.g6e3a78c723\n\n"},{"id":"534900","messageId":"xmqqecn76nca.fsf@gitster.g","threadId":"64784","inReplyTo":"20260130-toon-last-modified-tree-v6-0-db827e5df985@iotcl.com","subject":"Re: [PATCH v6 0/4] Fix git-last-modified(1) bug triggered when passing a tree-ish","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-01-30T17:07:33Z","receivedAt":"2026-01-30T17:07:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Toon Claes writes:\n\n> Changes in v6:\n> - Fix CI failure with Meson on Windows.\n> - Link to v5: https://patch.msgid.link/20260127-toon-last-modified-tree-v5-0-38d18a0956d4@iotcl.com\n\nReplaced. Thanks.\n"},{"id":"535354","messageId":"aYYO8Ph5yTE1ip-q@pks.im","threadId":"64784","inReplyTo":"20260130-toon-last-modified-tree-v6-4-db827e5df985@iotcl.com","subject":"Re: [PATCH v6 4/4] last-modified: verify revision argument is a commit-ish","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-02-06T15:55:28Z","receivedAt":"2026-02-06T15:55:35Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Jan 30, 2026 at 03:26:38PM +0100, Toon Claes wrote:\n> diff --git a/t/t8020-last-modified.sh b/t/t8020-last-modified.sh\n> index d1aad12319..ec5bdc6aa0 100755\n> --- a/t/t8020-last-modified.sh\n> +++ b/t/t8020-last-modified.sh\n> @@ -30,7 +31,7 @@ check_last_modified() {\n> \n> \tcat >expect &&\n> \tgit ${indir:+-C \"$indir\"} last-modified \"$@\" >tmp.1 &&\n> -\tgit name-rev --annotate-stdin --name-only --tags \\\n> +\tgit name-rev --annotate-stdin --name-only --tags --exclude=t2 \\\n> \t\ttmp.2 &&\n> \ttr '\\t' ' ' actual &&\n> \ttest_cmp expect actual\n\nIt's quite curious that we need to explicitly exclude t2 here to get a\ndeterministic result. But both of the results are correct, and in fact\nI've seen it once before that we got different results on different\nplatforms.\n\nSooo... weird, but I'd say we can live with this weirdness. Doubly so\nbecause it's not the fault of this patch series.\n\nPatrick\n"},{"id":"535355","messageId":"aYYO9SxiC5zFbB71@pks.im","threadId":"64784","inReplyTo":"xmqqecn76nca.fsf@gitster.g","subject":"Re: [PATCH v6 0/4] Fix git-last-modified(1) bug triggered when passing a tree-ish","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-02-06T15:55:33Z","receivedAt":"2026-02-06T15:55:39Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Jan 30, 2026 at 09:07:33AM -0800, Junio C Hamano wrote:\n> Toon Claes writes:\n> \n> > Changes in v6:\n> > - Fix CI failure with Meson on Windows.\n> > - Link to v5: https://patch.msgid.link/20260127-toon-last-modified-tree-v5-0-38d18a0956d4@iotcl.com\n> \n> Replaced. Thanks.\n\nFrom my point of view this series looks good to be merged down to 'next'\nnow. Thanks!\n\nPatrick\n"}]}