{"thread":{"id":"57983","subject":"git filter bug","startedAt":"2022-06-10T22:19:38Z","lastAt":"2022-06-23T12:13:54Z","messageCount":6,"participants":["Udoff, Marc","Johannes Sixt","Junio C Hamano","Shupak, Vitaly"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"457048","messageId":"101027c97a9b40ce97192b1cee203b07@deshaw.com","threadId":"57983","inReplyTo":null,"subject":"git filter bug","fromName":"Udoff, Marc","fromEmail":"marc.udoff@deshaw.com","sentAt":"2022-06-10T22:19:31Z","receivedAt":"2022-06-10T22:19:38Z","isPatch":false,"sender":{"key":"marc.udoff@deshaw.com","avatar":null},"body":"Hi,\n\nI believe there is a bug in git status that happens when a file changes but the filtered version of the file does not. Correctly, git diff does not show anything as different and git commit believes there is nothing to commit.\n\nReproducer:\n$ git init\n$ touch bar\n$ git add bar\n$ git commit -am 'Bar'\n[main (root-commit) dd12b3e] Bar\n1 file changed, 0 insertions(+), 0 deletions(-)\ncreate mode 100644 bar\n$ echo -en '\\n[filter \"noat\"]\\n     clean = grep -v \"@\"\\n' >> .git/config\n$ cat .git/config \n[core]\n        repositoryformatversion = 0\n        filemode = true\n        bare = false\n        logallrefupdates = true\n\n[filter \"noat\"]\n     clean = grep -v \"@\"\n$ echo -en 'abc\\n@def\\nghi\\n' > bar \n$ cat bar \nabc\n@def\nghi\n$ echo \"* filter=noat\" > .gitattributes\n$ git commit -am 'No at bar'\n[main e81ee3b] No at bar\n2 files changed, 3 insertions(+)\ncreate mode 100644 .gitattributes\n$ git show HEAD:bar\nabc\nghi\n$ echo \"@another line\" >> bar # Add another @ which will be filtered. touch doesn't cause this bug\n$ git status --porcelain\nM bar\n$ git diff # no output as there is no diff\n$ git commit -am \"I did not update\"\nOn branch main\nnothing to commit, working tree clean\n\n\nWhile this reproducer is a bit contrived, the real world examples are with Jupyter notebooks filtering output, so I expect this is a somewhat common occurrence. I think it also may be the same as https://stackoverflow.com/questions/62641222/how-to-make-git-status-consider-the-clean-filter.\n\nGit version: 2.35.1\nOS: Linux\n\n\nNote: I also logged this here, but I believe this mailing list is the correct place to raise the issue: https://github.com/gitgitgadget/git/issues/1256\n\nThanks,\nMarc\n\n"},{"id":"457060","messageId":"442e3166-4f18-3ee0-e3bc-d24687471d5c@kdbg.org","threadId":"57983","inReplyTo":"101027c97a9b40ce97192b1cee203b07@deshaw.com","subject":"Re: git filter bug","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2022-06-11T07:43:12Z","receivedAt":"2022-06-11T08:21:43Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 11.06.22 um 00:19 schrieb Udoff, Marc:\n> Hi,\n> \n> I believe there is a bug in git status that happens when a file\n> changes but the filtered version of the file does not. Correctly,\n> git diff does not show anything as different and git commit believes\n> there is nothing to commit.\n> \n> Reproducer:\n> $ git init\n> $ touch bar\n> $ git add bar\n> $ git commit -am 'Bar'\n> [main (root-commit) dd12b3e] Bar\n> 1 file changed, 0 insertions(+), 0 deletions(-)\n> create mode 100644 bar\n> $ echo -en '\\n[filter \"noat\"]\\n     clean = grep -v \"@\"\\n' >> .git/config\n> $ cat .git/config \n> [core]\n>         repositoryformatversion = 0\n>         filemode = true\n>         bare = false\n>         logallrefupdates = true\n> \n> [filter \"noat\"]\n>      clean = grep -v \"@\"\n> $ echo -en 'abc\\n@def\\nghi\\n' > bar \n> $ cat bar \n> abc\n> @def\n> ghi\n> $ echo \"* filter=noat\" > .gitattributes\n> $ git commit -am 'No at bar'\n> [main e81ee3b] No at bar\n> 2 files changed, 3 insertions(+)\n> create mode 100644 .gitattributes\n> $ git show HEAD:bar\n> abc\n> ghi\n> $ echo \"@another line\" >> bar # Add another @ which will be filtered. touch doesn't cause this bug\n> $ git status --porcelain\n> M bar\n\ngit status does not compute differences; it only looks at the stat\ninformation, and that is by design for performance reasons. So, IMO,\nthis is working as designed and not a bug.\n\n-- Hannes\n"},{"id":"457109","messageId":"xmqqsfo879r7.fsf@gitster.g","threadId":"57983","inReplyTo":"442e3166-4f18-3ee0-e3bc-d24687471d5c@kdbg.org","subject":"Re: git filter bug","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-06-13T17:29:00Z","receivedAt":"2022-06-13T19:17:48Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt <j6t@kdbg.org> writes:\n\n> git status does not compute differences; it only looks at the stat\n> information, and that is by design for performance reasons. So, IMO,\n> this is working as designed and not a bug.\n\nHmph, is that true?  I thought \"git status\" did an equivalent of\ndiff.autoRefreshIndex just like other commands like \"git diff\" at\nthe Porcelain level.\n\nIs this more like the commonly seen \"after you futzed the attributes\nto affect normalization, \"--renormalize\" is needed to force the\nindex to match the cleaned version of working tree under the new\nclean filter rules\", I wonder?\n\n"},{"id":"457124","messageId":"c2f49b4f-8588-bae1-97cf-91a36b3f16f9@kdbg.org","threadId":"57983","inReplyTo":"xmqqsfo879r7.fsf@gitster.g","subject":"Re: git filter bug","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2022-06-13T21:15:12Z","receivedAt":"2022-06-13T21:26:16Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 13.06.22 um 19:29 schrieb Junio C Hamano:\n> Johannes Sixt <j6t@kdbg.org> writes:\n> \n>> git status does not compute differences; it only looks at the stat\n>> information, and that is by design for performance reasons. So, IMO,\n>> this is working as designed and not a bug.\n> \n> Hmph, is that true?  I thought \"git status\" did an equivalent of\n> diff.autoRefreshIndex just like other commands like \"git diff\" at\n> the Porcelain level.\n\nIs it true? I don't know; you tell me ;) git status certainly does\nautoRefreshIndex, but is that based on a diff computation? I thought git\nstatus looks only at stat information.\n\n> Is this more like the commonly seen \"after you futzed the attributes\n> to affect normalization, \"--renormalize\" is needed to force the\n> index to match the cleaned version of working tree under the new\n> clean filter rules\", I wonder?\n\nNot in this case. The modified file that git status reports happens long\nafter git commit -a has already applied the new filter.\n\n-- Hannes\n"},{"id":"457208","messageId":"d8ae6210ddf146d7bbd9c78d170fb803@deshaw.com","threadId":"57983","inReplyTo":"c2f49b4f-8588-bae1-97cf-91a36b3f16f9@kdbg.org","subject":"RE: git filter bug","fromName":"Shupak, Vitaly","fromEmail":"vitaly.shupak@deshaw.com","sentAt":"2022-06-14T19:11:56Z","receivedAt":"2022-06-14T19:22:06Z","isPatch":false,"sender":{"key":"vitaly.shupak@deshaw.com","avatar":null},"body":"Here's the behavior that I observe:\n- If the mtime of the normal file changes from what's in the index but the content doesn't change, \"git status\" updates the index with the latest timestamp of the file.\n- If the filtered file changes, but the size stays the same, git status also triggers an index update.\n- BUT if the size of the filtered file changes, then the index does NOT get updated and the file appears modified on every git status run until you explicitly run \"git add <filename>\" again. This is true even if the post-clean filter content is the same as what's currently in the index.\n- The clean filter runs on every \"git status\" call anyway, so this behavior does not appear to be an optimization.\n\nSo if a file is modified such that the post-clean filter content is the same as what's in the index, \"git status\" will show the file as modified only if the file size has also changed. It seems that perhaps \"git status\" is comparing file sizes before applying the clean filter to see if the index entry needs to be refreshed? \n\nVitaly\n\n-----Original Message-----\nFrom: Johannes Sixt <j6t@kdbg.org> \nSent: Monday, June 13, 2022 5:15 PM\nTo: Junio C Hamano <gitster@pobox.com>\nCc: Udoff, Marc <Marc.Udoff@deshaw.com>; Shupak, Vitaly <Vitaly.Shupak@deshaw.com>; git@vger.kernel.org\nSubject: Re: git filter bug\n\nThis message was sent by an external party.\n\n\nAm 13.06.22 um 19:29 schrieb Junio C Hamano:\n> Johannes Sixt <j6t@kdbg.org> writes:\n>\n>> git status does not compute differences; it only looks at the stat \n>> information, and that is by design for performance reasons. So, IMO, \n>> this is working as designed and not a bug.\n>\n> Hmph, is that true?  I thought \"git status\" did an equivalent of \n> diff.autoRefreshIndex just like other commands like \"git diff\" at the \n> Porcelain level.\n\nIs it true? I don't know; you tell me ;) git status certainly does autoRefreshIndex, but is that based on a diff computation? I thought git status looks only at stat information.\n\n> Is this more like the commonly seen \"after you futzed the attributes \n> to affect normalization, \"--renormalize\" is needed to force the index \n> to match the cleaned version of working tree under the new clean \n> filter rules\", I wonder?\n\nNot in this case. The modified file that git status reports happens long after git commit -a has already applied the new filter.\n\n-- Hannes\n"},{"id":"457782","messageId":"1d01a877d86f4e0583e1bc617349b48a@deshaw.com","threadId":"57983","inReplyTo":"d8ae6210ddf146d7bbd9c78d170fb803@deshaw.com","subject":"RE: git filter bug","fromName":"Udoff, Marc","fromEmail":"marc.udoff@deshaw.com","sentAt":"2022-06-23T12:13:41Z","receivedAt":"2022-06-23T12:13:54Z","isPatch":false,"sender":{"key":"marc.udoff@deshaw.com","avatar":null},"body":"Based on the conversation below, I'm not quite sure if there is consensus this is a bug and not a feature. If this is a bug, what are the next steps to getting it fixed? If this is something that someone with minimal codebase experience can fix, we'd be happy to submit a PR if you could give us a few pointers.\n\nThanks!\n\n-----Original Message-----\nFrom: Shupak, Vitaly <Vitaly.Shupak@deshaw.com> \nSent: Tuesday, June 14, 2022 3:12 PM\nTo: Johannes Sixt <j6t@kdbg.org>; Junio C Hamano <gitster@pobox.com>\nCc: Udoff, Marc <Marc.Udoff@deshaw.com>; git@vger.kernel.org\nSubject: RE: git filter bug\n\nHere's the behavior that I observe:\n- If the mtime of the normal file changes from what's in the index but the content doesn't change, \"git status\" updates the index with the latest timestamp of the file.\n- If the filtered file changes, but the size stays the same, git status also triggers an index update.\n- BUT if the size of the filtered file changes, then the index does NOT get updated and the file appears modified on every git status run until you explicitly run \"git add <filename>\" again. This is true even if the post-clean filter content is the same as what's currently in the index.\n- The clean filter runs on every \"git status\" call anyway, so this behavior does not appear to be an optimization.\n\nSo if a file is modified such that the post-clean filter content is the same as what's in the index, \"git status\" will show the file as modified only if the file size has also changed. It seems that perhaps \"git status\" is comparing file sizes before applying the clean filter to see if the index entry needs to be refreshed? \n\nVitaly\n\n-----Original Message-----\nFrom: Johannes Sixt <j6t@kdbg.org>\nSent: Monday, June 13, 2022 5:15 PM\nTo: Junio C Hamano <gitster@pobox.com>\nCc: Udoff, Marc <Marc.Udoff@deshaw.com>; Shupak, Vitaly <Vitaly.Shupak@deshaw.com>; git@vger.kernel.org\nSubject: Re: git filter bug\n\nThis message was sent by an external party.\n\n\nAm 13.06.22 um 19:29 schrieb Junio C Hamano:\n> Johannes Sixt <j6t@kdbg.org> writes:\n>\n>> git status does not compute differences; it only looks at the stat \n>> information, and that is by design for performance reasons. So, IMO, \n>> this is working as designed and not a bug.\n>\n> Hmph, is that true?  I thought \"git status\" did an equivalent of \n> diff.autoRefreshIndex just like other commands like \"git diff\" at the \n> Porcelain level.\n\nIs it true? I don't know; you tell me ;) git status certainly does autoRefreshIndex, but is that based on a diff computation? I thought git status looks only at stat information.\n\n> Is this more like the commonly seen \"after you futzed the attributes \n> to affect normalization, \"--renormalize\" is needed to force the index \n> to match the cleaned version of working tree under the new clean \n> filter rules\", I wonder?\n\nNot in this case. The modified file that git status reports happens long after git commit -a has already applied the new filter.\n\n-- Hannes\n"}]}