From: Toon Claes <toon@iotcl.com>
Date: Wed, 14 Jan 2026 10:24:44 GMT
Subject: [PATCH v2 0/3] Fix git-last-modified(1) bug triggered when passing a tree-ish
Message-ID: <20260114-toon-last-modified-tree-v2-0-ba3b1860898f@iotcl.com>
In-Reply-To: <20260112-toon-last-modified-tree-v1-1-ecbc78341f76@iotcl.com>

Recently there was a bug reported[1] passing a tree OID triggers a BUG:

    $ git last-modified fb06ce04173d47aaaa498385621cba8b8dfd7584
    BUG: builtin/last-modified.c:456: paths remaining beyond boundary in last-modified
    [1]    690163 IOT instruction (core dumped)  git last-modified

    `fb06ce04173d47aaaa498385621cba8b8dfd7584` is the tree commit id of web_src. I
    suppose this should've returned a nice error message or blank output.

Fix this bug by checking the revision argument.

[1]: https://lore.kernel.org/git/03f96860-29fc-42a7-a220-c3ec65eb8516@codeberg.org/

---
Changes in v2:
- Prepend the change with a commit that modifies the error message
  when more than one revision is given.
- Prepend another commit that removes double error message.
- Add test to ensure the command works with annotated tags too.
- Link to v1: https://patch.msgid.link/20260112-toon-last-modified-tree-v1-1-ecbc78341f76@iotcl.com

---
Toon Claes (3):
      last-modified: rewrite error message when more than one revision given
      last-modified: remove double error message
      last-modified: verify revision argument is a commit-ish

 builtin/last-modified.c  | 19 ++++++++++++++-----
 t/t8020-last-modified.sh | 15 ++++++++++++++-
 2 files changed, 28 insertions(+), 6 deletions(-)

Range-diff versus v1:

1:  ebd05211ab ! 1:  70baa9b4eb last-modified: verify revision argument is a commit-ish
    @@ Metadata
     Author: Toon Claes <toon@iotcl.com>
     
      ## Commit message ##
    -    last-modified: verify revision argument is a commit-ish
    +    last-modified: rewrite error message when more than one revision given
     
    -    Passing a tree OID to git-last-modified(1) would trigger BUG behavior.
    +    When more than one revision is passed to the git-last-modified(1)
    +    command, this error message was printed:
     
    -        git last-modified HEAD^{tree}
    -        BUG: builtin/last-modified.c:456: paths remaining beyond boundary in last-modified
    +        error: last-modified can only operate on one tree at a time
     
    -    Fix this error by verifying the parsed revision is peels to a
    -    commit-ish.
    +    Calling these a "tree" is technically not correct. git-last-modified(1)
    +    expects revisions that peel to a commit.
     
    -    While at it, also fix a memory leak in populate_paths_from_revs().
    +    Rephrase the error message to:
    +
    +        error: last-modified can only operate on one revision at a time
    +
    +    While at it, also fix a memory leak that remained uncovered so far.
     
    -    Reported-by: Gusted <gusted@codeberg.org>
         Signed-off-by: Toon Claes <toon@iotcl.com>
     
      ## builtin/last-modified.c ##
    @@ builtin/last-modified.c: static int populate_paths_from_revs(struct last_modifie
     -		if (num_interesting++)
     -			return error(_("last-modified can only operate on one tree at a time"));
     +		if (num_interesting++) {
    -+			ret = error(_("last-modified can only operate on one tree at a time"));
    -+			break;
    -+		}
    -+
    -+		if (!repo_peel_to_type(lm->rev.repo, obj->path, 0, obj->item, OBJ_COMMIT)) {
    -+			ret = error(_("revision argument is not a commit-ish"));
    -+			break;
    ++			ret = error(_("last-modified can only operate on one revision at a time"));
    ++			goto out;
     +		}
      
      		diff_tree_oid(lm->rev.repo->hash_algo->empty_tree,
      			      &obj->item->oid, "", &diffopt);
    -@@ builtin/last-modified.c: static int populate_paths_from_revs(struct last_modified *lm)
    + 		diff_flush(&diffopt);
      	}
    ++
    ++out:
      	clear_pathspec(&diffopt.pathspec);
      
     -	return 0;
    @@ builtin/last-modified.c: static int populate_paths_from_revs(struct last_modifie
      static void last_modified_emit(struct last_modified *lm,
     
      ## t/t8020-last-modified.sh ##
    -@@ t/t8020-last-modified.sh: test_expect_success 'last-modified complains about unknown arguments' '
    - 	grep "unknown last-modified argument: --foo" err
    +@@ t/t8020-last-modified.sh: test_expect_success 'setup' '
    + 	test_commit 3 a/b/file
    + '
    + 
    +-test_expect_success 'cannot run last-modified on two trees' '
    ++test_expect_success 'cannot run last-modified on two revision' '
    + 	test_must_fail git last-modified HEAD HEAD~1
      '
      
    -+test_expect_success 'last-modified expects commit-ish' '
    -+	test_must_fail git last-modified HEAD^{tree} 2>err &&
    -+	grep "revision argument is not a commit-ish" err
    -+'
    -+
    - test_done
-:  ---------- > 2:  150c43580d last-modified: remove double error message
-:  ---------- > 3:  ab89cb1ef3 last-modified: verify revision argument is a commit-ish


---
base-commit: d529f3a197364881746f558e5652f0236131eb86
change-id: 20260112-toon-last-modified-tree-fdd96b2feaf7