object-name: fix reversed ordering with ":/<text>" revisions

Recently it was reported [1] that "look for the youngest commit
reachable from any ref with log message that match the given
pattern" syntax (i.e. ':/<text>') started to return results in
reverse recency order. This regression was introduced in Git v2.47.0
and is caused by a memory leak fix done in 57fb139b5e (object-name:
fix leaking commit list items, 2024-08-01).

The intent of the identified commit is to stop modifying the commit list
provided by the caller such that the caller can properly free all commit
list items, including those that the called function might potentially
remove from the list. This was done by creating a copy of the passed-in
commit list and modifying this copy instead of the caller-provided list.

We already knew to create such a copy beforehand with the `backup` list,
which was used to clear the `ONELINE_SEEN` commit mark after we were
done. So the refactoring simply renamed that list to `copy` and started
to operate on that list instead. There is a gotcha though: the backup
list, and thus now also the copied list, is always being prepended to,
so the resulting list is in reverse order! The end result is that we
pop commits from the wrong end of the commit list, returning commits in
reverse recency order.

Fix the bug by appending to the list instead.

[1]: <CAKOEJdcPYn3O01p29rVa+xv=Qr504FQyKJeSB-Moze04ViCGGg@mail.gmail.com>

Reported-by: Aarni Koskela <aarni@valohai.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/object-name.c b/object-name.c
index 240a93e7cef52f45c6354a7019417c7946b6b27a..4c50559ee8cdaafb172c1cf9d2c397b263186f31 100644 (file)
--- a/object-name.c
+++ b/object-name.c
@@ -1393,7 +1393,7 @@ static int get_oid_oneline(struct repository *r,
                           const char *prefix, struct object_id *oid,
                           const struct commit_list *list)
 {
-       struct commit_list *copy = NULL;
+       struct commit_list *copy = NULL, **copy_tail = ©
        const struct commit_list *l;
        int found = 0;
        int negative = 0;
@@ -1415,7 +1415,7 @@ static int get_oid_oneline(struct repository *r,
 
        for (l = list; l; l = l->next) {
                l->item->object.flags |= ONELINE_SEEN;
-               commit_list_insert(l->item, &copy);
+               copy_tail = &commit_list_insert(l->item, copy_tail)->next;
        }
        while (copy) {
                const char *p, *buf;

diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh
index 30c31918fde6539d52800e18dfbb3423b5b73491..42c4a63cb95eed781ed7d3029c4ff5e600e6f8b8 100755 (executable)
--- a/t/t1500-rev-parse.sh
+++ b/t/t1500-rev-parse.sh
@@ -310,4 +310,19 @@ test_expect_success '--short= truncates to the actual hash length' '
        test_cmp expect actual
 '
 
+test_expect_success ':/ and HEAD^{/} favor more recent matching commits' '
+       test_when_finished "rm -rf repo" &&
+       git init repo &&
+       (
+               cd repo &&
+               test_commit common-old &&
+               test_commit --no-tag common-new &&
+               git rev-parse HEAD >expect &&
+               git rev-parse :/common >actual &&
+               test_cmp expect actual &&
+               git rev-parse HEAD^{/common} >actual &&
+               test_cmp expect actual
+       )
+'
+
 test_done