From 7da62d2266f04ec066fce8c4c468eca81ca64680 Mon Sep 17 00:00:00 2001 From: Toon Claes Date: Mon, 12 Jan 2026 16:38:06 +0100 Subject: [PATCH 1/6] Fix git-last-modified(1) bug triggered when passing a tree-ish 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/ Cc: Patrick Steinhardt Cc: Kristoffer Haugsbakk --- Changes in v6: - EDITME: describe what is new in this series revision. - EDITME: use bulletpoints and terse descriptions. - Link to v5: https://patch.msgid.link/20260127-toon-last-modified-tree-v5-0-38d18a0956d4@iotcl.com Changes in v5: - Use 'commit' instead of 'revision'. - Small typo fixes. - Link to v4: https://patch.msgid.link/20260123-toon-last-modified-tree-v4-0-86bf97bad4e2@iotcl.com Changes in v4: - Extend the error message when revision is not a tree - Extend the test to verify the error message when more than one revision is given. - Link to v3: https://patch.msgid.link/20260116-toon-last-modified-tree-v3-0-e6ade4dc35ab@iotcl.com Changes in v3: - Split the change to plug the leak into a separate commit. - Small changes to commit messages. - Link to v2: https://patch.msgid.link/20260114-toon-last-modified-tree-v2-0-ba3b1860898f@iotcl.com 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 --- b4-submit-tracking --- # This section is used internally by b4 prep for tracking purposes. { "series": { "revision": 6, "change-id": "20260112-toon-last-modified-tree-fdd96b2feaf7", "prefixes": [], "presubject": "", "history": { "v1": [ "20260112-toon-last-modified-tree-v1-1-ecbc78341f76@iotcl.com" ], "v2": [ "20260114-toon-last-modified-tree-v2-0-ba3b1860898f@iotcl.com" ], "v3": [ "20260116-toon-last-modified-tree-v3-0-e6ade4dc35ab@iotcl.com" ], "v4": [ "20260123-toon-last-modified-tree-v4-0-86bf97bad4e2@iotcl.com" ], "v5": [ "20260127-toon-last-modified-tree-v5-0-38d18a0956d4@iotcl.com" ] } } } From 195da376429fabc8d2ad4fa050064675af7aafd4 Mon Sep 17 00:00:00 2001 From: Toon Claes Date: Fri, 16 Jan 2026 13:36:00 +0100 Subject: [PATCH 2/6] last-modified: rewrite error message when more than one commit given When more than one commit is passed to the git-last-modified(1) command, this error message was printed: error: last-modified can only operate on one tree at a time Calling these a "tree" is technically not correct. git-last-modified(1) expects revisions that peel to a commit. Rephrase the error message to: error: last-modified can only operate on one commit at a time While at it, modify the test to ensure the correct error message is printed. Signed-off-by: Toon Claes --- builtin/last-modified.c | 2 +- t/t8020-last-modified.sh | 11 ++++++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/builtin/last-modified.c b/builtin/last-modified.c index c80f0535f6a503..1219f6802e62a9 100644 --- a/builtin/last-modified.c +++ b/builtin/last-modified.c @@ -146,7 +146,7 @@ static int populate_paths_from_revs(struct last_modified *lm) continue; if (num_interesting++) - return error(_("last-modified can only operate on one tree at a time")); + return error(_("last-modified can only operate on one commit at a time")); diff_tree_oid(lm->rev.repo->hash_algo->empty_tree, &obj->item->oid, "", &diffopt); diff --git a/t/t8020-last-modified.sh b/t/t8020-last-modified.sh index 50f4312f715f41..d1aad1231938f6 100755 --- a/t/t8020-last-modified.sh +++ b/t/t8020-last-modified.sh @@ -12,10 +12,6 @@ test_expect_success 'setup' ' test_commit 3 a/b/file ' -test_expect_success 'cannot run last-modified on two trees' ' - test_must_fail git last-modified HEAD HEAD~1 -' - check_last_modified() { local indir= && while test $# != 0 @@ -230,9 +226,14 @@ test_expect_success 'last-modified merge undoes changes' ' EOF ' +test_expect_success 'cannot run last-modified on two commits' ' + test_must_fail git last-modified HEAD HEAD~1 2>err && + test_grep "last-modified can only operate on one commit at a time" err +' + test_expect_success 'last-modified complains about unknown arguments' ' test_must_fail git last-modified --foo 2>err && - grep "unknown last-modified argument: --foo" err + test_grep "unknown last-modified argument: --foo" err ' test_done From 9a7d62cf3970e0cf42e3924a42e566fa034bfda5 Mon Sep 17 00:00:00 2001 From: Toon Claes Date: Fri, 16 Jan 2026 13:36:51 +0100 Subject: [PATCH 3/6] last-modified: fix memory leak when more than one commit is given When more than one commit is given, the function populate_paths_from_revs() leaks a `struct pathspec`. Plug it. Signed-off-by: Toon Claes --- builtin/last-modified.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/builtin/last-modified.c b/builtin/last-modified.c index 1219f6802e62a9..31dea975a023a3 100644 --- a/builtin/last-modified.c +++ b/builtin/last-modified.c @@ -123,7 +123,7 @@ static void add_path_from_diff(struct diff_queue_struct *q, static int populate_paths_from_revs(struct last_modified *lm) { - int num_interesting = 0; + int num_interesting = 0, ret = 0; struct diff_options diffopt; /* @@ -145,16 +145,20 @@ static int populate_paths_from_revs(struct last_modified *lm) if (obj->item->flags & UNINTERESTING) continue; - if (num_interesting++) - return error(_("last-modified can only operate on one commit at a time")); + if (num_interesting++) { + ret = error(_("last-modified can only operate on one commit at a time")); + goto out; + } diff_tree_oid(lm->rev.repo->hash_algo->empty_tree, &obj->item->oid, "", &diffopt); diff_flush(&diffopt); } + +out: clear_pathspec(&diffopt.pathspec); - return 0; + return ret; } static void last_modified_emit(struct last_modified *lm, From fe6c9c904e2dbed896becf240208c8a88abdb76c Mon Sep 17 00:00:00 2001 From: Toon Claes Date: Wed, 14 Jan 2026 09:39:17 +0100 Subject: [PATCH 4/6] last-modified: remove double error message When the user passes two revisions, they get the following output: $ git last-modified HEAD HEAD~ error: last-modified can only operate on one revision at a time error: unable to setup last-modified The error message about "unable to setup" is not very informative, remove it. Signed-off-by: Toon Claes --- builtin/last-modified.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/last-modified.c b/builtin/last-modified.c index 31dea975a023a3..e02ec8428b1c36 100644 --- a/builtin/last-modified.c +++ b/builtin/last-modified.c @@ -495,7 +495,7 @@ static int last_modified_init(struct last_modified *lm, struct repository *r, lm->rev.bloom_filter_settings = get_bloom_filter_settings(lm->rev.repo); if (populate_paths_from_revs(lm) < 0) - return error(_("unable to setup last-modified")); + return -1; CALLOC_ARRAY(lm->all_paths, hashmap_get_size(&lm->paths)); lm->all_paths_nr = 0; From 5732e9d95f80ea8624b0d40d3f148bc9ad5c390b Mon Sep 17 00:00:00 2001 From: Toon Claes Date: Mon, 12 Jan 2026 16:49:25 +0100 Subject: [PATCH 5/6] last-modified: verify revision argument is a commit-ish Passing a non-committish revision to git-last-modified(1) triggers the following BUG: git last-modified HEAD^{tree} BUG: builtin/last-modified.c:456: paths remaining beyond boundary in last-modified Fix this error by ensuring that the given revision peels to a commit. Reported-by: Gusted Signed-off-by: Toon Claes --- builtin/last-modified.c | 5 +++++ t/t8020-last-modified.sh | 13 +++++++++++++ 2 files changed, 18 insertions(+) diff --git a/builtin/last-modified.c b/builtin/last-modified.c index e02ec8428b1c36..d0944673f080f7 100644 --- a/builtin/last-modified.c +++ b/builtin/last-modified.c @@ -150,6 +150,11 @@ static int populate_paths_from_revs(struct last_modified *lm) goto out; } + if (!repo_peel_to_type(lm->rev.repo, obj->path, 0, obj->item, OBJ_COMMIT)) { + ret = error(_("revision argument '%s' is a %s, not a commit-ish"), obj->name, type_name(obj->item->type)); + goto out; + } + diff_tree_oid(lm->rev.repo->hash_algo->empty_tree, &obj->item->oid, "", &diffopt); diff_flush(&diffopt); diff --git a/t/t8020-last-modified.sh b/t/t8020-last-modified.sh index d1aad1231938f6..b8cedd1118ffbe 100755 --- a/t/t8020-last-modified.sh +++ b/t/t8020-last-modified.sh @@ -8,6 +8,7 @@ test_expect_success 'setup' ' test_commit 1 file && mkdir a && test_commit 2 a/file && + git tag -mA t2 2 && mkdir a/b && test_commit 3 a/b/file ' @@ -51,6 +52,13 @@ test_expect_success 'last-modified recursive' ' EOF ' +test_expect_success 'last-modified on annotated tag' ' + check_last_modified t2 <<-\EOF + 2 a + 1 file + EOF +' + test_expect_success 'last-modified recursive with show-trees' ' check_last_modified -r -t <<-\EOF 3 a/b @@ -236,4 +244,9 @@ test_expect_success 'last-modified complains about unknown arguments' ' test_grep "unknown last-modified argument: --foo" err ' +test_expect_success 'last-modified expects commit-ish' ' + test_must_fail git last-modified HEAD^{tree} 2>err && + test_grep "revision argument ${SQ}HEAD^{tree}${SQ} is a tree, not a commit-ish" err +' + test_done From 16c4761a42c60f98a3bb95f95e7431dc22a213be Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Wed, 28 Jan 2026 14:53:06 +0100 Subject: [PATCH 6/6] ci: all the fixes always upload reports, not only on failure upload meson logs remove unused script print failures no verbose log with meson it causes us to write somewhere else, but we want it in the meson test log --- .gitlab-ci.yml | 11 ++++++++--- ci/lib.sh | 10 +++++++++- ci/run-test-slice-meson.sh | 13 ------------- 3 files changed, 17 insertions(+), 17 deletions(-) delete mode 100755 ci/run-test-slice-meson.sh diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index b419a84e2cc660..58e2ca43e475e8 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -69,10 +69,11 @@ test:linux: CC: gcc artifacts: paths: + - build/meson-logs - t/failed-test-artifacts reports: junit: build/meson-logs/testlog.junit.xml - when: on_failure + when: always test:osx: image: $image @@ -111,10 +112,11 @@ test:osx: CC: clang artifacts: paths: + - build/meson-logs - t/failed-test-artifacts reports: junit: build/meson-logs/testlog.junit.xml - when: on_failure + when: always .windows_before_script: &windows_before_script # Disabling realtime monitoring fails on some of the runners, but it @@ -183,11 +185,14 @@ test:msvc-meson: - job: "build:msvc-meson" artifacts: true script: - - meson test -C build --no-rebuild --print-errorlogs --slice $Env:CI_NODE_INDEX/$Env:CI_NODE_TOTAL + - meson test -C build --test-args=-x --no-rebuild --print-errorlogs --slice $Env:CI_NODE_INDEX/$Env:CI_NODE_TOTAL parallel: 10 artifacts: + paths: + - build/meson-logs reports: junit: build/meson-logs/testlog.junit.xml + when: always test:fuzz-smoke-tests: image: ubuntu:latest diff --git a/ci/lib.sh b/ci/lib.sh index 3ecbf147db4fc1..9f796af319c925 100755 --- a/ci/lib.sh +++ b/ci/lib.sh @@ -282,7 +282,15 @@ fi MAKEFLAGS="$MAKEFLAGS --jobs=$JOBS" GIT_PROVE_OPTS="--timer --jobs $JOBS" -GIT_TEST_OPTS="$GIT_TEST_OPTS --verbose-log -x" +GIT_TEST_OPTS="$GIT_TEST_OPTS -x" + +case "$jobname" in +*-meson) + ;; +*) + GIT_TEST_OPTS="$GIT_TEST_OPTS --verbose-log";; +esac + case "$CI_OS_NAME" in windows|windows_nt) GIT_TEST_OPTS="$GIT_TEST_OPTS --no-chain-lint --no-bin-wrappers" diff --git a/ci/run-test-slice-meson.sh b/ci/run-test-slice-meson.sh deleted file mode 100755 index 961c94fba0b2ee..00000000000000 --- a/ci/run-test-slice-meson.sh +++ /dev/null @@ -1,13 +0,0 @@ -#!/bin/sh - -# We must load the build options so we know where to find -# things like TEST_OUTPUT_DIRECTORY. This has to come before -# loading lib.sh, though, because it may clobber some CI lib -# variables like our custom GIT_TEST_OPTS. -. "$1"/GIT-BUILD-OPTIONS -. ${0%/*}/lib.sh - -group "Run tests" \ - meson test -C "$1" --no-rebuild --print-errorlogs \ - --test-args="$GIT_TEST_OPTS" --slice "$((1+$2))/$3" || -handle_failed_tests