threads / discuss / 62601

Regression in :/ commit selection between 2.43.0 and 2.47.1

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

## tl;dr

3 messages between Dec 5, 2024 and Dec 6, 2024.

replies: 2people: 3as markdown or json

Aarni Koskela· Dec 5, 2024, 12:22 UTC · lore

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)

Jeff King· Dec 5, 2024, 17:33 UTC · re: Aarni Koskela · lore

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

On Thu, Dec 05, 2024 at 02:22:13PM +0200, Aarni Koskela wrote:
Show 10 quoted lines
> 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
Patrick Steinhardt· Dec 6, 2024, 09:52 UTC · re: Jeff King · lore

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

On Thu, Dec 05, 2024 at 12:33:42PM -0500, Jeff King wrote:
Show 16 quoted lines
> 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

← back to recent threads