{"thread":{"id":"56937","subject":"git 2.34.0: Behavior of `**` in gitignore is different from previous versions.","startedAt":"2021-11-18T16:41:17Z","lastAt":"2021-11-23T21:13:43Z","messageCount":13,"participants":["Danial Alihosseini","Jeff King","Derrick Stolee","Johannes Sixt","Ævar Arnfjörð Bjarmason","Chris Torek","Junio C Hamano","Philip Oakley"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"441616","messageId":"CACLOEFZz7bunO2S5-ec1K10B9AJU4-m50j3j9c=12R6d1D+-dg@mail.gmail.com","threadId":"56937","inReplyTo":null,"subject":"git 2.34.0: Behavior of `**` in gitignore is different from previous versions.","fromName":"Danial Alihosseini","fromEmail":"danial.alihosseini@gmail.com","sentAt":"2021-11-18T16:41:04Z","receivedAt":"2021-11-18T16:41:17Z","isPatch":false,"sender":{"key":"danial.alihosseini@gmail.com","avatar":null},"body":"Thank you for filling out a Git bug report!\nPlease answer the following questions to help us understand your issue.\n\nWhat did you do before the bug happened? (Steps to reproduce your issue)\nConsider the following project structure\n- data\n  - data1\n    - file1\n    - file1.txt\n  - data2\n    - file2\n    - file2.txt\n- .gitignore\n\n\n`.gitignore` is as follows:\n```\ndata/**\n!data/**/\n!data/**/*.txt\n```\nWhat did you expect to happen? (Expected behavior)\n\nI expect all files in `data` folder to be ignored except `.txt` files.\n\nWhat happened instead? (Actual behavior)\n\n`file1` and `file2` are not ignored.\nHere is the `check-ignore` output:\n```\n$ git check-ignore -v data/data1/file1\n.gitignore:2:!/data/**/ data/data1/file1\n```\n\nWhat's different between what you expected and what actually happened?\nIt seems that the behavior of `**` in gitignore is different from\nprevious versions.\n\nAnything else you want to add:\n\nPlease review the rest of the bug report below.\nYou can delete any lines you don't wish to share.\n\n\n[System Info]\ngit version:\ngit version 2.34.0\ncpu: x86_64\nno commit associated with this build\nsizeof-long: 8\nsizeof-size_t: 8\nshell-path: /bin/sh\nuname: Linux 5.15.2-arch1-1\ncompiler info: gnuc: 11.1\nlibc info: glibc: 2.33\n$SHELL (typically, interactive shell): /usr/bin/zsh\n\n\n[Enabled Hooks]\n"},{"id":"441645","messageId":"YZaHpJKeyDEY8qKW@coredump.intra.peff.net","threadId":"56937","inReplyTo":"CACLOEFZz7bunO2S5-ec1K10B9AJU4-m50j3j9c=12R6d1D+-dg@mail.gmail.com","subject":"Re: git 2.34.0: Behavior of `**` in gitignore is different from previous versions.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-11-18T17:04:36Z","receivedAt":"2021-11-18T17:04:38Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Nov 18, 2021 at 08:11:04PM +0330, Danial Alihosseini wrote:\n\n> What did you do before the bug happened? (Steps to reproduce your issue)\n> Consider the following project structure\n> - data\n>   - data1\n>     - file1\n>     - file1.txt\n>   - data2\n>     - file2\n>     - file2.txt\n> - .gitignore\n> \n> \n> `.gitignore` is as follows:\n> ```\n> data/**\n> !data/**/\n> !data/**/*.txt\n> ```\n> What did you expect to happen? (Expected behavior)\n> \n> I expect all files in `data` folder to be ignored except `.txt` files.\n> \n> What happened instead? (Actual behavior)\n> \n> `file1` and `file2` are not ignored.\n> Here is the `check-ignore` output:\n> ```\n> $ git check-ignore -v data/data1/file1\n> .gitignore:2:!/data/**/ data/data1/file1\n> ```\n\nThanks for an easy reproduction. It looks like this changed in\nf6526728f9 (dir: select directories correctly, 2021-09-24). Author cc'd.\n\nThe key thing seems to be that the second line of your .gitignore should\nmatch only directories (because of the trailing slash), but no longer\ndoes.\n\n-Peff\n"},{"id":"441665","messageId":"2bd2269f-c7f1-7afb-7052-48fac148dffd@gmail.com","threadId":"56937","inReplyTo":"YZaHpJKeyDEY8qKW@coredump.intra.peff.net","subject":"Re: git 2.34.0: Behavior of `**` in gitignore is different from previous versions.","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2021-11-18T22:09:58Z","receivedAt":"2021-11-18T22:10:04Z","isPatch":false,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 11/18/2021 12:04 PM, Jeff King wrote:\n> On Thu, Nov 18, 2021 at 08:11:04PM +0330, Danial Alihosseini wrote:\n> \n>> What did you do before the bug happened? (Steps to reproduce your issue)\n>> Consider the following project structure\n>> - data\n>>   - data1\n>>     - file1\n>>     - file1.txt\n>>   - data2\n>>     - file2\n>>     - file2.txt\n>> - .gitignore\n>>\n>>\n>> `.gitignore` is as follows:\n>> ```\n>> data/**\n>> !data/**/\n>> !data/**/*.txt\n>> ```\n>> What did you expect to happen? (Expected behavior)\n>>\n>> I expect all files in `data` folder to be ignored except `.txt` files.\n>>\n>> What happened instead? (Actual behavior)\n>>\n>> `file1` and `file2` are not ignored.\n>> Here is the `check-ignore` output:\n>> ```\n>> $ git check-ignore -v data/data1/file1\n>> .gitignore:2:!/data/**/ data/data1/file1\n>> ```\n> \n> Thanks for an easy reproduction. It looks like this changed in\n> f6526728f9 (dir: select directories correctly, 2021-09-24). Author cc'd.\n\nThanks for the bisect and CC.\n\n> The key thing seems to be that the second line of your .gitignore should\n> match only directories (because of the trailing slash), but no longer\n> does.\n\nDoesn't \"matching only directories\" mean it would match everything\nwithin that directory? (It also means that \"data/file\" is not matched,\nwhich is still correct.)\n\nMy interpretation of these patterns is that everything in data/data1/\nand data/data2/ should not be ignored, making it seem like the change\nfixed a bug (it definitely changed behavior).\n\nJust for extra clarity, this test currently passes:\n\ntest_expect_success 'directories and ** matches' '\n\tcat >.gitignore <<-\\EOF &&\n\tdata/**\n\t!data/**/\n\t!data/**/*.txt\n\tEOF\n\tgit check-ignore file \\\n\t\tdata/file data/data1/file1 data/data1/file1.txt \\\n\t\tdata/data2/file2 data/data2/file2.txt >actual &&\n\tcat >expect <<-\\EOF &&\n\tdata/file\n\tEOF\n\ttest_cmp expect actual\n'\n\nbut the previous behavior would have passed this test:\n\ntest_expect_success 'directories and ** matches' '\n\tcat >.gitignore <<-\\EOF &&\n\tdata/**\n\t!data/**/\n\t!data/**/*.txt\n\tEOF\n\tgit check-ignore file \\\n\t\tdata/file data/data1/file1.txt \\\n\t\tdata/data2/file2.txt >actual &&\n\tcat >expect <<-\\EOF &&\n\tdata/file\n\tEOF\n\ttest_cmp expect actual\n'\n\nI seek more clarity on this. Specifically: if we match a directory\nthen should we not also match the contents within?\n\nThanks,\n-Stolee\n"},{"id":"441686","messageId":"CACLOEFbY3LwMa2uhc=9jmcGFf0mvWzEM=YityLyFcuGWXVmqbw@mail.gmail.com","threadId":"56937","inReplyTo":"2bd2269f-c7f1-7afb-7052-48fac148dffd@gmail.com","subject":"Re: git 2.34.0: Behavior of `**` in gitignore is different from previous versions.","fromName":"Danial Alihosseini","fromEmail":"danial.alihosseini@gmail.com","sentAt":"2021-11-19T04:01:45Z","receivedAt":"2021-11-19T04:01:58Z","isPatch":false,"sender":{"key":"danial.alihosseini@gmail.com","avatar":null},"body":"Thanks for your follow-up.\n\nI wanted to ignore all files in the \"data\" folder except \".txt\" ones.\n\nAs mentioned in the gitignore doc, there should be a difference\nbetween \"**\" and \"**/\".\n\n> A trailing \"/**\" matches everything inside. For example, \"abc/**\" matches all files inside directory \"abc\", relative to the location of the .gitignore file, with infinite depth.\n\nand,\n>\n> A slash followed by two consecutive asterisks then a slash matches zero or more directories. For example, \"a/**/b\" matches \"a/b\", \"a/x/b\", \"a/x/y/b\" and so on.\n\nand also,\n>\n> It is not possible to re-include a file if a parent directory of that file is excluded.\n\nSo, I excluded all files by \"data/**\", re-included just directories\n(at any depth) by \"!data/**/\" and, re-included \".txt\" files (at any\ndepth) by \"!data/**/*.txt\".\nHowever, if I wanted to re-include a directory and all of its\ncontents, I could use something like \"!data/data1/**\", without\ntrailing slash.\nAs I see, there is no separation between \"**\" and \"**/\" in the current version.\n\nI think there is another point that the previous behavior test case\nshould be like this:\ntest_expect_success 'directories and ** matches' '\n    cat >.gitignore <<-\\EOF &&\n    data/**\n    !data/**/\n    !data/**/*.txt\n    EOF\n    git check-ignore file \\\n        data/file data/data1/file1 data/data1/file1.txt \\\n        data/data2/file2 data/data2/file2.txt >actual &&\n    cat >expect <<-\\EOF &&\n    data/file\n    data/data1/file1\n    data/data2/file2\n    EOF\n    test_cmp expect actual\n'\nThanks\nDanial\n\n\nOn Fri, Nov 19, 2021 at 1:40 AM Derrick Stolee <stolee@gmail.com> wrote:\n>\n> On 11/18/2021 12:04 PM, Jeff King wrote:\n> > On Thu, Nov 18, 2021 at 08:11:04PM +0330, Danial Alihosseini wrote:\n> >\n> >> What did you do before the bug happened? (Steps to reproduce your issue)\n> >> Consider the following project structure\n> >> - data\n> >>   - data1\n> >>     - file1\n> >>     - file1.txt\n> >>   - data2\n> >>     - file2\n> >>     - file2.txt\n> >> - .gitignore\n> >>\n> >>\n> >> `.gitignore` is as follows:\n> >> ```\n> >> data/**\n> >> !data/**/\n> >> !data/**/*.txt\n> >> ```\n> >> What did you expect to happen? (Expected behavior)\n> >>\n> >> I expect all files in `data` folder to be ignored except `.txt` files.\n> >>\n> >> What happened instead? (Actual behavior)\n> >>\n> >> `file1` and `file2` are not ignored.\n> >> Here is the `check-ignore` output:\n> >> ```\n> >> $ git check-ignore -v data/data1/file1\n> >> .gitignore:2:!/data/**/ data/data1/file1\n> >> ```\n> >\n> > Thanks for an easy reproduction. It looks like this changed in\n> > f6526728f9 (dir: select directories correctly, 2021-09-24). Author cc'd.\n>\n> Thanks for the bisect and CC.\n>\n> > The key thing seems to be that the second line of your .gitignore should\n> > match only directories (because of the trailing slash), but no longer\n> > does.\n>\n> Doesn't \"matching only directories\" mean it would match everything\n> within that directory? (It also means that \"data/file\" is not matched,\n> which is still correct.)\n>\n> My interpretation of these patterns is that everything in data/data1/\n> and data/data2/ should not be ignored, making it seem like the change\n> fixed a bug (it definitely changed behavior).\n>\n> Just for extra clarity, this test currently passes:\n>\n> test_expect_success 'directories and ** matches' '\n>         cat >.gitignore <<-\\EOF &&\n>         data/**\n>         !data/**/\n>         !data/**/*.txt\n>         EOF\n>         git check-ignore file \\\n>                 data/file data/data1/file1 data/data1/file1.txt \\\n>                 data/data2/file2 data/data2/file2.txt >actual &&\n>         cat >expect <<-\\EOF &&\n>         data/file\n>         EOF\n>         test_cmp expect actual\n> '\n>\n> but the previous behavior would have passed this test:\n>\n> test_expect_success 'directories and ** matches' '\n>         cat >.gitignore <<-\\EOF &&\n>         data/**\n>         !data/**/\n>         !data/**/*.txt\n>         EOF\n>         git check-ignore file \\\n>                 data/file data/data1/file1.txt \\\n>                 data/data2/file2.txt >actual &&\n>         cat >expect <<-\\EOF &&\n>         data/file\n>         EOF\n>         test_cmp expect actual\n> '\n>\n> I seek more clarity on this. Specifically: if we match a directory\n> then should we not also match the contents within?\n>\n> Thanks,\n> -Stolee\n"},{"id":"441741","messageId":"72fffbff-16f7-fa17-b212-67aae9e1b034@gmail.com","threadId":"56937","inReplyTo":"CACLOEFbY3LwMa2uhc=9jmcGFf0mvWzEM=YityLyFcuGWXVmqbw@mail.gmail.com","subject":"Re: git 2.34.0: Behavior of `**` in gitignore is different from previous versions.","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2021-11-19T14:51:12Z","receivedAt":"2021-11-19T14:51:16Z","isPatch":false,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 11/18/2021 11:01 PM, Danial Alihosseini wrote:\n> Thanks for your follow-up.\n> \n> I wanted to ignore all files in the \"data\" folder except \".txt\" ones.\n> \n> As mentioned in the gitignore doc, there should be a difference\n> between \"**\" and \"**/\".\n> \n>> A trailing \"/**\" matches everything inside. For example, \"abc/**\" matches all files inside directory \"abc\", relative to the location of the .gitignore file, with infinite depth.\n> \n> and,\n>>\n>> A slash followed by two consecutive asterisks then a slash matches zero or more directories. For example, \"a/**/b\" matches \"a/b\", \"a/x/b\", \"a/x/y/b\" and so on.\n> \n> and also,\n>>\n>> It is not possible to re-include a file if a parent directory of that file is excluded.\n\nSo this is the key point of why you can't just omit the !data/**/\nline.\n\nWhat is unclear to me is what exactly \"match a directory\" means.\nIf we ignore a directory, then we ignore everything inside it (until\nanother pattern says we should care about it), but the converse\nshould also hold: if we have a pattern like \"!data/**/\", then that\nshould mean \"include everything inside data/<A>/ where <A> is any\ndirectory name\".\n\nMy inability to form a mental model where the existing behavior\nmatches the documented specification is an indicator that this was\nchanged erroneously. A revert patch is included at the end of this\nmessage.\n\nIf anyone could help clarify my understanding here, then maybe\nthere is room for improving the documentation.\n\n> So, I excluded all files by \"data/**\", re-included just directories\n> (at any depth) by \"!data/**/\" and, re-included \".txt\" files (at any\n> depth) by \"!data/**/*.txt\".\n> However, if I wanted to re-include a directory and all of its\n> contents, I could use something like \"!data/data1/**\", without\n> trailing slash.\n> As I see, there is no separation between \"**\" and \"**/\" in the current version.\n> \n> I think there is another point that the previous behavior test case\n> should be like this:\n> test_expect_success 'directories and ** matches' '\n>     cat >.gitignore <<-\\EOF &&\n>     data/**\n>     !data/**/\n>     !data/**/*.txt\n>     EOF\n>     git check-ignore file \\\n>         data/file data/data1/file1 data/data1/file1.txt \\\n>         data/data2/file2 data/data2/file2.txt >actual &&\n>     cat >expect <<-\\EOF &&\n>     data/file\n>     data/data1/file1\n>     data/data2/file2\n>     EOF\n>     test_cmp expect actual\n> '\n\nThank you for seeing my poorly-edited test and understanding\nwhat it _should_ have been.\n\nHere is a revert including this new test. I further tested\nwith the Scalar functional tests [1] to see if I could find\nwhat I was thinking when making this original change. Since\nthis is so similar to another revert we caught before the\n2.34.0 release, I regret not checking adjacent commits for\na similar \"reverting does not affect the test suite\" case.\n\n---- >8 ----\n\nFrom f833967da83ecd51090ebb75a4b8173a775d16f1 Mon Sep 17 00:00:00 2001\nFrom: Derrick Stolee <dstolee@microsoft.com>\nDate: Fri, 19 Nov 2021 09:13:49 -0500\nSubject: [PATCH] dir: revert \"dir: select directories correctly\"\n\nThis reverts commit f6526728f950cacfd5b5e42bcc65f2c47f3da654.\n\nThe change in f652672 (dir: select directories correctly, 2021-09-24)\ncaused a regression in directory-based matches with non-cone-mode\npatterns, especially for .gitignore patterns. A test is included to\nprevent this regression in the future.\n\nThe commit ed495847 (dir: fix pattern matching on dirs, 2021-09-24) was\nreverted in 5ceb663 (dir: fix directory-matching bug, 2021-11-02) for\nsimilar reasons. Neither commit changed tests, and tests added later in\nthe series continue to pass when these commits are reverted.\n\nReported-by: Danial Alihosseini <danial.alihosseini@gmail.com>\nSigned-off-by: Derrick Stolee <dstolee@microsoft.com>\n---\n dir.c              | 54 +++++-----------------------------------------\n t/t0008-ignores.sh | 17 +++++++++++++++\n 2 files changed, 22 insertions(+), 49 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex 9ea6cfe61c..86afa2eae0 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -1303,44 +1303,6 @@ int match_pathname(const char *pathname, int pathlen,\n \t\t\t\t WM_PATHNAME) == 0;\n }\n \n-static int path_matches_dir_pattern(const char *pathname,\n-\t\t\t\t    int pathlen,\n-\t\t\t\t    struct strbuf **path_parent,\n-\t\t\t\t    int *dtype,\n-\t\t\t\t    struct path_pattern *pattern,\n-\t\t\t\t    struct index_state *istate)\n-{\n-\tif (!*path_parent) {\n-\t\tchar *slash;\n-\t\tCALLOC_ARRAY(*path_parent, 1);\n-\t\tstrbuf_add(*path_parent, pathname, pathlen);\n-\t\tslash = find_last_dir_sep((*path_parent)->buf);\n-\n-\t\tif (slash)\n-\t\t\tstrbuf_setlen(*path_parent, slash - (*path_parent)->buf);\n-\t\telse\n-\t\t\tstrbuf_setlen(*path_parent, 0);\n-\t}\n-\n-\t/*\n-\t * If the parent directory matches the pattern, then we do not\n-\t * need to check for dtype.\n-\t */\n-\tif ((*path_parent)->len &&\n-\t    match_pathname((*path_parent)->buf, (*path_parent)->len,\n-\t\t\t   pattern->base,\n-\t\t\t   pattern->baselen ? pattern->baselen - 1 : 0,\n-\t\t\t   pattern->pattern, pattern->nowildcardlen,\n-\t\t\t   pattern->patternlen, pattern->flags))\n-\t\treturn 1;\n-\n-\t*dtype = resolve_dtype(*dtype, istate, pathname, pathlen);\n-\tif (*dtype != DT_DIR)\n-\t\treturn 0;\n-\n-\treturn 1;\n-}\n-\n /*\n  * Scan the given exclude list in reverse to see whether pathname\n  * should be ignored.  The first match (i.e. the last on the list), if\n@@ -1356,7 +1318,6 @@ static struct path_pattern *last_matching_pattern_from_list(const char *pathname\n {\n \tstruct path_pattern *res = NULL; /* undecided */\n \tint i;\n-\tstruct strbuf *path_parent = NULL;\n \n \tif (!pl->nr)\n \t\treturn NULL;\t/* undefined */\n@@ -1366,10 +1327,11 @@ static struct path_pattern *last_matching_pattern_from_list(const char *pathname\n \t\tconst char *exclude = pattern->pattern;\n \t\tint prefix = pattern->nowildcardlen;\n \n-\t\tif (pattern->flags & PATTERN_FLAG_MUSTBEDIR &&\n-\t\t    !path_matches_dir_pattern(pathname, pathlen, &path_parent,\n-\t\t\t\t\t      dtype, pattern, istate))\n-\t\t\tcontinue;\n+\t\tif (pattern->flags & PATTERN_FLAG_MUSTBEDIR) {\n+\t\t\t*dtype = resolve_dtype(*dtype, istate, pathname, pathlen);\n+\t\t\tif (*dtype != DT_DIR)\n+\t\t\t\tcontinue;\n+\t\t}\n \n \t\tif (pattern->flags & PATTERN_FLAG_NODIR) {\n \t\t\tif (match_basename(basename,\n@@ -1393,12 +1355,6 @@ static struct path_pattern *last_matching_pattern_from_list(const char *pathname\n \t\t\tbreak;\n \t\t}\n \t}\n-\n-\tif (path_parent) {\n-\t\tstrbuf_release(path_parent);\n-\t\tfree(path_parent);\n-\t}\n-\n \treturn res;\n }\n \ndiff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh\nindex 46a5038fed..fce7c7b408 100755\n--- a/t/t0008-ignores.sh\n+++ b/t/t0008-ignores.sh\n@@ -828,6 +828,23 @@ test_expect_success 'exact prefix matching (without root)' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'directories and ** matches' '\n+\tcat >.gitignore <<-\\EOF &&\n+\tdata/**\n+\t!data/**/\n+\t!data/**/*.txt\n+\tEOF\n+\tgit check-ignore file \\\n+\t\tdata/file data/data1/file1 data/data1/file1.txt \\\n+\t\tdata/data2/file2 data/data2/file2.txt >actual &&\n+\tcat >expect <<-\\EOF &&\n+\tdata/file\n+\tdata/data1/file1\n+\tdata/data2/file2\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n ############################################################################\n #\n # test whitespace handling\n-- \n2.34.0.dirty\n\n"},{"id":"441762","messageId":"CACLOEFb41ZLvvkaF6xivFShOJfSHcNupzstiShKHiNCg4DCEhQ@mail.gmail.com","threadId":"56937","inReplyTo":"72fffbff-16f7-fa17-b212-67aae9e1b034@gmail.com","subject":"Re: git 2.34.0: Behavior of `**` in gitignore is different from previous versions.","fromName":"Danial Alihosseini","fromEmail":"danial.alihosseini@gmail.com","sentAt":"2021-11-19T17:06:04Z","receivedAt":"2021-11-19T17:06:18Z","isPatch":false,"sender":{"key":"danial.alihosseini@gmail.com","avatar":null},"body":"Thanks Derrick.\n\n> What is unclear to me is what exactly \"match a directory\" means.\n> If we ignore a directory, then we ignore everything inside it (until\n> another pattern says we should care about it), but the converse\n> should also hold: if we have a pattern like \"!data/**/\", then that\n> should mean \"include everything inside data/<A>/ where <A> is any\n> directory name\".\n\nI think if we consider directory and files separately, it would be more clear.\nIf a pattern in gitignore matches a directory, it is just about that\ndirectory and not about its contents.\nThe contents are ignored by another rule which states:\n> It is not possible to re-include a file if a parent directory of that file is excluded.\n\nIn our test case, the \"data1\" directory is ignored by the \"data/**\" pattern.\nHowever, the contents are ignored by two rules. One of them is that\nthe parent (\"data1\") is not tracked. Another one is that the \"data/**\"\npattern matches them.\nBy adding the \"!data/**/\" pattern, the \"data1\" directory is\nre-included, but its contents are still ignored by the \"data/**\"\npattern.\n\nI hope this will help to make the rules more clear.\n\nThanks\nDanial\n"},{"id":"441779","messageId":"190a1fea-124d-2e85-38ea-9dab87f3e377@kdbg.org","threadId":"56937","inReplyTo":"72fffbff-16f7-fa17-b212-67aae9e1b034@gmail.com","subject":"Re: git 2.34.0: Behavior of `**` in gitignore is different from previous versions.","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2021-11-19T20:05:40Z","receivedAt":"2021-11-19T20:05:45Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 19.11.21 um 15:51 schrieb Derrick Stolee:\n> What is unclear to me is what exactly \"match a directory\" means.\n> If we ignore a directory, then we ignore everything inside it (until\n> another pattern says we should care about it), but the converse\n> should also hold: if we have a pattern like \"!data/**/\", then that\n> should mean \"include everything inside data/<A>/ where <A> is any\n> directory name\".\n> \n> My inability to form a mental model where the existing behavior\n> matches the documented specification is an indicator that this was\n> changed erroneously. A revert patch is included at the end of this\n> message.\n> \n> If anyone could help clarify my understanding here, then maybe\n> there is room for improving the documentation.\n\nYou form a wrong mental model when you start with the grand picture of a\nworking tree. That is, when you say\n\n- here I have theeeeeese many files and directories,\n- and I want to ignore some: foo/**/,\n- but I don't want to ignore others: !bar/**/.\n\nThis forms the wrong mental model because that is not how Git sees the\nworking tree: it never has a grand picture of all of its contents.\n\nGit only ever sees the contents of one directory. When Git determines\nthat a sub-directory is ignored, then that one's contents are never\ninspected, and there is no opportunity to un-ignore some of the\nsub-directory's contents.\n\n-- Hannes\n"},{"id":"441786","messageId":"429375f7-ec3e-596f-5f79-c724570c8397@gmail.com","threadId":"56937","inReplyTo":"190a1fea-124d-2e85-38ea-9dab87f3e377@kdbg.org","subject":"Re: git 2.34.0: Behavior of `**` in gitignore is different from previous versions.","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2021-11-19T20:33:41Z","receivedAt":"2021-11-19T20:33:45Z","isPatch":false,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 11/19/2021 3:05 PM, Johannes Sixt wrote:\n> Am 19.11.21 um 15:51 schrieb Derrick Stolee:\n>> What is unclear to me is what exactly \"match a directory\" means.\n>> If we ignore a directory, then we ignore everything inside it (until\n>> another pattern says we should care about it), but the converse\n>> should also hold: if we have a pattern like \"!data/**/\", then that\n>> should mean \"include everything inside data/<A>/ where <A> is any\n>> directory name\".\n>>\n>> My inability to form a mental model where the existing behavior\n>> matches the documented specification is an indicator that this was\n>> changed erroneously. A revert patch is included at the end of this\n>> message.\n>>\n>> If anyone could help clarify my understanding here, then maybe\n>> there is room for improving the documentation.\n> \n> You form a wrong mental model when you start with the grand picture of a\n> working tree. That is, when you say\n> \n> - here I have theeeeeese many files and directories,\n> - and I want to ignore some: foo/**/,\n> - but I don't want to ignore others: !bar/**/.\n> \n> This forms the wrong mental model because that is not how Git sees the\n> working tree: it never has a grand picture of all of its contents.\n> \n> Git only ever sees the contents of one directory. When Git determines\n> that a sub-directory is ignored, then that one's contents are never\n> inspected, and there is no opportunity to un-ignore some of the\n> sub-directory's contents.\n\nSo the problem is this: I want to know \"I have a file named <X>, and\na certain pattern set, does <X> match the patterns or not?\" but in\nfact it's not just \"check <X> against the patterns in order\" but\nactually \"check every parent directory of <X> in order to see if\nany directory is unmatched, which would preclude any later matches\nto other parents of <X>\"\n\nSo really, to check a path, we really want to first iterate on the\nparent directories. If we get a match on a positive pattern on level\ni, then we check level (i+1) for a match on a negative pattern. If\nwe find that negative pattern match, then continue. If we do not see\na negative match, then we terminate by matching the entire path <X>.\n\nI'm still not seeing a clear way of describing the matching procedure\nhere for a single path, and that's fine. Me understanding is not a\nnecessary condition for fixing this bug.\n\nThanks,\n-Stolee\n"},{"id":"441791","messageId":"211119.86zgpz272g.gmgdl@evledraar.gmail.com","threadId":"56937","inReplyTo":"429375f7-ec3e-596f-5f79-c724570c8397@gmail.com","subject":"Re: git 2.34.0: Behavior of `**` in gitignore is different from previous versions.","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-19T20:57:25Z","receivedAt":"2021-11-19T21:01:52Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Nov 19 2021, Derrick Stolee wrote:\n\n> On 11/19/2021 3:05 PM, Johannes Sixt wrote:\n>> Am 19.11.21 um 15:51 schrieb Derrick Stolee:\n>>> What is unclear to me is what exactly \"match a directory\" means.\n>>> If we ignore a directory, then we ignore everything inside it (until\n>>> another pattern says we should care about it), but the converse\n>>> should also hold: if we have a pattern like \"!data/**/\", then that\n>>> should mean \"include everything inside data/<A>/ where <A> is any\n>>> directory name\".\n>>>\n>>> My inability to form a mental model where the existing behavior\n>>> matches the documented specification is an indicator that this was\n>>> changed erroneously. A revert patch is included at the end of this\n>>> message.\n>>>\n>>> If anyone could help clarify my understanding here, then maybe\n>>> there is room for improving the documentation.\n>> \n>> You form a wrong mental model when you start with the grand picture of a\n>> working tree. That is, when you say\n>> \n>> - here I have theeeeeese many files and directories,\n>> - and I want to ignore some: foo/**/,\n>> - but I don't want to ignore others: !bar/**/.\n>> \n>> This forms the wrong mental model because that is not how Git sees the\n>> working tree: it never has a grand picture of all of its contents.\n>> \n>> Git only ever sees the contents of one directory. When Git determines\n>> that a sub-directory is ignored, then that one's contents are never\n>> inspected, and there is no opportunity to un-ignore some of the\n>> sub-directory's contents.\n>\n> So the problem is this: I want to know \"I have a file named <X>, and\n> a certain pattern set, does <X> match the patterns or not?\" but in\n> fact it's not just \"check <X> against the patterns in order\" but\n> actually \"check every parent directory of <X> in order to see if\n> any directory is unmatched, which would preclude any later matches\n> to other parents of <X>\"\n>\n> So really, to check a path, we really want to first iterate on the\n> parent directories. If we get a match on a positive pattern on level\n> i, then we check level (i+1) for a match on a negative pattern. If\n> we find that negative pattern match, then continue. If we do not see\n> a negative match, then we terminate by matching the entire path <X>.\n>\n> I'm still not seeing a clear way of describing the matching procedure\n> here for a single path, and that's fine. Me understanding is not a\n> necessary condition for fixing this bug.\n\nJust watching this thread on the sidelines I think it would help if it\ncan be distilled down to a wildatch() test that doesn't have to do with\nthe pathspec matching code.\n\nI.e. can you stick the \"should this match?\" into t3070 and it does the\nsame thing, or is this to do with the pathspec-specific sugar on top,\neither that it splits paths and then matches them, that there's some\ninformation about the path type in there added on top, or that it's to\ndo with the specifics of the exclude/include gitignore matching?\n\nFWIW I have some old WIP patches somewhere where I made this match\nbehavior much faster by compiling the (using a mode PCREv2 has) glob\nsyntax into PCRE's, which are then JIT'ed, and matched.\n\nTo do that I had to unpeel this whole truncation of the pattern thing,\nand IIRC it didn't matter for speed (or maybe it did just with the\nwildmatch code?).\n\nMaybe all of this is irrelevant, sorry. I haven't looked into this issue\nat all, just skimmed this growing thread over the past day, maybe some\nof the above helps, or not...\n"},{"id":"441865","messageId":"CAPx1GveHusi8n8n03hUwBu6B3=_j0ZY6FWXWKDs8mLGwHEN-Qg@mail.gmail.com","threadId":"56937","inReplyTo":"429375f7-ec3e-596f-5f79-c724570c8397@gmail.com","subject":"Re: git 2.34.0: Behavior of `**` in gitignore is different from previous versions.","fromName":"Chris Torek","fromEmail":"chris.torek@gmail.com","sentAt":"2021-11-20T22:41:44Z","receivedAt":"2021-11-20T22:42:01Z","isPatch":false,"sender":{"key":"chris.torek@gmail.com","avatar":"https://avatars.githubusercontent.com/u/16826774?v=4"},"body":"On Fri, Nov 19, 2021 at 12:44 PM Derrick Stolee <stolee@gmail.com> wrote:\n> So the problem is this: I want to know \"I have a file named <X>, and\n> a certain pattern set, does <X> match the patterns or not?\" but in\n> fact it's not just \"check <X> against the patterns in order\"  ...\n\nRight.  You're starting with the wrong problem by assuming that\nyou *do* have file named <X> in the first place.  You would not\nhave it at all in some cases.\n\nI've always found this somewhat puzzling, or at least very tricky\nto explain, in the presence of a file that is tracked because it's\nalready in the index, despite having a parent directory that would\nhave caused the file to not have been found.  So the standard\nexplanation -- at least, the one I use -- is this:\n\n * Git opens and reads the working tree directory.  For each file\n   or directory that is actually present here, Git checks it\n   against the ignore rules.  Some rules match only directories\n   and others match both directories and files.  Some rules say\n   \"do ignore\" and some say \"do not ignore\".\n\n * The *last* applicable rule wins.\n\n * If this is a file and the file is ignored, it's ignored.\n   Unless, that is, it's in the index already, because then it's\n   tracked and can't be ignored.\n\n * If this is a directory and the directory is ignored, it's\n   not even opened and read.  It's not in the index because\n   directories are never in the index (at least nominally).\n   If it is opened and read, the entire set of rules here\n   apply recursively.\n\nThis works, but skips over files that are in the index and are in\na directory that won't be read.  So I add one last rule, which is\nthat already-tracked files are checked despite not being scanned\nduring the above process.  (That's because the process above is\nonly used to determine which files to *complain about* at `git\nstatus` time, or auto-add with `git add --all` or the like.)\n\nSo: these files are not ignored ... but is their directory ever\nread?  The actual answer, per testing, is \"no\", which matches with\nthe parenthetical in the paragraph just above.  But no\ndocumentation says this, explicitly, one way or another.\n\nIncidentally, while I have no patches to contribute at this point,\nI do think it would be sensible for Git to read a `.gitignore`\nthat says:\n\n    *\n    !a/b/c\n\nas meaning:\n\n    *\n    !a/\n    !a/b/\n    !a/b/c\n\nThat is, declaring an un-ignored file within some ignored\ndirectory should automatically imply that we *must* un-ignore the\nparents.  It doesn't seem like it should be that hard to insert\nsome extra rules internally here (though without `*` we'd want\nto ignore `a/*`, i.e., the above is optimized beyond what I\nwould expect from automated rule insertion).\n\nChris\n"},{"id":"441866","messageId":"xmqq5ysmnxnx.fsf@gitster.g","threadId":"56937","inReplyTo":"CAPx1GveHusi8n8n03hUwBu6B3=_j0ZY6FWXWKDs8mLGwHEN-Qg@mail.gmail.com","subject":"Re: git 2.34.0: Behavior of `**` in gitignore is different from previous versions.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-11-21T00:46:10Z","receivedAt":"2021-11-21T00:46:15Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Chris Torek <chris.torek@gmail.com> writes:\n\n> ...  So the standard\n> explanation -- at least, the one I use -- is this:\n>\n>  * Git opens and reads the working tree directory.  For each file\n>    or directory that is actually present here, Git checks it\n>    against the ignore rules.  Some rules match only directories\n>    and others match both directories and files.  Some rules say\n>    \"do ignore\" and some say \"do not ignore\".\n>\n>  * The *last* applicable rule wins.\n>\n>  * If this is a file and the file is ignored, it's ignored.\n>    Unless, that is, it's in the index already, because then it's\n>    tracked and can't be ignored.\n>\n>  * If this is a directory and the directory is ignored, it's\n>    not even opened and read.  It's not in the index because\n>    directories are never in the index (at least nominally).\n>    If it is opened and read, the entire set of rules here\n>    apply recursively.\n>\n> This works, but skips over files that are in the index and are in\n> a directory that won't be read.  So I add one last rule, which is\n\nAll of the above is sensible.  If you deal with a path that is in\nthe index upfront, you can simplify the later rules somewhat, I\nwould think.  I.e. add a first rule before everything else that\nsays:\n\n * A file in the index is not ignored.  Everything below is about a\n   path not in the index.\n\nThen your third rule can lose \"Unless...\", and you do not have to\nadd one last rule outside the bulleted list, either.\n\n;-)\n\n"},{"id":"442140","messageId":"09cbc27d-8fdf-ed84-716f-23fd8dbb11de@iee.email","threadId":"56937","inReplyTo":"xmqq5ysmnxnx.fsf@gitster.g","subject":"Re: git 2.34.0: Behavior of `**` in gitignore is different from previous versions.","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2021-11-23T12:21:00Z","receivedAt":"2021-11-23T12:21:13Z","isPatch":false,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"On 21/11/2021 00:46, Junio C Hamano wrote:\n> Chris Torek <chris.torek@gmail.com> writes:\n>\n>> ...  So the standard\n>> explanation -- at least, the one I use -- is this:\n>>\n>>  * Git opens and reads the working tree directory.  For each file\n>>    or directory that is actually present here, Git checks it\n>>    against the ignore rules.  Some rules match only directories\n>>    and others match both directories and files.  Some rules say\n>>    \"do ignore\" and some say \"do not ignore\".\n>>\n>>  * The *last* applicable rule wins.\n>>\n>>  * If this is a file and the file is ignored, it's ignored.\n>>    Unless, that is, it's in the index already, because then it's\n>>    tracked and can't be ignored.\n>>\n>>  * If this is a directory and the directory is ignored, it's\n>>    not even opened and read.  It's not in the index because\n>>    directories are never in the index (at least nominally).\n>>    If it is opened and read, the entire set of rules here\n>>    apply recursively.\n>>\n>> This works, but skips over files that are in the index and are in\n>> a directory that won't be read.  So I add one last rule, which is\n> All of the above is sensible.  If you deal with a path that is in\n> the index upfront, you can simplify the later rules somewhat, I\n> would think.  I.e. add a first rule before everything else that\n> says:\n>\n>  * A file in the index is not ignored.  Everything below is about a\n>    path not in the index.\n>\n> Then your third rule can lose \"Unless...\", and you do not have to\n> add one last rule outside the bulleted list, either.\n>\n> ;-)\n>\nPerhaps, the clarifications are an opportunity to improve the\ndocumentation, should the Danial think that the explanation would have\nhelped. The 'ignore' matching rules does appear to show up quite often\non the list.\n\nAny thoughts, Danial?\n\nPhilip\n"},{"id":"442201","messageId":"CACLOEFYoqDsAhhHEMGXvtrkJLSpKdj8Rp1WtoBrPtJZbZN-s7Q@mail.gmail.com","threadId":"56937","inReplyTo":"09cbc27d-8fdf-ed84-716f-23fd8dbb11de@iee.email","subject":"Re: git 2.34.0: Behavior of `**` in gitignore is different from previous versions.","fromName":"Danial Alihosseini","fromEmail":"danial.alihosseini@gmail.com","sentAt":"2021-11-23T21:13:29Z","receivedAt":"2021-11-23T21:13:43Z","isPatch":false,"sender":{"key":"danial.alihosseini@gmail.com","avatar":null},"body":">  * Git opens and reads the working tree directory.  For each file\n>\n>    or directory that is actually present here, Git checks it\n>\n>    against the ignore rules.  Some rules match only directories\n>\n>    and others match both directories and files.  Some rules say\n>    \"do ignore\" and some say \"do not ignore\".\n>  * The *last* applicable rule wins.\n>  * If this is a file and the file is ignored, it's ignored.\n>    Unless, that is, it's in the index already, because then it's\n>    tracked and can't be ignored.\n>  * If this is a directory and the directory is ignored, it's\n>    not even opened and read.  It's not in the index because\n>    directories are never in the index (at least nominally).\n>    If it is opened and read, the entire set of rules here\n>    apply recursively.\n\n> I do think it would be sensible for Git to read a `.gitignore`\n>\n> that says:\n>\n>     *\n>\n>     !a/b/c\n>\n> as meaning:\n>     *\n>     !a/\n>     !a/b/\n>     !a/b/c\n> That is, declaring an un-ignored file within some ignored\n> directory should automatically imply that we *must* un-ignore the\n> parents.\n\nThanks, Chris. Yes, it mostly makes sense to me too.\nI think this will better describe the gitignore behavior.\nHowever, checking the indexed files at first may enhance it.\n\nIn addition to that, there should be an exception about negating patterns.\nFor example, for the \"!**/file.txt\" pattern, it might be tricky to\nre-include all possible parents of \"file.txt\" files. You have to\ntraverse directories to reach files and find out which parents to\nun-ignore. Any negating pattern with \"**\" at the beginning or in the\nmiddle of the pattern can make trouble.\nThe existing rule which states the file cannot be re-included in case\nof ignored parents may help to prevent this.\n\nThanks\nDanial\n"}]}