{"thread":{"id":"62601","subject":"Regression in :/ commit selection between 2.43.0 and 2.47.1","startedAt":"2024-12-05T12:22:25Z","lastAt":"2024-12-06T09:52:26Z","messageCount":3,"participants":["Aarni Koskela","Jeff King","Patrick Steinhardt"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"508669","messageId":"CAKOEJdcPYn3O01p29rVa+xv=Qr504FQyKJeSB-Moze04ViCGGg@mail.gmail.com","threadId":"62601","inReplyTo":null,"subject":"Regression in :/ commit selection between 2.43.0 and 2.47.1","fromName":"Aarni Koskela","fromEmail":"aarni@valohai.com","sentAt":"2024-12-05T12:22:13Z","receivedAt":"2024-12-05T12:22:25Z","isPatch":false,"sender":{"key":"aarni@valohai.com","avatar":null},"body":"The behavior of the `:/` notation to select commits seems to have\nchanged to no longer\nprioritize younger commits on the current `HEAD`. Instead, if an older\ncommit is reachable\nfrom a named ref (e.g. a tag), it will be selected instead of the\nyounger commit.\n\nSee below for a reproduction:\n\n* Initialize a repository\n* Create a commit with message \"mystery commit 1\"\n* Tag the commit with `git tag -a a-tag -m a-tag`\n* Create another commit with message \"mystery commit 2\" (which should\nbe found from HEAD with `:/mystery`)\n* Show the commit with `git show :/mystery` – it shows the older commit!\n* Delete the tag with `git tag -d a-tag`\n* Show the commit with `git show :/mystery` – it now shows the newer commit.\n\n```\n$ git init\n$ git commit -m \"mystery commit 1\" -m \"this is old\" --allow-empty\n[master (root-commit) 2613e98] mystery commit 1\n$ git tag -a a-tag -m a-tag\n$ git commit -m \"mystery commit 2\" -m \"this is newer\" --allow-empty\n[master 705a642] mystery commit 2\n$ git show :/mystery --oneline\n2613e98 mystery commit 1\n```\n\nIf one deletes the tag, the younger commit is prioritized.\n\n```\n$ git tag -d a-tag\nDeleted tag 'a-tag' (was 652eb72)\n$ git show :/mystery --oneline\n705a642 (HEAD -> master) mystery commit 2\n```\n\nhttps://git-scm.com/docs/gitrevisions documents `:/` to find\n\"the youngest matching commit which is reachable from any ref, including HEAD\",\nwhich is how has worked before.\n\nThis seems to have regressed somewhere between Git 2.43.0 and 2.47.1.\nAccessing the same repository with 2.43.0 (in a Docker container) shows\nthe expected behavior:\n\n```\nroot@db58673dabff:/w# git version\ngit version 2.43.0\nroot@db58673dabff:/w# git log --oneline\n705a642 (HEAD -> master) mystery commit 2\n2613e98 (tag: a-tag) mystery commit 1\nroot@db58673dabff:/w# git show :/mystery --oneline\n705a642 (HEAD -> master) mystery commit 2\n```\n\nI can also verify the same buggy behavior with 2.47.1 in an Arch Linux\nenvironment, so it's not specific to a macOS Homebrew build:\n\n```\nsh-5.2# git version\ngit version 2.47.1\nsh-5.2# git log --oneline\n705a642 (HEAD -> master) mystery commit 2\n2613e98 (tag: a-tag) mystery commit 1\nsh-5.2# git show :/mystery --oneline\n2613e98 (tag: a-tag) mystery commit 1\n```\n\n(This can also be reproduced with a fresh repository created within\nthat Arch Linux box with 2.47.1, so it's not e.g. a quirk of a given\nrepository.)\n\nThis breaks my usual `git commit --fixup :/something` workflow, as a\n`something` commit I know I've committed just recently is no longer selected\nas the fixup target :-)\n\nBest regards,\n\nAarni Koskela\n\n[System Info]\ngit version: git version 2.47.1 (from Homebrew)\ncpu: arm64\nsizeof-long: 8\nsizeof-size_t: 8\nshell-path: /bin/sh\nfeature: fsmonitor--daemon\nlibcurl: 8.6.0\nzlib: 1.2.12\nuname: Darwin 23.6.0 Darwin Kernel Version 23.6.0: Thu Sep 12 23:36:12\nPDT 2024; root:xnu-10063.141.1.701.1~1/RELEASE_ARM64_T6020 arm64\ncompiler info: clang: 16.0.0 (clang-1600.0.26.4)\n"},{"id":"508674","messageId":"20241205173342.GA2593033@coredump.intra.peff.net","threadId":"62601","inReplyTo":"CAKOEJdcPYn3O01p29rVa+xv=Qr504FQyKJeSB-Moze04ViCGGg@mail.gmail.com","subject":"Re: Regression in :/ commit selection between 2.43.0 and 2.47.1","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-12-05T17:33:42Z","receivedAt":"2024-12-05T17:33:47Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Dec 05, 2024 at 02:22:13PM +0200, Aarni Koskela wrote:\n\n> See below for a reproduction:\n> \n> * Initialize a repository\n> * Create a commit with message \"mystery commit 1\"\n> * Tag the commit with `git tag -a a-tag -m a-tag`\n> * Create another commit with message \"mystery commit 2\" (which should\n> be found from HEAD with `:/mystery`)\n> * Show the commit with `git show :/mystery` – it shows the older commit!\n> * Delete the tag with `git tag -d a-tag`\n> * Show the commit with `git show :/mystery` – it now shows the newer commit.\n\nThis bisects to 57fb139b5e (object-name: fix leaking commit list items,\n2024-08-01). Looks like an unintended side effect, maybe related to\nswapping \"list\" and \"copy\" in get_oid_oneline()?\n\n-Peff\n"},{"id":"508717","messageId":"Z1LJSADiStlFicTL@pks.im","threadId":"62601","inReplyTo":"20241205173342.GA2593033@coredump.intra.peff.net","subject":"Re: Regression in :/ commit selection between 2.43.0 and 2.47.1","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-12-06T09:52:08Z","receivedAt":"2024-12-06T09:52:26Z","isPatch":false,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, Dec 05, 2024 at 12:33:42PM -0500, Jeff King wrote:\n> On Thu, Dec 05, 2024 at 02:22:13PM +0200, Aarni Koskela wrote:\n> \n> > See below for a reproduction:\n> > \n> > * Initialize a repository\n> > * Create a commit with message \"mystery commit 1\"\n> > * Tag the commit with `git tag -a a-tag -m a-tag`\n> > * Create another commit with message \"mystery commit 2\" (which should\n> > be found from HEAD with `:/mystery`)\n> > * Show the commit with `git show :/mystery` – it shows the older commit!\n> > * Delete the tag with `git tag -d a-tag`\n> > * Show the commit with `git show :/mystery` – it now shows the newer commit.\n> \n> This bisects to 57fb139b5e (object-name: fix leaking commit list items,\n> 2024-08-01). Looks like an unintended side effect, maybe related to\n> swapping \"list\" and \"copy\" in get_oid_oneline()?\n\nIndeed, that's it. We have this hunk in that commit:\n\n@@ -1411,14 +1415,14 @@ static int get_oid_oneline(struct repository *r,\n        for (l = list; l; l = l->next) {\n                l->item->object.flags |= ONELINE_SEEN;\n-               commit_list_insert(l->item, &backup);\n+               commit_list_insert(l->item, &copy);\n        }\n\nThe problem with this code is that it's _prepending_ to the copy every\ntime, and thus we accidentally started to reverse the list. To fix this\nwe again have to _append_ to it:\n\ndiff --git a/object-name.c b/object-name.c\nindex 240a93e7cef..4c50559ee8c 100644\n--- a/object-name.c\n+++ b/object-name.c\n@@ -1393,7 +1393,7 @@ static int get_oid_oneline(struct repository *r,\n \t\t\t   const char *prefix, struct object_id *oid,\n \t\t\t   const struct commit_list *list)\n {\n-\tstruct commit_list *copy = NULL;\n+\tstruct commit_list *copy = NULL, **copy_tail = &copy;\n \tconst struct commit_list *l;\n \tint found = 0;\n \tint negative = 0;\n@@ -1415,7 +1415,7 @@ static int get_oid_oneline(struct repository *r,\n \n \tfor (l = list; l; l = l->next) {\n \t\tl->item->object.flags |= ONELINE_SEEN;\n-\t\tcommit_list_insert(l->item, &copy);\n+\t\tcopy_tail = &commit_list_insert(l->item, copy_tail)->next;\n \t}\n \twhile (copy) {\n \t\tconst char *p, *buf;\n\nI've sent a patch via [1].\n\n[1]: <20241206-pks-rev-parse-fix-reversed-list-v1-1-95a96564a4d7@pks.im>\n\nPatrick\n"}]}