{"thread":{"id":"32531","subject":"t7061: comments and one failure","startedAt":"2013-01-05T11:07:24Z","lastAt":"2013-01-07T20:24:35Z","messageCount":13,"participants":["Torsten Bögershausen","Jeff King","Antoine Pelisse","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"206018","messageId":"50E8096C.7000501@web.de","threadId":"32531","inReplyTo":null,"subject":"t7061: comments and one failure","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2013-01-05T11:07:24Z","receivedAt":"2013-01-05T11:07:24Z","isPatch":false,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"Hej,\nTC 9 is failing (Mac OS X 10.6),\n==========================\nexpecting success: \n >tracked/uncommitted &&\n git status --porcelain --ignored >actual &&\n test_cmp expected actual\n\n--- expected 2013-01-05 11:01:00.000000000 +0000\n+++ actual 2013-01-05 11:01:00.000000000 +0000\n@@ -1,4 +1,3 @@\n ?? .gitignore\n ?? actual\n ?? expected\n-!! tracked/\nnot ok 9 - status ignored tracked directory and uncommitted file with --ignore\n# \n# >tracked/uncommitted &&\n# git status --porcelain --ignored >actual &&\n# test_cmp expected actual\n# \n=======================\nI haven't been able to dig further into this,\n(I can volonteer to do some more debugging).\nLooking into the code, there are 2 questions:\n\n1) echo \"ignored\" >.gitignore &&\n We don't need the quoting of a simple string which does not have space in it.\n2) : >untracked/ignored &&\nDo we need the colon here?\n\nWould it make sence to do the following:\n\n\ndiff --git a/t/t7061-wtstatus-ignore.sh b/t/t7061-wtstatus-ignore.sh\nindex 0da1214..761a2e7 100755\n--- a/t/t7061-wtstatus-ignore.sh\n+++ b/t/t7061-wtstatus-ignore.sh\n@@ -12,10 +12,10 @@ cat >expected <<\\EOF\n EOF\n \n test_expect_success 'status untracked directory with --ignored' '\n-\techo \"ignored\" >.gitignore &&\n+\techo ignored >.gitignore &&\n \tmkdir untracked &&\n-\t: >untracked/ignored &&\n-\t: >untracked/uncommitted &&\n+\t>untracked/ignored &&\n+\t>untracked/uncommitted &&\n \tgit status --porcelain --ignored >actual &&\n \ttest_cmp expected actual\n '\n@@ -43,7 +43,7 @@ EOF\n test_expect_success 'status ignored directory with --ignore' '\n \trm -rf untracked &&\n \tmkdir ignored &&\n-\t: >ignored/uncommitted &&\n+\t>ignored/uncommitted &&\n \tgit status --porcelain --ignored >actual &&\n \ttest_cmp expected actual\n '\n@@ -71,8 +71,8 @@ test_expect_success 'status untracked directory with ignored files with --ignore\n \trm -rf ignored &&\n \tmkdir untracked-ignored &&\n \tmkdir untracked-ignored/test &&\n-\t: >untracked-ignored/ignored &&\n-\t: >untracked-ignored/test/ignored &&\n+\t>untracked-ignored/ignored &&\n+\t>untracked-ignored/test/ignored &&\n \tgit status --porcelain --ignored >actual &&\n \ttest_cmp expected actual\n '\n@@ -99,10 +99,10 @@ EOF\n test_expect_success 'status ignored tracked directory with --ignore' '\n \trm -rf untracked-ignored &&\n \tmkdir tracked &&\n-\t: >tracked/committed &&\n+\t>tracked/committed &&\n \tgit add tracked/committed &&\n \tgit commit -m. &&\n-\techo \"tracked\" >.gitignore &&\n+\techo tracked >.gitignore &&\n \tgit status --porcelain --ignored >actual &&\n \ttest_cmp expected actual\n '\n@@ -126,7 +126,7 @@ cat >expected <<\\EOF\n EOF\n \n test_expect_success 'status ignored tracked directory and uncommitted file with --ignore' '\n-\t: >tracked/uncommitted &&\n+\t>tracked/uncommitted &&\n \tgit status --porcelain --ignored >actual &&\n \ttest_cmp expected actual\n '\n\n\n/Torsten\n"},{"id":"206019","messageId":"20130105112432.GA14666@sigill.intra.peff.net","threadId":"32531","inReplyTo":"50E8096C.7000501@web.de","subject":"Re: t7061: comments and one failure","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-01-05T11:24:33Z","receivedAt":"2013-01-05T11:24:33Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Jan 05, 2013 at 12:07:24PM +0100, Torsten Bögershausen wrote:\n\n> TC 9 is failing (Mac OS X 10.6),\n\nYeah, I can reproduce the problem here on my OS X test box. It seems to\nbe related to core.ignorecase. If you put\n\n git config core.ignorecase false\n\nat the top of t7061, it passes on OS X. Likewise, if you set it to true,\nit will start failing on Linux.\n\nSo it looks like a real bug, not a test-portability issue.\n\n> Looking into the code, there are 2 questions:\n> \n> 1) echo \"ignored\" >.gitignore &&\n> We don't need the quoting of a simple string which does not have space in it.\n\nNo, but it does not hurt anything.\n\n> 2) : >untracked/ignored &&\n> Do we need the colon here?\n\nNo, but it does not hurt anything.\n\n-Peff\n"},{"id":"206020","messageId":"CALWbr2ytEBacYKnhVLZ7VHzmuAqp0zwq==LUYZW3WLR0yhJaiA@mail.gmail.com","threadId":"32531","inReplyTo":"20130105112432.GA14666@sigill.intra.peff.net","subject":"Re: t7061: comments and one failure","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2013-01-05T11:29:33Z","receivedAt":"2013-01-05T11:29:33Z","isPatch":false,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"> Yeah, I can reproduce the problem here on my OS X test box. It seems to\n> be related to core.ignorecase. If you put\n>\n> git config core.ignorecase false\n>\n> at the top of t7061, it passes on OS X. Likewise, if you set it to true,\n> it will start failing on Linux.\n\nGreat, so I have a chance to reproduce and fix it. (and it gives me a\nmassive hint on where the bug can stand).\n"},{"id":"206046","messageId":"1357418563-6626-1-git-send-email-apelisse@gmail.com","threadId":"32531","inReplyTo":"20130105112432.GA14666@sigill.intra.peff.net","subject":"[PATCH] status: report ignored yet tracked directories","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2013-01-05T20:42:43Z","receivedAt":"2013-01-05T20:42:43Z","isPatch":true,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"Tracked directories (i.e. directories containing tracked files) that\nare ignored must be reported as ignored if they contain untracked files.\n\nCurrently, tracked files or directories can't be reported untracked or ignored.\nRemove that constraint when searching ignored files.\n\nSigned-off-by: Antoine Pelisse \n---\nTorsten, Jeff,\n\nCan you please test this patch and tell me if this is better ? (t7061 is now\nsuccessful with core.ignorecase=true)\n\nThis patch applies on top of ap/status-ignored-in-ignored-directory (but\n should also apply cleanly on top of next for testing purpose).\n\nThanks,\n\n dir.c | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/dir.c b/dir.c\nindex 9b80348..eefa8ab 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -672,7 +672,8 @@ static struct dir_entry *dir_entry_new(const char *pathname, int len)\n\n static struct dir_entry *dir_add_name(struct dir_struct *dir, const char *pathname, int len)\n {\n-\tif (cache_name_exists(pathname, len, ignore_case))\n+\tif (!(dir->flags & DIR_SHOW_IGNORED) &&\n+\t cache_name_exists(pathname, len, ignore_case))\n \t\treturn NULL;\n\n \tALLOC_GROW(dir->entries, dir->nr+1, dir->alloc);\n--\n1.7.12.4.2.geb8c5b8.dirty\n"},{"id":"206060","messageId":"50E89AC7.9070600@web.de","threadId":"32531","inReplyTo":"1357418563-6626-1-git-send-email-apelisse@gmail.com","subject":"Re: [PATCH] status: report ignored yet tracked directories","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2013-01-05T21:27:35Z","receivedAt":"2013-01-05T21:27:35Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 05.01.13 21:42, Antoine Pelisse wrote:\n> Tracked directories (i.e. directories containing tracked files) that\n> are ignored must be reported as ignored if they contain untracked files.\n>\n> Currently, tracked files or directories can't be reported untracked or ignored.\n> Remove that constraint when searching ignored files.\n>\n> Signed-off-by: Antoine Pelisse \n> ---\n> Torsten, Jeff,\n>\n> Can you please test this patch and tell me if this is better ? (t7061 is now\n> successful with core.ignorecase=true)\n>\n> This patch applies on top of ap/status-ignored-in-ignored-directory (but\n> should also apply cleanly on top of next for testing purpose).\n>\n> Thanks,\n>\n> dir.c | 3 ++-\n> 1 file changed, 2 insertions(+), 1 deletion(-)\n>\n> diff --git a/dir.c b/dir.c\n> index 9b80348..eefa8ab 100644\n> --- a/dir.c\n> +++ b/dir.c\n> @@ -672,7 +672,8 @@ static struct dir_entry *dir_entry_new(const char *pathname, int len)\n>\n> static struct dir_entry *dir_add_name(struct dir_struct *dir, const char *pathname, int len)\n> {\n> -\tif (cache_name_exists(pathname, len, ignore_case))\n> +\tif (!(dir->flags & DIR_SHOW_IGNORED) &&\n> +\t cache_name_exists(pathname, len, ignore_case))\n> \t\treturn NULL;\n>\n> \tALLOC_GROW(dir->entries, dir->nr+1, dir->alloc);\n> --\n> 1.7.12.4.2.geb8c5b8.dirty\n>\n(BTW: thanks for contributing to git)\nAntoine, the test is OK:\n# passed all 10 test(s)\n================\nI'm not sure if I am happy with the commit message, so I try to have an improved\nversion below, which may be a starting point for a discussion:\n\ngit status: report ignored directories correctly\n\nA directory containing tracked files where the directory\nis ignored must be reported as ignored even if it contains untracked files.\n\n/Torsten\n"},{"id":"206069","messageId":"20130105230303.GA5195@sigill.intra.peff.net","threadId":"32531","inReplyTo":"1357418563-6626-1-git-send-email-apelisse@gmail.com","subject":"Re: [PATCH] status: report ignored yet tracked directories","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-01-05T23:03:03Z","receivedAt":"2013-01-05T23:03:03Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Jan 05, 2013 at 09:42:43PM +0100, Antoine Pelisse wrote:\n\n> Tracked directories (i.e. directories containing tracked files) that\n> are ignored must be reported as ignored if they contain untracked files.\n> \n> Currently, tracked files or directories can't be reported untracked or ignored.\n> Remove that constraint when searching ignored files.\n> \n> Signed-off-by: Antoine Pelisse \n> ---\n\nI was expecting to see some explanation of the user-visible bug here. In\nother words, what does this fix, and why does the bug only happen when\ncore.ignorecase is set.\n\nLooking at your fix and remembering how the index hashing works, I think\nthe answer is that:\n\n 1. This bug only affects directories, because they are the only thing\n that can be simultaneously \"ignored and untracked\" and \"tracked\"\n (i.e., they have entries of both, and we are using\n DIR_SHOW_OTHER_DIRECTORIES).\n\n 2. When core.ignorecase is false, the index name hash contains only\n the file entries, and cache_name_exists returns an exact match. So\n it doesn't matter if we make an extra check when adding the\n directory via dir_add_name; we know that it will not be there, and\n the final check is a no-op.\n\n 3. When core.ignorecase is true, we also store directory entries in\n the index name hash, and this extra check is harmful; the entry\n does not really exist in the index, and we still need to add it.\n\nBut that makes me wonder. In the ignorecase=false case, I claimed that\nthe check in dir_add_name is a no-op for mixed tracked/ignored\ndirectories. But it is presumably not a no-op for other cases. Your\npatch only turns it off when DIR_SHOW_IGNORED is set. But is it possible\nfor us to have DIR_SHOW_IGNORED set, _and_ to pass in a path that exists\nin the index as a regular file?\n\nI think in the normal file case, we'd expect treat_path to just tell us\nthat it is handled, and we would not ever call dir_add_name in the first\nplace. But what if we have an index entry for a file, but the working\ntree now contains a directory?\n\nI _think_ we still do not hit this code path in that instance, because\nwe will end up in treat_directory, and we will end up checking\ndirectory_exists_in_index. And I cannot get it to misbehave in practice.\nSo I think your fix is correct, but the exact how and why is a bit\nsubtle.\n\n-Peff\n"},{"id":"206145","messageId":"CALWbr2yRQai2Z08G2qFbA3AvsgivR-8kQ64SZ4pEktyrf+ZXiQ@mail.gmail.com","threadId":"32531","inReplyTo":"20130105230303.GA5195@sigill.intra.peff.net","subject":"Re: [PATCH] status: report ignored yet tracked directories","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2013-01-06T16:40:46Z","receivedAt":"2013-01-06T16:40:46Z","isPatch":true,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"On Sun, Jan 6, 2013 at 12:03 AM, Jeff King wrote:\n> On Sat, Jan 05, 2013 at 09:42:43PM +0100, Antoine Pelisse wrote:\n>\n>> Tracked directories (i.e. directories containing tracked files) that\n>> are ignored must be reported as ignored if they contain untracked files.\n>>\n>> Currently, tracked files or directories can't be reported untracked or ignored.\n>> Remove that constraint when searching ignored files.\n>>\n>> Signed-off-by: Antoine Pelisse \n>> ---\n>\n> I was expecting to see some explanation of the user-visible bug here. In\n> other words, what does this fix, and why does the bug only happen when\n> core.ignorecase is set.\n\nI spent a couple of hours trying to understand that issue, and even if\nI ended-up with pretty much the same points as you do below, I was not\nconfident enough to phrase it like you just did.\n\n> Looking at your fix and remembering how the index hashing works, I think\n> the answer is that:\n>\n> 1. This bug only affects directories, because they are the only thing\n> that can be simultaneously \"ignored and untracked\" and \"tracked\"\n> (i.e., they have entries of both, and we are using\n> DIR_SHOW_OTHER_DIRECTORIES).\n>\n> 2. When core.ignorecase is false, the index name hash contains only\n> the file entries, and cache_name_exists returns an exact match. So\n> it doesn't matter if we make an extra check when adding the\n> directory via dir_add_name; we know that it will not be there, and\n> the final check is a no-op.\n>\n> 3. When core.ignorecase is true, we also store directory entries in\n> the index name hash, and this extra check is harmful; the entry\n> does not really exist in the index, and we still need to add it.\n\nYes, because of this couple of lines I guess (name-hash.c, hash_index_entry()):\n\n if (ignore_case)\n hash_index_entry_directories(istate, ce);\n\n> But that makes me wonder. In the ignorecase=false case, I claimed that\n> the check in dir_add_name is a no-op for mixed tracked/ignored\n> directories. But it is presumably not a no-op for other cases. Your\n> patch only turns it off when DIR_SHOW_IGNORED is set. But is it possible\n> for us to have DIR_SHOW_IGNORED set, _and_ to pass in a path that exists\n> in the index as a regular file?\n\nI don't think so, because of the optimization I added in my previous\npatch, in treat_file():\n\n /*\n * Optimization:\n * Don't spend time on indexed files, they won't be\n * added to the list anyway\n */\n struct cache_entry *ce = index_name_exists(&the_index,\n path->buf, path->len, ignore_case);\n\nIt's no longer an optimization but a required step, I will update the comment.\n\n> I think in the normal file case, we'd expect treat_path to just tell us\n> that it is handled, and we would not ever call dir_add_name in the first\n> place. But what if we have an index entry for a file, but the working\n> tree now contains a directory?\n\nThe directory is treated as any other untracked directory (it never\nmatches indexed file because of the trailing /).\n\n> I _think_ we still do not hit this code path in that instance, because\n> we will end up in treat_directory, and we will end up checking\n> directory_exists_in_index. And I cannot get it to misbehave in practice.\n> So I think your fix is correct, but the exact how and why is a bit\n> subtle.\n\nThanks a lot for the help, I will try to come up with a better commit\nmessage now.\n\n> -Peff\n"},{"id":"206182","messageId":"1357510179-22852-1-git-send-email-apelisse@gmail.com","threadId":"32531","inReplyTo":"20130105230303.GA5195@sigill.intra.peff.net","subject":"[PATCH v2] status: always report ignored tracked directories","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2013-01-06T22:09:39Z","receivedAt":"2013-01-06T22:09:39Z","isPatch":true,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"Tracked directories (i.e. directories containing tracked files) that\nare ignored must be reported as ignored if they contain untracked files.\n\nCurrently, files in the index can't be reported as ignored and are\nautomatically dropped from the list:\n\n - When core.ignorecase is false, directories (which are not directly\n tracked) are not listed as part of the index, and the directory can be\n shown as ignored.\n\n - When core.ignorecase is true on the other hand, directories are\n reported as part of the index, and the directory is dropped, thus not\n displayed as ignored.\n\nFix that behavior by allowing indexed file to be added when looking for\nignored files.\n\n - Ignored tracked and untracked directories are treated the same way\n when looking for ignored files, so we should not care about their index\n status.\n - Files are dismissed by treat_file() if they belong to the\n index, so that step would have been a no-op anyway.\n\nSigned-off-by: Antoine Pelisse \n---\n dir.c | 9 +++------\n 1 file changed, 3 insertions(+), 6 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex 9b80348..f836590 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -672,7 +672,8 @@ static struct dir_entry *dir_entry_new(const char *pathname, int len)\n\n static struct dir_entry *dir_add_name(struct dir_struct *dir, const char *pathname, int len)\n {\n-\tif (cache_name_exists(pathname, len, ignore_case))\n+\tif (!(dir->flags & DIR_SHOW_IGNORED) &&\n+\t cache_name_exists(pathname, len, ignore_case))\n \t\treturn NULL;\n\n \tALLOC_GROW(dir->entries, dir->nr+1, dir->alloc);\n@@ -877,11 +878,7 @@ static int treat_file(struct dir_struct *dir, struct strbuf *path, int exclude,\n \tif (exclude)\n \t\texclude_file = !(dir->flags & DIR_SHOW_IGNORED);\n \telse if (dir->flags & DIR_SHOW_IGNORED) {\n-\t\t/*\n-\t\t * Optimization:\n-\t\t * Don't spend time on indexed files, they won't be\n-\t\t * added to the list anyway\n-\t\t */\n+\t\t/* Always exclude indexed files */\n \t\tstruct cache_entry *ce = index_name_exists(&the_index,\n \t\t path->buf, path->len, ignore_case);\n\n--\n1.7.12.4.3.g90f5e2d\n"},{"id":"206216","messageId":"20130107083318.GA18052@sigill.intra.peff.net","threadId":"32531","inReplyTo":"CALWbr2yRQai2Z08G2qFbA3AvsgivR-8kQ64SZ4pEktyrf+ZXiQ@mail.gmail.com","subject":"Re: [PATCH] status: report ignored yet tracked directories","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-01-07T08:33:18Z","receivedAt":"2013-01-07T08:33:18Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Jan 06, 2013 at 05:40:46PM +0100, Antoine Pelisse wrote:\n\n> > Looking at your fix and remembering how the index hashing works, I think\n> > the answer is that:\n> >\n> > 1. This bug only affects directories, because they are the only thing\n> > that can be simultaneously \"ignored and untracked\" and \"tracked\"\n> > (i.e., they have entries of both, and we are using\n> > DIR_SHOW_OTHER_DIRECTORIES).\n> >\n> > 2. When core.ignorecase is false, the index name hash contains only\n> > the file entries, and cache_name_exists returns an exact match. So\n> > it doesn't matter if we make an extra check when adding the\n> > directory via dir_add_name; we know that it will not be there, and\n> > the final check is a no-op.\n> >\n> > 3. When core.ignorecase is true, we also store directory entries in\n> > the index name hash, and this extra check is harmful; the entry\n> > does not really exist in the index, and we still need to add it.\n> \n> Yes, because of this couple of lines I guess (name-hash.c, hash_index_entry()):\n> \n> if (ignore_case)\n> hash_index_entry_directories(istate, ce);\n\nExactly. I couldn't remember at first why this was the case, but after\nreading 5102c61 (Add case insensitivity support for directories when\nusing git status, 2010-10-03) again, I think it is because we cannot do\na partial-name lookup via the hash (i.e., the hash for \"foo/\" and\n\"foo/bar\" have no relation to each other). Not related to your patch,\nobviously, but it was the missing piece for me to understand why the\ncode was doing what it does.\n\n> > I think in the normal file case, we'd expect treat_path to just tell us\n> > that it is handled, and we would not ever call dir_add_name in the first\n> > place. But what if we have an index entry for a file, but the working\n> > tree now contains a directory?\n> \n> The directory is treated as any other untracked directory (it never\n> matches indexed file because of the trailing /).\n\nAh, right. That makes sense.\n\n> > I _think_ we still do not hit this code path in that instance, because\n> > we will end up in treat_directory, and we will end up checking\n> > directory_exists_in_index. And I cannot get it to misbehave in practice.\n> > So I think your fix is correct, but the exact how and why is a bit\n> > subtle.\n> \n> Thanks a lot for the help, I will try to come up with a better commit\n> message now.\n\nThanks. I think the patch is right, but the reasoning is just a bit\nsubtle.\n\n-Peff\n"},{"id":"206236","messageId":"50EB0409.1090307@web.de","threadId":"32531","inReplyTo":"1357510179-22852-1-git-send-email-apelisse@gmail.com","subject":"Re: [PATCH v2] status: always report ignored tracked directories","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2013-01-07T17:21:13Z","receivedAt":"2013-01-07T17:21:13Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 06.01.13 23:09, Antoine Pelisse wrote:\n[snip]\n> Signed-off-by: Antoine Pelisse \n> ---\n> dir.c | 9 +++------\n> 1 file changed, 3 insertions(+), 6 deletions(-)\n>\n> diff --git a/dir.c b/dir.c\n> index 9b80348..f836590 100644\n> --- a/dir.c\n> +++ b/dir.c\n> @@ -672,7 +672,8 @@ static struct dir_entry *dir_entry_new(const char *pathname, int len)\n>\n> static struct dir_entry *dir_add_name(struct dir_struct *dir, const char *pathname, int len)\n> {\n> -\tif (cache_name_exists(pathname, len, ignore_case))\n> +\tif (!(dir->flags & DIR_SHOW_IGNORED) &&\n> +\t cache_name_exists(pathname, len, ignore_case))\n> \t\treturn NULL;\n>\n> \tALLOC_GROW(dir->entries, dir->nr+1, dir->alloc);\n> @@ -877,11 +878,7 @@ static int treat_file(struct dir_struct *dir, struct strbuf *path, int exclude,\n> \tif (exclude)\n> \t\texclude_file = !(dir->flags & DIR_SHOW_IGNORED);\n> \telse if (dir->flags & DIR_SHOW_IGNORED) {\n> -\t\t/*\n> -\t\t * Optimization:\n> -\t\t * Don't spend time on indexed files, they won't be\n> -\t\t * added to the list anyway\n> -\t\t */\n> +\t\t/* Always exclude indexed files */\n> \t\tstruct cache_entry *ce = index_name_exists(&the_index,\n> \t\t path->buf, path->len, ignore_case);\n>\n> --\n> 1.7.12.4.3.g90f5e2d\n>\nThe bad news: the patch does not apply.\nThe good news: t7061 passes on pu,\nand dir.c seems to be changes as needed:\n\ncommit 1f4e17c6c9833f17dc6bbf045f8a8d6378dcb417\nMerge: dee1fa4 cc37e5b\nAuthor: Junio C Hamano \nDate: Sun Jan 6 23:46:29 2013 -0800\n\nMerge branch 'nd/parse-pathspec' into pu\n\nwhich comes from Duy:\n\ncommit cc37e5bf18ca11d9a884bddfebcdff61df3e6279\nAuthor: Nguyễn Thái Ngọc Duy \nDate: Sun Jan 6 13:21:08 2013 +0700\n\nConvert more init_pathspec() to parse_pathspec()\n"},{"id":"206238","messageId":"7v4niskhlb.fsf@alter.siamese.dyndns.org","threadId":"32531","inReplyTo":"50EB0409.1090307@web.de","subject":"Re: [PATCH v2] status: always report ignored tracked directories","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-07T17:50:40Z","receivedAt":"2013-01-07T17:50:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Torsten Bögershausen writes:\n\n> The bad news: the patch does not apply.\n\nHuh, isn't this already queued as 22ccf86 (status: always report\nignored tracked directories, 2013-01-06)?\n\n> The good news: t7061 passes on pu,\n> and dir.c seems to be changes as needed:\n>\n> commit 1f4e17c6c9833f17dc6bbf045f8a8d6378dcb417\n> Merge: dee1fa4 cc37e5b\n> Author: Junio C Hamano \n> Date: Sun Jan 6 23:46:29 2013 -0800\n>\n> Merge branch 'nd/parse-pathspec' into pu\n>\n> which comes from Duy:\n>\n> commit cc37e5bf18ca11d9a884bddfebcdff61df3e6279\n> Author: Nguyễn Thái Ngọc Duy \n> Date: Sun Jan 6 13:21:08 2013 +0700\n>\n> Convert more init_pathspec() to parse_pathspec()\n\nYes, it needs conflict resolution with other topics in flight, but\nthe thing to test is to see if the result of merging 22ccf86 into\nthe 'master' branch does what we want; the newer topic is still in\nflux and we know it will be rerolled before it gets into testable\nshape.\n"},{"id":"206244","messageId":"7vip78izir.fsf@alter.siamese.dyndns.org","threadId":"32531","inReplyTo":"1357510179-22852-1-git-send-email-apelisse@gmail.com","subject":"Re: [PATCH v2] status: always report ignored tracked directories","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-07T19:06:20Z","receivedAt":"2013-01-07T19:06:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Antoine Pelisse writes:\n\nFirst, thanks for working on this.\n\nThe explanation is a bit confusing, especially for people like me,\nas it does not make it clear that there are two kinds of \"paths in\nthe index\". Files can be added to the index (\"git add\" and then\nshown via \"ls-files\") and these are what we usually call \"files in\nthe index\". But there is a separate \"name hash\" that keeps track of\n\"paths the index knows about\" in play, and that is what is discussed\nin the description.\n\n> Tracked directories (i.e. directories containing tracked files)\n> that are ignored must be reported as ignored if they contain\n> untracked files.\n>\n> Currently, files in the index can't be reported as ignored and are\n> automatically dropped from the list:\n\n\"file in the index\" will never be ignored, period. Some paths the\nindex knows about, namely, directory names in name hash, may need to\nbe reported as ignored.\n\nIt is not clear what \"list\" the above \"dropped from the list\" refers\nto. The list of paths that becomes the result of \"status\"?\n\n> - When core.ignorecase is false, directories (which are not directly\n> tracked) are not listed as part of the index, and the directory can be\n> shown as ignored.\n>\n> - When core.ignorecase is true on the other hand, directories are\n> reported as part of the index, and the directory is dropped, thus not\n> displayed as ignored.\n>\n> Fix that behavior by allowing indexed file to be added when looking for\n> ignored files.\n>\n> - Ignored tracked and untracked directories are treated the same way\n> when looking for ignored files, so we should not care about their index\n> status.\n> - Files are dismissed by treat_file() if they belong to the\n> index, so that step would have been a no-op anyway.\n\nHere is my attempt...\n\n When enumerating paths that are ignored, paths the index knows\n about are not included in the result. The \"index knows about\"\n check is done by consulting the name hash, not the actual\n contents of the index:\n\n - When core.ignorecase is false, directory names are not in the\n name hash, and ignored ones are shown as ignored (directories\n can never be tracked anyway).\n\n - When core.ignorecase is true, however, the name hash keeps\n track of the names of directories, in order to detect\n additions of the paths under different cases. This causes\n ignored directories to be mistakenly excluded when\n enumerating ignored paths.\n\n Stop excluding directories that are in the name hash when\n looking for ignored files in dir_add_name(); the names that are\n actually in the index are excluded much earlier in the callchain\n in treat_file(), so this fix will not make them mistakenly\n identified as ignored.\n\n Signed-off-by: Antoine Pelisse \n Reviewed-by: Jeff King \n Signed-off-by: Junio C Hamano \n\ndiff --git a/dir.c b/dir.c\nindex 9b80348..f836590 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -672,7 +672,8 @@ static struct dir_entry *dir_entry_new(const char *pathname, int len)\n \n static struct dir_entry *dir_add_name(struct dir_struct *dir, const char *pathname, int len)\n {\n-\tif (cache_name_exists(pathname, len, ignore_case))\n+\tif (!(dir->flags & DIR_SHOW_IGNORED) &&\n+\t cache_name_exists(pathname, len, ignore_case))\n \t\treturn NULL;\n \n \tALLOC_GROW(dir->entries, dir->nr+1, dir->alloc);\n@@ -877,11 +878,7 @@ static int treat_file(struct dir_struct *dir, struct strbuf *path, int exclude,\n \tif (exclude)\n \t\texclude_file = !(dir->flags & DIR_SHOW_IGNORED);\n \telse if (dir->flags & DIR_SHOW_IGNORED) {\n-\t\t/*\n-\t\t * Optimization:\n-\t\t * Don't spend time on indexed files, they won't be\n-\t\t * added to the list anyway\n-\t\t */\n+\t\t/* Always exclude indexed files */\n \t\tstruct cache_entry *ce = index_name_exists(&the_index,\n \t\t path->buf, path->len, ignore_case);\n \n"},{"id":"206248","messageId":"CALWbr2xQePMwPARTNu566=OU9KtKo9nWPo=0j2MSpjBjAqegyg@mail.gmail.com","threadId":"32531","inReplyTo":"7vip78izir.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2] status: always report ignored tracked directories","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2013-01-07T20:24:35Z","receivedAt":"2013-01-07T20:24:35Z","isPatch":true,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"> Here is my attempt...\n>\n> When enumerating paths that are ignored, paths the index knows\n> about are not included in the result. The \"index knows about\"\n> check is done by consulting the name hash, not the actual\n> contents of the index:\n>\n> - When core.ignorecase is false, directory names are not in the\n> name hash, and ignored ones are shown as ignored (directories\n> can never be tracked anyway).\n>\n> - When core.ignorecase is true, however, the name hash keeps\n> track of the names of directories, in order to detect\n> additions of the paths under different cases. This causes\n> ignored directories to be mistakenly excluded when\n> enumerating ignored paths.\n>\n> Stop excluding directories that are in the name hash when\n> looking for ignored files in dir_add_name(); the names that are\n> actually in the index are excluded much earlier in the callchain\n> in treat_file(), so this fix will not make them mistakenly\n> identified as ignored.\n>\n> Signed-off-by: Antoine Pelisse \n> Reviewed-by: Jeff King \n> Signed-off-by: Junio C Hamano \n\nI like it a lot, thanks to both of you\n"}]}