{"thread":{"id":"58847","subject":"[bug] git log --invert-grep --grep=[sufficiently complicated regex] prints nothing","startedAt":"2022-11-23T20:18:57Z","lastAt":"2022-11-24T16:35:21Z","messageCount":5,"participants":["Zack Weinberg","Phillip Wood","Ævar Arnfjörð Bjarmason","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"467884","messageId":"71fcf215-101e-4398-bffa-4ed7e286d9c8@app.fastmail.com","threadId":"58847","inReplyTo":null,"subject":"[bug] git log --invert-grep --grep=[sufficiently complicated regex] prints nothing","fromName":"Zack Weinberg","fromEmail":"zack@owlfolio.org","sentAt":"2022-11-23T20:17:02Z","receivedAt":"2022-11-23T20:18:57Z","isPatch":false,"sender":{"key":"zack@owlfolio.org","avatar":null},"body":"I’m attempting to have my blog, which is a static site generated from\na bunch of Markdown files stored in git, automatically pull the\nthe most recent modification date for each page out of the git history.\nThe idea is to use ‘git log --follow --pretty=tformat:'%ct' <file>‘ on\neach file and then use the oldest reported timestamp as the creation\ndate and the newest reported timestamp as the last modification date.\n\nBut there’s a catch: there are commits I want to ignore in this\ncalculation, such as mechanical changes applied to the entire site.\nAnd this brings me to the bug: --invert-grep doesn’t work correctly\nwhen the --grep regex is sufficiently complicated.\n\nHere's the complete set of commits that modified an example file:\n\n$ git log --follow --pretty=tformat:'%ct %s' \\\n  src/posts/uncat/unearthed-arcana-music-division.md\n1668545053 Begin restoring the site structure.\n1668545051 Reorganize directory tree prior to setting up Metalsmith\n1668545032 Mechanically convert Pandoc to standard YAML metadata delimiters.\n1610735533 Mechanically convert to properly delimited YAML metadata.\n1417101173 Correct slug for Uncategorized.\n1417050416 The Great Dead and Moved Link Cleanup of 2014.\n1416938128 Use category_meta plugin to fix category slugs.\n1416763607 Initial import of content and Pelican skeleton.\n\nAnd here's an application of --grep that prints only the commits I _don't_ want:\n\n$ git log --follow -E --pretty=tformat:'%ct %s' \\\n  --grep='^(Mechanically convert|Begin restoring the site structure|Reorganize directory tree)' \\\n src/posts/uncat/unearthed-arcana-music-division.md\n1668545053 Begin restoring the site structure.\n1668545051 Reorganize directory tree prior to setting up Metalsmith\n1668545032 Mechanically convert Pandoc to standard YAML metadata delimiters.\n1610735533 Mechanically convert to properly delimited YAML metadata.\n\nTheoretically, adding --invert-grep to that should make it print the\ncommits I do want, but instead it prints nothing at all:\n\n$ git log --follow -E --pretty=tformat:'%ct %s' --invert-grep \\\n  --grep='^(Mechanically convert|Begin restoring the site structure|Reorganize directory tree)' \\\n src/posts/uncat/unearthed-arcana-music-division.md\n[no output]\n\nFor clarity, I expected that to print\n\n1417101173 Correct slug for Uncategorized.\n1417050416 The Great Dead and Moved Link Cleanup of 2014.\n1416938128 Use category_meta plugin to fix category slugs.\n1416763607 Initial import of content and Pelican skeleton.\n\nThe problem seems to be related to the complexity of the regex, e.g.\n(all examples with -E --invert-grep)\n\n--grep='^(Mechanically convert|Begin restoring)'             # works correctly\n--grep='^(Mechanically convert|Begin restoring|Reorganize)'  # prints nothing\n--grep='^(Mechanically convert|Begin restoring|Correct slug)'# prints incorrect subset\n\nIncidentally, if there is a better way to query the first and last\ncommit (with filtering) that touched a particular file — or even\na way to do the query for many files at once — please tell me about\nit.\n\nThanks for your attention,\nzw\n\n[System Info]\ngit version:\ngit version 2.37.2\ncpu: x86_64\nno commit associated with this build\nsizeof-long: 8\nsizeof-size_t: 8\nshell-path: /bin/sh\nuname: Linux 6.0.0-4-amd64 #1 SMP PREEMPT_DYNAMIC Debian 6.0.8-1 (2022-11-11) x86_64\ncompiler info: gnuc: 12.1\nlibc info: glibc: 2.36\n$SHELL (typically, interactive shell): /bin/bash\n\n\n[Enabled Hooks]\n"},{"id":"467922","messageId":"909b0770-2ac2-1a87-498b-0537a94e94ac@dunelm.org.uk","threadId":"58847","inReplyTo":"71fcf215-101e-4398-bffa-4ed7e286d9c8@app.fastmail.com","subject":"Re: [bug] git log --invert-grep --grep=[sufficiently complicated regex] prints nothing","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2022-11-24T10:31:46Z","receivedAt":"2022-11-24T10:31:56Z","isPatch":false,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Zack\n\nOn 23/11/2022 20:17, Zack Weinberg wrote:\n> I’m attempting to have my blog, which is a static site generated from\n> a bunch of Markdown files stored in git, automatically pull the\n> the most recent modification date for each page out of the git history.\n> The idea is to use ‘git log --follow --pretty=tformat:'%ct' <file>‘ on\n> each file and then use the oldest reported timestamp as the creation\n> date and the newest reported timestamp as the last modification date.\n> \n> But there’s a catch: there are commits I want to ignore in this\n> calculation, such as mechanical changes applied to the entire site.\n> And this brings me to the bug: --invert-grep doesn’t work correctly\n> when the --grep regex is sufficiently complicated.\n> \n> Here's the complete set of commits that modified an example file:\n> \n> $ git log --follow --pretty=tformat:'%ct %s' \\\n>    src/posts/uncat/unearthed-arcana-music-division.md\n> 1668545053 Begin restoring the site structure.\n> 1668545051 Reorganize directory tree prior to setting up Metalsmith\n> 1668545032 Mechanically convert Pandoc to standard YAML metadata delimiters.\n> 1610735533 Mechanically convert to properly delimited YAML metadata.\n> 1417101173 Correct slug for Uncategorized.\n> 1417050416 The Great Dead and Moved Link Cleanup of 2014.\n> 1416938128 Use category_meta plugin to fix category slugs.\n> 1416763607 Initial import of content and Pelican skeleton.\n> \n> And here's an application of --grep that prints only the commits I _don't_ want:\n> \n> $ git log --follow -E --pretty=tformat:'%ct %s' \\\n>    --grep='^(Mechanically convert|Begin restoring the site structure|Reorganize directory tree)' \\\n>   src/posts/uncat/unearthed-arcana-music-division.md\n> 1668545053 Begin restoring the site structure.\n> 1668545051 Reorganize directory tree prior to setting up Metalsmith\n> 1668545032 Mechanically convert Pandoc to standard YAML metadata delimiters.\n> 1610735533 Mechanically convert to properly delimited YAML metadata.\n> \n> Theoretically, adding --invert-grep to that should make it print the\n> commits I do want, but instead it prints nothing at all:\n\nI think the problem is that you are excluding the commit that renames \nthe file and that stops --follow from following the rename. See below \nfor a simple reproduction using git's test suite. I'm afraid I'm not \nfamiliar with the --follow code so I've no idea how to fix this.\n\nBest Wishes\n\nPhillip\n\n---- >8 ----\n#!/bin/sh\n\ntest_description='Demonstrate `git log --follow --grep` failure'\n\n. ./test-lib.sh\n\ntest_expect_failure '--grep breaks --follow' '\n\ttest_commit first file-a \"$(seq 1 100)\" &&\n\tgit rm file-a &&\n\ttest_commit second file-b \"$(seq 2 101)\" &&\n\ttest_commit third file-b \"$(seq 3 102)\" &&\n\tgit log --follow --grep \"^first\" file-b >actual &&\n\tgit log first >expect &&\n\ttest_cmp expect actual\n'\n\ntest_expect_success '--grep breaks --follow' '\n\tgit log --follow -E --grep \"^(first|second)\" file-b >actual &&\n\tgit log second >expect &&\n\ttest_cmp expect actual\n'\n\ntest_done\n"},{"id":"467935","messageId":"cfbd647e-0e66-4417-8c94-c8cac905a78a@app.fastmail.com","threadId":"58847","inReplyTo":"909b0770-2ac2-1a87-498b-0537a94e94ac@dunelm.org.uk","subject":"Re: [bug] git log --invert-grep --grep=[sufficiently complicated regex] prints nothing","fromName":"Zack Weinberg","fromEmail":"zack@owlfolio.org","sentAt":"2022-11-24T13:36:55Z","receivedAt":"2022-11-24T13:37:21Z","isPatch":false,"sender":{"key":"zack@owlfolio.org","avatar":null},"body":"On Thu, Nov 24, 2022, at 5:31 AM, Phillip Wood wrote:\n> I think the problem is that you are excluding the commit that renames \n> the file and that stops --follow from following the rename. See below \n> for a simple reproduction using git's test suite.\n\nYes, I can confirm.  Both of the \"incorrect output\" examples I gave\ninvolved excluding a commit that renames the file.  If I don't do that\nthere is no problem with an `a|b|c` regex.\n\n> I'm afraid I'm not \n> familiar with the --follow code so I've no idea how to fix this.\n\nI'm honestly unsure what the right behavior _should_ be, now.  I\nexpected --grep to be applied very late in the process, after the\nset of commits touching the specified file had already been computed\n(including all of its previous names, because of --follow) but the\ndocumentation is ambiguous.\n\nzw\n"},{"id":"467941","messageId":"221124.861qpspckm.gmgdl@evledraar.gmail.com","threadId":"58847","inReplyTo":"cfbd647e-0e66-4417-8c94-c8cac905a78a@app.fastmail.com","subject":"Re: [bug] git log --invert-grep --grep=[sufficiently complicated regex] prints nothing","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-24T15:53:28Z","receivedAt":"2022-11-24T16:02:46Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Nov 24 2022, Zack Weinberg wrote:\n\n> On Thu, Nov 24, 2022, at 5:31 AM, Phillip Wood wrote:\n> [...]\n>> I'm afraid I'm not \n>> familiar with the --follow code so I've no idea how to fix this.\n>\n> I'm honestly unsure what the right behavior _should_ be, now.  I\n> expected --grep to be applied very late in the process, after the\n> set of commits touching the specified file had already been computed\n> (including all of its previous names, because of --follow) but the\n> documentation is ambiguous.\n\nThis doesn't help with your case, but I remember there being some\nsimilar confusions and/or expectations mismatches reported in the\npatch. E.g. \"--since\" here:\nhttps://lore.kernel.org/git/220401.86pmm1nmvh.gmgdl@evledraar.gmail.com/\n\nI couldn't find a reference quickly, but I seem to recall (but perhaps\nI'm imagining it) that we had a report/discussion semi-recently about:\n\n\tgit log --reverse --follow -- path\n\nWhich has a similar edge case, e.g. try on git.git:\n\n\tgit log --reverse --follow -- object-name.c\n\nThat's also \"correct\" if you look at it from the POV of how we implement\nit, i.e. we'll traverse, and the rename to object-name.c is the first\ntime we encounter the name from the POV of the walking logic.\n\nHrm, but shouldn't we show all commits *after* the rename then? Anyway,\nI haven't thought about it this time around, just wanted to provide some\nrabbit-hole references in case you're interested.\n\nIn terms of optimization it's *very* useful that we take these\nshortcuts, but as your (and some of these) examples show it can yield\nthe wrong or unexpected result in some cases, and in those cases we\nusually have no non-brute-force way of getting the \"right\" (or\n\"desired\") result other (brute force being: parse \"git log -p\"\nyourself).\n\nSo it would be nice in general if we had some ability to say what\nfilters apply at what stage in the walk, but I suspect that would\nrequire a rather large UX overhaul...\n"},{"id":"467945","messageId":"Y3+dRELMa8AXZ0tZ@coredump.intra.peff.net","threadId":"58847","inReplyTo":"221124.861qpspckm.gmgdl@evledraar.gmail.com","subject":"Re: [bug] git log --invert-grep --grep=[sufficiently complicated regex] prints nothing","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-11-24T16:35:16Z","receivedAt":"2022-11-24T16:35:21Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Nov 24, 2022 at 04:53:28PM +0100, Ævar Arnfjörð Bjarmason wrote:\n\n> This doesn't help with your case, but I remember there being some\n> similar confusions and/or expectations mismatches reported in the\n> patch. E.g. \"--since\" here:\n> https://lore.kernel.org/git/220401.86pmm1nmvh.gmgdl@evledraar.gmail.com/\n> \n> I couldn't find a reference quickly, but I seem to recall (but perhaps\n> I'm imagining it) that we had a report/discussion semi-recently about:\n> \n> \tgit log --reverse --follow -- path\n> \n> Which has a similar edge case, e.g. try on git.git:\n> \n> \tgit log --reverse --follow -- object-name.c\n> \n> That's also \"correct\" if you look at it from the POV of how we implement\n> it, i.e. we'll traverse, and the rename to object-name.c is the first\n> time we encounter the name from the POV of the walking logic.\n\nI think all of this goes back to Linus's original \"--follow is a hack I\nbolted on\" implementation. It probably should happen up-front as part of\nthe history simplification. I think it's a combination of nobody wanting\nto do the work to extract that, and that it may produce less \"streaming\"\noutput, as we have to do a lot more work before producing the first line\nof output.\n\n-Peff\n"}]}