{"thread":{"id":"41721","subject":"bug: sparse config interpretation incorrectness in 2.8.0-rc2","startedAt":"2016-03-17T00:09:23Z","lastAt":"2016-03-19T01:03:46Z","messageCount":30,"participants":["Durham Goode","Duy Nguyen","Junio C Hamano","Nguyễn Thái Ngọc Duy","Johannes Schindelin","Eric Sunshine"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"280990","messageId":"56E9F5B3.6030903@fb.com","threadId":"41721","inReplyTo":null,"subject":"bug: sparse config interpretation incorrectness in 2.8.0-rc2","fromName":"Durham Goode","fromEmail":"durham@fb.com","sentAt":"2016-03-17T00:09:23Z","receivedAt":"2016-03-17T00:09:23Z","isPatch":false,"sender":{"key":"durham@fb.com","avatar":null},"body":"Using git 2.8.0-rc2, given a repo with the following files:\n\n- one/hideme\n- one/donthide\n- two/foo\n\nA sparse config of:\n\ncat > .git/info/sparse-checkout <<EOF\n/*\n!one/hideme\nEOF\n\nResults in a repository that only has `one/donthide` in it.  I would \nexpect `two/foo`to be present as well.  This worked in 2.6, and \nbisecting it points to d589a67eceacd1cc171bbe94906ca7c9a0edd8c5 \"dir.c: \ndon't exclude whole dir prematurely\" (author cc'd).\n\nThe script I used to repro and for bisecting is pasted below:\n\n\n\n\n\n#!/bin/bash\n\nset -x\nrm -rf sparse-test\nGIT=git\n\n$GIT init sparse-test\ncd sparse-test\n$GIT config --add core.sparsecheckout true\n\nmkdir one two\ntouch one/hideme\ntouch one/donthide\ntouch two/foo\n\n$GIT add .\n$GIT commit -m \"initial commit\"\n$GIT read-tree --reset -u HEAD\n\nmkdir .git/info\ncat > .git/info/sparse-checkout <<EOF\n/*\n!one/hideme\nEOF\n$GIT read-tree --reset -u HEAD\n\nls -R one two\nset +x\necho\necho expected: see one/donthide and two/foo\necho actual: see only one/donthide\n\n[ -d two ] && exit 0\nexit 1\n"},{"id":"280992","messageId":"20160317005632.GA17778@duynguyen-vnpc.dek-tpc.internal","threadId":"41721","inReplyTo":"56E9F5B3.6030903@fb.com","subject":"Re: bug: sparse config interpretation incorrectness in 2.8.0-rc2","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-03-17T00:56:32Z","receivedAt":"2016-03-17T00:56:32Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Mar 16, 2016 at 05:09:23PM -0700, Durham Goode wrote:\n> Using git 2.8.0-rc2, given a repo with the following files:\n> \n> - one/hideme\n> - one/donthide\n> - two/foo\n> \n> A sparse config of:\n> \n> cat > .git/info/sparse-checkout <<EOF\n> /*\n> !one/hideme\n> EOF\n> \n> Results in a repository that only has `one/donthide` in it.  I would\n> expect `two/foo`to be present as well.  This worked in 2.6, and\n> bisecting it points to d589a67eceacd1cc171bbe94906ca7c9a0edd8c5\n> \"dir.c: don't exclude whole dir prematurely\" (author cc'd).\n\nThank you. This should fix it. I think I understand why it goes\nwrong. I'm going to run some more tests and post a proper patch later.\n\n-- 8< --\ndiff --git a/dir.c b/dir.c\nindex 69e0be6..77f38a5 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -1027,7 +1027,6 @@ static struct exclude *last_exclude_matching_from_list(const char *pathname,\n \t\t\t\texc = x;\n \t\t\t\tbreak;\n \t\t\t}\n-\t\t\tcontinue;\n \t\t}\n \n \t\tif (x->flags & EXC_FLAG_MUSTBEDIR) {\n-- 8< --\n--\nDuy\n"},{"id":"281005","messageId":"56EA5378.4070001@fb.com","threadId":"41721","inReplyTo":"20160317005632.GA17778@duynguyen-vnpc.dek-tpc.internal","subject":"Re: bug: sparse config interpretation incorrectness in 2.8.0-rc2","fromName":"Durham Goode","fromEmail":"durham@fb.com","sentAt":"2016-03-17T06:49:28Z","receivedAt":"2016-03-17T06:49:28Z","isPatch":false,"sender":{"key":"durham@fb.com","avatar":null},"body":"On 3/16/16 5:56 PM, Duy Nguyen wrote:\n> On Wed, Mar 16, 2016 at 05:09:23PM -0700, Durham Goode wrote:\n>> Using git 2.8.0-rc2, given a repo with the following files:\n>>\n>> - one/hideme\n>> - one/donthide\n>> - two/foo\n>>\n>> A sparse config of:\n>>\n>> cat > .git/info/sparse-checkout <<EOF\n>> /*\n>> !one/hideme\n>> EOF\n>>\n>> Results in a repository that only has `one/donthide` in it.  I would\n>> expect `two/foo`to be present as well.  This worked in 2.6, and\n>> bisecting it points to d589a67eceacd1cc171bbe94906ca7c9a0edd8c5\n>> \"dir.c: don't exclude whole dir prematurely\" (author cc'd).\n> Thank you. This should fix it. I think I understand why it goes\n> wrong. I'm going to run some more tests and post a proper patch later.\n>\n> -- 8< --\n> diff --git a/dir.c b/dir.c\n> index 69e0be6..77f38a5 100644\n> --- a/dir.c\n> +++ b/dir.c\n> @@ -1027,7 +1027,6 @@ static struct exclude *last_exclude_matching_from_list(const char *pathname,\n>   \t\t\t\texc = x;\n>   \t\t\t\tbreak;\n>   \t\t\t}\n> -\t\t\tcontinue;\n>   \t\t}\n>   \n>   \t\tif (x->flags & EXC_FLAG_MUSTBEDIR) {\n> -- 8< --\n> --\n> Duy\nTo provide some perspective on the severity of this bug: all of our \nusers who were using sparse checkouts had their working copy mostly \ndeleted when they did a checkout after we upgraded to 2.8.0-rc2.  So I'd \nthink this is a fix that should get in to 2.8.\n"},{"id":"281006","messageId":"xmqqfuvp36wu.fsf@gitster.mtv.corp.google.com","threadId":"41721","inReplyTo":"56E9F5B3.6030903@fb.com","subject":"Re: bug: sparse config interpretation incorrectness in 2.8.0-rc2","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-03-17T07:22:41Z","receivedAt":"2016-03-17T07:22:41Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Durham Goode <durham@fb.com> writes:\n\n> Using git 2.8.0-rc2, given a repo with the following files:\n>\n> - one/hideme\n> - one/donthide\n> - two/foo\n>\n> A sparse config of:\n>\n> cat > .git/info/sparse-checkout <<EOF\n> /*\n> !one/hideme\n> EOF\n>\n> Results in a repository that only has `one/donthide` in it.  I would\n> expect `two/foo`to be present as well.  This worked in 2.6,...\n\n2.6 is a tad too old as a reference, as the \"!reinclusion\" has been\nin flux in recent releases.  Can you test these?\n\n - 2.7.0\n - 2.7.1\n - e79112d\n\nI suspect that v2.7.0 would be broken, v2.7.1 is OK and e79112d\nwould also be OK (I am guessing this from [1]).  e79112d is the last\nversion on the 'master' branch without the topic that contains the\ncommit you bisected down to, but between 2.7.0 and 2.7.1 there was a\nreversion of a commit that introduced the original issue.\n\nThe commit you bisected down to that is on 'master', IIUC, was a\n(faulty) attempt to fix the breakage in a different way.\n\n\n[Reference]\n\n*1* http://thread.gmane.org/gmane.comp.version-control.git/288228/focus=288273\n"},{"id":"281011","messageId":"xmqq37rp35k1.fsf@gitster.mtv.corp.google.com","threadId":"41721","inReplyTo":"20160317005632.GA17778@duynguyen-vnpc.dek-tpc.internal","subject":"Re: bug: sparse config interpretation incorrectness in 2.8.0-rc2","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-03-17T07:51:58Z","receivedAt":"2016-03-17T07:51:58Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen <pclouds@gmail.com> writes:\n\n> On Wed, Mar 16, 2016 at 05:09:23PM -0700, Durham Goode wrote:\n>> Using git 2.8.0-rc2, given a repo with the following files:\n>> \n>> - one/hideme\n>> - one/donthide\n>> - two/foo\n>> \n>> A sparse config of:\n>> \n>> cat > .git/info/sparse-checkout <<EOF\n>> /*\n>> !one/hideme\n>> EOF\n>> \n>> Results in a repository that only has `one/donthide` in it.  I would\n>> expect `two/foo`to be present as well.  This worked in 2.6, and\n>> bisecting it points to d589a67eceacd1cc171bbe94906ca7c9a0edd8c5\n>> \"dir.c: don't exclude whole dir prematurely\" (author cc'd).\n>\n> Thank you. This should fix it. I think I understand why it goes\n> wrong. I'm going to run some more tests and post a proper patch later.\n\nI admit that I've always considered \"sparse checkout\" was an\nuninteresting experimental feature that I do not want to pay too\nmuch attention to, and the only review I did carefully myself was to\nmake sure that patches around that area would not change the\nbehaviour of the original code in repositories that do not use that\nfeature, so please do not laugh too loudly at me if I ask the\nobvious ;-) So the pattern list in that info/sparse-checkout file\nshares the same logic with the gitignore mechanism, and the paths\nthat would have been \"ignored\" if the pattern list were in the\n.gitignore file are the ones that would be included in the checkout?\n\nIf so, among the three paths Durham lists:\n\n * one/hideme matches !one/hideme the last, i.e. it would not be\n   ignored if the pattern appeared in .gitignore, hence it should not\n   be in the checkout;\n\n * one/donthide matches * the last, i.e. would be ignored, hence it\n   should be in the checkout;\n\n * two/foo (or really anything) matches * the last, i.e. would be\n   ignored, hence it should be in the checkout.\n\nAm I reading the bug correctly?\n\n> -- 8< --\n> diff --git a/dir.c b/dir.c\n> index 69e0be6..77f38a5 100644\n> --- a/dir.c\n> +++ b/dir.c\n> @@ -1027,7 +1027,6 @@ static struct exclude *last_exclude_matching_from_list(const char *pathname,\n>  \t\t\t\texc = x;\n>  \t\t\t\tbreak;\n>  \t\t\t}\n> -\t\t\tcontinue;\n>  \t\t}\n>  \n>  \t\tif (x->flags & EXC_FLAG_MUSTBEDIR) {\n> -- 8< --\n> --\n> Duy\n"},{"id":"281020","messageId":"CACsJy8A2D8CqeWZ6CFdTVXzXiWdonBE=3tWFPpe7-ZmDtNGYNw@mail.gmail.com","threadId":"41721","inReplyTo":"xmqq37rp35k1.fsf@gitster.mtv.corp.google.com","subject":"Re: bug: sparse config interpretation incorrectness in 2.8.0-rc2","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-03-17T10:17:43Z","receivedAt":"2016-03-17T10:17:43Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Mar 17, 2016 at 2:51 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Duy Nguyen <pclouds@gmail.com> writes:\n>\n>> On Wed, Mar 16, 2016 at 05:09:23PM -0700, Durham Goode wrote:\n>>> Using git 2.8.0-rc2, given a repo with the following files:\n>>>\n>>> - one/hideme\n>>> - one/donthide\n>>> - two/foo\n>>>\n>>> A sparse config of:\n>>>\n>>> cat > .git/info/sparse-checkout <<EOF\n>>> /*\n>>> !one/hideme\n>>> EOF\n>>>\n>>> Results in a repository that only has `one/donthide` in it.  I would\n>>> expect `two/foo`to be present as well.  This worked in 2.6, and\n>>> bisecting it points to d589a67eceacd1cc171bbe94906ca7c9a0edd8c5\n>>> \"dir.c: don't exclude whole dir prematurely\" (author cc'd).\n>>\n>> Thank you. This should fix it. I think I understand why it goes\n>> wrong. I'm going to run some more tests and post a proper patch later.\n>\n> I admit that I've always considered \"sparse checkout\" was an\n> uninteresting experimental feature that I do not want to pay too\n> much attention to, and the only review I did carefully myself was to\n> make sure that patches around that area would not change the\n> behaviour of the original code in repositories that do not use that\n> feature, so please do not laugh too loudly at me if I ask the\n> obvious ;-)\n\nGood news for you is there's \"sparse checkout v2\" in the work, that\nwould not rely on exclude engine and should be both faster and more\nelegant. That should reduce \"sparse checkout v1\" usage to really small\ncases.\n\n> So the pattern list in that info/sparse-checkout file\n> shares the same logic with the gitignore mechanism, and the paths\n> that would have been \"ignored\" if the pattern list were in the\n> .gitignore file are the ones that would be included in the checkout?\n>\n> If so, among the three paths Durham lists:\n>\n>  * one/hideme matches !one/hideme the last, i.e. it would not be\n>    ignored if the pattern appeared in .gitignore, hence it should not\n>    be in the checkout;\n>\n>  * one/donthide matches * the last, i.e. would be ignored, hence it\n>    should be in the checkout;\n>\n>  * two/foo (or really anything) matches * the last, i.e. would be\n>    ignored, hence it should be in the checkout.\n>\n> Am I reading the bug correctly?\n\nYes that's how I read it too. The patterns basically say \"include\neverything (/*) except one/hideme (!one/hideme)\" so two/foo should be\nincluded.\n\n>> -- 8< --\n>> diff --git a/dir.c b/dir.c\n>> index 69e0be6..77f38a5 100644\n>> --- a/dir.c\n>> +++ b/dir.c\n>> @@ -1027,7 +1027,6 @@ static struct exclude *last_exclude_matching_from_list(const char *pathname,\n>>                               exc = x;\n>>                               break;\n>>                       }\n>> -                     continue;\n>>               }\n>>\n>>               if (x->flags & EXC_FLAG_MUSTBEDIR) {\n>> -- 8< --\n>> --\n>> Duy\n\n\n\n-- \nDuy\n"},{"id":"281027","messageId":"1458218744-15810-1-git-send-email-pclouds@gmail.com","threadId":"41721","inReplyTo":"56E9F5B3.6030903@fb.com","subject":"[PATCH 1/2] dir.c: fix bug in 'nd/exclusion-regression-fix' topic","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-03-17T12:45:41Z","receivedAt":"2016-03-17T12:45:41Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"The topic in question introduces \"sticky\" path list for this purpose:\ngiven a path 'abc/def', if 'abc' already matches a pattern X, it's added\nto X's sticky path list. When we need to check if 'abc/def' matches\npattern X and see that 'abc' is already in the list, we conclude right\naway that 'abc/def' also matches X.\n\nThe short reason (*) for sticky path list is to workaround limitations\nof matching code that will return \"not match\" when we compare\n'abc/def' and pattern X.\n\nThe bug is in this code. Not only it does \"when we need to check if\n'abc/def' matches...\", it does an extra thing: if 'foo/bar' is _not_ in\nthe list, return 'not matched' by bypassing all matching code with the\n\"continue;\" statement. It should let the remaining code decide match\nstatus instead.\n\nThis bug affects both .gitignore and sparse checkout, but it's reported\nas a sparse checkout bug, so let's examine how it happens. The\nsparse-checkout pattern has two rules\n\n    /*\n    !one/hideme\n\nand the worktree has three tracked files, one/hideme, one/showme and\ntwo/showme. What happens is this\n\n* check \"one\", it matches the first pattern -> positive -> keep\n  examining.\n\n*1* \"thanks\" to 'nd/exclusion-regression-fix' we detect this pair of\n  patterns, so we put \"one\" in the sticky list of pattern \"/*\"\n\n* enter \"one\", check \"one/hideme\", it matches the second pattern\n  first (we search from bottom up) -> negative -> excluded\n\n* check \"one/showme\", it does not match the second pattern.\n\n*2* We then check it against the first pattern and notice the sticky list\n  that includes \"one\", so we decide right away that \"one/showme\" is\n  included.\n\n* leave \"one\", check \"two\" which does not match the second pattern.\n\n*3* then we check \"two\" against the first pattern and notice that this\n   pattern has a non-empty sticky list, which contains \"one\", not \"two\".\n   This bug kicks in and bypasses the true matching logic for pattern\n   \"/*\". As a result, we exclude \"two/showme\".\n\nOne may notice that the order of these steps matter. If *3* occurs\nbefore *1*, then the sticky list at that moment is empty and the bug\ndoes not kick in. Sparse checkout always examines entries in\nalphabetical order, so \"abc/showme\" would be examined before \"one\" and\nnot hit this bug!\n\nThe last remark is important when we move to .gitignore. We receive the\nlist of entries with readdir() and cannot control the order of\nentries. Which means we can't write a test for .gitignore that will\nreliably fail without this fix. Which is why this patch only adds a test\nfor sparse checkout, even though the same above steps happen to\n.gitignore.\n\n(*) The problem is known and will be fixed later and described in\ndetail then. For this commit, it's sufficient to see the following\nlink because the long reason is really long:\n\nhttp://article.gmane.org/gmane.comp.version-control.git/288479\n\nReported-by: Durham Goode <durham@fb.com>\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n dir.c                                |  1 -\n t/t1011-read-tree-sparse-checkout.sh | 20 ++++++++++++++++++++\n 2 files changed, 20 insertions(+), 1 deletion(-)\n\ndiff --git a/dir.c b/dir.c\nindex 69e0be6..77f38a5 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -1027,7 +1027,6 @@ static struct exclude *last_exclude_matching_from_list(const char *pathname,\n \t\t\t\texc = x;\n \t\t\t\tbreak;\n \t\t\t}\n-\t\t\tcontinue;\n \t\t}\n \n \t\tif (x->flags & EXC_FLAG_MUSTBEDIR) {\ndiff --git a/t/t1011-read-tree-sparse-checkout.sh b/t/t1011-read-tree-sparse-checkout.sh\nindex 0c74bee..ecc5e93 100755\n--- a/t/t1011-read-tree-sparse-checkout.sh\n+++ b/t/t1011-read-tree-sparse-checkout.sh\n@@ -274,4 +274,24 @@ test_expect_success 'checkout with --ignore-skip-worktree-bits' '\n \tgit diff --exit-code HEAD\n '\n \n+test_expect_success 'sparse checkout and dir.c sticky bits' '\n+\tgit init sticky &&\n+\t(\n+\t\tcd sticky &&\n+\t\tmkdir one two &&\n+\t\ttouch one/hideme one/showme two/showme &&\n+\t\tgit add . &&\n+\t\tgit commit -m initial &&\n+\t\tcat >.git/info/sparse-checkout <<-\\EOF &&\n+\t\t/*\n+\t\t!one/hideme\n+\t\tEOF\n+\t\tgit config core.sparsecheckout true &&\n+\t\tgit checkout &&\n+\t\ttest_path_is_missing one/hideme &&\n+\t\ttest_path_is_file    one/showme &&\n+\t\ttest_path_is_file    two/showme\n+\t)\n+'\n+\n test_done\n-- \n2.8.0.rc0.210.gd302cd2\n"},{"id":"281028","messageId":"1458218744-15810-2-git-send-email-pclouds@gmail.com","threadId":"41721","inReplyTo":"1458218744-15810-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 1/2] dir.c: fix bug in 'nd/exclusion-regression-fix' topic","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-03-17T12:45:42Z","receivedAt":"2016-03-17T12:45:42Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"The topic in question introduces \"sticky\" path list for this purpose:\ngiven a path 'abc/def', if 'abc' already matches a pattern X, it's added\nto X's sticky path list. When we need to check if 'abc/def' matches\npattern X and see that 'abc' is already in the list, we conclude right\naway that 'abc/def' also matches X.\n\nThe short reason for sticky path list is to workaround limitations of\nmatching code (*) that will return \"not match\" when we compare 'abc/def'\nand pattern X.\n\nThe bug is in this code. Not only it does \"when we need to check if\n'abc/def' matches...\", it does an extra thing: if 'foo/bar' is _not_ in\nthe list, return 'not matched' by bypassing all matching code with the\n\"continue;\" statement. It should let the remaining code decide match\nstatus instead.\n\nThis bug affects both .gitignore and sparse checkout, but it's reported\nas a sparse checkout bug, so let's examine how it happens. The\nsparse-checkout pattern has two rules\n\n    /*\n    !one/hideme\n\nand the worktree has three tracked files, one/hideme, one/showme and\ntwo/showme. What happens is this\n\n* check \"one\", it matches the first pattern -> positive -> keep\n  examining.\n\n*1* \"thanks\" to 'nd/exclusion-regression-fix' we detect this pair of\n  patterns, so we put \"one\" in the sticky list of pattern \"/*\"\n\n* enter \"one\", check \"one/hideme\", it matches the second pattern\n  first (we search from bottom up) -> negative -> excluded\n\n* check \"one/showme\", it does not match the second pattern.\n\n*2* We then check it against the first pattern and notice the sticky list\n  that includes \"one\", so we decide right away that \"one/showme\" is\n  included.\n\n* leave \"one\", check \"two\" which does not match the second pattern.\n\n*3* then we check \"two\" against the first pattern and notice that this\n   pattern has a non-empty sticky list, which contains \"one\", not \"two\".\n   This bug kicks in and bypasses the true matching logic for pattern\n   \"/*\". As a result, we exclude \"two/showme\".\n\nOne may notice that the order of these steps matter. If *3* occurs\nbefore *1*, then the sticky list at that moment is empty and the bug\ndoes not kick in. Sparse checkout always examines entries in\nalphabetical order, so \"abc/showme\" would be examined before \"one\" and\nnot hit this bug!\n\nThe last remark is important when we move to .gitignore. We receive the\nlist of entries with readdir() and cannot control the order of\nentries. Which means we can't write a test for .gitignore that will\nreliably fail without this fix. Which is why this patch only adds a test\nfor sparse checkout, even though the same above steps happen to\n.gitignore.\n\n(*) which will be fixed later and described in detail then. For this\ncommit, it's sufficient to see the following link because the\nexplanation is long:\n\nhttp://article.gmane.org/gmane.comp.version-control.git/288479\n\nReported-by: Durham Goode <durham@fb.com>\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n dir.c                                |  1 -\n t/t1011-read-tree-sparse-checkout.sh | 20 ++++++++++++++++++++\n 2 files changed, 20 insertions(+), 1 deletion(-)\n\ndiff --git a/dir.c b/dir.c\nindex 69e0be6..77f38a5 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -1027,7 +1027,6 @@ static struct exclude *last_exclude_matching_from_list(const char *pathname,\n \t\t\t\texc = x;\n \t\t\t\tbreak;\n \t\t\t}\n-\t\t\tcontinue;\n \t\t}\n \n \t\tif (x->flags & EXC_FLAG_MUSTBEDIR) {\ndiff --git a/t/t1011-read-tree-sparse-checkout.sh b/t/t1011-read-tree-sparse-checkout.sh\nindex 0c74bee..ecc5e93 100755\n--- a/t/t1011-read-tree-sparse-checkout.sh\n+++ b/t/t1011-read-tree-sparse-checkout.sh\n@@ -274,4 +274,24 @@ test_expect_success 'checkout with --ignore-skip-worktree-bits' '\n \tgit diff --exit-code HEAD\n '\n \n+test_expect_success 'sparse checkout and dir.c sticky bits' '\n+\tgit init sticky &&\n+\t(\n+\t\tcd sticky &&\n+\t\tmkdir one two &&\n+\t\ttouch one/hideme one/showme two/showme &&\n+\t\tgit add . &&\n+\t\tgit commit -m initial &&\n+\t\tcat >.git/info/sparse-checkout <<-\\EOF &&\n+\t\t/*\n+\t\t!one/hideme\n+\t\tEOF\n+\t\tgit config core.sparsecheckout true &&\n+\t\tgit checkout &&\n+\t\ttest_path_is_missing one/hideme &&\n+\t\ttest_path_is_file    one/showme &&\n+\t\ttest_path_is_file    two/showme\n+\t)\n+'\n+\n test_done\n-- \n2.8.0.rc0.210.gd302cd2\n"},{"id":"281029","messageId":"1458218744-15810-3-git-send-email-pclouds@gmail.com","threadId":"41721","inReplyTo":"1458218744-15810-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 2/2] dir.c: correct \"stuck\" logging logic","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-03-17T12:45:43Z","receivedAt":"2016-03-17T12:45:43Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Before the last patch, the loop in last_exclude_matching_from_list()\nlooks like this (greatly simplified of course)\n\n   exc = NULL;\n   for (...) {\n      if (sticky_paths.nr) {\n         if (matched) {\n            exc = non-NULL;\n            break;\n         }\n         continue;\n      }\n      ...\n   }\n\nWith this loop, if sticky_paths.nr is non-zero and exc is not NULL, we\nknow we have found a sticky path and can log \" (stuck)\".\n\nWith the last patch, the \"continue;\" line is removed and that won't work\nanymore. So explicitly keep track of when to print \" (stuck)\".\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n dir.c                                | 4 +++-\n t/t1011-read-tree-sparse-checkout.sh | 8 ++++++--\n 2 files changed, 9 insertions(+), 3 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex 77f38a5..2028094 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -1005,6 +1005,7 @@ static struct exclude *last_exclude_matching_from_list(const char *pathname,\n {\n \tstruct exclude *exc = NULL; /* undecided */\n \tint i, maybe_descend = 0;\n+\tconst char *stuck = \"\";\n \n \tif (!el->nr)\n \t\treturn NULL;\t/* undefined */\n@@ -1024,6 +1025,7 @@ static struct exclude *last_exclude_matching_from_list(const char *pathname,\n \t\t\tif (*dtype == DT_UNKNOWN)\n \t\t\t\t*dtype = get_dtype(NULL, pathname, pathlen);\n \t\t\tif (match_sticky(x, pathname, pathlen, *dtype)) {\n+\t\t\t\tstuck = \" (stuck)\";\n \t\t\t\texc = x;\n \t\t\t\tbreak;\n \t\t\t}\n@@ -1093,7 +1095,7 @@ static struct exclude *last_exclude_matching_from_list(const char *pathname,\n \ttrace_printf_key(&trace_exclude, \"exclude: %.*s vs %s at line %d => %s%s\\n\",\n \t\t\t pathlen, pathname, exc->pattern, exc->srcpos,\n \t\t\t exc->flags & EXC_FLAG_NEGATIVE ? \"no\" : \"yes\",\n-\t\t\t exc->sticky_paths.nr ? \" (stuck)\" : \"\");\n+\t\t\t stuck);\n \treturn exc;\n }\n \ndiff --git a/t/t1011-read-tree-sparse-checkout.sh b/t/t1011-read-tree-sparse-checkout.sh\nindex ecc5e93..f0b856f 100755\n--- a/t/t1011-read-tree-sparse-checkout.sh\n+++ b/t/t1011-read-tree-sparse-checkout.sh\n@@ -287,10 +287,14 @@ test_expect_success 'sparse checkout and dir.c sticky bits' '\n \t\t!one/hideme\n \t\tEOF\n \t\tgit config core.sparsecheckout true &&\n-\t\tgit checkout &&\n+\t\tGIT_TRACE_EXCLUDE=2 git checkout 2>&1 | grep stuck >stuck-list &&\n \t\ttest_path_is_missing one/hideme &&\n \t\ttest_path_is_file    one/showme &&\n-\t\ttest_path_is_file    two/showme\n+\t\ttest_path_is_file    two/showme &&\n+\t\tcat >expected <<-\\EOF &&\n+\t\texclude: one/showme vs /* at line 1 => yes (stuck)\n+\t\tEOF\n+\t\ttest_cmp expected stuck-list\n \t)\n '\n \n-- \n2.8.0.rc0.210.gd302cd2\n"},{"id":"281030","messageId":"1458218744-15810-4-git-send-email-pclouds@gmail.com","threadId":"41721","inReplyTo":"1458218744-15810-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 2/2] dir.c: correct \"stuck\" logging logic","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-03-17T12:45:44Z","receivedAt":"2016-03-17T12:45:44Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Before the last patch, the loop in last_exclude_matching_from_list()\nlooks like this (greatly simplified of course)\n\n   exc = NULL;\n   for (...) {\n      if (sticky_paths.nr) {\n         if (matched) {\n            exc = something;\n            break;\n         }\n         continue;\n      }\n      ...\n   }\n\nWith this loop, if sticky_paths.nr is non-zero and exc is not NULL, we\nknow we have found a sticky path and can log \" (stuck)\".\n\nWith the last patch, the \"continue;\" line is removed and that won't work\nanymore. So explicitly keep track of when to print \" (stuck)\".\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n dir.c                                | 4 +++-\n t/t1011-read-tree-sparse-checkout.sh | 8 ++++++--\n 2 files changed, 9 insertions(+), 3 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex 77f38a5..2028094 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -1005,6 +1005,7 @@ static struct exclude *last_exclude_matching_from_list(const char *pathname,\n {\n \tstruct exclude *exc = NULL; /* undecided */\n \tint i, maybe_descend = 0;\n+\tconst char *stuck = \"\";\n \n \tif (!el->nr)\n \t\treturn NULL;\t/* undefined */\n@@ -1024,6 +1025,7 @@ static struct exclude *last_exclude_matching_from_list(const char *pathname,\n \t\t\tif (*dtype == DT_UNKNOWN)\n \t\t\t\t*dtype = get_dtype(NULL, pathname, pathlen);\n \t\t\tif (match_sticky(x, pathname, pathlen, *dtype)) {\n+\t\t\t\tstuck = \" (stuck)\";\n \t\t\t\texc = x;\n \t\t\t\tbreak;\n \t\t\t}\n@@ -1093,7 +1095,7 @@ static struct exclude *last_exclude_matching_from_list(const char *pathname,\n \ttrace_printf_key(&trace_exclude, \"exclude: %.*s vs %s at line %d => %s%s\\n\",\n \t\t\t pathlen, pathname, exc->pattern, exc->srcpos,\n \t\t\t exc->flags & EXC_FLAG_NEGATIVE ? \"no\" : \"yes\",\n-\t\t\t exc->sticky_paths.nr ? \" (stuck)\" : \"\");\n+\t\t\t stuck);\n \treturn exc;\n }\n \ndiff --git a/t/t1011-read-tree-sparse-checkout.sh b/t/t1011-read-tree-sparse-checkout.sh\nindex ecc5e93..f0b856f 100755\n--- a/t/t1011-read-tree-sparse-checkout.sh\n+++ b/t/t1011-read-tree-sparse-checkout.sh\n@@ -287,10 +287,14 @@ test_expect_success 'sparse checkout and dir.c sticky bits' '\n \t\t!one/hideme\n \t\tEOF\n \t\tgit config core.sparsecheckout true &&\n-\t\tgit checkout &&\n+\t\tGIT_TRACE_EXCLUDE=2 git checkout 2>&1 | grep stuck >stuck-list &&\n \t\ttest_path_is_missing one/hideme &&\n \t\ttest_path_is_file    one/showme &&\n-\t\ttest_path_is_file    two/showme\n+\t\ttest_path_is_file    two/showme &&\n+\t\tcat >expected <<-\\EOF &&\n+\t\texclude: one/showme vs /* at line 1 => yes (stuck)\n+\t\tEOF\n+\t\ttest_cmp expected stuck-list\n \t)\n '\n \n-- \n2.8.0.rc0.210.gd302cd2\n"},{"id":"281031","messageId":"1458219254-16343-1-git-send-email-pclouds@gmail.com","threadId":"41721","inReplyTo":"1458218744-15810-2-git-send-email-pclouds@gmail.com","subject":"[PATCH 3/2] dir.c: fix dir re-inclusion rules with \"NODIR\" and \"MUSTBEDIR\"","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-03-17T12:54:14Z","receivedAt":"2016-03-17T12:54:14Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"For NODIR case, the patterns look like this\n\n    *          # exclude dir, dir/file1 and dir/file2..\n    !dir       # ..except that dir and everything inside is re-included..\n    dir/file2  # ..except (again!) that dir/file2 is excluded\n               # ..which means dir/file1 stays included\n\nWhen we stay at topdir and test \"dir\", everything is hunky dory, current\ncode returns \"re-include dir\" as expected. When we stay in \"dir\" and\nexamine \"dir/file1\", however, match_basename() checks if the base name\ncomponent, which is \"file1\", matches \"dir\" from the second rule.\n\nThis is wrong. We contradict ourselves because earlier we decided to\nre-include dir/file1 (from the second rule) when we were at toplevel.\nBecause the second rule is ignored, we hit the first one and return\n\"excluded\" even though \"dir/file1\" should be re-included.\n\nIn the MUSTBEDIR case, the patterns look like this\n\n    *          # exclude dir, dir/file1 and dir/file2..\n    !dir/      #  ..except that dir and everything inside is re-included..\n    dir/file2  # ..except (again!) that dir/file2 is excluded\n               # ..which means dir/file1 stays included\n\nAgain, we're ok at the toplevel, then we enter \"dir\" and test\n\"dir/file1\". The MUSTBEDIR code tests if the _full_ path (ie. \"dir/file1\")\nis a directory. We want it to test the \"dir\" part from \"dir/file1\"\ninstead.\n\nIn both cases, the second decision on \"dir/file1\" is wrong and\ncontradicts with the first one on \"dir\". This is a perfect use case for\n\"sticky path list\" added earlier to solve a different (but quite\nsimilar) problem. So, when we have the right decision the first time, we\nmark the path sticky to override the coming wrong decision.\n\nThe reason to do this, instead of actually fixing the code to make the\nright second decision in the first place, is because it's soooooo\ncomplicated. There are many combinations to take care of, many\noptimizations involved to keep the cost on normal/common case (and\nwhat's being described here is NOT normal) down to minimum. On top of\nthat, exclude code is already complicated as it is. It's best to not\nturn the code upside down. Not until this approach is proved\ninsufficient.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n This is NOT for 2.8.0! Posted here to give you some context on the\n problem mentioned in the first patch.\n\n If you actually read the link in 1/2, you'll notice this patch is\n completely different. \"soooooo complicated\" is not an exaggeration. I\n found some problem with that old patch, which ended up with a\n combination explosion of cases I would have to cover separately,\n essentially the same thing I encountered in my first try before that\n patch. I finally admitted I could not fit all that in my brain\n anymore.\n\n dir.c                              |  5 ++++\n t/t3007-ls-files-other-negative.sh | 51 ++++++++++++++++++++++++++++++++++++++\n 2 files changed, 56 insertions(+)\n\ndiff --git a/dir.c b/dir.c\nindex 2028094..a704e8a 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -1090,6 +1090,11 @@ static struct exclude *last_exclude_matching_from_list(const char *pathname,\n \t\t\t\t\t x->pattern, x->srcpos);\n \t\t\treturn NULL;\n \t\t}\n+\t} else if (exc->flags & EXC_FLAG_NEGATIVE) {\n+\t\tif (*dtype == DT_UNKNOWN)\n+\t\t\t*dtype = get_dtype(NULL, pathname, pathlen);\n+\t\tif (*dtype == DT_DIR)\n+\t\t\tadd_sticky(exc, pathname, pathlen);\n \t}\n \n \ttrace_printf_key(&trace_exclude, \"exclude: %.*s vs %s at line %d => %s%s\\n\",\ndiff --git a/t/t3007-ls-files-other-negative.sh b/t/t3007-ls-files-other-negative.sh\nindex 0797b86..c8f39dd 100755\n--- a/t/t3007-ls-files-other-negative.sh\n+++ b/t/t3007-ls-files-other-negative.sh\n@@ -150,4 +150,55 @@ test_expect_success 'match, literal pathname, nested negatives' '\n \ttest_cmp tmp/expected tmp/actual\n '\n \n+test_expect_success 're-include case, NODIR' '\n+\tgit init re-include-nodir &&\n+\t(\n+\t\tcd re-include-nodir &&\n+\t\tmkdir dir &&\n+\t\ttouch dir/file1 dir/file2 &&\n+\t\tcat >.gitignore <<-\\EOF &&\n+\t\t*\n+\t\t!dir\n+\t\tdir/file2\n+\t\tEOF\n+\t\tgit ls-files -o --exclude-standard >../actual &&\n+\t\techo dir/file1 >../expected &&\n+\t\ttest_cmp ../expected ../actual\n+\t)\n+'\n+\n+test_expect_success 're-include case, MUSTBEDIR with NODIR' '\n+\tgit init re-include-mustbedir &&\n+\t(\n+\t\tcd re-include-mustbedir &&\n+\t\tmkdir dir &&\n+\t\ttouch dir/file1 dir/file2 &&\n+\t\tcat >.gitignore <<-\\EOF &&\n+\t\t*\n+\t\t!dir/\n+\t\tdir/file2\n+\t\tEOF\n+\t\tgit ls-files -o --exclude-standard >../actual &&\n+\t\techo dir/file1 >../expected &&\n+\t\ttest_cmp ../expected ../actual\n+\t)\n+'\n+\n+test_expect_success 're-include case, MUSTBEDIR without NODIR' '\n+\tgit init re-include-pathname &&\n+\t(\n+\t\tcd re-include-pathname &&\n+\t\tmkdir -p dir1/dir2 &&\n+\t\ttouch dir1/dir2/file1 dir1/dir2/file2 &&\n+\t\tcat >.gitignore <<-\\EOF &&\n+\t\t*\n+\t\t!dir1/dir2/\n+\t\tdir1/dir2/file2\n+\t\tEOF\n+\t\tgit ls-files -o --exclude-standard >../actual &&\n+\t\techo dir1/dir2/file1 >../expected &&\n+\t\ttest_cmp ../expected ../actual\n+\t)\n+'\n+\n test_done\n-- \n2.8.0.rc0.210.gd302cd2\n"},{"id":"281035","messageId":"alpine.DEB.2.20.1603171402330.4690@virtualbox","threadId":"41721","inReplyTo":"CACsJy8A2D8CqeWZ6CFdTVXzXiWdonBE=3tWFPpe7-ZmDtNGYNw@mail.gmail.com","subject":"Re: bug: sparse config interpretation incorrectness in 2.8.0-rc2","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-03-17T13:04:51Z","receivedAt":"2016-03-17T13:04:51Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Duy,\n\nOn Thu, 17 Mar 2016, Duy Nguyen wrote:\n\n> Good news for you is there's \"sparse checkout v2\" in the work, that\n> would not rely on exclude engine and should be both faster and more\n> elegant. That should reduce \"sparse checkout v1\" usage to really small\n> cases.\n\nI dabbled myself with speeding up the entire exclude engine (essentially,\nI use a hash map of the non-wildcard prefixes to the corresponding line\nnumber). So I am interested to see what your approach is. Could you point\nme to it (I did not see any obvious branch in your GitHub space)?\n\nThanks,\nDscho\n"},{"id":"281038","messageId":"CACsJy8DH297aV2pYL7xGNZ5X24rR5PxvrpJQ+15YxpDNvqgS5Q@mail.gmail.com","threadId":"41721","inReplyTo":"alpine.DEB.2.20.1603171402330.4690@virtualbox","subject":"Re: bug: sparse config interpretation incorrectness in 2.8.0-rc2","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-03-17T13:20:07Z","receivedAt":"2016-03-17T13:20:07Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Mar 17, 2016 at 8:04 PM, Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n> Hi Duy,\n>\n> On Thu, 17 Mar 2016, Duy Nguyen wrote:\n>\n>> Good news for you is there's \"sparse checkout v2\" in the work, that\n>> would not rely on exclude engine and should be both faster and more\n>> elegant. That should reduce \"sparse checkout v1\" usage to really small\n>> cases.\n>\n> I dabbled myself with speeding up the entire exclude engine (essentially,\n> I use a hash map of the non-wildcard prefixes to the corresponding line\n> number). So I am interested to see what your approach is. Could you point\n> me to it (I did not see any obvious branch in your GitHub space)?\n\nFirst of all it's not about speeding up exclude engine. I think I did\nthat (sort of) already with untracked cache (have you tried it on\nWindows?)\n\nIt's Junio's idea actually, what I'm implementing. The index contains\na list of _files_ yes?\nWhat if the index contains also _directories_? Of course we can't\n\"refresh\" or check if the whole directory is \"dirty\" like a file, so\nthose dirs will stay registered in the index, but be entirely missing\nfrom worktree. The effect is the same as sparse checkout, except that\nwe keep all \"hidden\" files of sparse checkout in the index. In fact I\nplan to reuse CE_SKIP_WORKTREE (from sparse checkout) to mark those\ndirs.\n\nThe advantage is obvious: the index can be much smaller, which leads\nto faster update and access. Of course now you can only hide either\nwhole directory, or hide nothing. It's less flexible than sparse\ncheckout \"v1\", but you can use \"v1\" on top of \"v2\" for that. Changing\ncheckout regions involves unfolding or folding certain directories,\nsparse patterns are not used.\n\nIf you are still curious, you can check out my 'narrow-checkout'\nbranch on github. The basic is working. Folding/unfolding is not. Be\nwarned that it's even dirtier than your rebase-helper branch (and will\nnot be released any time soon either, lots of other stuff to be\nfinished first).\n-- \nDuy\n"},{"id":"281041","messageId":"alpine.DEB.2.20.1603171444210.4690@virtualbox","threadId":"41721","inReplyTo":"CACsJy8DH297aV2pYL7xGNZ5X24rR5PxvrpJQ+15YxpDNvqgS5Q@mail.gmail.com","subject":"Re: bug: sparse config interpretation incorrectness in 2.8.0-rc2","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-03-17T13:46:59Z","receivedAt":"2016-03-17T13:46:59Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Duy,\n\nOn Thu, 17 Mar 2016, Duy Nguyen wrote:\n\n> On Thu, Mar 17, 2016 at 8:04 PM, Johannes Schindelin\n> <Johannes.Schindelin@gmx.de> wrote:\n> >\n> > On Thu, 17 Mar 2016, Duy Nguyen wrote:\n> >\n> >> Good news for you is there's \"sparse checkout v2\" in the work, that\n> >> would not rely on exclude engine and should be both faster and more\n> >> elegant. That should reduce \"sparse checkout v1\" usage to really\n> >> small cases.\n> >\n> > I dabbled myself with speeding up the entire exclude engine\n> > (essentially, I use a hash map of the non-wildcard prefixes to the\n> > corresponding line number). So I am interested to see what your\n> > approach is. Could you point me to it (I did not see any obvious\n> > branch in your GitHub space)?\n> \n> First of all it's not about speeding up exclude engine. I think I did\n> that (sort of) already with untracked cache (have you tried it on\n> Windows?)\n\nYes, I tested the untracked cache and enabled it in a couple of my\nrepositories. It works great. But it has little to do with the exclude\nengine ;-)\n\n> It's Junio's idea actually, what I'm implementing. The index contains\n> a list of _files_ yes?\n> What if the index contains also _directories_? Of course we can't\n> \"refresh\" or check if the whole directory is \"dirty\" like a file, so\n> those dirs will stay registered in the index, but be entirely missing\n> from worktree. The effect is the same as sparse checkout, except that\n> we keep all \"hidden\" files of sparse checkout in the index. In fact I\n> plan to reuse CE_SKIP_WORKTREE (from sparse checkout) to mark those\n> dirs.\n\nOh, I see.\n\nUnfortunately, this does not help me at all. In the use case I am trying\nto get to work fast, we have tons and tons of directories and need *one*\nfile in pretty much *all* of those directories, and exclude most of the\nother files.\n\nTo make matters even worse, the list of excluded (or included) files is\nconstantly changing.\n\n> The advantage is obvious: the index can be much smaller, which leads\n> to faster update and access. Of course now you can only hide either\n> whole directory, or hide nothing. It's less flexible than sparse\n> checkout \"v1\", but you can use \"v1\" on top of \"v2\" for that. Changing\n> checkout regions involves unfolding or folding certain directories,\n> sparse patterns are not used.\n> \n> If you are still curious, you can check out my 'narrow-checkout'\n> branch on github. The basic is working. Folding/unfolding is not. Be\n> warned that it's even dirtier than your rebase-helper branch (and will\n> not be released any time soon either, lots of other stuff to be\n> finished first).\n\nThanks!\nDscho\n"},{"id":"281042","messageId":"CACsJy8D1e5zd-b6CsszHGcNjMPedyZSrZpqdWeCUGdMD5UP7qQ@mail.gmail.com","threadId":"41721","inReplyTo":"alpine.DEB.2.20.1603171444210.4690@virtualbox","subject":"Re: bug: sparse config interpretation incorrectness in 2.8.0-rc2","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-03-17T14:00:38Z","receivedAt":"2016-03-17T14:00:38Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Mar 17, 2016 at 8:46 PM, Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n> Unfortunately, this does not help me at all. In the use case I am trying\n> to get to work fast, we have tons and tons of directories and need *one*\n> file in pretty much *all* of those directories, and exclude most of the\n> other files.\n>\n> To make matters even worse, the list of excluded (or included) files is\n> constantly changing.\n\nOK very weird use case to me :) There's something you might try. [1]\ncan help trimming unrelated patterns, fewer patterns to match for a\ndir, faster matching.\n\nI knew it might help speed up sparse checkout (because we can't spread\nsparse patterns over many .gitignore files,  we only have one place\nfor all patterns). I did try at some point. I just don't remember if I\ngave up because I didn't think it's worth the effort or I found out\nsomething wrong with it for sparse checkout case. It may or may help\nyour case, depending on the patterns, of course.\n\n[1] http://article.gmane.org/gmane.comp.version-control.git/217958\n-- \nDuy\n"},{"id":"281058","messageId":"56EAEEB2.4020002@fb.com","threadId":"41721","inReplyTo":"xmqqfuvp36wu.fsf@gitster.mtv.corp.google.com","subject":"Re: bug: sparse config interpretation incorrectness in 2.8.0-rc2","fromName":"Durham Goode","fromEmail":"durham@fb.com","sentAt":"2016-03-17T17:51:46Z","receivedAt":"2016-03-17T17:51:46Z","isPatch":false,"sender":{"key":"durham@fb.com","avatar":null},"body":"On 3/17/16 12:22 AM, Junio C Hamano wrote:\n> Durham Goode <durham@fb.com> writes:\n>\n>> Using git 2.8.0-rc2, given a repo with the following files:\n>>\n>> - one/hideme\n>> - one/donthide\n>> - two/foo\n>>\n>> A sparse config of:\n>>\n>> cat > .git/info/sparse-checkout <<EOF\n>> /*\n>> !one/hideme\n>> EOF\n>>\n>> Results in a repository that only has `one/donthide` in it.  I would\n>> expect `two/foo`to be present as well.  This worked in 2.6,...\n> 2.6 is a tad too old as a reference, as the \"!reinclusion\" has been\n> in flux in recent releases.  Can you test these?\n>\n>   - 2.7.0\n>   - 2.7.1\n>   - e79112d\n>\n> I suspect that v2.7.0 would be broken, v2.7.1 is OK and e79112d\n> would also be OK (I am guessing this from [1]).  e79112d is the last\n> version on the 'master' branch without the topic that contains the\n> commit you bisected down to, but between 2.7.0 and 2.7.1 there was a\n> reversion of a commit that introduced the original issue.\n>\n> The commit you bisected down to that is on 'master', IIUC, was a\n> (faulty) attempt to fix the breakage in a different way.\nSince Duy has already sent a fix, I assume it's a real bug and he \nunderstands what's going on.  If you still want me to test those hashes \nlet me know.\n"},{"id":"281098","messageId":"xmqqfuvoy89q.fsf@gitster.mtv.corp.google.com","threadId":"41721","inReplyTo":"1458219254-16343-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 3/2] dir.c: fix dir re-inclusion rules with \"NODIR\" and \"MUSTBEDIR\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-03-17T23:49:53Z","receivedAt":"2016-03-17T23:49:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks for these 5 patches, two of which need to be discarded ;-).\nI think you can pick either one of 1/2, pick the one that says\n\"non-NULL\" (as opposed to \"something\") in the log message for 2/2.\n\nDurham, does it fix your issues if you apply the 1/2 and 2/2 (but\nnot 3/2) on top of 2.8-rc?\n\nDuy, how comfortable are you with the idea of including this two in\n2.8 final?  We have long passed the final -rc, and while it is\nprobably OK to prolong the cycle and do another -rc, we cannot keep\ngoing like \"oops, there is another thing discovered by somebod new\"\nforever.\n\nThanks.\n"},{"id":"281100","messageId":"CACsJy8Dm3_w6TT6FP-my9fsRJ8F+StK8dBPid9zxQv4OzoZfcw@mail.gmail.com","threadId":"41721","inReplyTo":"xmqqfuvoy89q.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 3/2] dir.c: fix dir re-inclusion rules with \"NODIR\" and \"MUSTBEDIR\"","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-03-18T00:15:48Z","receivedAt":"2016-03-18T00:15:48Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Mar 18, 2016 at 6:49 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Thanks for these 5 patches, two of which need to be discarded ;-).\n> I think you can pick either one of 1/2, pick the one that says\n> \"non-NULL\" (as opposed to \"something\") in the log message for 2/2.\n\nSorry, I did \"git send-email ... 00*\" and it picked up *.patch~ as\nwell. non-NULL is the non-backup version.\n\n> Durham, does it fix your issues if you apply the 1/2 and 2/2 (but\n> not 3/2) on top of 2.8-rc?\n>\n> Duy, how comfortable are you with the idea of including this two in\n> 2.8 final?  We have long passed the final -rc, and while it is\n> probably OK to prolong the cycle and do another -rc, we cannot keep\n> going like \"oops, there is another thing discovered by somebody new\"\n> forever.\n\nThe fix is well understood. I did feel unsure about that \"continue;\"\nwhen I first wrote it and should have given it more thought. No other\n\"unsure\" feelings, so we're probably good from now on.\n-- \nDuy\n"},{"id":"281111","messageId":"56EB8961.70302@fb.com","threadId":"41721","inReplyTo":"xmqqfuvoy89q.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 3/2] dir.c: fix dir re-inclusion rules with \"NODIR\" and \"MUSTBEDIR\"","fromName":"Durham Goode","fromEmail":"durham@fb.com","sentAt":"2016-03-18T04:51:45Z","receivedAt":"2016-03-18T04:51:45Z","isPatch":true,"sender":{"key":"durham@fb.com","avatar":null},"body":"On 3/17/16 4:49 PM, Junio C Hamano wrote:\n> Thanks for these 5 patches, two of which need to be discarded ;-).\n> I think you can pick either one of 1/2, pick the one that says\n> \"non-NULL\" (as opposed to \"something\") in the log message for 2/2.\n>\n> Durham, does it fix your issues if you apply the 1/2 and 2/2 (but\n> not 3/2) on top of 2.8-rc?\n>\n> Duy, how comfortable are you with the idea of including this two in\n> 2.8 final?  We have long passed the final -rc, and while it is\n> probably OK to prolong the cycle and do another -rc, we cannot keep\n> going like \"oops, there is another thing discovered by somebod new\"\n> forever.\n>\n> Thanks.\nPatches 1+2 fix the repro steps in the report, yes.  But I've found \nanother case that produces different results in 2.8 than in 2.7:\n\nGiven a repo with files:\n\ndir1/dir2/show/file\ndir1/dir2/hide/file\n\nand a sparse-checkout of\n\n/*\n/dir1/dir2/show\n!/dir1/dir2/\n\nthe working copy still contains dir1/dir2/hide/file when run from \n2.8.0-rc2. In git 2.6 and 2.7.3 it does not show up (which is the \nexpected behavior, from what I understand of the docs).  Repro script is \nbelow.  Notice, the 'dir2/' part of the paths is important.  If I drop \nthat directory, the issue doesn't repro.\n\n\n#!/bin/bash\n\nset -x\nrm -rf sparse-test\nGIT=git\n$GIT init sparse-test\ncd sparse-test\n$GIT config --add core.sparsecheckout true\n\nmkdir -p dir1/dir2/show dir1/dir2/hide\ntouch dir1/dir2/show/file1\ntouch dir1/dir2/hide/file2\n\n$GIT add .\n$GIT commit -m \"initial commit\"\n$GIT read-tree --reset -u HEAD\n\nmkdir .git/info\ncat > .git/info/sparse-checkout <<EOF\n/*\n/dir1/dir2/show\n!/dir1/dir2/\nEOF\n$GIT read-tree --reset -u HEAD\n\nls -R dir1/dir2\nset +x\necho\necho expected: see only \"dir1/dir2/show/file\"\necho actual: see \"dir1/dir2/hide/file\" as well\n"},{"id":"281119","messageId":"xmqqbn6cs5sa.fsf@gitster.mtv.corp.google.com","threadId":"41721","inReplyTo":"CACsJy8Dm3_w6TT6FP-my9fsRJ8F+StK8dBPid9zxQv4OzoZfcw@mail.gmail.com","subject":"Re: [PATCH 3/2] dir.c: fix dir re-inclusion rules with \"NODIR\" and \"MUSTBEDIR\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-03-18T05:40:05Z","receivedAt":"2016-03-18T05:40:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen <pclouds@gmail.com> writes:\n\n> On Fri, Mar 18, 2016 at 6:49 AM, Junio C Hamano <gitster@pobox.com> wrote:\n>> Thanks for these 5 patches, two of which need to be discarded ;-).\n>> I think you can pick either one of 1/2, pick the one that says\n>> \"non-NULL\" (as opposed to \"something\") in the log message for 2/2.\n>\n> Sorry, I did \"git send-email ... 00*\" and it picked up *.patch~ as\n> well. non-NULL is the non-backup version.\n\nOff-topic.  I wonder what people think about doing something like\nthis patch.\n\n-- >8 --\nsend-email: detect and offer to skip backup files\n\nDiligent people save output from format-patch to files, proofread\nand edit them and then finally send the result out.  If the\nresulting files are sent out with \"git send-email 0*\", this ends up\nsending backup files (e.g. 0001-X.patch.backup or 0001-X.patch~)\nleft by their editors next to the final version.  Sending them with\n\"git send-email 0*.patch\" (if format-patch was run with the standard\nsuffix) would avoid such an embarrassment, but not everybody is\ncareful.\n\nAfter collecting files to be sent (and sorting them if read from a\ndirectory), notice when the file being sent out has the same name as\nthe previous file, plus some suffix (e.g. 0001-X.patch was sent, and\nwe are looking at 0001-X.patch.backup or 0001-X.patch~), and the\nsuffix begins with a non-alnum (e.g. \".backup\" or \"~\") and ask if\nthe user really wants to send it out.  Once the user skips sending\nsuch a \"backup\" file, remember the suffix and stop asking the same\nquestion (e.g. after skipping 0001-X.patch~, skip 0002-Y.patch~\nwithout asking).\n\n\n git-send-email.perl | 40 ++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 40 insertions(+)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex d356901..74ed01a 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -621,6 +621,8 @@ sub is_format_patch_arg {\n \tpush @files, $repo->command('format-patch', '-o', tempdir(CLEANUP => 1), @rev_list_opts);\n }\n \n+@files = handle_backup_files(@files);\n+\n if ($validate) {\n \tforeach my $f (@files) {\n \t\tunless (-p $f) {\n@@ -1726,6 +1728,44 @@ sub validate_patch {\n \treturn;\n }\n \n+sub handle_backup {\n+\tmy ($last, $lastlen, $file, $known_suffix) = @_;\n+\tmy ($suffix, $skip);\n+\n+\t$skip = 0;\n+\tif (defined $last &&\n+\t    ($lastlen < length($file)) &&\n+\t    (substr($file, 0, $lastlen) eq $last) &&\n+\t    ($suffix = substr($file, $lastlen)) !~ /^[a-z0-9]/i) {\n+\t\tif (defined $known_suffix && $suffix eq $known_suffix) {\n+\t\t\tprint \"Skipping $file with backup suffix '$known_suffix'.\\n\";\n+\t\t\t$skip = 1;\n+\t\t} else {\n+\t\t\tmy $answer = ask(\"Do you really want to send $file? (y|N): \",\n+\t\t\t\t\t valid_re => qr/^(?:y|n)/i,\n+\t\t\t\t\t default => 'y');\n+\t\t\t$skip = ($answer ne 'y');\n+\t\t\tif ($skip) {\n+\t\t\t\t$known_suffix = $suffix;\n+\t\t\t}\n+\t\t}\n+\t}\n+\treturn ($skip, $known_suffix);\n+}\n+\n+sub handle_backup_files {\n+\tmy @file = @_;\n+\tmy ($last, $lastlen, $known_suffix, $skip, @result);\n+\tfor my $file (@file) {\n+\t\t($skip, $known_suffix) = handle_backup($last, $lastlen,\n+\t\t\t\t\t\t       $file, $known_suffix);\n+\t\tpush @result, $file unless $skip;\n+\t\t$last = $file;\n+\t\t$lastlen = length($file);\n+\t}\n+\treturn @result;\n+}\n+\n sub file_has_nonascii {\n \tmy $fn = shift;\n \topen(my $fh, '<', $fn)\n"},{"id":"281120","messageId":"CACsJy8A2FKn-8nWtK4QPMHDCDYvTZBrQs1RVMApnuejXQis19g@mail.gmail.com","threadId":"41721","inReplyTo":"56EB8961.70302@fb.com","subject":"Re: [PATCH 3/2] dir.c: fix dir re-inclusion rules with \"NODIR\" and \"MUSTBEDIR\"","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-03-18T05:40:09Z","receivedAt":"2016-03-18T05:40:09Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Mar 18, 2016 at 11:51 AM, Durham Goode <durham@fb.com> wrote:\n> On 3/17/16 4:49 PM, Junio C Hamano wrote:\n>>\n>> Thanks for these 5 patches, two of which need to be discarded ;-).\n>> I think you can pick either one of 1/2, pick the one that says\n>> \"non-NULL\" (as opposed to \"something\") in the log message for 2/2.\n>>\n>> Durham, does it fix your issues if you apply the 1/2 and 2/2 (but\n>> not 3/2) on top of 2.8-rc?\n>>\n>> Duy, how comfortable are you with the idea of including this two in\n>> 2.8 final?  We have long passed the final -rc, and while it is\n>> probably OK to prolong the cycle and do another -rc, we cannot keep\n>> going like \"oops, there is another thing discovered by somebod new\"\n>> forever.\n>>\n>> Thanks.\n>\n> Patches 1+2 fix the repro steps in the report, yes.  But I've found another\n> case that produces different results in 2.8 than in 2.7:\n>\n> Given a repo with files:\n>\n> dir1/dir2/show/file\n> dir1/dir2/hide/file\n>\n> and a sparse-checkout of\n>\n> /*\n> /dir1/dir2/show\n> !/dir1/dir2/\n\nI would say this is \"undefined behavior\" patterns. The intention of\n\"!\" pattern is to revert a subset of a matched pattern, e.g.\n!/dir1/dir2/show/something. Combining lines 2 and 3 together,\n\"!dir1/dir2/\" is not only supposed to revert dir1/dir2/show entirely,\nbut extend the reversion outside of it. At least to me that's not\nintended.\n\nSkipping that tricky pair, the pairs \"/*\" and \"!/dir1/dir2/\" means\n\"exclude everything except dir1/dir2\" (in .gitignore sense) or\n\"include everything except dir1/dir2\" in sparse checkout sense. Which\nresults in empty worktree. 1+2 trips when the trailing slash in the\nlast rule exists and includes both files in show/hide. Patch 3/2 fixes\nthe tripping and exclude both. If the last rule is \"!/dir1/dir2\" then\n1+2 results in empty worktree as well.\n\nIf I swap the last two rules, then it behaves the way we expect,\ndir1/dir2/show stays, dir1/dir2/hide is gone (again, the trailing \"/\"\nin !dir1/dir2/ requires patch 3/2).\n\nv2.7.3 differs in the way \"!\" is handled. It does extend reversion\noutside dir1/dir2/show, back to dir1/dir2. While 2.8+1+2 recognizes\nand follows the \"/*\" and \"!dir1/dir2/\" pair.\n\nThe way I interpreted the rules above, though, may be because I'm just\ntrying to defend the current code. Junio, your call on whether to\nrevert this whole patch series.\n\n> the working copy still contains dir1/dir2/hide/file when run from 2.8.0-rc2.\n> In git 2.6 and 2.7.3 it does not show up (which is the expected behavior,\n> from what I understand of the docs).  Repro script is below.  Notice, the\n> 'dir2/' part of the paths is important.  If I drop that directory, the issue\n> doesn't repro.\n-- \nDuy\n"},{"id":"281122","messageId":"CACsJy8BHVJPHv-Tap8DYUYGn0NH9s2rXPfxV7FUiMyVGQMU4yQ@mail.gmail.com","threadId":"41721","inReplyTo":"xmqqbn6cs5sa.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 3/2] dir.c: fix dir re-inclusion rules with \"NODIR\" and \"MUSTBEDIR\"","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-03-18T05:51:35Z","receivedAt":"2016-03-18T05:51:35Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Mar 18, 2016 at 12:40 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> send-email: detect and offer to skip backup files\n>\n> Diligent people save output from format-patch to files, proofread\n> and edit them and then finally send the result out.  If the\n> resulting files are sent out with \"git send-email 0*\", this ends up\n> sending backup files (e.g. 0001-X.patch.backup or 0001-X.patch~)\n> left by their editors next to the final version.  Sending them with\n> \"git send-email 0*.patch\" (if format-patch was run with the standard\n> suffix) would avoid such an embarrassment, but not everybody is\n> careful.\n>\n> After collecting files to be sent (and sorting them if read from a\n> directory), notice when the file being sent out has the same name as\n> the previous file, plus some suffix (e.g. 0001-X.patch was sent, and\n> we are looking at 0001-X.patch.backup or 0001-X.patch~), and the\n> suffix begins with a non-alnum (e.g. \".backup\" or \"~\") and ask if\n> the user really wants to send it out.  Once the user skips sending\n> such a \"backup\" file, remember the suffix and stop asking the same\n> question (e.g. after skipping 0001-X.patch~, skip 0002-Y.patch~\n> without asking).\n\nThe problem I see is it's hard to review the to-send list.\ngit-send-email does list it, but the the dashes in file names make\nthem really hard to split out words. Maybe highlighting can help, or\nmaybe we can show subject lines instead of file names.\n\nBack to the patch, another case you can catch is when people have some\nleftover patches in current dir, then 0*.patch may include unrelated\npatches too. A check on patch numbering (contiguous and no duplicates)\nmight help.\n\n>  git-send-email.perl | 40 ++++++++++++++++++++++++++++++++++++++++\n>  1 file changed, 40 insertions(+)\n>\n> diff --git a/git-send-email.perl b/git-send-email.perl\n> index d356901..74ed01a 100755\n> --- a/git-send-email.perl\n> +++ b/git-send-email.perl\n> @@ -621,6 +621,8 @@ sub is_format_patch_arg {\n>         push @files, $repo->command('format-patch', '-o', tempdir(CLEANUP => 1), @rev_list_opts);\n>  }\n>\n> +@files = handle_backup_files(@files);\n> +\n>  if ($validate) {\n>         foreach my $f (@files) {\n>                 unless (-p $f) {\n> @@ -1726,6 +1728,44 @@ sub validate_patch {\n>         return;\n>  }\n>\n> +sub handle_backup {\n> +       my ($last, $lastlen, $file, $known_suffix) = @_;\n> +       my ($suffix, $skip);\n> +\n> +       $skip = 0;\n> +       if (defined $last &&\n> +           ($lastlen < length($file)) &&\n> +           (substr($file, 0, $lastlen) eq $last) &&\n> +           ($suffix = substr($file, $lastlen)) !~ /^[a-z0-9]/i) {\n> +               if (defined $known_suffix && $suffix eq $known_suffix) {\n> +                       print \"Skipping $file with backup suffix '$known_suffix'.\\n\";\n> +                       $skip = 1;\n> +               } else {\n> +                       my $answer = ask(\"Do you really want to send $file? (y|N): \",\n> +                                        valid_re => qr/^(?:y|n)/i,\n> +                                        default => 'y');\n> +                       $skip = ($answer ne 'y');\n> +                       if ($skip) {\n> +                               $known_suffix = $suffix;\n> +                       }\n> +               }\n> +       }\n> +       return ($skip, $known_suffix);\n> +}\n> +\n> +sub handle_backup_files {\n> +       my @file = @_;\n> +       my ($last, $lastlen, $known_suffix, $skip, @result);\n> +       for my $file (@file) {\n> +               ($skip, $known_suffix) = handle_backup($last, $lastlen,\n> +                                                      $file, $known_suffix);\n> +               push @result, $file unless $skip;\n> +               $last = $file;\n> +               $lastlen = length($file);\n> +       }\n> +       return @result;\n> +}\n> +\n>  sub file_has_nonascii {\n>         my $fn = shift;\n>         open(my $fh, '<', $fn)\n-- \nDuy\n"},{"id":"281123","messageId":"CAPig+cRp3ghX8VUvR8Tfb6rsnbOM4eQiQ8Hw1hs4=BvXORREYA@mail.gmail.com","threadId":"41721","inReplyTo":"xmqqbn6cs5sa.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 3/2] dir.c: fix dir re-inclusion rules with \"NODIR\" and \"MUSTBEDIR\"","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-03-18T05:58:15Z","receivedAt":"2016-03-18T05:58:15Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Mar 18, 2016 at 1:40 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> send-email: detect and offer to skip backup files\n>\n> Diligent people save output from format-patch to files, proofread\n> and edit them and then finally send the result out.  If the\n> resulting files are sent out with \"git send-email 0*\", this ends up\n> sending backup files (e.g. 0001-X.patch.backup or 0001-X.patch~)\n> left by their editors next to the final version.  Sending them with\n> \"git send-email 0*.patch\" (if format-patch was run with the standard\n> suffix) would avoid such an embarrassment, but not everybody is\n> careful.\n>\n> After collecting files to be sent (and sorting them if read from a\n> directory), notice when the file being sent out has the same name as\n> the previous file, plus some suffix (e.g. 0001-X.patch was sent, and\n> we are looking at 0001-X.patch.backup or 0001-X.patch~), and the\n> suffix begins with a non-alnum (e.g. \".backup\" or \"~\") and ask if\n> the user really wants to send it out.  Once the user skips sending\n> such a \"backup\" file, remember the suffix and stop asking the same\n> question (e.g. after skipping 0001-X.patch~, skip 0002-Y.patch~\n> without asking).\n>\n> diff --git a/git-send-email.perl b/git-send-email.perl\n> @@ -1726,6 +1728,44 @@ sub validate_patch {\n> +sub handle_backup {\n> +       my ($last, $lastlen, $file, $known_suffix) = @_;\n\nIs $lastlen a micro-optimization or does it have some other purpose\nI'm overlooking?\n\n> +       my ($suffix, $skip);\n> +\n> +       $skip = 0;\n> +       if (defined $last &&\n> +           ($lastlen < length($file)) &&\n> +           (substr($file, 0, $lastlen) eq $last) &&\n> +           ($suffix = substr($file, $lastlen)) !~ /^[a-z0-9]/i) {\n> +               if (defined $known_suffix && $suffix eq $known_suffix) {\n> +                       print \"Skipping $file with backup suffix '$known_suffix'.\\n\";\n> +                       $skip = 1;\n> +               } else {\n> +                       my $answer = ask(\"Do you really want to send $file? (y|N): \",\n> +                                        valid_re => qr/^(?:y|n)/i,\n> +                                        default => 'y');\n\nIs this 'default' correct or am I reading the code incorrectly?\n\n> +                       $skip = ($answer ne 'y');\n> +                       if ($skip) {\n> +                               $known_suffix = $suffix;\n> +                       }\n> +               }\n> +       }\n> +       return ($skip, $known_suffix);\n> +}\n> +\n> +sub handle_backup_files {\n> +       my @file = @_;\n> +       my ($last, $lastlen, $known_suffix, $skip, @result);\n> +       for my $file (@file) {\n> +               ($skip, $known_suffix) = handle_backup($last, $lastlen,\n> +                                                      $file, $known_suffix);\n> +               push @result, $file unless $skip;\n> +               $last = $file;\n> +               $lastlen = length($file);\n> +       }\n> +       return @result;\n> +}\n"},{"id":"281128","messageId":"56EB9E67.1000306@fb.com","threadId":"41721","inReplyTo":"CACsJy8A2FKn-8nWtK4QPMHDCDYvTZBrQs1RVMApnuejXQis19g@mail.gmail.com","subject":"Re: [PATCH 3/2] dir.c: fix dir re-inclusion rules with \"NODIR\" and \"MUSTBEDIR\"","fromName":"Durham Goode","fromEmail":"durham@fb.com","sentAt":"2016-03-18T06:21:27Z","receivedAt":"2016-03-18T06:21:27Z","isPatch":true,"sender":{"key":"durham@fb.com","avatar":null},"body":"\n\nOn 3/17/16 10:40 PM, Duy Nguyen wrote:\n> On Fri, Mar 18, 2016 at 11:51 AM, Durham Goode <durham@fb.com> wrote:\n>> On 3/17/16 4:49 PM, Junio C Hamano wrote:\n>>> Thanks for these 5 patches, two of which need to be discarded ;-).\n>>> I think you can pick either one of 1/2, pick the one that says\n>>> \"non-NULL\" (as opposed to \"something\") in the log message for 2/2.\n>>>\n>>> Durham, does it fix your issues if you apply the 1/2 and 2/2 (but\n>>> not 3/2) on top of 2.8-rc?\n>>>\n>>> Duy, how comfortable are you with the idea of including this two in\n>>> 2.8 final?  We have long passed the final -rc, and while it is\n>>> probably OK to prolong the cycle and do another -rc, we cannot keep\n>>> going like \"oops, there is another thing discovered by somebod new\"\n>>> forever.\n>>>\n>>> Thanks.\n>> Patches 1+2 fix the repro steps in the report, yes.  But I've found another\n>> case that produces different results in 2.8 than in 2.7:\n>>\n>> Given a repo with files:\n>>\n>> dir1/dir2/show/file\n>> dir1/dir2/hide/file\n>>\n>> and a sparse-checkout of\n>>\n>> /*\n>> /dir1/dir2/show\n>> !/dir1/dir2/\n> I would say this is \"undefined behavior\" patterns. The intention of\n> \"!\" pattern is to revert a subset of a matched pattern, e.g.\n> !/dir1/dir2/show/something. Combining lines 2 and 3 together,\n> \"!dir1/dir2/\" is not only supposed to revert dir1/dir2/show entirely,\n> but extend the reversion outside of it. At least to me that's not\n> intended.\nAfter looking at it further, I agree with you.  The new behavior is as \njustifiable as the old. In addition, my description of this secondary \nissue was wrong (I said it left the files around, when in reality it \nresulted in an empty working copy, so git read-tree failed and left the \nfiles).  So we can ignore that case. Thanks.\n>\n> Skipping that tricky pair, the pairs \"/*\" and \"!/dir1/dir2/\" means\n> \"exclude everything except dir1/dir2\" (in .gitignore sense) or\n> \"include everything except dir1/dir2\" in sparse checkout sense. Which\n> results in empty worktree. 1+2 trips when the trailing slash in the\n> last rule exists and includes both files in show/hide. Patch 3/2 fixes\n> the tripping and exclude both. If the last rule is \"!/dir1/dir2\" then\n> 1+2 results in empty worktree as well.\n>\nI'm not sure I fully understand.  Here's what I'm seeing, with patch 1, \n2, and 3 applied:  with patterns \"/*', \"!/dir1/dir2\", \"/dir1/dir2/show\", \nI see the contents of dir1/dir2/show/ (good). If I add a trailing slash \nto the last pattern (so it becomes \"/dir1/dir2/show/\"), it now results \nin an empty working copy. That seems funky, given that the last rule is \nto include that directory. Am I misunderstanding the trailing slash?\n\nWe're pretty far beyond my ability to understand ignore patterns now.  \nThe patches seem to make the behavior better though, so thanks for that.\n"},{"id":"281130","messageId":"CACsJy8CrXFUPqJz4kt6fsvFhGDavOfZBQtHD98FuEW14UMmMtg@mail.gmail.com","threadId":"41721","inReplyTo":"56EB9E67.1000306@fb.com","subject":"Re: [PATCH 3/2] dir.c: fix dir re-inclusion rules with \"NODIR\" and \"MUSTBEDIR\"","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-03-18T06:28:01Z","receivedAt":"2016-03-18T06:28:01Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Mar 18, 2016 at 1:21 PM, Durham Goode <durham@fb.com> wrote:\n>> Skipping that tricky pair, the pairs \"/*\" and \"!/dir1/dir2/\" means\n>> \"exclude everything except dir1/dir2\" (in .gitignore sense) or\n>> \"include everything except dir1/dir2\" in sparse checkout sense. Which\n>> results in empty worktree. 1+2 trips when the trailing slash in the\n>> last rule exists and includes both files in show/hide. Patch 3/2 fixes\n>> the tripping and exclude both. If the last rule is \"!/dir1/dir2\" then\n>> 1+2 results in empty worktree as well.\n>>\n> I'm not sure I fully understand.  Here's what I'm seeing, with patch 1, 2,\n> and 3 applied:  with patterns \"/*', \"!/dir1/dir2\", \"/dir1/dir2/show\", I see\n> the contents of dir1/dir2/show/ (good). If I add a trailing slash to the\n> last pattern (so it becomes \"/dir1/dir2/show/\"), it now results in an empty\n> working copy. That seems funky, given that the last rule is to include that\n> directory. Am I misunderstanding the trailing slash?\n\nAh.. I meant the last slash in !dir1/dir2/, not in dir1/dir2/show. But\nthat's interesting, will need to run some more tests.\n\n> We're pretty far beyond my ability to understand ignore patterns now.\n\nI'm right there with you. I'm starting to fear that I don't (and\nprobably won't) understand this thing.\n-- \nDuy\n"},{"id":"281152","messageId":"alpine.DEB.2.20.1603181647160.4690@virtualbox","threadId":"41721","inReplyTo":"CACsJy8D1e5zd-b6CsszHGcNjMPedyZSrZpqdWeCUGdMD5UP7qQ@mail.gmail.com","subject":"Re: bug: sparse config interpretation incorrectness in 2.8.0-rc2","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-03-18T15:49:03Z","receivedAt":"2016-03-18T15:49:03Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Duy,\n\nOn Thu, 17 Mar 2016, Duy Nguyen wrote:\n\n> On Thu, Mar 17, 2016 at 8:46 PM, Johannes Schindelin\n> <Johannes.Schindelin@gmx.de> wrote:\n> > Unfortunately, this does not help me at all. In the use case I am trying\n> > to get to work fast, we have tons and tons of directories and need *one*\n> > file in pretty much *all* of those directories, and exclude most of the\n> > other files.\n> >\n> > To make matters even worse, the list of excluded (or included) files is\n> > constantly changing.\n> \n> OK very weird use case to me :) There's something you might try. [1]\n> can help trimming unrelated patterns, fewer patterns to match for a\n> dir, faster matching.\n\nSadly, [1] (exclude: filter out patterns not applicable to the current\ndirectory) would not help at all, because the regular use case really\nwould use the top-level directory as current one.\n\nSo I really need to speed up the sparse machinery from O(m*n) (where m is\nthe number of entries in the sparse-checkout file and n is the number of\nfiles in the index).\n\nCiao,\nDscho\n"},{"id":"281160","messageId":"xmqqegb7r8jn.fsf@gitster.mtv.corp.google.com","threadId":"41721","inReplyTo":"1458218744-15810-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 1/2] dir.c: fix bug in 'nd/exclusion-regression-fix' topic","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-03-18T17:38:04Z","receivedAt":"2016-03-18T17:38:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n\n> The topic in question introduces \"sticky\" path list for this purpose:\n> given a path 'abc/def', if 'abc' already matches a pattern X, it's added\n> to X's sticky path list. When we need to check if 'abc/def' matches\n> pattern X and see that 'abc' is already in the list, we conclude right\n> away that 'abc/def' also matches X.\n\nIt took me two reads to realize that I can get what this paragraph\nwants to say if I ignore \"given a path 'abc/def'\" at the beginning.\nIn short, if we already know that a directory 'abc' matches a\npattern, the pattern remembers that fact in the list and further\nqueries about anything in that directory, e.g. 'abc/def', is\nanswered with \"we already know abc matches, so it also does match\".\n\n\"Sticky\" is probably better understood if called \"ancestor directory\".\n\n> The short reason (*) for sticky path list is to workaround limitations\n> of matching code that will return \"not match\" when we compare\n> 'abc/def' and pattern X.\n\nIf one asks about 'abc/def' first, you ask \"does '*' match\n'abc/def'?\"--if there is a limitation with the matching code, this\nlist would not work at all as a workaround.  But we can safely\nassume that before asking about 'abc/def' we would always ask about\n'abc', so it is OK.  Is that what happens here?\n\n> The bug is in this code. Not only it does \"when we need to check if\n> 'abc/def' matches...\", it does an extra thing: if 'foo/bar' is _not_ in\n> the list, return 'not matched' by bypassing all matching code with the\n> \"continue;\" statement. It should let the remaining code decide match\n> status instead.\n\nSo once a pattern has a non-empty \"sticky\" list after examining\n'abc' and if you ask about 'foo', because it is not in the list and\ninstead of saying \"it is not in the list, so we need to try matching\nthe pattern against 'foo' to decide if it matches\", it incorrectly\nsays \"'foo' does not match\".  Is that what happens here?\n\nAn abrupt appearance of 'foo/bar' in the description made me read\nthe above three times and that is why I dropped '/bar' part in the\nabove.  The correctness of the 'sticky' hack seems to depend on the\nassumption that we would always ask about 'foo' before asking about\n'foo/bar', so the scenario presented with 'foo/bar' is implausible;\neven when asking about 'foo/bar', we would have 'foo' in the list,\nno?\n\nThe remainder, assuming that I read the above correctly, made sense\nto me and justifies what the update code does.\n\nThanks.\n\n> This bug affects both .gitignore and sparse checkout, but it's reported\n> as a sparse checkout bug, so let's examine how it happens. The\n> sparse-checkout pattern has two rules\n>\n>     /*\n>     !one/hideme\n>\n> and the worktree has three tracked files, one/hideme, one/showme and\n> two/showme. What happens is this\n>\n> * check \"one\", it matches the first pattern -> positive -> keep\n>   examining.\n>\n> *1* \"thanks\" to 'nd/exclusion-regression-fix' we detect this pair of\n>   patterns, so we put \"one\" in the sticky list of pattern \"/*\"\n>\n> * enter \"one\", check \"one/hideme\", it matches the second pattern\n>   first (we search from bottom up) -> negative -> excluded\n>\n> * check \"one/showme\", it does not match the second pattern.\n>\n> *2* We then check it against the first pattern and notice the sticky list\n>   that includes \"one\", so we decide right away that \"one/showme\" is\n>   included.\n>\n> * leave \"one\", check \"two\" which does not match the second pattern.\n>\n> *3* then we check \"two\" against the first pattern and notice that this\n>    pattern has a non-empty sticky list, which contains \"one\", not \"two\".\n>    This bug kicks in and bypasses the true matching logic for pattern\n>    \"/*\". As a result, we exclude \"two/showme\".\n>\n> One may notice that the order of these steps matter. If *3* occurs\n> before *1*, then the sticky list at that moment is empty and the bug\n> does not kick in. Sparse checkout always examines entries in\n> alphabetical order, so \"abc/showme\" would be examined before \"one\" and\n> not hit this bug!\n>\n> The last remark is important when we move to .gitignore. We receive the\n> list of entries with readdir() and cannot control the order of\n> entries. Which means we can't write a test for .gitignore that will\n> reliably fail without this fix. Which is why this patch only adds a test\n> for sparse checkout, even though the same above steps happen to\n> .gitignore.\n>\n> (*) The problem is known and will be fixed later and described in\n> detail then. For this commit, it's sufficient to see the following\n> link because the long reason is really long:\n>\n> http://article.gmane.org/gmane.comp.version-control.git/288479\n>\n> Reported-by: Durham Goode <durham@fb.com>\n> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n> ---\n>  dir.c                                |  1 -\n>  t/t1011-read-tree-sparse-checkout.sh | 20 ++++++++++++++++++++\n>  2 files changed, 20 insertions(+), 1 deletion(-)\n>\n> diff --git a/dir.c b/dir.c\n> index 69e0be6..77f38a5 100644\n> --- a/dir.c\n> +++ b/dir.c\n> @@ -1027,7 +1027,6 @@ static struct exclude *last_exclude_matching_from_list(const char *pathname,\n>  \t\t\t\texc = x;\n>  \t\t\t\tbreak;\n>  \t\t\t}\n> -\t\t\tcontinue;\n>  \t\t}\n>  \n>  \t\tif (x->flags & EXC_FLAG_MUSTBEDIR) {\n> diff --git a/t/t1011-read-tree-sparse-checkout.sh b/t/t1011-read-tree-sparse-checkout.sh\n> index 0c74bee..ecc5e93 100755\n> --- a/t/t1011-read-tree-sparse-checkout.sh\n> +++ b/t/t1011-read-tree-sparse-checkout.sh\n> @@ -274,4 +274,24 @@ test_expect_success 'checkout with --ignore-skip-worktree-bits' '\n>  \tgit diff --exit-code HEAD\n>  '\n>  \n> +test_expect_success 'sparse checkout and dir.c sticky bits' '\n> +\tgit init sticky &&\n> +\t(\n> +\t\tcd sticky &&\n> +\t\tmkdir one two &&\n> +\t\ttouch one/hideme one/showme two/showme &&\n> +\t\tgit add . &&\n> +\t\tgit commit -m initial &&\n> +\t\tcat >.git/info/sparse-checkout <<-\\EOF &&\n> +\t\t/*\n> +\t\t!one/hideme\n> +\t\tEOF\n> +\t\tgit config core.sparsecheckout true &&\n> +\t\tgit checkout &&\n> +\t\ttest_path_is_missing one/hideme &&\n> +\t\ttest_path_is_file    one/showme &&\n> +\t\ttest_path_is_file    two/showme\n> +\t)\n> +'\n> +\n>  test_done\n"},{"id":"281162","messageId":"xmqqa8lvr7ix.fsf@gitster.mtv.corp.google.com","threadId":"41721","inReplyTo":"CACsJy8A2FKn-8nWtK4QPMHDCDYvTZBrQs1RVMApnuejXQis19g@mail.gmail.com","subject":"Re: [PATCH 3/2] dir.c: fix dir re-inclusion rules with \"NODIR\" and \"MUSTBEDIR\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-03-18T18:00:06Z","receivedAt":"2016-03-18T18:00:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen <pclouds@gmail.com> writes:\n\n> v2.7.3 differs in the way \"!\" is handled. It does extend reversion\n> outside dir1/dir2/show, back to dir1/dir2. While 2.8+1+2 recognizes\n> and follows the \"/*\" and \"!dir1/dir2/\" pair.\n>\n> The way I interpreted the rules above, though, may be because I'm just\n> trying to defend the current code. Junio, your call on whether to\n> revert this whole patch series.\n\nIt seems that the more you dig into piling changes on this topic\nthere is somebody else who comes back with yet another corner case.\n\nAt this point in the release cycle, I am inclined to say that we\nshould revert the whole thing from 2.8-rc.  Even with that plan, we\nprobably need an extra rc as the reverting a topic this late in the\ncycle is something I feel uncomfortable with.\n\nWhat is the ultimate goal of nd/exclusion-regression-fix topic over\nthe behaviour before 2.7.0 and after 2.7.1 (there was a regression\nin 2.7.0 that was reverted in 2.7.1)?  From the cover letter of the\nseries:\n\n    Take one was bad and reverted in commit 8c72236. Take two provides a\n    more complete solution to the pair of rules\n\n      exclude/this\n      !exclude/this/except/this\n\n    3/4 should do a better job at stopping regressions in take 1. 4/4\n    provides the solution. I think I have tested (and wrote tests) for all\n    the cases I can imagine.\n\n\"solution to the pair of rules\" hints there are some problem in the\npair of rules, without stating what it is trying to solve.\n\nIsn't the root cause of the issue that treat_one_path() when\ndeciding if it is worth descending into a directory check if the\ndirectory itself is excluded and returns path_excluded, even if some\npaths inside it may have a countermanding ! entries that would match\nthem?  What if we change that part smarter and allow it to descend?\n"},{"id":"281165","messageId":"xmqq60wjr5ss.fsf_-_@gitster.mtv.corp.google.com","threadId":"41721","inReplyTo":"xmqqa8lvr7ix.fsf@gitster.mtv.corp.google.com","subject":"Extending this cycle by a week and reverting !reinclusion topic","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-03-18T18:37:23Z","receivedAt":"2016-03-18T18:37:23Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"It seems that people are still finding regressions in the topic for\nthis item in the 2.8-rc3:\n\n * Another try to improve the ignore mechanism that lets you say\n   \"this is excluded\" and then later say \"oh, no, this part (that is\n   a subset of the previous part) is not excluded\".  This has still\n   a known limitation, though.\n\nKnown limitations in a new feature are somewhat OK, but I am getting\nan impression that this is not nearly complete yet and it needs more\ntime to be tested before it inflicts pain to the general public.\n\nI've reverted the offending changes from 'master' and will be\ntagging an extra -rc (2.8-rc4) early next week, as reverting a topic\nthis late in the cycle is something I feel uncomfortable with.\n\nThanks.\n"},{"id":"281190","messageId":"CACsJy8BBwiH9YtDUZRYwHm4t2ou84B6Ltx37ruXkM-vDoSGG9A@mail.gmail.com","threadId":"41721","inReplyTo":"xmqqa8lvr7ix.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 3/2] dir.c: fix dir re-inclusion rules with \"NODIR\" and \"MUSTBEDIR\"","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-03-19T01:03:46Z","receivedAt":"2016-03-19T01:03:46Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sat, Mar 19, 2016 at 1:00 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> What is the ultimate goal of nd/exclusion-regression-fix topic over\n> the behaviour before 2.7.0 and after 2.7.1 (there was a regression\n> in 2.7.0 that was reverted in 2.7.1)?  From the cover letter of the\n> series:\n>\n>     Take one was bad and reverted in commit 8c72236. Take two provides a\n>     more complete solution to the pair of rules\n>\n>       exclude/this\n>       !exclude/this/except/this\n>\n>     3/4 should do a better job at stopping regressions in take 1. 4/4\n>     provides the solution. I think I have tested (and wrote tests) for all\n>     the cases I can imagine.\n>\n> \"solution to the pair of rules\" hints there are some problem in the\n> pair of rules, without stating what it is trying to solve.\n>\n> Isn't the root cause of the issue that treat_one_path() when\n> deciding if it is worth descending into a directory check if the\n> directory itself is excluded and returns path_excluded, even if some\n> paths inside it may have a countermanding ! entries that would match\n> them?  What if we change that part smarter and allow it to descend?\n\nThat's the easy part, detecting a pair and continue descending. The\nproblem is after you descend, exclude engine may return contradicting\nresults on old patterns. It's the same thing that 3/2 describes (after\nyou change patterns from \"!dir\\ndir/file2\" to \"dir\\n!dir/file2\").\n-- \nDuy\n"}]}