{"thread":{"id":"59064","subject":"Bugreport: Prefix - is ignored when sorting (on committerdate)","startedAt":"2023-01-10T09:04:53Z","lastAt":"2023-01-10T13:47:36Z","messageCount":6,"participants":["Fredrik Öberg","Martin Ågren","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"470050","messageId":"CALDp=1EVmQE2NZAd7ccAT0z=R=2S8FvyJzsPNrDeXwdJkEW3Lg@mail.gmail.com","threadId":"59064","inReplyTo":null,"subject":"Bugreport: Prefix - is ignored when sorting (on committerdate)","fromName":"Fredrik Öberg","fromEmail":"fredrik@bakskuru.se","sentAt":"2023-01-10T09:02:28Z","receivedAt":"2023-01-10T09:04:53Z","isPatch":false,"sender":{"key":"fredrik@bakskuru.se","avatar":null},"body":"Thank you for filling out a Git bug report!\nPlease answer the following questions to help us understand your issue.\n\nWhat did you do before the bug happened? (Steps to reproduce your issue)\n- Create a new repository\n- Add a new file to the repository and commit\n- Create a tag\n$ git tag -a TagOne -m \"\"\n- Update the file and commit the change\n- Create a tag\n$ git tag -a TagTwo -m \"\"\n- List tags by committerdate:\n$ git tag -l --sort=committerdate\n(oldest tag is listed first)\n- List tags by reversed committerdate:\n$ git tag -l --sort=-committerdate\n(oldest tag it still listed first)\n\nWhat did you expect to happen? (Expected behavior)\nUsing the minus-sign as prefix is supposed to reverse the sorting\norder, listing the newest item first. I expected the newest tag to be\nlisted first\n\nWhat happened instead? (Actual behavior)\nThe minus-sign was ignored, and the oldest tag was still listed first\n\nWhat's different between what you expected and what actually happened?\nI expected the newest tag to be listed first when the minus-sign was\nused as a prefix, but the oldest tag was still listed first.\n\nAnything else you want to add:\nThe correct behaviour (reversing sort-order by using the minus-prefix)\nhas been verified in\nversion 2.11.0, 2.11.1, 2.14.2, 2.19.1, 2.19.2, 2.23.0 on CentOS 7.4\nversion 2.31.1 and 2.35.1 on CentOS 7.4 contains the error\nversion 2.34.1.windows.1 contains the error\n\nPlease review the rest of the bug report below.\nYou can delete any lines you don't wish to share.\n\n[System Info]\ngit version:\ngit version 2.34.1.windows.1\ncpu: x86_64\nbuilt from commit: 2ca94ab318509b3c271e82889938816bad76dfea\nsizeof-long: 4\nsizeof-size_t: 8\nshell-path: /bin/sh\nfeature: fsmonitor--daemon\nuname: Windows 10.0 19042\ncompiler info: gnuc: 11.2\nlibc info: no libc information available\n$SHELL (typically, interactive shell): C:\\Program Files\\Git\\usr\\bin\\bash.exe\n\n\n[Enabled Hooks]\n"},{"id":"470056","messageId":"CAN0heSprKD35P-D7GTVQ20eRsj0AriYEA7iCDnrA9GuiwX0snw@mail.gmail.com","threadId":"59064","inReplyTo":"CALDp=1EVmQE2NZAd7ccAT0z=R=2S8FvyJzsPNrDeXwdJkEW3Lg@mail.gmail.com","subject":"Re: Bugreport: Prefix - is ignored when sorting (on committerdate)","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2023-01-10T10:54:16Z","receivedAt":"2023-01-10T10:54:56Z","isPatch":false,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"Hej Fredrik,\n\nOn Tue, 10 Jan 2023 at 10:50, Fredrik Öberg <fredrik@bakskuru.se> wrote:\n> - Create a new repository\n> - Add a new file to the repository and commit\n> - Create a tag\n> $ git tag -a TagOne -m \"\"\n> - Update the file and commit the change\n> - Create a tag\n> $ git tag -a TagTwo -m \"\"\n> - List tags by committerdate:\n> $ git tag -l --sort=committerdate\n> (oldest tag is listed first)\n> - List tags by reversed committerdate:\n> $ git tag -l --sort=-committerdate\n> (oldest tag it still listed first)\n\n> Anything else you want to add:\n> The correct behaviour (reversing sort-order by using the minus-prefix)\n> has been verified in\n> version 2.11.0, 2.11.1, 2.14.2, 2.19.1, 2.19.2, 2.23.0 on CentOS 7.4\n> version 2.31.1 and 2.35.1 on CentOS 7.4 contains the error\n> version 2.34.1.windows.1 contains the error\n\nThis bisects to 7c5045fc18 (\"ref-filter: apply fallback refname sort\nonly after all user sorts\", 2020-05-03). I've cc-ed the author. That\ncommit does change the behavior for the kind of test repo you describe.\n\nThat said, you're using \"committerdate\" here. If you use \"taggerdate\"\n(or \"creatordate\") I think you'll get the output you expect, even for\nnewer Git versions. Does that help?\n\nSince you just have two commits in the reproducer, there's a strong\ncorrelation between tag names and the timestamps involved. You actually\nend up sorting by refname: because there is no committerdate for these\ntags, the refnames are compared as a fallback. While the old code then\napplied the reversal ('-') to *that*, the new code first fails to find\nany difference, so doesn't have anything to reverse, then falls back to\ncomparing the refnames, at which point it doesn't consider reversing the\nresult.\n\nAll of this is based on my understanding. I could obviously be wrong.\n\nI suppose it could be argued that the '-' should be applied to the\nfallback as well, e.g., to uphold some sort of \"using '-' should give\nthe same result as piping the whole thing through tac\" (i.e., respecting\n`s->reverse` in `compare_refs()`, if you're following along in\n7c5045fc18). With multiple sort keys, some with '-' and some\nwithout, we'd grab the '-' from the first key. It seems like that could\nmake sense, actually.\n\nI might post such a patch, but as noted above, I think what you really\nwant to use is \"taggerdate\". I hope that helps.\n\nMartin\n"},{"id":"470057","messageId":"Y71Xnx2vmznV913I@coredump.intra.peff.net","threadId":"59064","inReplyTo":"CAN0heSprKD35P-D7GTVQ20eRsj0AriYEA7iCDnrA9GuiwX0snw@mail.gmail.com","subject":"Re: Bugreport: Prefix - is ignored when sorting (on committerdate)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-01-10T12:18:39Z","receivedAt":"2023-01-10T12:20:37Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jan 10, 2023 at 11:54:16AM +0100, Martin Ågren wrote:\n\n> On Tue, 10 Jan 2023 at 10:50, Fredrik Öberg <fredrik@bakskuru.se> wrote:\n> > - Create a new repository\n> > - Add a new file to the repository and commit\n> > - Create a tag\n> > $ git tag -a TagOne -m \"\"\n> > - Update the file and commit the change\n> > - Create a tag\n> > $ git tag -a TagTwo -m \"\"\n> > - List tags by committerdate:\n> > $ git tag -l --sort=committerdate\n> > (oldest tag is listed first)\n> > - List tags by reversed committerdate:\n> > $ git tag -l --sort=-committerdate\n> > (oldest tag it still listed first)\n> [...]\n> \n> This bisects to 7c5045fc18 (\"ref-filter: apply fallback refname sort\n> only after all user sorts\", 2020-05-03). I've cc-ed the author. That\n> commit does change the behavior for the kind of test repo you describe.\n> \n> That said, you're using \"committerdate\" here. If you use \"taggerdate\"\n> (or \"creatordate\") I think you'll get the output you expect, even for\n> newer Git versions. Does that help?\n\nYes, I think that's the crux of the issue here; there is no date sorting\nhappening at all, before or after that patch. Depending on what you want\nto do, I think \"creatordate\" is probably the most common option (it's\nbetter than taggerdate in that it handles lightweight tags, too). But if\nyou really want committer dates, then \"--sort=-=*committerdate\" would\nwork.\n\n> Since you just have two commits in the reproducer, there's a strong\n> correlation between tag names and the timestamps involved. You actually\n> end up sorting by refname: because there is no committerdate for these\n> tags, the refnames are compared as a fallback. While the old code then\n> applied the reversal ('-') to *that*, the new code first fails to find\n> any difference, so doesn't have anything to reverse, then falls back to\n> comparing the refnames, at which point it doesn't consider reversing the\n> result.\n> \n> All of this is based on my understanding. I could obviously be wrong.\n\nI think that explanation is exactly correct. The only difference before\nand after that patch is whether the \"-\" is applied to the fallback\nrefname sort.\n\n> I suppose it could be argued that the '-' should be applied to the\n> fallback as well, e.g., to uphold some sort of \"using '-' should give\n> the same result as piping the whole thing through tac\" (i.e., respecting\n> `s->reverse` in `compare_refs()`, if you're following along in\n> 7c5045fc18). With multiple sort keys, some with '-' and some\n> without, we'd grab the '-' from the first key. It seems like that could\n> make sense, actually.\n\nI dunno. Just because you are reverse-sorting on one field doesn't\nnecessarily imply that you want the tie-breaker to reverse-sort, too. I\nget that it's kind of a \"DWIM\" when there's a single sorting key, but I\nthink the multi-key behavior is harder to explain. In:\n\n  git for-each-ref --sort=committername --sort=-committerdate\n\nshould a refname tie breaker follow the reverse-chronological sort, or\nthe in-order name sort? The date sort is the primary key here, so\nrespecting it would be most like \"tac\". But the inner tie-breaker is\nbreaking ties on committername, so should it be most like that?\n\nI could see it depending on exactly what you're trying to do. Which\nleads me to think the rule should be as simple as possible. You can\nalways do:\n\n  git for-each-ref --sort=-refname --sort=-committerdate\n\nto specify exactly what you want.\n\n-Peff\n"},{"id":"470061","messageId":"CAN0heSqvqnCHPa-RQ3JuxxP7M_k1ORYOAV9aG+8EuXvB1GZukw@mail.gmail.com","threadId":"59064","inReplyTo":"Y71Xnx2vmznV913I@coredump.intra.peff.net","subject":"Re: Bugreport: Prefix - is ignored when sorting (on committerdate)","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2023-01-10T13:05:54Z","receivedAt":"2023-01-10T13:06:13Z","isPatch":false,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"On Tue, 10 Jan 2023 at 13:18, Jeff King <peff@peff.net> wrote:\n>\n> On Tue, Jan 10, 2023 at 11:54:16AM +0100, Martin Ågren wrote:\n>\n> > I suppose it could be argued that the '-' should be applied to the\n> > fallback as well, e.g., to uphold some sort of \"using '-' should give\n> > the same result as piping the whole thing through tac\" (i.e., respecting\n> > `s->reverse` in `compare_refs()`, if you're following along in\n> > 7c5045fc18). With multiple sort keys, some with '-' and some\n> > without, we'd grab the '-' from the first key. It seems like that could\n> > make sense, actually.\n>\n> I dunno. Just because you are reverse-sorting on one field doesn't\n> necessarily imply that you want the tie-breaker to reverse-sort, too. I\n[...]\n\n> I could see it depending on exactly what you're trying to do. Which\n> leads me to think the rule should be as simple as possible. You can\n> always do:\n>\n>   git for-each-ref --sort=-refname --sort=-committerdate\n>\n> to specify exactly what you want.\n\nIndeed. So probably best to leave it as-is, then.\n\nMartin\n"},{"id":"470069","messageId":"CALDp=1GBtibTR5SvG8sOX14aODtdEJWhL_PCP1mzTLmX9V0sow@mail.gmail.com","threadId":"59064","inReplyTo":"CAN0heSqvqnCHPa-RQ3JuxxP7M_k1ORYOAV9aG+8EuXvB1GZukw@mail.gmail.com","subject":"Re: Bugreport: Prefix - is ignored when sorting (on committerdate)","fromName":"Fredrik Öberg","fromEmail":"fredrik@bakskuru.se","sentAt":"2023-01-10T13:33:14Z","receivedAt":"2023-01-10T13:34:13Z","isPatch":false,"sender":{"key":"fredrik@bakskuru.se","avatar":null},"body":"Thanks for the  quick response! I've been playing with creatordate and\nit seems to work with with both older and newer versions of git.\n\nI am trying to understand what I did wrong. I guess you are saying\nthat I tried to use the field committerdate, which is not available on\nthe tags? And that the handling of this situation has changed since\ngit/2.23? Hence I got different results?\n\n/Fredrik\n\nDen tis 10 jan. 2023 kl 14:05 skrev Martin Ågren <martin.agren@gmail.com>:\n>\n> On Tue, 10 Jan 2023 at 13:18, Jeff King <peff@peff.net> wrote:\n> >\n> > On Tue, Jan 10, 2023 at 11:54:16AM +0100, Martin Ågren wrote:\n> >\n> > > I suppose it could be argued that the '-' should be applied to the\n> > > fallback as well, e.g., to uphold some sort of \"using '-' should give\n> > > the same result as piping the whole thing through tac\" (i.e., respecting\n> > > `s->reverse` in `compare_refs()`, if you're following along in\n> > > 7c5045fc18). With multiple sort keys, some with '-' and some\n> > > without, we'd grab the '-' from the first key. It seems like that could\n> > > make sense, actually.\n> >\n> > I dunno. Just because you are reverse-sorting on one field doesn't\n> > necessarily imply that you want the tie-breaker to reverse-sort, too. I\n> [...]\n>\n> > I could see it depending on exactly what you're trying to do. Which\n> > leads me to think the rule should be as simple as possible. You can\n> > always do:\n> >\n> >   git for-each-ref --sort=-refname --sort=-committerdate\n> >\n> > to specify exactly what you want.\n>\n> Indeed. So probably best to leave it as-is, then.\n>\n> Martin\n"},{"id":"470071","messageId":"Y71scwT5d74Daw0E@coredump.intra.peff.net","threadId":"59064","inReplyTo":"CALDp=1GBtibTR5SvG8sOX14aODtdEJWhL_PCP1mzTLmX9V0sow@mail.gmail.com","subject":"Re: Bugreport: Prefix - is ignored when sorting (on committerdate)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-01-10T13:47:31Z","receivedAt":"2023-01-10T13:47:36Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jan 10, 2023 at 02:33:14PM +0100, Fredrik Öberg wrote:\n\n> I am trying to understand what I did wrong. I guess you are saying\n> that I tried to use the field committerdate, which is not available on\n> the tags?\n\nRight. The committerdate of a tag is a blank string, so sorting on it\nwas not doing anything.\n\n> And that the handling of this situation has changed since\n> git/2.23? Hence I got different results?\n\nSort of. Your committerdate sort was always doing nothing, but due to\nthe buggy way that the sort handled ties in 2.23 and older, it happened\nto do a reverse refname sort instead of a forward refname sort. Now it\ndoes a forward refname sort to break ties.\n\nBut the most important change for you is to actually do a real date\nsort, so the tie-breaking should not be important either way.\n\n-Peff\n"}]}