{"thread":{"id":"48093","subject":".gitattributes override behavior (possible bug, or documentation bug)","startedAt":"2018-03-20T01:49:36Z","lastAt":"2018-03-23T09:12:29Z","messageCount":28,"participants":["Dakota Hawkins","Jeff King","Junio C Hamano","Duy Nguyen"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"342272","messageId":"CAHnyXxRX4+sMJCNG6f9xtsDO6bdqRS-U6TAYO47OKQjH8bGzbg@mail.gmail.com","threadId":"48093","inReplyTo":null,"subject":".gitattributes override behavior (possible bug, or documentation bug)","fromName":"Dakota Hawkins","fromEmail":"dakota@dakotahawkins.com","sentAt":"2018-03-20T01:49:28Z","receivedAt":"2018-03-20T01:49:36Z","isPatch":false,"sender":{"key":"dakota@dakotahawkins.com","avatar":null},"body":"According to the gitattributes docs:\n\n\"When more than one pattern matches the path, a later line overrides\nan earlier line. This overriding is done per attribute.\"\n\nI had a need to do this recently, and while the attributes are git-lfs\nspecific, I think the issue may be generic.\n\nSummary: Trying to apply attributes to file extensions everywhere\nexcept in one directory.\n\n.gitattributes:\n\n *.[Pp][Nn][Gg] filter=lfs diff=lfs merge=lfs -text\n /.readme-docs/ -filter=lfs -diff=lfs -merge=lfs\n\nMake some data:\n\n echo \"asldkjfa;sldkjf;alsdjf\" > ./.readme-docs/test.png\n git add -A\n\nResult:\n\n \"git check-attr -a --cached -- ./readme-docs/test.png\" still shows\n\"filter=lfs diff=lfs merge=lfs\" attributes.\n\nFull details: https://github.com/git-lfs/git-lfs/issues/2120#issuecomment-374432354\n\nIs this me misunderstanding something in the documentation? I would\nexpect \"./.readme-docs/\" to match \"./.readme-docs/test.png\" and\noverride the earlier \"*.[Pp][Nn][Gg]\" attributes.\n\nI have found the following overrides to work in lieu of the directory match:\n\n /.readme-docs/* -filter=lfs -diff=lfs -merge=lfs\n /.readme-docs/**/* -filter=lfs -diff=lfs -merge=lfs\n\n...but I don't see a justification in the documentation for this\nworking and the original directory filter not working.\n\nThanks for any clarification or help you can provide,\n\n- Dakota\n"},{"id":"342273","messageId":"20180320023423.GA10143@sigill.intra.peff.net","threadId":"48093","inReplyTo":"CAHnyXxRX4+sMJCNG6f9xtsDO6bdqRS-U6TAYO47OKQjH8bGzbg@mail.gmail.com","subject":"Re: .gitattributes override behavior (possible bug, or documentation bug)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-03-20T02:34:24Z","receivedAt":"2018-03-20T02:34:33Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 19, 2018 at 09:49:28PM -0400, Dakota Hawkins wrote:\n\n> Summary: Trying to apply attributes to file extensions everywhere\n> except in one directory.\n> \n> .gitattributes:\n> \n> *.[Pp][Nn][Gg] filter=lfs diff=lfs merge=lfs -text\n> /.readme-docs/ -filter=lfs -diff=lfs -merge=lfs\n> \n> Make some data:\n> \n> echo \"asldkjfa;sldkjf;alsdjf\" > ./.readme-docs/test.png\n> git add -A\n\nAs you noted below, that second line does not match your path, because\nattributes on a directory aren't applied recursively. And it has nothing\nto do with overriding. If you remove the png line entirely, you can see\nthat we still do not match it. You need to use \"*\" to match the paths.\n\nYou may also find that \"-diff=lfs\" does not do quite what you want.\nThere is no way to say \"cancel any previous attribute\", which I think is\nwhat you're trying for here. You can only override it with a new value.\nSo:\n\n /.readme-docs/* -diff\n\nsays \"do not diff this\". And:\n\n /.readme-docs/* diff\n\nsays \"diff this as text, even if it looks binary\".\n\nThe best you can probably do is:\n\n /.readme-docs/* diff=foo\n\nSince you have no diff.foo.* config, that will behave in the default way\n(including respecting the usual \"is it binary\" checks). So a bit hacky,\nbut I think it would work as \"ignore prior diff\".\n\nAnd I think filter and merge drivers should work the same.\n\n> Is this me misunderstanding something in the documentation? I would\n> expect \"./.readme-docs/\" to match \"./.readme-docs/test.png\" and\n> override the earlier \"*.[Pp][Nn][Gg]\" attributes.\n>\n> I have found the following overrides to work in lieu of the directory match:\n> \n> /.readme-docs/* -filter=lfs -diff=lfs -merge=lfs\n> /.readme-docs/**/* -filter=lfs -diff=lfs -merge=lfs\n> \n> ...but I don't see a justification in the documentation for this\n> working and the original directory filter not working.\n\nI could not find anything useful in gitattributes(5). There's some old\ndiscussion here:\n\n https://public-inbox.org/git/slrnkldd3g.1l4.jan@majutsushi.net/\n\nwhich makes it clear that attributes aren't recursive, but it's probably\nworth calling out in the documentation. In fact, I think the current\ndocumentation is a bit misleading in that it says \"patterns are matched\nas in .gitignore\", which is clearly not the case here.\n\nI think just \"/.readme-docs/**\" should be sufficient for your case. You\ncould also probably write \"*\" inside \".readme-docs/.gitattributes\",\nwhich may be simpler (you don't need \"**\" there because patterns without\na slash are just matched directly against the basename).\n\n-Peff\n"},{"id":"342274","messageId":"CAHnyXxRcwq40W4tKm=Kscrsnb77yh7=eGDE=r5AZq073MPX9AQ@mail.gmail.com","threadId":"48093","inReplyTo":"20180320023423.GA10143@sigill.intra.peff.net","subject":"Re: .gitattributes override behavior (possible bug, or documentation bug)","fromName":"Dakota Hawkins","fromEmail":"dakota@dakotahawkins.com","sentAt":"2018-03-20T03:10:26Z","receivedAt":"2018-03-20T03:10:41Z","isPatch":false,"sender":{"key":"dakota@dakotahawkins.com","avatar":null},"body":"Thanks for the quick reply!\n\nOn Mon, Mar 19, 2018 at 10:34 PM, Jeff King wrote:\n> On Mon, Mar 19, 2018 at 09:49:28PM -0400, Dakota Hawkins wrote:\n>\n>> Summary: Trying to apply attributes to file extensions everywhere\n>> except in one directory.\n>>\n>> .gitattributes:\n>>\n>> *.[Pp][Nn][Gg] filter=lfs diff=lfs merge=lfs -text\n>> /.readme-docs/ -filter=lfs -diff=lfs -merge=lfs\n>>\n>> Make some data:\n>>\n>> echo \"asldkjfa;sldkjf;alsdjf\" > ./.readme-docs/test.png\n>> git add -A\n>\n> As you noted below, that second line does not match your path, because\n> attributes on a directory aren't applied recursively. And it has nothing\n> to do with overriding. If you remove the png line entirely, you can see\n> that we still do not match it. You need to use \"*\" to match the paths.\n\nAh, yes, I see that. Inconsistent with .gitignore (more below), right?\n\n> You may also find that \"-diff=lfs\" does not do quite what you want.\n> There is no way to say \"cancel any previous attribute\", which I think is\n> what you're trying for here. You can only override it with a new value.\n> So:\n>\n> /.readme-docs/* -diff\n>\n> says \"do not diff this\". And:\n>\n> /.readme-docs/* diff\n>\n> says \"diff this as text, even if it looks binary\".\n>\n> The best you can probably do is:\n>\n> /.readme-docs/* diff=foo\n>\n> Since you have no diff.foo.* config, that will behave in the default way\n> (including respecting the usual \"is it binary\" checks). So a bit hacky,\n> but I think it would work as \"ignore prior diff\".\n>\n> And I think filter and merge drivers should work the same.\n\nThat's interesting... in this case I was taking my advice on how this\nshould work from the git-lfs folks. I have promised to share what I\nfind here with them, so that will help at least :)\n\nI think that makes sense to me -- there would be no good way to tell\nit what the default should have been without explicitly telling it\nwhat to use instead.\n\n>> Is this me misunderstanding something in the documentation? I would\n>> expect \"./.readme-docs/\" to match \"./.readme-docs/test.png\" and\n>> override the earlier \"*.[Pp][Nn][Gg]\" attributes.\n>>\n>> I have found the following overrides to work in lieu of the directory match:\n>>\n>> /.readme-docs/* -filter=lfs -diff=lfs -merge=lfs\n>> /.readme-docs/**/* -filter=lfs -diff=lfs -merge=lfs\n>>\n>> ...but I don't see a justification in the documentation for this\n>> working and the original directory filter not working.\n>\n> I could not find anything useful in gitattributes(5). There's some old\n> discussion here:\n>\n> https://public-inbox.org/git/slrnkldd3g.1l4.jan@majutsushi.net/\n\nIf I follow that correctly: There's some initial speculation that it\nwould be OK to apply the attributes recursively, which is then shot\ndown because it wasn't designed to be recursive (though I don't see a\ndifferent, technical reason for that), followed by finding a (this\nsame?) solution/workaround for the original problem. Is that about\nright?\n\n> which makes it clear that attributes aren't recursive, but it's probably\n> worth calling out in the documentation. In fact, I think the current\n> documentation is a bit misleading in that it says \"patterns are matched\n> as in .gitignore\", which is clearly not the case here.\n\nI was indeed going off of the suggestion to consult the .gitignore\npattern matching documentation.\n\n> I think just \"/.readme-docs/**\" should be sufficient for your case. You\n> could also probably write \"*\" inside \".readme-docs/.gitattributes\",\n> which may be simpler (you don't need \"**\" there because patterns without\n> a slash are just matched directly against the basename).\n\nWouldn't that make the \"*\" inside \".readme-docs/.gitattributes\",\ntechnically recursive when \"*\" matches a directory? It's always seemed\nto me that both were necessary to explicitly match things in a\ndirectory and its subdirectories (example, IIRC: \"git ls-files --\n.gitattributes\" vs \"git ls-files -- .gitattributes\n**/.gitattributes\"). Maybe that example is peculiar in that its a\ndotfile and can't have a wildcard before the dot?\n\nI guess my takeaway is that it would be _good_ if the gitattributes\ndocumentation contained the caveat about not matching directories\nrecursively, but _great_ if gitattributes and gitignore (and whatever\nelse there is) were consistent.\n\nAt any rate, thanks for the great, quick help!\n\n-Dakota\n"},{"id":"342275","messageId":"CAHnyXxR+03P8vG02dSuDCnRG0vsbOEWWsNBsKVsedgpkPNu=fQ@mail.gmail.com","threadId":"48093","inReplyTo":"CAHnyXxRcwq40W4tKm=Kscrsnb77yh7=eGDE=r5AZq073MPX9AQ@mail.gmail.com","subject":"Re: .gitattributes override behavior (possible bug, or documentation bug)","fromName":"Dakota Hawkins","fromEmail":"dakota@dakotahawkins.com","sentAt":"2018-03-20T03:17:04Z","receivedAt":"2018-03-20T03:17:15Z","isPatch":false,"sender":{"key":"dakota@dakotahawkins.com","avatar":null},"body":"Sorry to tack on to my previous email, but I just thought of this:\n\nIf something like \"-diff=lfs\" won't do what I (and git-lfs) thought it\nwould, do you think it would be prudent/reasonable to suggest git-lfs\nadd a \"no-lfs\" filter for exactly this case? That way I could have\nexplicit exclusions without any \"diff=foo\" shenanigans.\n\nThanks again,\n\n- Dakota\n\nOn Mon, Mar 19, 2018 at 11:10 PM, Dakota Hawkins\n wrote:\n> Thanks for the quick reply!\n>\n> On Mon, Mar 19, 2018 at 10:34 PM, Jeff King wrote:\n>> On Mon, Mar 19, 2018 at 09:49:28PM -0400, Dakota Hawkins wrote:\n>>\n>>> Summary: Trying to apply attributes to file extensions everywhere\n>>> except in one directory.\n>>>\n>>> .gitattributes:\n>>>\n>>> *.[Pp][Nn][Gg] filter=lfs diff=lfs merge=lfs -text\n>>> /.readme-docs/ -filter=lfs -diff=lfs -merge=lfs\n>>>\n>>> Make some data:\n>>>\n>>> echo \"asldkjfa;sldkjf;alsdjf\" > ./.readme-docs/test.png\n>>> git add -A\n>>\n>> As you noted below, that second line does not match your path, because\n>> attributes on a directory aren't applied recursively. And it has nothing\n>> to do with overriding. If you remove the png line entirely, you can see\n>> that we still do not match it. You need to use \"*\" to match the paths.\n>\n> Ah, yes, I see that. Inconsistent with .gitignore (more below), right?\n>\n>> You may also find that \"-diff=lfs\" does not do quite what you want.\n>> There is no way to say \"cancel any previous attribute\", which I think is\n>> what you're trying for here. You can only override it with a new value.\n>> So:\n>>\n>> /.readme-docs/* -diff\n>>\n>> says \"do not diff this\". And:\n>>\n>> /.readme-docs/* diff\n>>\n>> says \"diff this as text, even if it looks binary\".\n>>\n>> The best you can probably do is:\n>>\n>> /.readme-docs/* diff=foo\n>>\n>> Since you have no diff.foo.* config, that will behave in the default way\n>> (including respecting the usual \"is it binary\" checks). So a bit hacky,\n>> but I think it would work as \"ignore prior diff\".\n>>\n>> And I think filter and merge drivers should work the same.\n>\n> That's interesting... in this case I was taking my advice on how this\n> should work from the git-lfs folks. I have promised to share what I\n> find here with them, so that will help at least :)\n>\n> I think that makes sense to me -- there would be no good way to tell\n> it what the default should have been without explicitly telling it\n> what to use instead.\n>\n>>> Is this me misunderstanding something in the documentation? I would\n>>> expect \"./.readme-docs/\" to match \"./.readme-docs/test.png\" and\n>>> override the earlier \"*.[Pp][Nn][Gg]\" attributes.\n>>>\n>>> I have found the following overrides to work in lieu of the directory match:\n>>>\n>>> /.readme-docs/* -filter=lfs -diff=lfs -merge=lfs\n>>> /.readme-docs/**/* -filter=lfs -diff=lfs -merge=lfs\n>>>\n>>> ...but I don't see a justification in the documentation for this\n>>> working and the original directory filter not working.\n>>\n>> I could not find anything useful in gitattributes(5). There's some old\n>> discussion here:\n>>\n>> https://public-inbox.org/git/slrnkldd3g.1l4.jan@majutsushi.net/\n>\n> If I follow that correctly: There's some initial speculation that it\n> would be OK to apply the attributes recursively, which is then shot\n> down because it wasn't designed to be recursive (though I don't see a\n> different, technical reason for that), followed by finding a (this\n> same?) solution/workaround for the original problem. Is that about\n> right?\n>\n>> which makes it clear that attributes aren't recursive, but it's probably\n>> worth calling out in the documentation. In fact, I think the current\n>> documentation is a bit misleading in that it says \"patterns are matched\n>> as in .gitignore\", which is clearly not the case here.\n>\n> I was indeed going off of the suggestion to consult the .gitignore\n> pattern matching documentation.\n>\n>> I think just \"/.readme-docs/**\" should be sufficient for your case. You\n>> could also probably write \"*\" inside \".readme-docs/.gitattributes\",\n>> which may be simpler (you don't need \"**\" there because patterns without\n>> a slash are just matched directly against the basename).\n>\n> Wouldn't that make the \"*\" inside \".readme-docs/.gitattributes\",\n> technically recursive when \"*\" matches a directory? It's always seemed\n> to me that both were necessary to explicitly match things in a\n> directory and its subdirectories (example, IIRC: \"git ls-files --\n> .gitattributes\" vs \"git ls-files -- .gitattributes\n> **/.gitattributes\"). Maybe that example is peculiar in that its a\n> dotfile and can't have a wildcard before the dot?\n>\n> I guess my takeaway is that it would be _good_ if the gitattributes\n> documentation contained the caveat about not matching directories\n> recursively, but _great_ if gitattributes and gitignore (and whatever\n> else there is) were consistent.\n>\n> At any rate, thanks for the great, quick help!\n>\n> -Dakota\n"},{"id":"342276","messageId":"xmqq8tanto90.fsf@gitster-ct.c.googlers.com","threadId":"48093","inReplyTo":"20180320023423.GA10143@sigill.intra.peff.net","subject":"Re: .gitattributes override behavior (possible bug, or documentation bug)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-20T03:33:15Z","receivedAt":"2018-03-20T03:33:23Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> The best you can probably do is:\n>\n> /.readme-docs/* diff=foo\n>\n> Since you have no diff.foo.* config, that will behave in the default way\n> (including respecting the usual \"is it binary\" checks). So a bit hacky,\n> but I think it would work as \"ignore prior diff\".\n\nYou can say\n\n\t/.readme-docs/* !diff\n\nI think. Thre is a difference between setting it to \"false\"\n(i.e. Unset) and reverting it to unspecified state.\n\nIsn't that what you want here?\n"},{"id":"342277","messageId":"CAHnyXxRjqKKyp0U7ryxUk4BMHGdqN1pMYC66BDG+8UYPaF7a3w@mail.gmail.com","threadId":"48093","inReplyTo":"xmqq8tanto90.fsf@gitster-ct.c.googlers.com","subject":"Re: .gitattributes override behavior (possible bug, or documentation bug)","fromName":"Dakota Hawkins","fromEmail":"dakota@dakotahawkins.com","sentAt":"2018-03-20T03:40:24Z","receivedAt":"2018-03-20T03:40:31Z","isPatch":false,"sender":{"key":"dakota@dakotahawkins.com","avatar":null},"body":"On Mon, Mar 19, 2018 at 11:33 PM, Junio C Hamano wrote:\n> Jeff King writes:\n>\n>> The best you can probably do is:\n>>\n>> /.readme-docs/* diff=foo\n>>\n>> Since you have no diff.foo.* config, that will behave in the default way\n>> (including respecting the usual \"is it binary\" checks). So a bit hacky,\n>> but I think it would work as \"ignore prior diff\".\n>\n> You can say\n>\n> /.readme-docs/* !diff\n>\n> I think. Thre is a difference between setting it to \"false\"\n> (i.e. Unset) and reverting it to unspecified state.\n>\n> Isn't that what you want here?\n\nIn this case, I think so? In this context I don't necessarily know the\nfiles in /.readme-docs/ are binary (though that's its intent).\nIdeally, I just want it to do whatever it did before the match gave it\n\"diff=lfs\". I realize that that's not possible/feasible, so I asked in\na separate reply whether it might be a good idea to ask git-lfs for a\n\"no-lfs\" filter for exactly this situation.\n\nThe actual \"lfs\" section of my .gitattributes file is about 65 lines\nfor 65 different extensions, so I'd prefer to handle the exclusion of\na directory without having to repeat that whole block with different\noptions, if that makes sense :)\n"},{"id":"342278","messageId":"20180320034545.GA12938@sigill.intra.peff.net","threadId":"48093","inReplyTo":"xmqq8tanto90.fsf@gitster-ct.c.googlers.com","subject":"Re: .gitattributes override behavior (possible bug, or documentation bug)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-03-20T03:45:46Z","receivedAt":"2018-03-20T03:45:52Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 19, 2018 at 08:33:15PM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > The best you can probably do is:\n> >\n> > /.readme-docs/* diff=foo\n> >\n> > Since you have no diff.foo.* config, that will behave in the default way\n> > (including respecting the usual \"is it binary\" checks). So a bit hacky,\n> > but I think it would work as \"ignore prior diff\".\n> \n> You can say\n> \n> \t/.readme-docs/* !diff\n> \n> I think. Thre is a difference between setting it to \"false\"\n> (i.e. Unset) and reverting it to unspecified state.\n> \n> Isn't that what you want here?\n\nAh, yes, I totally forgot that existed. That's much better than the\nhackery I proposed.\n\n-Peff\n"},{"id":"342279","messageId":"20180320040411.GB12938@sigill.intra.peff.net","threadId":"48093","inReplyTo":"CAHnyXxRcwq40W4tKm=Kscrsnb77yh7=eGDE=r5AZq073MPX9AQ@mail.gmail.com","subject":"Re: .gitattributes override behavior (possible bug, or documentation bug)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-03-20T04:04:11Z","receivedAt":"2018-03-20T04:04:18Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 19, 2018 at 11:10:26PM -0400, Dakota Hawkins wrote:\n\n> > As you noted below, that second line does not match your path, because\n> > attributes on a directory aren't applied recursively. And it has nothing\n> > to do with overriding. If you remove the png line entirely, you can see\n> > that we still do not match it. You need to use \"*\" to match the paths.\n> \n> Ah, yes, I see that. Inconsistent with .gitignore (more below), right?\n\nYes, it is.\n\n> > I could not find anything useful in gitattributes(5). There's some old\n> > discussion here:\n> >\n> > https://public-inbox.org/git/slrnkldd3g.1l4.jan@majutsushi.net/\n> \n> If I follow that correctly: There's some initial speculation that it\n> would be OK to apply the attributes recursively, which is then shot\n> down because it wasn't designed to be recursive (though I don't see a\n> different, technical reason for that), followed by finding a (this\n> same?) solution/workaround for the original problem. Is that about\n> right?\n\nRight. The technical reason is mostly \"that is not how it was designed,\nand it would possibly break some corner cases if we switched it now\".\n\n> > I think just \"/.readme-docs/**\" should be sufficient for your case. You\n> > could also probably write \"*\" inside \".readme-docs/.gitattributes\",\n> > which may be simpler (you don't need \"**\" there because patterns without\n> > a slash are just matched directly against the basename).\n> \n> Wouldn't that make the \"*\" inside \".readme-docs/.gitattributes\",\n> technically recursive when \"*\" matches a directory?\n\nSort of. The pattern is applied recursively to each basename, but the\npattern itself does not match recursively. That's probably splitting\nhairs, though. :)\n\n> It's always seemed to me that both were necessary to explicitly match\n> things in a directory and its subdirectories (example, IIRC: \"git\n> ls-files -- .gitattributes\" vs \"git ls-files -- .gitattributes\n> **/.gitattributes\"). Maybe that example is peculiar in that its a\n> dotfile and can't have a wildcard before the dot?\n\nThose are pathspecs, which are not quite the same as gitattributes. They\ndon't do the magic basename matching.\n\nFor pathspecs a \"*\" matches across slashes, which is what allows \"git\nlog -- '*.h'\" to work (but not a suffix wildcard like 'foo*').\n\n> I guess my takeaway is that it would be _good_ if the gitattributes\n> documentation contained the caveat about not matching directories\n> recursively, but _great_ if gitattributes and gitignore (and whatever\n> else there is) were consistent.\n\nI agree it would be nice if they were consistent (and pathspecs, too).\nBut unfortunately at this point there's a maze of backwards\ncompatibility to deal with.\n\n-Peff\n"},{"id":"342280","messageId":"20180320041205.GC12938@sigill.intra.peff.net","threadId":"48093","inReplyTo":"CAHnyXxR+03P8vG02dSuDCnRG0vsbOEWWsNBsKVsedgpkPNu=fQ@mail.gmail.com","subject":"Re: .gitattributes override behavior (possible bug, or documentation bug)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-03-20T04:12:06Z","receivedAt":"2018-03-20T04:12:12Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 19, 2018 at 11:17:04PM -0400, Dakota Hawkins wrote:\n\n> Sorry to tack on to my previous email, but I just thought of this:\n> \n> If something like \"-diff=lfs\" won't do what I (and git-lfs) thought it\n> would, do you think it would be prudent/reasonable to suggest git-lfs\n> add a \"no-lfs\" filter for exactly this case? That way I could have\n> explicit exclusions without any \"diff=foo\" shenanigans.\n\nIt might be if my earlier email weren't totally wrong. ;) I think the\n\"!filter\" syntax that Junio mentioned would do what you want.\n\n-Peff\n"},{"id":"342281","messageId":"20180320041454.GA15213@sigill.intra.peff.net","threadId":"48093","inReplyTo":"20180320040411.GB12938@sigill.intra.peff.net","subject":"[PATCH] doc/gitattributes: mention non-recursive behavior","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-03-20T04:14:54Z","receivedAt":"2018-03-20T04:15:00Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 20, 2018 at 12:04:11AM -0400, Jeff King wrote:\n\n> > I guess my takeaway is that it would be _good_ if the gitattributes\n> > documentation contained the caveat about not matching directories\n> > recursively, but _great_ if gitattributes and gitignore (and whatever\n> > else there is) were consistent.\n> \n> I agree it would be nice if they were consistent (and pathspecs, too).\n> But unfortunately at this point there's a maze of backwards\n> compatibility to deal with.\n\nSo let's not forget to do the easy half there. Here's a patch.\n\n-- >8 --\nSubject: [PATCH] doc/gitattributes: mention non-recursive behavior\n\nThe gitattributes documentation claims that the pattern\nrules are largely the same as for gitignore. However, the\nrules for recursion are different.\n\nIn an ideal world, we would make them the same (if for\nnothing else than consistency and simplicity), but that\nwould create backwards compatibility issues. For some\ndiscussion, see this thread:\n\n https://public-inbox.org/git/slrnkldd3g.1l4.jan@majutsushi.net/\n\nBut let's at least document the differences instead of\nactively misleading the user by claiming that they're the\nsame.\n\nSigned-off-by: Jeff King \n---\n Documentation/gitattributes.txt | 13 ++++++++++---\n 1 file changed, 10 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt\nindex d52b254a22..1094fe2b5b 100644\n--- a/Documentation/gitattributes.txt\n+++ b/Documentation/gitattributes.txt\n@@ -56,9 +56,16 @@ Unspecified::\n \n When more than one pattern matches the path, a later line\n overrides an earlier line. This overriding is done per\n-attribute. The rules how the pattern matches paths are the\n-same as in `.gitignore` files; see linkgit:gitignore[5].\n-Unlike `.gitignore`, negative patterns are forbidden.\n+attribute.\n+\n+The rules by which the pattern matches paths are the same as in\n+`.gitignore` files (see linkgit:gitignore[5]), with a few exceptions:\n+\n+ - negative patterns are forbidden\n+\n+ - patterns that match a directory do not recursively match paths\n+ inside that directory (so using the trailing-slash `path/` syntax is\n+ pointless in an attributes file; use `path/**` instead)\n \n When deciding what attributes are assigned to a path, Git\n consults `$GIT_DIR/info/attributes` file (which has the highest\n-- \n2.17.0.rc0.402.ged0b3fd1ee\n\n"},{"id":"342283","messageId":"CAHnyXxQMbnNam=oB_B57xDZBaMPRn_8hfKtostHPV6nBHbTBog@mail.gmail.com","threadId":"48093","inReplyTo":"20180320040411.GB12938@sigill.intra.peff.net","subject":"Re: .gitattributes override behavior (possible bug, or documentation bug)","fromName":"Dakota Hawkins","fromEmail":"dakota@dakotahawkins.com","sentAt":"2018-03-20T04:25:27Z","receivedAt":"2018-03-20T04:25:33Z","isPatch":false,"sender":{"key":"dakota@dakotahawkins.com","avatar":null},"body":"> Right. The technical reason is mostly \"that is not how it was designed,\n> and it would possibly break some corner cases if we switched it now\".\n\nI'm just spitballing here, but do you guys think there's any subset of\nthe combined .gitignore and .gitattributes matching functionality that\ncould at least serve as a good \"best-practices, going forward\"\n(because of consistency) for both? I will say every time I do this for\na new repo and have to do something even slightly complicated or\ndifferent from what I've done before with .gitattributes/.gitignore\nthat it takes me a long-ish time to figure it out. It's like I'm\nvaguely aware of pitfalls I've encountered in the past in certain\nareas but don't remember exactly what they are, so I consult the docs,\nwhich are (in sum) confusing and lead to more time spent\ntrying/failing/trying/works/fails-later/etc.\n\nOne \"this subset of rules will work for both this way\" would be\nawesome even if the matching capabilities are technically divergent,\nbut on the other hand that might paint both into a corner in terms of\nfunctionality.\n\n>> > I think just \"/.readme-docs/**\" should be sufficient for your case. You\n>> > could also probably write \"*\" inside \".readme-docs/.gitattributes\",\n>> > which may be simpler (you don't need \"**\" there because patterns without\n>> > a slash are just matched directly against the basename).\n>>\n>> Wouldn't that make the \"*\" inside \".readme-docs/.gitattributes\",\n>> technically recursive when \"*\" matches a directory?\n>\n> Sort of. The pattern is applied recursively to each basename, but the\n> pattern itself does not match recursively. That's probably splitting\n> hairs, though. :)\n\nI understand, but if I think about it too much I feel the overwhelming\nurge to stop thinking about it :)\n\n>> It's always seemed to me that both were necessary to explicitly match\n>> things in a directory and its subdirectories (example, IIRC: \"git\n>> ls-files -- .gitattributes\" vs \"git ls-files -- .gitattributes\n>> **/.gitattributes\"). Maybe that example is peculiar in that its a\n>> dotfile and can't have a wildcard before the dot?\n>\n> Those are pathspecs, which are not quite the same as gitattributes. They\n> don't do the magic basename matching.\n>\n> For pathspecs a \"*\" matches across slashes, which is what allows \"git\n> log -- '*.h'\" to work (but not a suffix wildcard like 'foo*').\n\nSame comment -- makes sense but hard to want to think too much about.\n\n>> I guess my takeaway is that it would be _good_ if the gitattributes\n>> documentation contained the caveat about not matching directories\n>> recursively, but _great_ if gitattributes and gitignore (and whatever\n>> else there is) were consistent.\n>\n> I agree it would be nice if they were consistent (and pathspecs, too).\n> But unfortunately at this point there's a maze of backwards\n> compatibility to deal with.\n>\n> -Peff\n\nAgain, as above, what if there were a subset of functionality git\ncould recommend to avoid inconsistencies?\n"},{"id":"342284","messageId":"CAHnyXxSOvU9aokdyo_UBcHPLeCSogyFCRjxXi9NkrmQbT0Qtig@mail.gmail.com","threadId":"48093","inReplyTo":"20180320041454.GA15213@sigill.intra.peff.net","subject":"Re: [PATCH] doc/gitattributes: mention non-recursive behavior","fromName":"Dakota Hawkins","fromEmail":"dakota@dakotahawkins.com","sentAt":"2018-03-20T04:28:03Z","receivedAt":"2018-03-20T04:28:09Z","isPatch":true,"sender":{"key":"dakota@dakotahawkins.com","avatar":null},"body":"That's perfect, thank you so much!\n\nOn Tue, Mar 20, 2018 at 12:14 AM, Jeff King wrote:\n> On Tue, Mar 20, 2018 at 12:04:11AM -0400, Jeff King wrote:\n>\n>> > I guess my takeaway is that it would be _good_ if the gitattributes\n>> > documentation contained the caveat about not matching directories\n>> > recursively, but _great_ if gitattributes and gitignore (and whatever\n>> > else there is) were consistent.\n>>\n>> I agree it would be nice if they were consistent (and pathspecs, too).\n>> But unfortunately at this point there's a maze of backwards\n>> compatibility to deal with.\n>\n> So let's not forget to do the easy half there. Here's a patch.\n>\n> -- >8 --\n> Subject: [PATCH] doc/gitattributes: mention non-recursive behavior\n>\n> The gitattributes documentation claims that the pattern\n> rules are largely the same as for gitignore. However, the\n> rules for recursion are different.\n>\n> In an ideal world, we would make them the same (if for\n> nothing else than consistency and simplicity), but that\n> would create backwards compatibility issues. For some\n> discussion, see this thread:\n>\n> https://public-inbox.org/git/slrnkldd3g.1l4.jan@majutsushi.net/\n>\n> But let's at least document the differences instead of\n> actively misleading the user by claiming that they're the\n> same.\n>\n> Signed-off-by: Jeff King \n> ---\n> Documentation/gitattributes.txt | 13 ++++++++++---\n> 1 file changed, 10 insertions(+), 3 deletions(-)\n>\n> diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt\n> index d52b254a22..1094fe2b5b 100644\n> --- a/Documentation/gitattributes.txt\n> +++ b/Documentation/gitattributes.txt\n> @@ -56,9 +56,16 @@ Unspecified::\n>\n> When more than one pattern matches the path, a later line\n> overrides an earlier line. This overriding is done per\n> -attribute. The rules how the pattern matches paths are the\n> -same as in `.gitignore` files; see linkgit:gitignore[5].\n> -Unlike `.gitignore`, negative patterns are forbidden.\n> +attribute.\n> +\n> +The rules by which the pattern matches paths are the same as in\n> +`.gitignore` files (see linkgit:gitignore[5]), with a few exceptions:\n> +\n> + - negative patterns are forbidden\n> +\n> + - patterns that match a directory do not recursively match paths\n> + inside that directory (so using the trailing-slash `path/` syntax is\n> + pointless in an attributes file; use `path/**` instead)\n>\n> When deciding what attributes are assigned to a path, Git\n> consults `$GIT_DIR/info/attributes` file (which has the highest\n> --\n> 2.17.0.rc0.402.ged0b3fd1ee\n>\n"},{"id":"342287","messageId":"20180320044036.GD13302@sigill.intra.peff.net","threadId":"48093","inReplyTo":"CAHnyXxQMbnNam=oB_B57xDZBaMPRn_8hfKtostHPV6nBHbTBog@mail.gmail.com","subject":"Re: .gitattributes override behavior (possible bug, or documentation bug)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-03-20T04:40:36Z","receivedAt":"2018-03-20T04:40:43Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 20, 2018 at 12:25:27AM -0400, Dakota Hawkins wrote:\n\n> > Right. The technical reason is mostly \"that is not how it was designed,\n> > and it would possibly break some corner cases if we switched it now\".\n> \n> I'm just spitballing here, but do you guys think there's any subset of\n> the combined .gitignore and .gitattributes matching functionality that\n> could at least serve as a good \"best-practices, going forward\"\n> (because of consistency) for both? I will say every time I do this for\n> a new repo and have to do something even slightly complicated or\n> different from what I've done before with .gitattributes/.gitignore\n> that it takes me a long-ish time to figure it out. It's like I'm\n> vaguely aware of pitfalls I've encountered in the past in certain\n> areas but don't remember exactly what they are, so I consult the docs,\n> which are (in sum) confusing and lead to more time spent\n> trying/failing/trying/works/fails-later/etc.\n> \n> One \"this subset of rules will work for both this way\" would be\n> awesome even if the matching capabilities are technically divergent,\n> but on the other hand that might paint both into a corner in terms of\n> functionality.\n\nAs far as I know, they should be the same with the exception of this\nrecursion, and the negative-pattern thing. But I'm cc-ing Duy, who is\nthe resident expert on ignore and attributes matching (whether he wants\nto be or not ;) ). I wouldn't be surprised if there's something I don't\nknow about.\n\nSo I think the \"recommended subset\" is basically \"everything but these\nfew constructs\". We just need to document them. ;)\n\nI probably should cc'd Duy on the documentation patch, too:\n\n https://public-inbox.org/git/20180320041454.GA15213@sigill.intra.peff.net/\n\n-Peff\n"},{"id":"342288","messageId":"CAHnyXxTj_TzUz13cK76D_hGF4FmBo40Q1RypvOLC8Q1eNk7FmA@mail.gmail.com","threadId":"48093","inReplyTo":"20180320044036.GD13302@sigill.intra.peff.net","subject":"Re: .gitattributes override behavior (possible bug, or documentation bug)","fromName":"Dakota Hawkins","fromEmail":"dakota@dakotahawkins.com","sentAt":"2018-03-20T04:49:38Z","receivedAt":"2018-03-20T04:49:43Z","isPatch":false,"sender":{"key":"dakota@dakotahawkins.com","avatar":null},"body":"> So I think the \"recommended subset\" is basically \"everything but these\n> few constructs\". We just need to document them. ;)\n\nMentioned so-far/running list?\n\n- Matching directories recursively, or at all I guess (e.g. \"/\")\n - (???) Instead: \"/*\"\n- Negative matches\n - (???) Instead: Is there any longer-form attributes-OK equivalent?\nJust positive-match with \"!s\"?\n"},{"id":"342337","messageId":"CACsJy8BP+ov0jeHp6QZoi=etPDQX5qEFqEfktyA-U-ffUE92vA@mail.gmail.com","threadId":"48093","inReplyTo":"20180320044036.GD13302@sigill.intra.peff.net","subject":"Re: .gitattributes override behavior (possible bug, or documentation bug)","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2018-03-20T16:28:46Z","receivedAt":"2018-03-20T16:29:21Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Mar 20, 2018 at 5:40 AM, Jeff King wrote:\n> On Tue, Mar 20, 2018 at 12:25:27AM -0400, Dakota Hawkins wrote:\n>\n>> > Right. The technical reason is mostly \"that is not how it was designed,\n>> > and it would possibly break some corner cases if we switched it now\".\n>>\n>> I'm just spitballing here, but do you guys think there's any subset of\n>> the combined .gitignore and .gitattributes matching functionality that\n>> could at least serve as a good \"best-practices, going forward\"\n>> (because of consistency) for both? I will say every time I do this for\n>> a new repo and have to do something even slightly complicated or\n>> different from what I've done before with .gitattributes/.gitignore\n>> that it takes me a long-ish time to figure it out. It's like I'm\n>> vaguely aware of pitfalls I've encountered in the past in certain\n>> areas but don't remember exactly what they are, so I consult the docs,\n>> which are (in sum) confusing and lead to more time spent\n>> trying/failing/trying/works/fails-later/etc.\n>>\n>> One \"this subset of rules will work for both this way\" would be\n\nYou know, you (Dakota) could implement the new \"exclude\" attribute in\n.gitattributes and ignore .gitignore files completely. That makes it\nworks \"for both\" ;-) The effort is probably not small though.\n\n>> awesome even if the matching capabilities are technically divergent,\n>> but on the other hand that might paint both into a corner in terms of\n>> functionality.\n>\n> As far as I know, they should be the same with the exception of this\n> recursion, and the negative-pattern thing. But I'm cc-ing Duy, who is\n> the resident expert on ignore and attributes matching (whether he wants\n> to be or not ;) ).\n\nHa ha ha.\n\n> I wouldn't be surprised if there's something I don't know about.\n\nThe only thing from the top of my head is what made me fail to unify\nthe implementation of the two. It's basically different order of\nevaluation [1] when your patterns are spread out in multiple files. I\nthink it makes gitattr and gitignore behavior different too (but I\ndidn't try to verify).\n\nApart from that, the two should behave the same way besides the\nexceptions you pointed out.\n\n[1] https://public-inbox.org/git/%3CCACsJy8B8kYU7bkD8SiK354z4u=sY3hHbe4JVwNT_1pxod1cqUw@mail.gmail.com%3E/\n\n> So I think the \"recommended subset\" is basically \"everything but these\n> few constructs\". We just need to document them. ;)\n>\n> I probably should cc'd Duy on the documentation patch, too:\n>\n> https://public-inbox.org/git/20180320041454.GA15213@sigill.intra.peff.net/\n>\n> -Peff\n-- \nDuy\n"},{"id":"342339","messageId":"CACsJy8CpwtNfp9oQGvECBuWGcwLEKK609iPJVEiXH4cDD6mpEg@mail.gmail.com","threadId":"48093","inReplyTo":"20180320041454.GA15213@sigill.intra.peff.net","subject":"Re: [PATCH] doc/gitattributes: mention non-recursive behavior","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2018-03-20T16:41:52Z","receivedAt":"2018-03-20T16:42:29Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Mar 20, 2018 at 5:14 AM, Jeff King wrote:\n> On Tue, Mar 20, 2018 at 12:04:11AM -0400, Jeff King wrote:\n>\n>> > I guess my takeaway is that it would be _good_ if the gitattributes\n>> > documentation contained the caveat about not matching directories\n>> > recursively, but _great_ if gitattributes and gitignore (and whatever\n>> > else there is) were consistent.\n>>\n>> I agree it would be nice if they were consistent (and pathspecs, too).\n>> But unfortunately at this point there's a maze of backwards\n>> compatibility to deal with.\n>\n> So let's not forget to do the easy half there. Here's a patch.\n>\n> -- >8 --\n> Subject: [PATCH] doc/gitattributes: mention non-recursive behavior\n>\n> The gitattributes documentation claims that the pattern\n> rules are largely the same as for gitignore. However, the\n> rules for recursion are different.\n>\n> In an ideal world, we would make them the same (if for\n> nothing else than consistency and simplicity), but that\n> would create backwards compatibility issues. For some\n> discussion, see this thread:\n>\n> https://public-inbox.org/git/slrnkldd3g.1l4.jan@majutsushi.net/\n>\n> But let's at least document the differences instead of\n> actively misleading the user by claiming that they're the\n> same.\n>\n> Signed-off-by: Jeff King \n> ---\n> Documentation/gitattributes.txt | 13 ++++++++++---\n> 1 file changed, 10 insertions(+), 3 deletions(-)\n>\n> diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt\n> index d52b254a22..1094fe2b5b 100644\n> --- a/Documentation/gitattributes.txt\n> +++ b/Documentation/gitattributes.txt\n> @@ -56,9 +56,16 @@ Unspecified::\n>\n> When more than one pattern matches the path, a later line\n> overrides an earlier line. This overriding is done per\n> -attribute. The rules how the pattern matches paths are the\n> -same as in `.gitignore` files; see linkgit:gitignore[5].\n> -Unlike `.gitignore`, negative patterns are forbidden.\n> +attribute.\n> +\n> +The rules by which the pattern matches paths are the same as in\n> +`.gitignore` files (see linkgit:gitignore[5]), with a few exceptions:\n> +\n> + - negative patterns are forbidden\n\nAfter 8b1bd02415 (Make !pattern in .gitattributes non-fatal -\n2013-03-01) maybe we could use the verb \"ignored\" too instead of\n\"forbidden\"\n\n> +\n> + - patterns that match a directory do not recursively match paths\n> + inside that directory (so using the trailing-slash `path/` syntax is\n\nTechnically gitignore does not match paths inside either. It simply\nignores the whole dir and not traverse in (which is more of an\noptimization than anything). That is coincidentally perceived as\nrecursively ignoring. Anyway yes it's good to spell out the\ndifferences here for gitattributes.\n\n> + pointless in an attributes file; use `path/**` instead)\n\nWe probably could do this internally too (converting \"path/\" to\n\"path/**\") but we need to deal with corner cases (e.g. \"path\" without\nthe trailing slash, but is a directory). So yes, suggesting the user\nto do it instead may be easier.\n\n>\n> When deciding what attributes are assigned to a path, Git\n> consults `$GIT_DIR/info/attributes` file (which has the highest\n> --\n> 2.17.0.rc0.402.ged0b3fd1ee\n>\n\n\n\n-- \nDuy\n"},{"id":"342375","messageId":"CAHnyXxRsqR3A0FpHJwnfOWo29WNx05qWSXfxvmwPv1TUdqm0aQ@mail.gmail.com","threadId":"48093","inReplyTo":"CACsJy8BP+ov0jeHp6QZoi=etPDQX5qEFqEfktyA-U-ffUE92vA@mail.gmail.com","subject":"Re: .gitattributes override behavior (possible bug, or documentation bug)","fromName":"Dakota Hawkins","fromEmail":"dakota@dakotahawkins.com","sentAt":"2018-03-21T03:22:02Z","receivedAt":"2018-03-21T03:22:09Z","isPatch":false,"sender":{"key":"dakota@dakotahawkins.com","avatar":null},"body":"Thinking about this a little more, I'm now attracted to the idea that\nits .gitignore that's weird.\n\nAs I understand it, .gitignore stops recursion when there's a\ndirectory match (`somedir/`) but also explicitly allows nested\n.gitnore file _as well as_ exclusion (`!*.txt`).\n\nSo, in the following (contrived) example, the user doesn't get what they want:\n\n repo/\n |- .git/\n |- .gitignore # /ignore-most/\n |- ignore-most/\n | |- .gitignore # !*.txt\n | |- please_ignore.png\n | |- dont_ignore_me.txt\n\n`repo/ignore-most/dont_ignore_me.txt` is still ignored, despite what\nseems like the obvious intention of the user.\n\nMaybe a unified \"best-practices\" would first-and-foremost recommend\nagainst matching directories at all (makes sense, git doesn't manage\ndirectories). In the above example, changing `/ignore-most/` to\n`/ignore-most/*` has the \"desired\" effect.\n\nWhat do you think?\n\nOn Tue, Mar 20, 2018 at 12:28 PM, Duy Nguyen wrote:\n> On Tue, Mar 20, 2018 at 5:40 AM, Jeff King wrote:\n>> On Tue, Mar 20, 2018 at 12:25:27AM -0400, Dakota Hawkins wrote:\n>>\n>>> > Right. The technical reason is mostly \"that is not how it was designed,\n>>> > and it would possibly break some corner cases if we switched it now\".\n>>>\n>>> I'm just spitballing here, but do you guys think there's any subset of\n>>> the combined .gitignore and .gitattributes matching functionality that\n>>> could at least serve as a good \"best-practices, going forward\"\n>>> (because of consistency) for both? I will say every time I do this for\n>>> a new repo and have to do something even slightly complicated or\n>>> different from what I've done before with .gitattributes/.gitignore\n>>> that it takes me a long-ish time to figure it out. It's like I'm\n>>> vaguely aware of pitfalls I've encountered in the past in certain\n>>> areas but don't remember exactly what they are, so I consult the docs,\n>>> which are (in sum) confusing and lead to more time spent\n>>> trying/failing/trying/works/fails-later/etc.\n>>>\n>>> One \"this subset of rules will work for both this way\" would be\n>\n> You know, you (Dakota) could implement the new \"exclude\" attribute in\n> .gitattributes and ignore .gitignore files completely. That makes it\n> works \"for both\" ;-) The effort is probably not small though.\n>\n>>> awesome even if the matching capabilities are technically divergent,\n>>> but on the other hand that might paint both into a corner in terms of\n>>> functionality.\n>>\n>> As far as I know, they should be the same with the exception of this\n>> recursion, and the negative-pattern thing. But I'm cc-ing Duy, who is\n>> the resident expert on ignore and attributes matching (whether he wants\n>> to be or not ;) ).\n>\n> Ha ha ha.\n>\n>> I wouldn't be surprised if there's something I don't know about.\n>\n> The only thing from the top of my head is what made me fail to unify\n> the implementation of the two. It's basically different order of\n> evaluation [1] when your patterns are spread out in multiple files. I\n> think it makes gitattr and gitignore behavior different too (but I\n> didn't try to verify).\n>\n> Apart from that, the two should behave the same way besides the\n> exceptions you pointed out.\n>\n> [1] https://public-inbox.org/git/%3CCACsJy8B8kYU7bkD8SiK354z4u=sY3hHbe4JVwNT_1pxod1cqUw@mail.gmail.com%3E/\n>\n>> So I think the \"recommended subset\" is basically \"everything but these\n>> few constructs\". We just need to document them. ;)\n>>\n>> I probably should cc'd Duy on the documentation patch, too:\n>>\n>> https://public-inbox.org/git/20180320041454.GA15213@sigill.intra.peff.net/\n>>\n>> -Peff\n> --\n> Duy\n"},{"id":"342387","messageId":"20180321065005.GB16784@sigill.intra.peff.net","threadId":"48093","inReplyTo":"CACsJy8CpwtNfp9oQGvECBuWGcwLEKK609iPJVEiXH4cDD6mpEg@mail.gmail.com","subject":"Re: [PATCH] doc/gitattributes: mention non-recursive behavior","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-03-21T06:50:05Z","receivedAt":"2018-03-21T06:50:12Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 20, 2018 at 05:41:52PM +0100, Duy Nguyen wrote:\n\n> > +The rules by which the pattern matches paths are the same as in\n> > +`.gitignore` files (see linkgit:gitignore[5]), with a few exceptions:\n> > +\n> > + - negative patterns are forbidden\n> \n> After 8b1bd02415 (Make !pattern in .gitattributes non-fatal -\n> 2013-03-01) maybe we could use the verb \"ignored\" too instead of\n> \"forbidden\"\n\nMakes sense. The original is already in 'next', so do you want to send\nan incremental patch?\n\n> > + pointless in an attributes file; use `path/**` instead)\n> \n> We probably could do this internally too (converting \"path/\" to\n> \"path/**\") but we need to deal with corner cases (e.g. \"path\" without\n> the trailing slash, but is a directory). So yes, suggesting the user\n> to do it instead may be easier.\n\nYeah, I almost suggested that, but I worried about those corner cases.\nIt seems like documenting the current behavior is the right first step\nin any case.\n\nOne other maybe-difference I came across coincidentally today: you have\nto quote the pattern in .gitattributes if it contains spaces, but not in\n.gitignore. But that's more an artifact of the rest of the file syntax\nthan the pattern syntax (.gitignore has no other fields to confuse it\nwith).\n\n-Peff\n"},{"id":"342388","messageId":"20180321065212.GC16784@sigill.intra.peff.net","threadId":"48093","inReplyTo":"CAHnyXxRsqR3A0FpHJwnfOWo29WNx05qWSXfxvmwPv1TUdqm0aQ@mail.gmail.com","subject":"Re: .gitattributes override behavior (possible bug, or documentation bug)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-03-21T06:52:13Z","receivedAt":"2018-03-21T06:52:18Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 20, 2018 at 11:22:02PM -0400, Dakota Hawkins wrote:\n\n> Thinking about this a little more, I'm now attracted to the idea that\n> its .gitignore that's weird.\n> \n> As I understand it, .gitignore stops recursion when there's a\n> directory match (`somedir/`) but also explicitly allows nested\n> .gitnore file _as well as_ exclusion (`!*.txt`).\n> \n> So, in the following (contrived) example, the user doesn't get what they want:\n> \n> repo/\n> |- .git/\n> |- .gitignore # /ignore-most/\n> |- ignore-most/\n> | |- .gitignore # !*.txt\n> | |- please_ignore.png\n> | |- dont_ignore_me.txt\n> \n> `repo/ignore-most/dont_ignore_me.txt` is still ignored, despite what\n> seems like the obvious intention of the user.\n\nRight, I think this gets back to Duy's other email. We're not really\nrecursively applying the pattern, it's just that we don't bother\ndescending into ignore areas at all.\n\n> Maybe a unified \"best-practices\" would first-and-foremost recommend\n> against matching directories at all (makes sense, git doesn't manage\n> directories). In the above example, changing `/ignore-most/` to\n> `/ignore-most/*` has the \"desired\" effect.\n> \n> What do you think?\n\nI think that ignoring all of /ignore-most/ is much more efficient, since\nwe don't have to enumerate the paths inside it at all (which is why the\ncurrent behavior works as it does).\n\n-Peff\n"},{"id":"342389","messageId":"CAHnyXxTf16ukf8CFSfzjQjTEjf5yJXFqK5TnS9KiXZEeAD7dgg@mail.gmail.com","threadId":"48093","inReplyTo":"20180321065212.GC16784@sigill.intra.peff.net","subject":"Re: .gitattributes override behavior (possible bug, or documentation bug)","fromName":"Dakota Hawkins","fromEmail":"dakota@dakotahawkins.com","sentAt":"2018-03-21T07:36:09Z","receivedAt":"2018-03-21T07:36:16Z","isPatch":false,"sender":{"key":"dakota@dakotahawkins.com","avatar":null},"body":"> I think that ignoring all of /ignore-most/ is much more efficient, since\n> we don't have to enumerate the paths inside it at all (which is why the\n> current behavior works as it does).\n\nThat's definitely true, but I wonder what the impact would be for most\ncases (even for most cases with large repos and larges sets of ignore\nfiles).\n\nMost of my .gitignore patterns weren't hand-written\n(https://www.gitignore.io/ is pretty neat), but there are a ton of\npatterns like `dir/`...\n\nI think if I were designing it from scratch and knew what I know now\nI'd probably argue that behavior should be declarative (`dir/*\nrecurse=false` or something), but we can't really get there from here.\n\nAt any rate, would it at least be a good idea to make the \"trailing\nslash halts recursion, won't consider nested .gitignore files\"\nexplicit in the `.gitignore` doc? Unless I'm missing it, I don't think\nthat behavior is called out (or at least not called out concisely/in\none place). It looks like this is all there is:\n\n \"If the pattern ends with a slash, it is removed for the purpose\nof the following description, but it would only find a match with a\ndirectory. In other words, foo/ will match a directory foo and paths\nunderneath it, but will not match a regular file or a symbolic link\nfoo (this is consistent with the way how pathspec works in general in\nGit).\"\n\nAlso, I'm not sure what the \"following description\" is in \"it is\nremoved for the purpose of the following description\". Is that trying\nto imply \"excluded from the rest of the doc\"?\n\n- Dakota\n"},{"id":"342390","messageId":"CAHnyXxRpRjyuTg9v-m9c4ZCwRTM2_4335m-yzFYqALBLAy8=+g@mail.gmail.com","threadId":"48093","inReplyTo":"CAHnyXxTf16ukf8CFSfzjQjTEjf5yJXFqK5TnS9KiXZEeAD7dgg@mail.gmail.com","subject":"Re: .gitattributes override behavior (possible bug, or documentation bug)","fromName":"Dakota Hawkins","fromEmail":"dakota@dakotahawkins.com","sentAt":"2018-03-21T07:44:17Z","receivedAt":"2018-03-21T07:44:24Z","isPatch":false,"sender":{"key":"dakota@dakotahawkins.com","avatar":null},"body":"One extra note:\n\nI was burned by this just a few hours ago in a new repo (but because\nof this discussion I realized what the problem was pretty quickly).\n\nIn the top-level .gitignore I had\n\n build/\n ...\n !alpine/build/\n\nwhere `build/` was a stock ignore line among hundreds that I blindly\npasted in there, and the exclusion was an attempt to exclude some\nthings that shouldn't have been ignored.\n\nEven in the same file, the exclusion doesn't work. I had to change it to:\n\n build/*\n ...\n !alpine/build/\n\nGood times :)\n\n- Dakota\n\nOn Wed, Mar 21, 2018 at 3:36 AM, Dakota Hawkins\n wrote:\n>> I think that ignoring all of /ignore-most/ is much more efficient, since\n>> we don't have to enumerate the paths inside it at all (which is why the\n>> current behavior works as it does).\n>\n> That's definitely true, but I wonder what the impact would be for most\n> cases (even for most cases with large repos and larges sets of ignore\n> files).\n>\n> Most of my .gitignore patterns weren't hand-written\n> (https://www.gitignore.io/ is pretty neat), but there are a ton of\n> patterns like `dir/`...\n>\n> I think if I were designing it from scratch and knew what I know now\n> I'd probably argue that behavior should be declarative (`dir/*\n> recurse=false` or something), but we can't really get there from here.\n>\n> At any rate, would it at least be a good idea to make the \"trailing\n> slash halts recursion, won't consider nested .gitignore files\"\n> explicit in the `.gitignore` doc? Unless I'm missing it, I don't think\n> that behavior is called out (or at least not called out concisely/in\n> one place). It looks like this is all there is:\n>\n> \"If the pattern ends with a slash, it is removed for the purpose\n> of the following description, but it would only find a match with a\n> directory. In other words, foo/ will match a directory foo and paths\n> underneath it, but will not match a regular file or a symbolic link\n> foo (this is consistent with the way how pathspec works in general in\n> Git).\"\n>\n> Also, I'm not sure what the \"following description\" is in \"it is\n> removed for the purpose of the following description\". Is that trying\n> to imply \"excluded from the rest of the doc\"?\n>\n> - Dakota\n"},{"id":"342391","messageId":"20180321075041.GA24701@sigill.intra.peff.net","threadId":"48093","inReplyTo":"CAHnyXxTf16ukf8CFSfzjQjTEjf5yJXFqK5TnS9KiXZEeAD7dgg@mail.gmail.com","subject":"Re: .gitattributes override behavior (possible bug, or documentation bug)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-03-21T07:50:41Z","receivedAt":"2018-03-21T07:50:52Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Mar 21, 2018 at 03:36:09AM -0400, Dakota Hawkins wrote:\n\n> > I think that ignoring all of /ignore-most/ is much more efficient, since\n> > we don't have to enumerate the paths inside it at all (which is why the\n> > current behavior works as it does).\n> \n> That's definitely true, but I wonder what the impact would be for most\n> cases (even for most cases with large repos and larges sets of ignore\n> files).\n\nProbably not that much CPU, but it used to be quite annoying to fault in\nthe dcache for an infrequently used hierarchy. These days I generally\nhave an SSD and lots of memory for disk caching, so I don't know if it\nwould still be.\n\nOn systems where readdir/stat are slower than Linux it might be\nnoticeable for a big hierarchy.\n\n> At any rate, would it at least be a good idea to make the \"trailing\n> slash halts recursion, won't consider nested .gitignore files\"\n> explicit in the `.gitignore` doc? Unless I'm missing it, I don't think\n> that behavior is called out (or at least not called out concisely/in\n> one place). It looks like this is all there is:\n\nYeah, it's definitely come up multiple times over the years. I don't\nknow what all is in gitignore(5), but if it's not mentioned it probably\nshould be.\n\n> \"If the pattern ends with a slash, it is removed for the purpose\n> of the following description, but it would only find a match with a\n> directory. In other words, foo/ will match a directory foo and paths\n> underneath it, but will not match a regular file or a symbolic link\n> foo (this is consistent with the way how pathspec works in general in\n> Git).\"\n> \n> Also, I'm not sure what the \"following description\" is in \"it is\n> removed for the purpose of the following description\". Is that trying\n> to imply \"excluded from the rest of the doc\"?\n\nI think it means \"for the rest of the description of how the patterns\nwork\". I.e., \"foo/\" matches as \"foo\" when the rest of the matching rules\nare applied. I agree it's a bit awkward. Patches welcome. :)\n\n-Peff\n"},{"id":"342394","messageId":"CAHnyXxTP7i4G=4G_ihRQTSbLNzAWuqZ3FoX0NtesAX219Pa5-g@mail.gmail.com","threadId":"48093","inReplyTo":"20180321075041.GA24701@sigill.intra.peff.net","subject":"Re: .gitattributes override behavior (possible bug, or documentation bug)","fromName":"Dakota Hawkins","fromEmail":"dakota@dakotahawkins.com","sentAt":"2018-03-21T08:35:26Z","receivedAt":"2018-03-21T08:35:35Z","isPatch":false,"sender":{"key":"dakota@dakotahawkins.com","avatar":null},"body":"> I think it means \"for the rest of the description of how the patterns\n> work\". I.e., \"foo/\" matches as \"foo\" when the rest of the matching rules\n> are applied. I agree it's a bit awkward. Patches welcome. :)\n\nI'd be more than happy to do that!\n\nIt will take me a while, this (email and text-patches) is foreign to\nme and will take me some extra time, but I think I can figure it out.\n\nIs \"consistent with the way how pathspec works in general in Git\"\nabsolutely still true (and relevant?)\n"},{"id":"342395","messageId":"20180321083646.GA32578@sigill.intra.peff.net","threadId":"48093","inReplyTo":"CAHnyXxTP7i4G=4G_ihRQTSbLNzAWuqZ3FoX0NtesAX219Pa5-g@mail.gmail.com","subject":"Re: .gitattributes override behavior (possible bug, or documentation bug)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-03-21T08:36:46Z","receivedAt":"2018-03-21T08:36:53Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Mar 21, 2018 at 04:35:26AM -0400, Dakota Hawkins wrote:\n\n> > I think it means \"for the rest of the description of how the patterns\n> > work\". I.e., \"foo/\" matches as \"foo\" when the rest of the matching rules\n> > are applied. I agree it's a bit awkward. Patches welcome. :)\n> \n> I'd be more than happy to do that!\n> \n> It will take me a while, this (email and text-patches) is foreign to\n> me and will take me some extra time, but I think I can figure it out.\n> \n> Is \"consistent with the way how pathspec works in general in Git\"\n> absolutely still true (and relevant?)\n\nI think so. \"git log Makefile/\" will not match anything, for example.\n\n-Peff\n"},{"id":"342413","messageId":"CACsJy8A7k0d8wB_XWhvaaTXespg2y6G+Ab4b27Dc3iKY2Zjujw@mail.gmail.com","threadId":"48093","inReplyTo":"CAHnyXxRsqR3A0FpHJwnfOWo29WNx05qWSXfxvmwPv1TUdqm0aQ@mail.gmail.com","subject":"Re: .gitattributes override behavior (possible bug, or documentation bug)","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2018-03-21T16:07:02Z","receivedAt":"2018-03-21T16:07:40Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Mar 21, 2018 at 4:22 AM, Dakota Hawkins\n wrote:\n> Thinking about this a little more, I'm now attracted to the idea that\n> its .gitignore that's weird.\n>\n> As I understand it, .gitignore stops recursion when there's a\n> directory match (`somedir/`) but also explicitly allows nested\n> .gitnore file _as well as_ exclusion (`!*.txt`).\n>\n> So, in the following (contrived) example, the user doesn't get what they want:\n>\n> repo/\n> |- .git/\n> |- .gitignore # /ignore-most/\n> |- ignore-most/\n> | |- .gitignore # !*.txt\n> | |- please_ignore.png\n> | |- dont_ignore_me.txt\n>\n> `repo/ignore-most/dont_ignore_me.txt` is still ignored, despite what\n> seems like the obvious intention of the user.\n\nDon't get me started on this. I voiced this problem a couple times.\nAttempted to fix it once which made it to rc cycles and caused lots of\nregressions. I haven't taken another stab since.\n\n> Maybe a unified \"best-practices\" would first-and-foremost recommend\n> against matching directories at all (makes sense, git doesn't manage\n> directories). In the above example, changing `/ignore-most/` to\n> `/ignore-most/*` has the \"desired\" effect.\n\nI think it's actually more intuitive to think \"ignore recursively\"\n(with the trailing slash) These things make sense to you once you know\nexactly _how_ the matching machinery works. But normal users don't\nknow that. It's probably better that we add recursive matching support\nin gitattributes. Then both gitignore/gitattr are more or less\nconsistent again and also easy to use (most of the times)\n-- \nDuy\n"},{"id":"342415","messageId":"CACsJy8DCkh=kn1bmCQf4idPggsh5cEY936gfR-5qaQKRsmLBzA@mail.gmail.com","threadId":"48093","inReplyTo":"20180321065005.GB16784@sigill.intra.peff.net","subject":"Re: [PATCH] doc/gitattributes: mention non-recursive behavior","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2018-03-21T16:16:16Z","receivedAt":"2018-03-21T16:16:52Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Mar 21, 2018 at 7:50 AM, Jeff King wrote:\n> On Tue, Mar 20, 2018 at 05:41:52PM +0100, Duy Nguyen wrote:\n>\n>> > +The rules by which the pattern matches paths are the same as in\n>> > +`.gitignore` files (see linkgit:gitignore[5]), with a few exceptions:\n>> > +\n>> > + - negative patterns are forbidden\n>>\n>> After 8b1bd02415 (Make !pattern in .gitattributes non-fatal -\n>> 2013-03-01) maybe we could use the verb \"ignored\" too instead of\n>> \"forbidden\"\n>\n> Makes sense. The original is already in 'next', so do you want to send\n> an incremental patch?\n\nIt's up to you. After all it's you who's doing all the work :)\n\n>> > + pointless in an attributes file; use `path/**` instead)\n>>\n>> We probably could do this internally too (converting \"path/\" to\n>> \"path/**\") but we need to deal with corner cases (e.g. \"path\" without\n>> the trailing slash, but is a directory). So yes, suggesting the user\n>> to do it instead may be easier.\n>\n> Yeah, I almost suggested that, but I worried about those corner cases.\n> It seems like documenting the current behavior is the right first step\n> in any case.\n\nAgreed.\n\n> One other maybe-difference I came across coincidentally today: you have\n> to quote the pattern in .gitattributes if it contains spaces, but not in\n> .gitignore. But that's more an artifact of the rest of the file syntax\n> than the pattern syntax (.gitignore has no other fields to confuse it\n> with).\n\nYeah I forgot about that (and I was the one who started it). The\ndocument was updated in 860a74d9d9 (attr: support quoting pathname\npatterns in C style - 2017-01-27) though.\n-- \nDuy\n"},{"id":"342417","messageId":"xmqqa7v1qu5o.fsf@gitster-ct.c.googlers.com","threadId":"48093","inReplyTo":"CAHnyXxTf16ukf8CFSfzjQjTEjf5yJXFqK5TnS9KiXZEeAD7dgg@mail.gmail.com","subject":"Re: .gitattributes override behavior (possible bug, or documentation bug)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-21T16:18:27Z","receivedAt":"2018-03-21T16:18:35Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dakota Hawkins writes:\n\n> At any rate, would it at least be a good idea to make the \"trailing\n> slash halts recursion, won't consider nested .gitignore files\"\n> explicit in the `.gitignore` doc? Unless I'm missing it, I don't think\n> that behavior is called out (or at least not called out concisely/in\n> one place). It looks like this is all there is:\n>\n> \"If the pattern ends with a slash, it is removed for the purpose\n> of the following description, but it would only find a match with a\n> directory. In other words, foo/ will match a directory foo and paths\n> underneath it, but will not match a regular file or a symbolic link\n> foo (this is consistent with the way how pathspec works in general in\n> Git).\"\n\nThis is not saying \"trailing slash halts\" (and there is no need to\nsay \"trailing slash halts\"---the rule is \"directory is not recursed\ninto\"). The mention of '/' in the above paragraph is merely saying\nthat we chose to use a trailing slash as a syntax to say \"I want the\npath to match this pattern, but only when the path is a directory\".\nIn other words, if \"D\" (nothing else on the line) is on a line, it\nwould match a file whose path is \"D\", but if you write \"D/\", it\nwould not. It only would match a directory whose path is \"D\".\n\nWhat \"removed for the purpose of the following description\" wants to\nsay is about where in the path \"D\" in the above example can appear.\nAnother rule after the paragraph would say that a pattern with a\nslash in it will match only at the current level, and a pattern with\nno slash would match in any level down below. Imagine an entry \"D\"\nand another entry \"A/B\" in .gitignore at the top-level of the\nproject. The former has slash in it, the latter does not. These\npatterns match paths \"D\", \"X/D\", \"A/B\" but not \"X/A/B\". The first\ntwo match because the pattern \"D\" is not anchored with any slash,\nthe last one does not match because the pattern \"A/B\" has a slash in\nit.\n\nIf the top-level .gitignore had \"D/\" and \"A/B\" in the above example,\nand paths \"D\" and \"X/D\" were both directories, the pattern \"D/\"\nstill matches the directory \"X/D\", even though it is spelled with a\nslash in it. That slash is there merely for the purpose of marking\nthe pattern to only match directories, and does not trigger \"slash\nin a pattern anchors the pattern\" rule, because it is \"removed for\nthe purpose of the following description\".\n\n> Also, I'm not sure what the \"following description\" is in \"it is\n> removed for the purpose of the following description\". Is that trying\n> to imply \"excluded from the rest of the doc\"?\n\nSee above.\n"},{"id":"342570","messageId":"20180323091221.GB24416@sigill.intra.peff.net","threadId":"48093","inReplyTo":"CACsJy8DCkh=kn1bmCQf4idPggsh5cEY936gfR-5qaQKRsmLBzA@mail.gmail.com","subject":"Re: [PATCH] doc/gitattributes: mention non-recursive behavior","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-03-23T09:12:21Z","receivedAt":"2018-03-23T09:12:29Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Mar 21, 2018 at 05:16:16PM +0100, Duy Nguyen wrote:\n\n> >> After 8b1bd02415 (Make !pattern in .gitattributes non-fatal -\n> >> 2013-03-01) maybe we could use the verb \"ignored\" too instead of\n> >> \"forbidden\"\n> >\n> > Makes sense. The original is already in 'next', so do you want to send\n> > an incremental patch?\n> \n> It's up to you. After all it's you who's doing all the work :)\n\nI was trying to trick you into doing it. ;)\n\nAs I tried to write up the commit message, though, I had second\nthoughts. It's true that they are ignored in the current code. But I do\nnot think they are something we want to encourage, and certainly we do\nnot want to promise that the \"ignored\" behavior will last forever.\n\nSo I think it is actually best to declare them forbidden, and we just\nhappen to treat it as a non-fatal error in the current code.\n\n> > One other maybe-difference I came across coincidentally today: you have\n> > to quote the pattern in .gitattributes if it contains spaces, but not in\n> > .gitignore. But that's more an artifact of the rest of the file syntax\n> > than the pattern syntax (.gitignore has no other fields to confuse it\n> > with).\n> \n> Yeah I forgot about that (and I was the one who started it). The\n> document was updated in 860a74d9d9 (attr: support quoting pathname\n> patterns in C style - 2017-01-27) though.\n\nOK. I was considering adding a special note to the list of differences,\nbut I think the existing text is probably fine.\n\n-Peff\n"}]}