Regression in :/ commit selection between 2.43.0 and 2.47.1

Regression in :/ commit selection between 2.43.0 and 2.47.1

[Aarni Koskela]

The behavior of the `:/` notation to select commits seems to have
changed to no longer
prioritize younger commits on the current `HEAD`. Instead, if an older
commit is reachable
from a named ref (e.g. a tag), it will be selected instead of the
younger commit.

See below for a reproduction:

* Initialize a repository
* Create a commit with message "mystery commit 1"
* Tag the commit with `git tag -a a-tag -m a-tag`
* Create another commit with message "mystery commit 2" (which should
be found from HEAD with `:/mystery`)
* Show the commit with `git show :/mystery` – it shows the older commit!
* Delete the tag with `git tag -d a-tag`
* Show the commit with `git show :/mystery` – it now shows the newer commit.

```
$ git init
$ git commit -m "mystery commit 1" -m "this is old" --allow-empty
[master (root-commit) 2613e98] mystery commit 1
$ git tag -a a-tag -m a-tag
$ git commit -m "mystery commit 2" -m "this is newer" --allow-empty
[master 705a642] mystery commit 2
$ git show :/mystery --oneline
2613e98 mystery commit 1
```

If one deletes the tag, the younger commit is prioritized.

```
$ git tag -d a-tag
Deleted tag 'a-tag' (was 652eb72)
$ git show :/mystery --oneline
705a642 (HEAD -> master) mystery commit 2
```

https://git-scm.com/docs/gitrevisions documents `:/` to find
"the youngest matching commit which is reachable from any ref, including HEAD",
which is how has worked before.

This seems to have regressed somewhere between Git 2.43.0 and 2.47.1.
Accessing the same repository with 2.43.0 (in a Docker container) shows
the expected behavior:

```
root@db58673dabff:/w# git version
git version 2.43.0
root@db58673dabff:/w# git log --oneline
705a642 (HEAD -> master) mystery commit 2
2613e98 (tag: a-tag) mystery commit 1
root@db58673dabff:/w# git show :/mystery --oneline
705a642 (HEAD -> master) mystery commit 2
```

I can also verify the same buggy behavior with 2.47.1 in an Arch Linux
environment, so it's not specific to a macOS Homebrew build:

```
sh-5.2# git version
git version 2.47.1
sh-5.2# git log --oneline
705a642 (HEAD -> master) mystery commit 2
2613e98 (tag: a-tag) mystery commit 1
sh-5.2# git show :/mystery --oneline
2613e98 (tag: a-tag) mystery commit 1
```

(This can also be reproduced with a fresh repository created within
that Arch Linux box with 2.47.1, so it's not e.g. a quirk of a given
repository.)

This breaks my usual `git commit --fixup :/something` workflow, as a
`something` commit I know I've committed just recently is no longer selected
as the fixup target :-)

Best regards,

Aarni Koskela

[System Info]
git version: git version 2.47.1 (from Homebrew)
cpu: arm64
sizeof-long: 8
sizeof-size_t: 8
shell-path: /bin/sh
feature: fsmonitor--daemon
libcurl: 8.6.0
zlib: 1.2.12
uname: Darwin 23.6.0 Darwin Kernel Version 23.6.0: Thu Sep 12 23:36:12
PDT 2024; root:xnu-10063.141.1.701.1~1/RELEASE_ARM64_T6020 arm64
compiler info: clang: 16.0.0 (clang-1600.0.26.4)

Re: Regression in :/ commit selection between 2.43.0 and 2.47.1

[Jeff King]

On Thu, Dec 05, 2024 at 02:22:13PM +0200, Aarni Koskela wrote:

> See below for a reproduction:
> 
> * Initialize a repository
> * Create a commit with message "mystery commit 1"
> * Tag the commit with `git tag -a a-tag -m a-tag`
> * Create another commit with message "mystery commit 2" (which should
> be found from HEAD with `:/mystery`)
> * Show the commit with `git show :/mystery` – it shows the older commit!
> * Delete the tag with `git tag -d a-tag`
> * Show the commit with `git show :/mystery` – it now shows the newer commit.

This bisects to 57fb139b5e (object-name: fix leaking commit list items,
2024-08-01). Looks like an unintended side effect, maybe related to
swapping "list" and "copy" in get_oid_oneline()?

-Peff

Re: Regression in :/ commit selection between 2.43.0 and 2.47.1

[Patrick Steinhardt]

On Thu, Dec 05, 2024 at 12:33:42PM -0500, Jeff King wrote:
> On Thu, Dec 05, 2024 at 02:22:13PM +0200, Aarni Koskela wrote:
> 
> > See below for a reproduction:
> > 
> > * Initialize a repository
> > * Create a commit with message "mystery commit 1"
> > * Tag the commit with `git tag -a a-tag -m a-tag`
> > * Create another commit with message "mystery commit 2" (which should
> > be found from HEAD with `:/mystery`)
> > * Show the commit with `git show :/mystery` – it shows the older commit!
> > * Delete the tag with `git tag -d a-tag`
> > * Show the commit with `git show :/mystery` – it now shows the newer commit.
> 
> This bisects to 57fb139b5e (object-name: fix leaking commit list items,
> 2024-08-01). Looks like an unintended side effect, maybe related to
> swapping "list" and "copy" in get_oid_oneline()?

Indeed, that's it. We have this hunk in that commit:

@@ -1411,14 +1415,14 @@ 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, &backup);
+               commit_list_insert(l->item, &copy);
        }

The problem with this code is that it's _prepending_ to the copy every
time, and thus we accidentally started to reverse the list. To fix this
we again have to _append_ to it:

diff --git a/object-name.c b/object-name.c
index 240a93e7cef..4c50559ee8c 100644
--- 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;

I've sent a patch via [1].

[1]: <20241206-pks-rev-parse-fix-reversed-list-v1-1-95a96564a4d7@pks.im>

Patrick