{"thread":{"id":"45826","subject":"Bug Report: .gitignore behavior is not matching in git clean and git status","startedAt":"2017-04-28T22:56:32Z","lastAt":"2017-05-26T01:05:20Z","messageCount":89,"participants":["Chris Johnson","Junio C Hamano","Samuel Lijin","Stefan Beller","Torsten Bögershausen"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"318218","messageId":"CABfTTAchc61aB02sCD=Oa9gRMGr94h7mC53B9q6Qy2k2hDqzAQ@mail.gmail.com","threadId":"45826","inReplyTo":null,"subject":"Bug Report: .gitignore behavior is not matching in git clean and git status","fromName":"Chris Johnson","fromEmail":"chrisjohnson0@gmail.com","sentAt":"2017-04-28T22:56:02Z","receivedAt":"2017-04-28T22:56:32Z","isPatch":false,"sender":{"key":"chrisjohnson0@gmail.com","avatar":null},"body":"I am a mailing list noob so I’m sorry if this is the wrong format or\nthe wrong please.\n\nHere’s the setup for the bug (I will call it a bug but I half expect\nsomebody to tell me I’m an idiot):\n\ngit init\necho \"/A/B/\" > .gitignore\ngit add .gitignore && git commit -m 'Add ignore'\nmkdir -p A/B\ntouch A/B/C\ngit status\ngit clean -dn\n\n(my client may have screwed up the quotes, please bear with me)\n\nNow, this may just be a case of me misunderstanding the behavior of\n.gitignore, and that’s fine, but to me, git clean -dn should roughly\nreflect the same behavior as git status as far as ignored files go. As\nit is, git status does NOT report A/B/C as an untracked file, but git\nclean will remove the entire A dir. It seems to me that one or the\nother’s behavior ought to be tweaked. Which one is correct I am not\nsure.\n\nThis happens on 2.5.1 as well as 2.9.2\n\nChris\n"},{"id":"318280","messageId":"xmqq60hljqud.fsf@gitster.mtv.corp.google.com","threadId":"45826","inReplyTo":"CABfTTAchc61aB02sCD=Oa9gRMGr94h7mC53B9q6Qy2k2hDqzAQ@mail.gmail.com","subject":"Re: Bug Report: .gitignore behavior is not matching in git clean and git status","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-01T01:41:46Z","receivedAt":"2017-05-01T01:41:56Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Chris Johnson writes:\n\n> I am a mailing list noob so I’m sorry if this is the wrong format or\n> the wrong please.\n>\n> Here’s the setup for the bug (I will call it a bug but I half expect\n> somebody to tell me I’m an idiot):\n>\n> git init\n> echo \"/A/B/\" > .gitignore\n\nYou tell Git that anything in A/B/ are uninteresting.\n\n> git add .gitignore && git commit -m 'Add ignore'\n> mkdir -p A/B\n> touch A/B/C\n\nAnd create an uninteresting cruft.\n\n> git status\n\nAnd Git does not bug you about it.\n\n> git clean -dn\n\nThis incorrectly reports \"Would remove A/\" and if you gave 'f'\ninstead of 'n', it does remove A/, A/B, and A/B/C.\n\nDespite that \"git clean --help\" says 'only files unknown to Git are\nremoved' (with an undefined term 'unknown to Git'). What it wants\nthe term mean can be guessed by seeing 'if the -x option is\nspecified, ignored files are also removed'---so 'unknown to Git'\ndoes not include what you told .gitignore that they are\nuninteresting. IOW, Git knows they are not interesting.\n\nIt looks like a bug in \"git clean -d\" to me. Do you see the same\nissue if you use \"git clean\" without \"-d\"? IOW, does it offer to\nremove A/B/C?\n"},{"id":"318285","messageId":"CABfTTAdQPGei6CZoHGJGUtKHdJ4eT822pzc=DATynfeaZ94gxA@mail.gmail.com","threadId":"45826","inReplyTo":"xmqq60hljqud.fsf@gitster.mtv.corp.google.com","subject":"Re: Bug Report: .gitignore behavior is not matching in git clean and git status","fromName":"Chris Johnson","fromEmail":"chrisjohnson0@gmail.com","sentAt":"2017-05-01T01:56:43Z","receivedAt":"2017-05-01T01:57:10Z","isPatch":false,"sender":{"key":"chrisjohnson0@gmail.com","avatar":null},"body":"Good assessment/understanding of the issue. git clean -n does not\nreport anything as being targeted for removal, and git clean -f\nmatches that behavior. I agree with it probably being related\nspecifically to the -d flag.\n\nAs another experiment I modified .gitignore to ignore /A/B/C instead\nof /A/B/ and the same result occurs (-n reports nothing, -dn reports\nremoving A/)\n\nLastly, I changed .gitignore to just be /A/, and in doing so, clean\n-dn stops reporting that it will remove A/. I’m not exactly sure if\nthis last one is surprising or not.\n\nAlso, and sorry for the noise, but I did a reply-all here, but will a\nreply automatically include the rest of the list? Or was reply-all the\nright move?\n\nOn Sun, Apr 30, 2017 at 9:41 PM, Junio C Hamano wrote:\n> Chris Johnson writes:\n>\n>> I am a mailing list noob so I’m sorry if this is the wrong format or\n>> the wrong please.\n>>\n>> Here’s the setup for the bug (I will call it a bug but I half expect\n>> somebody to tell me I’m an idiot):\n>>\n>> git init\n>> echo \"/A/B/\" > .gitignore\n>\n> You tell Git that anything in A/B/ are uninteresting.\n>\n>> git add .gitignore && git commit -m 'Add ignore'\n>> mkdir -p A/B\n>> touch A/B/C\n>\n> And create an uninteresting cruft.\n>\n>> git status\n>\n> And Git does not bug you about it.\n>\n>> git clean -dn\n>\n> This incorrectly reports \"Would remove A/\" and if you gave 'f'\n> instead of 'n', it does remove A/, A/B, and A/B/C.\n>\n> Despite that \"git clean --help\" says 'only files unknown to Git are\n> removed' (with an undefined term 'unknown to Git'). What it wants\n> the term mean can be guessed by seeing 'if the -x option is\n> specified, ignored files are also removed'---so 'unknown to Git'\n> does not include what you told .gitignore that they are\n> uninteresting. IOW, Git knows they are not interesting.\n>\n> It looks like a bug in \"git clean -d\" to me. Do you see the same\n> issue if you use \"git clean\" without \"-d\"? IOW, does it offer to\n> remove A/B/C?\n"},{"id":"318343","messageId":"xmqqziexgtds.fsf@gitster.mtv.corp.google.com","threadId":"45826","inReplyTo":"CABfTTAdQPGei6CZoHGJGUtKHdJ4eT822pzc=DATynfeaZ94gxA@mail.gmail.com","subject":"Re: Bug Report: .gitignore behavior is not matching in git clean and git status","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-01T03:15:11Z","receivedAt":"2017-05-01T03:15:17Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Chris Johnson writes:\n\n> Also, and sorry for the noise, but I did a reply-all here, but will a\n> reply automatically include the rest of the list? Or was reply-all the\n> right move?\n\nThe convention around here is to do reply-all (in other words, make\nsure that Cc: line has git@vger.kernel.org and others involved in\nthe discussion and mentioned on To: or Cc: line of the message you\nare responding to). Your message (i.e. the one I am responding to)\nhas correct addresses on To:/Cc: lines AFAICS.\n\n"},{"id":"318370","messageId":"CAJZjrdXfcaUrJXbAoPtRtvigouZ2eNyNsZ=2WtSY20_D+Ow6qw@mail.gmail.com","threadId":"45826","inReplyTo":"CABfTTAdQPGei6CZoHGJGUtKHdJ4eT822pzc=DATynfeaZ94gxA@mail.gmail.com","subject":"Re: Bug Report: .gitignore behavior is not matching in git clean and git status","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-01T13:51:01Z","receivedAt":"2017-05-01T13:51:47Z","isPatch":false,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"On Sun, Apr 30, 2017 at 8:56 PM, Chris Johnson wrote:\n> Good assessment/understanding of the issue. git clean -n does not\n> report anything as being targeted for removal, and git clean -f\n> matches that behavior. I agree with it probably being related\n> specifically to the -d flag.\n>\n> As another experiment I modified .gitignore to ignore /A/B/C instead\n> of /A/B/ and the same result occurs (-n reports nothing, -dn reports\n> removing A/)\n>\n> Lastly, I changed .gitignore to just be /A/, and in doing so, clean\n> -dn stops reporting that it will remove A/. I’m not exactly sure if\n> this last one is surprising or not.\n\nIt doesn't seem so to me, since a trailing slash in .gitignore is an\nexplicit directive to ignore the entire directory, so when pruning the\ntree of files/dirs to ignore, it drops everything in A/ before even\ngetting to A/B/C. The issue is that ignoring A/B/C shouldn't leave A/\nto be cleaned.\n\n> Also, and sorry for the noise, but I did a reply-all here, but will a\n> reply automatically include the rest of the list? Or was reply-all the\n> right move?\n>\n> On Sun, Apr 30, 2017 at 9:41 PM, Junio C Hamano wrote:\n>> Chris Johnson writes:\n>>\n>>> I am a mailing list noob so I’m sorry if this is the wrong format or\n>>> the wrong please.\n>>>\n>>> Here’s the setup for the bug (I will call it a bug but I half expect\n>>> somebody to tell me I’m an idiot):\n>>>\n>>> git init\n>>> echo \"/A/B/\" > .gitignore\n>>\n>> You tell Git that anything in A/B/ are uninteresting.\n>>\n>>> git add .gitignore && git commit -m 'Add ignore'\n>>> mkdir -p A/B\n>>> touch A/B/C\n>>\n>> And create an uninteresting cruft.\n>>\n>>> git status\n>>\n>> And Git does not bug you about it.\n>>\n>>> git clean -dn\n>>\n>> This incorrectly reports \"Would remove A/\" and if you gave 'f'\n>> instead of 'n', it does remove A/, A/B, and A/B/C.\n>>\n>> Despite that \"git clean --help\" says 'only files unknown to Git are\n>> removed' (with an undefined term 'unknown to Git'). What it wants\n>> the term mean can be guessed by seeing 'if the -x option is\n>> specified, ignored files are also removed'---so 'unknown to Git'\n>> does not include what you told .gitignore that they are\n>> uninteresting. IOW, Git knows they are not interesting.\n>>\n>> It looks like a bug in \"git clean -d\" to me.\n\nI may be wrong (I'm not very familiar with the codebase), but I don't\nthink it's a bug in specifically git clean -d.\n\nThrowing gdb at it, when builtin/clean.c:cmd_clean() calls\ndir.c:fill_directory(), A/ gets appended to dir->entries, regardless\nof whether or not git clean is called with or without -d. The\ndifference is that if git clean is called without -d, the loop that\nstrips out directories strips out the A/ entry.\n\nWhen dir.c:fill_directory() is invoked through git status, A/ does\nnot, however, get appended to dir->entries. As best as I can tell,\nthis seems to be because git status sets the\nDIR_HIDE_EMPTY_DIRECTORIES flag, whereas git clean does not (which\nmakes sense), but the fact that DIR_HIDE_EMPTY_DIRECTORIES is\nresponsible for not adding A/ to dir->entries seems to be the issue.\n"},{"id":"318373","messageId":"CAJZjrdXVopdvDoDwnsmqvrLESKQq=-a2waDgu5HYYoP2WvSrPA@mail.gmail.com","threadId":"45826","inReplyTo":"CAJZjrdXfcaUrJXbAoPtRtvigouZ2eNyNsZ=2WtSY20_D+Ow6qw@mail.gmail.com","subject":"Re: Bug Report: .gitignore behavior is not matching in git clean and git status","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-01T15:34:08Z","receivedAt":"2017-05-01T15:43:35Z","isPatch":false,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"After some more digging (and familiarizing myself with the\nbehind-the-scenes logic) the issue is that dir.c has this implicit\nassumption that a directory which contains only untracked and ignored\nfiles should itself be considered untracked. While that works fine for\nuse cases where we're asking if a directory should be added to the git\ndatabase, that decidedly does not make sense when we're asking if a\ndirectory can be removed from the working tree.\n\nI'm not sure where to proceed from here. I see two ways forward: one,\nbuiltin/clean.c can collect ignored files when it calls\ndir.c:fill_directory(), and then clean -d can prune out directories\nthat contain ignored files; two, path_treatment can learn about\nuntracked directories which contain excluded (ignored) files.\n\nOn Mon, May 1, 2017 at 8:51 AM, Samuel Lijin wrote:\n> On Sun, Apr 30, 2017 at 8:56 PM, Chris Johnson wrote:\n>> Good assessment/understanding of the issue. git clean -n does not\n>> report anything as being targeted for removal, and git clean -f\n>> matches that behavior. I agree with it probably being related\n>> specifically to the -d flag.\n>>\n>> As another experiment I modified .gitignore to ignore /A/B/C instead\n>> of /A/B/ and the same result occurs (-n reports nothing, -dn reports\n>> removing A/)\n>>\n>> Lastly, I changed .gitignore to just be /A/, and in doing so, clean\n>> -dn stops reporting that it will remove A/. I’m not exactly sure if\n>> this last one is surprising or not.\n>\n> It doesn't seem so to me, since a trailing slash in .gitignore is an\n> explicit directive to ignore the entire directory, so when pruning the\n> tree of files/dirs to ignore, it drops everything in A/ before even\n> getting to A/B/C. The issue is that ignoring A/B/C shouldn't leave A/\n> to be cleaned.\n>\n>> Also, and sorry for the noise, but I did a reply-all here, but will a\n>> reply automatically include the rest of the list? Or was reply-all the\n>> right move?\n>>\n>> On Sun, Apr 30, 2017 at 9:41 PM, Junio C Hamano wrote:\n>>> Chris Johnson writes:\n>>>\n>>>> I am a mailing list noob so I’m sorry if this is the wrong format or\n>>>> the wrong please.\n>>>>\n>>>> Here’s the setup for the bug (I will call it a bug but I half expect\n>>>> somebody to tell me I’m an idiot):\n>>>>\n>>>> git init\n>>>> echo \"/A/B/\" > .gitignore\n>>>\n>>> You tell Git that anything in A/B/ are uninteresting.\n>>>\n>>>> git add .gitignore && git commit -m 'Add ignore'\n>>>> mkdir -p A/B\n>>>> touch A/B/C\n>>>\n>>> And create an uninteresting cruft.\n>>>\n>>>> git status\n>>>\n>>> And Git does not bug you about it.\n>>>\n>>>> git clean -dn\n>>>\n>>> This incorrectly reports \"Would remove A/\" and if you gave 'f'\n>>> instead of 'n', it does remove A/, A/B, and A/B/C.\n>>>\n>>> Despite that \"git clean --help\" says 'only files unknown to Git are\n>>> removed' (with an undefined term 'unknown to Git'). What it wants\n>>> the term mean can be guessed by seeing 'if the -x option is\n>>> specified, ignored files are also removed'---so 'unknown to Git'\n>>> does not include what you told .gitignore that they are\n>>> uninteresting. IOW, Git knows they are not interesting.\n>>>\n>>> It looks like a bug in \"git clean -d\" to me.\n>\n> I may be wrong (I'm not very familiar with the codebase), but I don't\n> think it's a bug in specifically git clean -d.\n>\n> Throwing gdb at it, when builtin/clean.c:cmd_clean() calls\n> dir.c:fill_directory(), A/ gets appended to dir->entries, regardless\n> of whether or not git clean is called with or without -d. The\n> difference is that if git clean is called without -d, the loop that\n> strips out directories strips out the A/ entry.\n>\n> When dir.c:fill_directory() is invoked through git status, A/ does\n> not, however, get appended to dir->entries. As best as I can tell,\n> this seems to be because git status sets the\n> DIR_HIDE_EMPTY_DIRECTORIES flag, whereas git clean does not (which\n> makes sense), but the fact that DIR_HIDE_EMPTY_DIRECTORIES is\n> responsible for not adding A/ to dir->entries seems to be the issue.\n"},{"id":"318431","messageId":"xmqqshkof6jd.fsf@gitster.mtv.corp.google.com","threadId":"45826","inReplyTo":"CAJZjrdXVopdvDoDwnsmqvrLESKQq=-a2waDgu5HYYoP2WvSrPA@mail.gmail.com","subject":"Re: Bug Report: .gitignore behavior is not matching in git clean and git status","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-02T00:26:14Z","receivedAt":"2017-05-02T00:26:22Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Samuel Lijin writes:\n\n> After some more digging (and familiarizing myself with the\n> behind-the-scenes logic) the issue is that dir.c has this implicit\n> assumption that a directory which contains only untracked and ignored\n> files should itself be considered untracked. While that works fine for\n> use cases where we're asking if a directory should be added to the git\n> database, that decidedly does not make sense when we're asking if a\n> directory can be removed from the working tree.\n\nThanks for digging.\n\n> I'm not sure where to proceed from here. I see two ways forward: one,\n> builtin/clean.c can collect ignored files when it calls\n> dir.c:fill_directory(), and then clean -d can prune out directories\n> that contain ignored files; two, path_treatment can learn about\n> untracked directories which contain excluded (ignored) files.\n\nMy gut feeling is that the former approach would be of lesser\nimpact. Directory A/ can be removed \"clean\" without \"-x\" when there\nis nothing tracked in there in the index and there is no ignored\npaths. Having zero untracked files there or one or more untracked\nfile there do not matter---they are all subject to removal by\n\"clean\".\n"},{"id":"318601","messageId":"20170503032932.16043-1-sxlijin@gmail.com","threadId":"45826","inReplyTo":"xmqqshkof6jd.fsf@gitster.mtv.corp.google.com","subject":"[PATCH 0/7] Keep git clean -d from inadvertently removing ignored files","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-03T03:29:25Z","receivedAt":"2017-05-03T03:29:52Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"This patch series fixes the bug where git clean -d culls directories containing\nonly untracked and ignored files, by first teaching read_directory() and\nread_directory_recursive() to search \"untracked\" directories (read: directories\n*treated* as untracked because they only contain untracked and ignored files)\nfor ignored contents, and then teaching cmd_clean() to skip untracked\ndirectories containing ignored files.\n\nThis does, however, introduce a breaking change in the behavior of git status:\nwhen invoked with --ignored, git status will now return ignored files in an\nuntracked directory, whereas previously it would not.\n\nFirst patches to the actual C code that I'm sending out! :D Looking forward to\nfeedback - the changes I made in read_directory_recursive() and read_directory()\nfeel a bit hacky, but I'm not sure how to get around that.\n\nSamuel Lijin (7):\n t7300: skip untracked dirs containing ignored files\n dir: recurse into untracked dirs for ignored files\n dir: add method to check if a dir_entry lexically contains another\n dir: hide untracked contents of untracked dirs\n dir: change linkage of cmp_name() and check_contains()\n builtin/clean: teach clean -d to skip dirs containing ignored files\n t7061: check for ignored file in untracked dir\n\n builtin/clean.c | 24 ++++++++++++++++--\n dir.c | 61 ++++++++++++++++++++++++++++++++++++++++++++--\n dir.h | 3 +++\n t/t7061-wtstatus-ignore.sh | 1 +\n t/t7300-clean.sh | 10 ++++++++\n 5 files changed, 95 insertions(+), 4 deletions(-)\n\n-- \n2.12.2\n\n"},{"id":"318602","messageId":"20170503032932.16043-2-sxlijin@gmail.com","threadId":"45826","inReplyTo":"20170503032932.16043-1-sxlijin@gmail.com","subject":"[PATCH 1/7] t7300: skip untracked dirs containing ignored files","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-03T03:29:26Z","receivedAt":"2017-05-03T03:30:08Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"If git sees a directory which contains only untracked and ignored\nfiles, clean -d should not remove that directory.\n---\n t/t7300-clean.sh | 10 ++++++++++\n 1 file changed, 10 insertions(+)\n\ndiff --git a/t/t7300-clean.sh b/t/t7300-clean.sh\nindex b89fd2a6a..948a455e8 100755\n--- a/t/t7300-clean.sh\n+++ b/t/t7300-clean.sh\n@@ -653,4 +653,14 @@ test_expect_success 'git clean -d respects pathspecs (pathspec is prefix of dir)\n \ttest_path_is_dir foobar\n '\n \n+test_expect_success 'git clean -d skips untracked dirs containing ignored files' '\n+\techo /foo/bar >.gitignore &&\n+\trm -rf foo &&\n+\tmkdir -p foo &&\n+\ttouch foo/bar &&\n+\tgit clean -df &&\n+\ttest_path_is_file foo/bar &&\n+\ttest_path_is_dir foo\n+'\n+\n test_done\n-- \n2.12.2\n\n"},{"id":"318603","messageId":"20170503032932.16043-8-sxlijin@gmail.com","threadId":"45826","inReplyTo":"20170503032932.16043-1-sxlijin@gmail.com","subject":"[PATCH 7/7] t7061: check for ignored file in untracked dir","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-03T03:29:32Z","receivedAt":"2017-05-03T03:30:09Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"---\n t/t7061-wtstatus-ignore.sh | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/t/t7061-wtstatus-ignore.sh b/t/t7061-wtstatus-ignore.sh\nindex cdc0747bf..fc6013ba3 100755\n--- a/t/t7061-wtstatus-ignore.sh\n+++ b/t/t7061-wtstatus-ignore.sh\n@@ -9,6 +9,7 @@ cat >expected <<\\EOF\n ?? actual\n ?? expected\n ?? untracked/\n+!! untracked/ignored\n EOF\n \n test_expect_success 'status untracked directory with --ignored' '\n-- \n2.12.2\n\n"},{"id":"318604","messageId":"20170503032932.16043-6-sxlijin@gmail.com","threadId":"45826","inReplyTo":"20170503032932.16043-1-sxlijin@gmail.com","subject":"[PATCH 5/7] dir: change linkage of cmp_name() and check_contains()","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-03T03:29:30Z","receivedAt":"2017-05-03T03:30:11Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"---\n dir.c | 4 ++--\n dir.h | 3 +++\n 2 files changed, 5 insertions(+), 2 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex f0ddb4608..91103b561 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -1844,7 +1844,7 @@ static enum path_treatment read_directory_recursive(struct dir_struct *dir,\n \treturn dir_state;\n }\n \n-static int cmp_name(const void *p1, const void *p2)\n+int cmp_name(const void *p1, const void *p2)\n {\n \tconst struct dir_entry *e1 = *(const struct dir_entry **)p1;\n \tconst struct dir_entry *e2 = *(const struct dir_entry **)p2;\n@@ -1853,7 +1853,7 @@ static int cmp_name(const void *p1, const void *p2)\n }\n \n // check if *out lexically contains *in\n-static int check_contains(const struct dir_entry *out, const struct dir_entry *in)\n+int check_contains(const struct dir_entry *out, const struct dir_entry *in)\n {\n \treturn (out->len < in->len) &&\n \t\t\t(out->name[out->len - 1] == '/') &&\ndiff --git a/dir.h b/dir.h\nindex bf23a470a..1ddd8b611 100644\n--- a/dir.h\n+++ b/dir.h\n@@ -326,6 +326,9 @@ static inline int dir_path_match(const struct dir_entry *ent,\n \t\t\t has_trailing_dir);\n }\n \n+int cmp_name(const void *p1, const void *p2);\n+int check_contains(const struct dir_entry *out, const struct dir_entry *in);\n+\n void untracked_cache_invalidate_path(struct index_state *, const char *);\n void untracked_cache_remove_from_index(struct index_state *, const char *);\n void untracked_cache_add_to_index(struct index_state *, const char *);\n-- \n2.12.2\n\n"},{"id":"318605","messageId":"20170503032932.16043-4-sxlijin@gmail.com","threadId":"45826","inReplyTo":"20170503032932.16043-1-sxlijin@gmail.com","subject":"[PATCH 3/7] dir: add method to check if a dir_entry lexically contains another","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-03T03:29:28Z","receivedAt":"2017-05-03T03:30:12Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"Introduce a method that allows us to check if one dir_entry corresponds\nto a path which contains the path corresponding to another dir_entry.\n---\n dir.c | 8 ++++++++\n 1 file changed, 8 insertions(+)\n\ndiff --git a/dir.c b/dir.c\nindex 6bd0350e9..25cb9eadf 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -1852,6 +1852,14 @@ static int cmp_name(const void *p1, const void *p2)\n \treturn name_compare(e1->name, e1->len, e2->name, e2->len);\n }\n \n+// check if *out lexically contains *in\n+static int check_contains(const struct dir_entry *out, const struct dir_entry *in)\n+{\n+\treturn (out->len < in->len) &&\n+\t\t\t(out->name[out->len - 1] == '/') &&\n+\t\t\t!memcmp(out->name, in->name, out->len);\n+}\n+\n static int treat_leading_path(struct dir_struct *dir,\n \t\t\t const char *path, int len,\n \t\t\t const struct pathspec *pathspec)\n-- \n2.12.2\n\n"},{"id":"318606","messageId":"20170503032932.16043-7-sxlijin@gmail.com","threadId":"45826","inReplyTo":"20170503032932.16043-1-sxlijin@gmail.com","subject":"[PATCH 6/7] builtin/clean: teach clean -d to skip dirs containing ignored files","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-03T03:29:31Z","receivedAt":"2017-05-03T03:30:14Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"There is an implicit assumption that a directory containing only\nuntracked and ignored files should itself be considered untracked. This\nmakes sense in use cases where we're asking if a directory should be\nadded to the git database, but not when we're asking if a directory can\nbe safely removed from the working tree; as a result, clean -d would\nassume that an \"untracked\" directory containing ignored files could be\ndeleted.\n\nTo get around this, we teach clean -d to collect ignored files and skip\nover so-called \"untracked\" directories if they contain any ignored\nfiles.\n---\n builtin/clean.c | 24 ++++++++++++++++++++++--\n 1 file changed, 22 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/clean.c b/builtin/clean.c\nindex d861f836a..368e19427 100644\n--- a/builtin/clean.c\n+++ b/builtin/clean.c\n@@ -859,7 +859,7 @@ static void interactive_main_loop(void)\n \n int cmd_clean(int argc, const char **argv, const char *prefix)\n {\n-\tint i, res;\n+\tint i, j, res;\n \tint dry_run = 0, remove_directories = 0, quiet = 0, ignored = 0;\n \tint ignored_only = 0, config_set = 0, errors = 0, gone = 1;\n \tint rm_flags = REMOVE_DIR_KEEP_NESTED_GIT;\n@@ -911,6 +911,9 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \t\t\t\t \" refusing to clean\"));\n \t}\n \n+\tif (remove_directories)\n+\t\tdir.flags |= DIR_SHOW_IGNORED_TOO;\n+\n \tif (force > 1)\n \t\trm_flags = 0;\n \n@@ -932,7 +935,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \n \tfill_directory(&dir, &pathspec);\n \n-\tfor (i = 0; i < dir.nr; i++) {\n+\tfor (j = i = 0; i < dir.nr; i++) {\n \t\tstruct dir_entry *ent = dir.entries[i];\n \t\tint matches = 0;\n \t\tstruct stat st;\n@@ -954,10 +957,27 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \t\t matches != MATCHED_EXACTLY)\n \t\t\tcontinue;\n \n+\t\t// skip any dir.entries which contains a dir.ignored\n+\t\tfor (; j < dir.ignored_nr; j++) {\n+\t\t\tif (cmp_name(&dir.entries[i],\n+\t\t\t\t\t\t&dir.ignored[j]) < 0)\n+\t\t\t\tbreak;\n+\t\t}\n+\t\tif ((j < dir.ignored_nr) &&\n+\t\t\t\tcheck_contains(dir.entries[i], dir.ignored[j])) {\n+\t\t\tcontinue;\n+\t\t}\n+\n \t\trel = relative_path(ent->name, prefix, &buf);\n \t\tstring_list_append(&del_list, rel);\n \t}\n \n+\tfor (i = 0; i < dir.nr; i++)\n+\t\tfree(dir.entries[i]);\n+\n+\tfor (i = 0; i < dir.ignored_nr; i++)\n+\t\tfree(dir.ignored[i]);\n+\n \tif (interactive && del_list.nr > 0)\n \t\tinteractive_main_loop();\n \n-- \n2.12.2\n\n"},{"id":"318607","messageId":"20170503032932.16043-3-sxlijin@gmail.com","threadId":"45826","inReplyTo":"20170503032932.16043-1-sxlijin@gmail.com","subject":"[PATCH 2/7] dir: recurse into untracked dirs for ignored files","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-03T03:29:27Z","receivedAt":"2017-05-03T03:30:15Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"We consider directories containing only untracked and ignored files to\nbe themselves untracked, which in the usual case means we don't have to\nsearch these directories. This is problematic when we want to collect\nignored files with DIR_SHOW_IGNORED_TOO, though, so we teach\nread_directory_recursive() to recurse into untracked directories to find\nthe ignored files they contain when DIR_SHOW_IGNORED_TOO is set. This\nhas the side effect of also collecting all untracked files in untracked\ndirectories as well.\n---\n dir.c | 7 ++++++-\n 1 file changed, 6 insertions(+), 1 deletion(-)\n\ndiff --git a/dir.c b/dir.c\nindex f451bfa48..6bd0350e9 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -1784,7 +1784,12 @@ static enum path_treatment read_directory_recursive(struct dir_struct *dir,\n \t\t\tdir_state = state;\n \n \t\t/* recurse into subdir if instructed by treat_path */\n-\t\tif (state == path_recurse) {\n+\t\tif ((state == path_recurse) ||\n+\t\t\t\t((get_dtype(cdir.de, path.buf, path.len) == DT_DIR) &&\n+\t\t\t\t (state == path_untracked) &&\n+\t\t\t\t (dir->flags & DIR_SHOW_IGNORED_TOO))\n+\t\t\t\t)\n+\t\t{\n \t\t\tstruct untracked_cache_dir *ud;\n \t\t\tud = lookup_untracked(dir->untracked, untracked,\n \t\t\t\t\t path.buf + baselen,\n-- \n2.12.2\n\n"},{"id":"318608","messageId":"20170503032932.16043-5-sxlijin@gmail.com","threadId":"45826","inReplyTo":"20170503032932.16043-1-sxlijin@gmail.com","subject":"[PATCH 4/7] dir: hide untracked contents of untracked dirs","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-03T03:29:29Z","receivedAt":"2017-05-03T03:30:16Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"When we taught read_directory_recursive() to recurse into untracked\ndirectories in search of ignored files given DIR_SHOW_IGNORED_TOO, that\nhad the side effect of teaching it to collect the untracked contents of\nuntracked directories. It does not make sense to return these, so we\nteach read_directory() to strip dir->entries of any such untracked\ncontents.\n---\n dir.c | 44 ++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 44 insertions(+)\n\ndiff --git a/dir.c b/dir.c\nindex 25cb9eadf..f0ddb4608 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -2075,6 +2075,50 @@ int read_directory(struct dir_struct *dir, const char *path,\n \t\tread_directory_recursive(dir, path, len, untracked, 0, pathspec);\n \tQSORT(dir->entries, dir->nr, cmp_name);\n \tQSORT(dir->ignored, dir->ignored_nr, cmp_name);\n+\n+\t// if collecting ignored files, never consider a directory containing\n+\t// ignored files to be untracked\n+\tif (dir->flags & DIR_SHOW_IGNORED_TOO) {\n+\t\tint i, j, nr_removed = 0;\n+\n+\t\t// remove from dir->entries untracked contents of untracked dirs\n+\t\tfor (i = 0; i < dir->nr; i++) {\n+\t\t\tif (!dir->entries[i])\n+\t\t\t\tcontinue;\n+\n+\t\t\tfor (j = i + 1; j < dir->nr; j++) {\n+\t\t\t\tif (!dir->entries[j])\n+\t\t\t\t\tcontinue;\n+\t\t\t\tif (check_contains(dir->entries[i], dir->entries[j])) {\n+\t\t\t\t\tnr_removed++;\n+\t\t\t\t\tfree(dir->entries[j]);\n+\t\t\t\t\tdir->entries[j] = NULL;\n+\t\t\t\t}\n+\t\t\t\telse {\n+\t\t\t\t\tbreak;\n+\t\t\t\t}\n+\t\t\t}\n+\t\t}\n+\n+\t\t// strip dir->entries of NULLs\n+\t\tif (nr_removed) {\n+\t\t\tfor (i = 0;;) {\n+\t\t\t\twhile (i < dir->nr && dir->entries[i])\n+\t\t\t\t\ti++;\n+\t\t\t\tif (i == dir->nr)\n+\t\t\t\t\tbreak;\n+\t\t\t\tj = i;\n+\t\t\t\twhile (j < dir->nr && !dir->entries[j])\n+\t\t\t\t\tj++;\n+\t\t\t\tif (j == dir->nr)\n+\t\t\t\t\tbreak;\n+\t\t\t\tdir->entries[i] = dir->entries[j];\n+\t\t\t\tdir->entries[j] = NULL;\n+\t\t\t}\n+\t\t\tdir->nr -= nr_removed;\n+\t\t}\n+\t}\n+\n \tif (dir->untracked) {\n \t\tstatic struct trace_key trace_untracked_stats = TRACE_KEY_INIT(UNTRACKED_STATS);\n \t\ttrace_printf_key(&trace_untracked_stats,\n-- \n2.12.2\n\n"},{"id":"318681","messageId":"CAGZ79kYie6F29R7HP0+MSpkHqSE7Cn=55EhGsNjoKxA_9Esz5Q@mail.gmail.com","threadId":"45826","inReplyTo":"20170503032932.16043-4-sxlijin@gmail.com","subject":"Re: [PATCH 3/7] dir: add method to check if a dir_entry lexically contains another","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-05-03T18:09:00Z","receivedAt":"2017-05-03T18:09:06Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, May 2, 2017 at 8:29 PM, Samuel Lijin wrote:\n> Introduce a method that allows us to check if one dir_entry corresponds\n> to a path which contains the path corresponding to another dir_entry.\n> ---\n> dir.c | 8 ++++++++\n> 1 file changed, 8 insertions(+)\n>\n> diff --git a/dir.c b/dir.c\n> index 6bd0350e9..25cb9eadf 100644\n> --- a/dir.c\n> +++ b/dir.c\n> @@ -1852,6 +1852,14 @@ static int cmp_name(const void *p1, const void *p2)\n> return name_compare(e1->name, e1->len, e2->name, e2->len);\n> }\n>\n> +// check if *out lexically contains *in\n\nThanks for adding a comment to describe what the function ought to do.\n\nHowever our Coding style prefers\n\n/* comments this way */\n\n/*\n * or in case of multi-\n * line comments,\n * this way.\n */\n\nI think one of the ancient compilers just dislikes // as comment style.\n\n> +static int check_contains(const struct dir_entry *out, const struct dir_entry *in)\n> +{\n> + return (out->len < in->len) &&\n> + (out->name[out->len - 1] == '/') &&\n> + !memcmp(out->name, in->name, out->len);\n> +}\n> +\n> static int treat_leading_path(struct dir_struct *dir,\n> const char *path, int len,\n> const struct pathspec *pathspec)\n> --\n> 2.12.2\n>\n"},{"id":"318682","messageId":"CAGZ79kauf5tAEv1JJCbTsuKhPYFKq0DVChBBt2EjHxRRZEzYAw@mail.gmail.com","threadId":"45826","inReplyTo":"20170503032932.16043-2-sxlijin@gmail.com","subject":"Re: [PATCH 1/7] t7300: skip untracked dirs containing ignored files","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-05-03T18:19:16Z","receivedAt":"2017-05-03T18:19:31Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, May 2, 2017 at 8:29 PM, Samuel Lijin wrote:\n> If git sees a directory which contains only untracked and ignored\n> files, clean -d should not remove that directory.\n\nYes that states a fact; it is not clear why we want to have this test here\nand now. (Is it testing for a recently fixed regression?)\nAre you just introducing the test to demonstrate it keeps working later on?\nDo you plan on changing this behavior in a later patch?)\n\n> ---\n> t/t7300-clean.sh | 10 ++++++++++\n> 1 file changed, 10 insertions(+)\n>\n> diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh\n> index b89fd2a6a..948a455e8 100755\n> --- a/t/t7300-clean.sh\n> +++ b/t/t7300-clean.sh\n> @@ -653,4 +653,14 @@ test_expect_success 'git clean -d respects pathspecs (pathspec is prefix of dir)\n> test_path_is_dir foobar\n> '\n>\n> +test_expect_success 'git clean -d skips untracked dirs containing ignored files' '\n> + echo /foo/bar >.gitignore &&\n> + rm -rf foo &&\n> + mkdir -p foo &&\n> + touch foo/bar &&\n> + git clean -df &&\n> + test_path_is_file foo/bar &&\n> + test_path_is_dir foo\n> +'\n\nThe test makes sense, though I am wondering if we can integrate\nthis test into another test e.g. \"ok 15 - git clean -d\".\n\nIs the -f flag needed?\n\nThanks,\nStefan\n"},{"id":"318684","messageId":"CAJZjrdWLuXF7c=kt4SM1CcOUPpmmZXmOZPUCuzcO0z2nNsVMLQ@mail.gmail.com","threadId":"45826","inReplyTo":"CAGZ79kauf5tAEv1JJCbTsuKhPYFKq0DVChBBt2EjHxRRZEzYAw@mail.gmail.com","subject":"Re: [PATCH 1/7] t7300: skip untracked dirs containing ignored files","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-03T18:26:39Z","receivedAt":"2017-05-03T18:27:25Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"On Wed, May 3, 2017 at 1:19 PM, Stefan Beller wrote:\n> On Tue, May 2, 2017 at 8:29 PM, Samuel Lijin wrote:\n>> If git sees a directory which contains only untracked and ignored\n>> files, clean -d should not remove that directory.\n>\n> Yes that states a fact; it is not clear why we want to have this test here\n> and now. (Is it testing for a recently fixed regression?)\n\nIt was recently discovered that this is not true of clean -d (and I'm\nnot sure if it ever was, to be honest). See\nhttp://public-inbox.org/git/xmqqshkof6jd.fsf@gitster.mtv.corp.google.com/T/#mf541c06250724bb000461d210b4ed157e415a596\n\n> Are you just introducing the test to demonstrate it keeps working later on?\n> Do you plan on changing this behavior in a later patch?)\n\nThe idea was to introduce the broken test and then have the rest of\nthe patch series fix it; Junio pointed out to me (off-list, since he\nwas responding from his phone, I think) that I got the convention\nbackwards, so I'll be changing this in the next version.\n\n>> ---\n>> t/t7300-clean.sh | 10 ++++++++++\n>> 1 file changed, 10 insertions(+)\n>>\n>> diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh\n>> index b89fd2a6a..948a455e8 100755\n>> --- a/t/t7300-clean.sh\n>> +++ b/t/t7300-clean.sh\n>> @@ -653,4 +653,14 @@ test_expect_success 'git clean -d respects pathspecs (pathspec is prefix of dir)\n>> test_path_is_dir foobar\n>> '\n>>\n>> +test_expect_success 'git clean -d skips untracked dirs containing ignored files' '\n>> + echo /foo/bar >.gitignore &&\n>> + rm -rf foo &&\n>> + mkdir -p foo &&\n>> + touch foo/bar &&\n>> + git clean -df &&\n>> + test_path_is_file foo/bar &&\n>> + test_path_is_dir foo\n>> +'\n>\n> The test makes sense, though I am wondering if we can integrate\n> this test into another test e.g. \"ok 15 - git clean -d\".\n>\n> Is the -f flag needed?\n\nWill look into both.\n\n> Thanks,\n> Stefan\n"},{"id":"318687","messageId":"CAGZ79katJKWOY=FdoU2DO-bwyZSA9KL3kcUgw1uKLyytNCXd-Q@mail.gmail.com","threadId":"45826","inReplyTo":"CAJZjrdWLuXF7c=kt4SM1CcOUPpmmZXmOZPUCuzcO0z2nNsVMLQ@mail.gmail.com","subject":"Re: [PATCH 1/7] t7300: skip untracked dirs containing ignored files","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-05-03T18:34:50Z","receivedAt":"2017-05-03T18:34:57Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, May 3, 2017 at 11:26 AM, Samuel Lijin wrote:\n> On Wed, May 3, 2017 at 1:19 PM, Stefan Beller wrote:\n>> On Tue, May 2, 2017 at 8:29 PM, Samuel Lijin wrote:\n>>> If git sees a directory which contains only untracked and ignored\n>>> files, clean -d should not remove that directory.\n>>\n>> Yes that states a fact; it is not clear why we want to have this test here\n>> and now. (Is it testing for a recently fixed regression?)\n>\n> It was recently discovered that this is not true of clean -d (and I'm\n> not sure if it ever was, to be honest). See\n> http://public-inbox.org/git/xmqqshkof6jd.fsf@gitster.mtv.corp.google.com/T/#mf541c06250724bb000461d210b4ed157e415a596\n>\n>> Are you just introducing the test to demonstrate it keeps working later on?\n>> Do you plan on changing this behavior in a later patch?)\n>\n> The idea was to introduce the broken test and then have the rest of\n> the patch series fix it;\n\nThis is valuable information for the commit message.\nAlso to keep git-bisect happy we want to have all tests passing on all commits,\nwhich this does not. However to further signal your intent, you can mark it\nas test_expect_failure. This is marking a test as \"if Git was not buggy, this\nis a test that would pass\", so it actually marks success when it\nfails, for example\nwhen running t7300:\n\n...\nok 31 - should not clean submodules\nok 32 - should avoid cleaning possible submodules\nok 33 - nested (empty) git should be kept\nok 34 - nested bare repositories should be cleaned\nnot ok 35 - nested (empty) bare repositories should be cleaned even\nwhen in .git # TODO known breakage\nnot ok 36 - nested (non-empty) bare repositories should be cleaned\neven when in .git # TODO known breakage\nok 37 - giving path in nested git work tree will remove it\nok 38 - giving path to nested .git will not remove it\nok 39 - giving path to nested .git/ will remove contents\nok 40 - force removal of nested git work tree\n...\n\nSimilarly if you were to mark that test as expecting failure, we'd see:\n\nno ok - 45 git clean -d skips untracked dirs containing ignored files\n# TODO known breakage\n\nAnd then in a later patch, when you actually fix the bug, you would just flip it\nto test_expect_success.\n\nThanks,\nStefan\n"},{"id":"318940","messageId":"20170505104611.17845-1-sxlijin@gmail.com","threadId":"45826","inReplyTo":"20170503032932.16043-1-sxlijin@gmail.com","subject":"[PATCH v2 0/9] Keep git clean -d from inadvertently removing ignored files","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-05T10:46:02Z","receivedAt":"2017-05-06T18:49:34Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"Addresses the issues raised by Stefan and Junio (thanks for your feedback) about not using C99-style comments and keeping tests working on every commit to prevent breaking git bisect. (About the latter one: is it necessary to prevent compiler warnings, in addition to compiler errors? Because if so I should probably squash some of the commits together.)\n\nNote that this introduces a breaking change in the behavior of git status: when invoked with --ignored, git status will now return ignored files in an untracked directory, whereas previously it would not.\n\nIt's possible that there are standard practices that I might have missed, so if there is anything along those lines, I'd appreciate you letting me know. (As an aside, about the git bisect thing: is there a script somewhere that people use to test patch series before sending them out?)\n\nSamuel Lijin (9):\n t7300: skip untracked dirs containing ignored files\n t7061: expect failure where expected behavior will change\n dir: recurse into untracked dirs for ignored files\n dir: add method to check if a dir_entry lexically contains another\n dir: hide untracked contents of untracked dirs\n dir: change linkage of cmp_name() and check_contains()\n builtin/clean: teach clean -d to skip dirs containing ignored files\n t7300: clean -d now skips untracked dirs containing ignored files\n t7061: expect ignored files in untracked dirs\n\n builtin/clean.c | 24 ++++++++++++++++--\n dir.c | 61 ++++++++++++++++++++++++++++++++++++++++++++--\n dir.h | 3 +++\n t/t7061-wtstatus-ignore.sh | 1 +\n t/t7300-clean.sh | 10 ++++++++\n 5 files changed, 95 insertions(+), 4 deletions(-)\n\n-- \n2.12.2\n\n"},{"id":"318941","messageId":"20170505104611.17845-2-sxlijin@gmail.com","threadId":"45826","inReplyTo":"20170505104611.17845-1-sxlijin@gmail.com","subject":"[PATCH v2 1/9] t7300: skip untracked dirs containing ignored files","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-05T10:46:03Z","receivedAt":"2017-05-06T18:49:37Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"If git sees a directory which contains only untracked and ignored\nfiles, clean -d should not remove that directory. It was recently\ndiscovered that this is *not* true of git clean -d, and it's possible\nthat this has never worked correctly; this test and its accompanying\npatch series aims to fix that.\n\nSigned-off-by: Samuel Lijin \n---\n t/t7300-clean.sh | 10 ++++++++++\n 1 file changed, 10 insertions(+)\n\ndiff --git a/t/t7300-clean.sh b/t/t7300-clean.sh\nindex b89fd2a6a..252c75b40 100755\n--- a/t/t7300-clean.sh\n+++ b/t/t7300-clean.sh\n@@ -653,4 +653,14 @@ test_expect_success 'git clean -d respects pathspecs (pathspec is prefix of dir)\n \ttest_path_is_dir foobar\n '\n \n+test_expect_failure 'git clean -d skips untracked dirs containing ignored files' '\n+\techo /foo/bar >.gitignore &&\n+\trm -rf foo &&\n+\tmkdir -p foo &&\n+\ttouch foo/bar &&\n+\tgit clean -df &&\n+\ttest_path_is_file foo/bar &&\n+\ttest_path_is_dir foo\n+'\n+\n test_done\n-- \n2.12.2\n\n"},{"id":"318942","messageId":"20170505104611.17845-3-sxlijin@gmail.com","threadId":"45826","inReplyTo":"20170505104611.17845-1-sxlijin@gmail.com","subject":"[PATCH v2 2/9] t7061: expect failure where expected behavior will change","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-05T10:46:04Z","receivedAt":"2017-05-06T18:49:38Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"This changes tests for `status --ignored` from test_expect_success to\ntest_expect_failure in preparation for a change in its expected behavior\n(namely, that ignored files in untracked dirs will be reported).\n\nSigned-off-by: Samuel Lijin \n---\n t/t7061-wtstatus-ignore.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t7061-wtstatus-ignore.sh b/t/t7061-wtstatus-ignore.sh\nindex cdc0747bf..dc3be92a2 100755\n--- a/t/t7061-wtstatus-ignore.sh\n+++ b/t/t7061-wtstatus-ignore.sh\n@@ -11,7 +11,7 @@ cat >expected <<\\EOF\n ?? untracked/\n EOF\n \n-test_expect_success 'status untracked directory with --ignored' '\n+test_expect_failure 'status untracked directory with --ignored' '\n \techo \"ignored\" >.gitignore &&\n \tmkdir untracked &&\n \t: >untracked/ignored &&\n@@ -20,7 +20,7 @@ test_expect_success 'status untracked directory with --ignored' '\n \ttest_cmp expected actual\n '\n \n-test_expect_success 'same with gitignore starting with BOM' '\n+test_expect_failure 'same with gitignore starting with BOM' '\n \tprintf \"\\357\\273\\277ignored\\n\" >.gitignore &&\n \tmkdir -p untracked &&\n \t: >untracked/ignored &&\n-- \n2.12.2\n\n"},{"id":"318943","messageId":"20170505104611.17845-5-sxlijin@gmail.com","threadId":"45826","inReplyTo":"20170505104611.17845-1-sxlijin@gmail.com","subject":"[PATCH v2 4/9] dir: add method to check if a dir_entry lexically contains another","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-05T10:46:06Z","receivedAt":"2017-05-06T18:49:40Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"Introduce a method that allows us to check if one dir_entry corresponds\nto a path which contains the path corresponding to another dir_entry.\n\nSigned-off-by: Samuel Lijin \n---\n dir.c | 8 ++++++++\n 1 file changed, 8 insertions(+)\n\ndiff --git a/dir.c b/dir.c\nindex 6bd0350e9..4739087f4 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -1852,6 +1852,14 @@ static int cmp_name(const void *p1, const void *p2)\n \treturn name_compare(e1->name, e1->len, e2->name, e2->len);\n }\n \n+/* check if *out lexically contains *in */\n+static int check_contains(const struct dir_entry *out, const struct dir_entry *in)\n+{\n+\treturn (out->len < in->len) &&\n+\t\t\t(out->name[out->len - 1] == '/') &&\n+\t\t\t!memcmp(out->name, in->name, out->len);\n+}\n+\n static int treat_leading_path(struct dir_struct *dir,\n \t\t\t const char *path, int len,\n \t\t\t const struct pathspec *pathspec)\n-- \n2.12.2\n\n"},{"id":"318944","messageId":"20170505104611.17845-7-sxlijin@gmail.com","threadId":"45826","inReplyTo":"20170505104611.17845-1-sxlijin@gmail.com","subject":"[PATCH v2 6/9] dir: change linkage of cmp_name() and check_contains()","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-05T10:46:08Z","receivedAt":"2017-05-06T18:49:43Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"Signed-off-by: Samuel Lijin \n---\n dir.c | 4 ++--\n dir.h | 3 +++\n 2 files changed, 5 insertions(+), 2 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex fd445ee9e..6a71683df 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -1844,7 +1844,7 @@ static enum path_treatment read_directory_recursive(struct dir_struct *dir,\n \treturn dir_state;\n }\n \n-static int cmp_name(const void *p1, const void *p2)\n+int cmp_name(const void *p1, const void *p2)\n {\n \tconst struct dir_entry *e1 = *(const struct dir_entry **)p1;\n \tconst struct dir_entry *e2 = *(const struct dir_entry **)p2;\n@@ -1853,7 +1853,7 @@ static int cmp_name(const void *p1, const void *p2)\n }\n \n /* check if *out lexically contains *in */\n-static int check_contains(const struct dir_entry *out, const struct dir_entry *in)\n+int check_contains(const struct dir_entry *out, const struct dir_entry *in)\n {\n \treturn (out->len < in->len) &&\n \t\t\t(out->name[out->len - 1] == '/') &&\ndiff --git a/dir.h b/dir.h\nindex bf23a470a..1ddd8b611 100644\n--- a/dir.h\n+++ b/dir.h\n@@ -326,6 +326,9 @@ static inline int dir_path_match(const struct dir_entry *ent,\n \t\t\t has_trailing_dir);\n }\n \n+int cmp_name(const void *p1, const void *p2);\n+int check_contains(const struct dir_entry *out, const struct dir_entry *in);\n+\n void untracked_cache_invalidate_path(struct index_state *, const char *);\n void untracked_cache_remove_from_index(struct index_state *, const char *);\n void untracked_cache_add_to_index(struct index_state *, const char *);\n-- \n2.12.2\n\n"},{"id":"318945","messageId":"20170505104611.17845-4-sxlijin@gmail.com","threadId":"45826","inReplyTo":"20170505104611.17845-1-sxlijin@gmail.com","subject":"[PATCH v2 3/9] dir: recurse into untracked dirs for ignored files","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-05T10:46:05Z","receivedAt":"2017-05-06T18:49:45Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"We consider directories containing only untracked and ignored files to\nbe themselves untracked, which in the usual case means we don't have to\nsearch these directories. This is problematic when we want to collect\nignored files with DIR_SHOW_IGNORED_TOO, though, so we teach\nread_directory_recursive() to recurse into untracked directories to find\nthe ignored files they contain when DIR_SHOW_IGNORED_TOO is set. This\nhas the side effect of also collecting all untracked files in untracked\ndirectories as well.\n\nSigned-off-by: Samuel Lijin \n---\n dir.c | 7 ++++++-\n 1 file changed, 6 insertions(+), 1 deletion(-)\n\ndiff --git a/dir.c b/dir.c\nindex f451bfa48..6bd0350e9 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -1784,7 +1784,12 @@ static enum path_treatment read_directory_recursive(struct dir_struct *dir,\n \t\t\tdir_state = state;\n \n \t\t/* recurse into subdir if instructed by treat_path */\n-\t\tif (state == path_recurse) {\n+\t\tif ((state == path_recurse) ||\n+\t\t\t\t((get_dtype(cdir.de, path.buf, path.len) == DT_DIR) &&\n+\t\t\t\t (state == path_untracked) &&\n+\t\t\t\t (dir->flags & DIR_SHOW_IGNORED_TOO))\n+\t\t\t\t)\n+\t\t{\n \t\t\tstruct untracked_cache_dir *ud;\n \t\t\tud = lookup_untracked(dir->untracked, untracked,\n \t\t\t\t\t path.buf + baselen,\n-- \n2.12.2\n\n"},{"id":"318946","messageId":"20170505104611.17845-8-sxlijin@gmail.com","threadId":"45826","inReplyTo":"20170505104611.17845-1-sxlijin@gmail.com","subject":"[PATCH v2 7/9] builtin/clean: teach clean -d to skip dirs containing ignored files","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-05T10:46:09Z","receivedAt":"2017-05-06T18:49:50Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"There is an implicit assumption that a directory containing only\nuntracked and ignored files should itself be considered untracked. This\nmakes sense in use cases where we're asking if a directory should be\nadded to the git database, but not when we're asking if a directory can\nbe safely removed from the working tree; as a result, clean -d would\nassume that an \"untracked\" directory containing ignored files could be\ndeleted.\n\nTo get around this, we teach clean -d to collect ignored files and skip\nover so-called \"untracked\" directories if they contain any ignored\nfiles.\n\nSigned-off-by: Samuel Lijin \n---\n builtin/clean.c | 24 ++++++++++++++++++++++--\n 1 file changed, 22 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/clean.c b/builtin/clean.c\nindex d861f836a..368e19427 100644\n--- a/builtin/clean.c\n+++ b/builtin/clean.c\n@@ -859,7 +859,7 @@ static void interactive_main_loop(void)\n \n int cmd_clean(int argc, const char **argv, const char *prefix)\n {\n-\tint i, res;\n+\tint i, j, res;\n \tint dry_run = 0, remove_directories = 0, quiet = 0, ignored = 0;\n \tint ignored_only = 0, config_set = 0, errors = 0, gone = 1;\n \tint rm_flags = REMOVE_DIR_KEEP_NESTED_GIT;\n@@ -911,6 +911,9 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \t\t\t\t \" refusing to clean\"));\n \t}\n \n+\tif (remove_directories)\n+\t\tdir.flags |= DIR_SHOW_IGNORED_TOO;\n+\n \tif (force > 1)\n \t\trm_flags = 0;\n \n@@ -932,7 +935,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \n \tfill_directory(&dir, &pathspec);\n \n-\tfor (i = 0; i < dir.nr; i++) {\n+\tfor (j = i = 0; i < dir.nr; i++) {\n \t\tstruct dir_entry *ent = dir.entries[i];\n \t\tint matches = 0;\n \t\tstruct stat st;\n@@ -954,10 +957,27 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \t\t matches != MATCHED_EXACTLY)\n \t\t\tcontinue;\n \n+\t\t// skip any dir.entries which contains a dir.ignored\n+\t\tfor (; j < dir.ignored_nr; j++) {\n+\t\t\tif (cmp_name(&dir.entries[i],\n+\t\t\t\t\t\t&dir.ignored[j]) < 0)\n+\t\t\t\tbreak;\n+\t\t}\n+\t\tif ((j < dir.ignored_nr) &&\n+\t\t\t\tcheck_contains(dir.entries[i], dir.ignored[j])) {\n+\t\t\tcontinue;\n+\t\t}\n+\n \t\trel = relative_path(ent->name, prefix, &buf);\n \t\tstring_list_append(&del_list, rel);\n \t}\n \n+\tfor (i = 0; i < dir.nr; i++)\n+\t\tfree(dir.entries[i]);\n+\n+\tfor (i = 0; i < dir.ignored_nr; i++)\n+\t\tfree(dir.ignored[i]);\n+\n \tif (interactive && del_list.nr > 0)\n \t\tinteractive_main_loop();\n \n-- \n2.12.2\n\n"},{"id":"318947","messageId":"20170505104611.17845-9-sxlijin@gmail.com","threadId":"45826","inReplyTo":"20170505104611.17845-1-sxlijin@gmail.com","subject":"[PATCH v2 8/9] t7300: clean -d now skips untracked dirs containing ignored files","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-05T10:46:10Z","receivedAt":"2017-05-06T18:50:03Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"This was previously broken (and likely never worked); this concludes the\npatch series fixing the behavior of clean -d.\n\nSigned-off-by: Samuel Lijin \n---\n t/t7300-clean.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t7300-clean.sh b/t/t7300-clean.sh\nindex 252c75b40..948a455e8 100755\n--- a/t/t7300-clean.sh\n+++ b/t/t7300-clean.sh\n@@ -653,7 +653,7 @@ test_expect_success 'git clean -d respects pathspecs (pathspec is prefix of dir)\n \ttest_path_is_dir foobar\n '\n \n-test_expect_failure 'git clean -d skips untracked dirs containing ignored files' '\n+test_expect_success 'git clean -d skips untracked dirs containing ignored files' '\n \techo /foo/bar >.gitignore &&\n \trm -rf foo &&\n \tmkdir -p foo &&\n-- \n2.12.2\n\n"},{"id":"318948","messageId":"20170505104611.17845-6-sxlijin@gmail.com","threadId":"45826","inReplyTo":"20170505104611.17845-1-sxlijin@gmail.com","subject":"[PATCH v2 5/9] dir: hide untracked contents of untracked dirs","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-05T10:46:07Z","receivedAt":"2017-05-06T18:50:05Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"When we taught read_directory_recursive() to recurse into untracked\ndirectories in search of ignored files given DIR_SHOW_IGNORED_TOO, that\nhad the side effect of teaching it to collect the untracked contents of\nuntracked directories. It does not make sense to return these, so we\nteach read_directory() to strip dir->entries of any such untracked\ncontents.\n\nSigned-off-by: Samuel Lijin \n---\n dir.c | 44 ++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 44 insertions(+)\n\ndiff --git a/dir.c b/dir.c\nindex 4739087f4..fd445ee9e 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -2075,6 +2075,50 @@ int read_directory(struct dir_struct *dir, const char *path,\n \t\tread_directory_recursive(dir, path, len, untracked, 0, pathspec);\n \tQSORT(dir->entries, dir->nr, cmp_name);\n \tQSORT(dir->ignored, dir->ignored_nr, cmp_name);\n+\n+\t// if collecting ignored files, never consider a directory containing\n+\t// ignored files to be untracked\n+\tif (dir->flags & DIR_SHOW_IGNORED_TOO) {\n+\t\tint i, j, nr_removed = 0;\n+\n+\t\t// remove from dir->entries untracked contents of untracked dirs\n+\t\tfor (i = 0; i < dir->nr; i++) {\n+\t\t\tif (!dir->entries[i])\n+\t\t\t\tcontinue;\n+\n+\t\t\tfor (j = i + 1; j < dir->nr; j++) {\n+\t\t\t\tif (!dir->entries[j])\n+\t\t\t\t\tcontinue;\n+\t\t\t\tif (check_contains(dir->entries[i], dir->entries[j])) {\n+\t\t\t\t\tnr_removed++;\n+\t\t\t\t\tfree(dir->entries[j]);\n+\t\t\t\t\tdir->entries[j] = NULL;\n+\t\t\t\t}\n+\t\t\t\telse {\n+\t\t\t\t\tbreak;\n+\t\t\t\t}\n+\t\t\t}\n+\t\t}\n+\n+\t\t// strip dir->entries of NULLs\n+\t\tif (nr_removed) {\n+\t\t\tfor (i = 0;;) {\n+\t\t\t\twhile (i < dir->nr && dir->entries[i])\n+\t\t\t\t\ti++;\n+\t\t\t\tif (i == dir->nr)\n+\t\t\t\t\tbreak;\n+\t\t\t\tj = i;\n+\t\t\t\twhile (j < dir->nr && !dir->entries[j])\n+\t\t\t\t\tj++;\n+\t\t\t\tif (j == dir->nr)\n+\t\t\t\t\tbreak;\n+\t\t\t\tdir->entries[i] = dir->entries[j];\n+\t\t\t\tdir->entries[j] = NULL;\n+\t\t\t}\n+\t\t\tdir->nr -= nr_removed;\n+\t\t}\n+\t}\n+\n \tif (dir->untracked) {\n \t\tstatic struct trace_key trace_untracked_stats = TRACE_KEY_INIT(UNTRACKED_STATS);\n \t\ttrace_printf_key(&trace_untracked_stats,\n-- \n2.12.2\n\n"},{"id":"318949","messageId":"20170505104611.17845-10-sxlijin@gmail.com","threadId":"45826","inReplyTo":"20170505104611.17845-1-sxlijin@gmail.com","subject":"[PATCH v2 9/9] t7061: expect ignored files in untracked dirs","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-05T10:46:11Z","receivedAt":"2017-05-06T18:50:06Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"We now expect `status --ignored` to list ignored files even if they are\nin an untracked directory.\n\nSigned-off-by: Samuel Lijin \n---\n t/t7061-wtstatus-ignore.sh | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t7061-wtstatus-ignore.sh b/t/t7061-wtstatus-ignore.sh\nindex dc3be92a2..fc6013ba3 100755\n--- a/t/t7061-wtstatus-ignore.sh\n+++ b/t/t7061-wtstatus-ignore.sh\n@@ -9,9 +9,10 @@ cat >expected <<\\EOF\n ?? actual\n ?? expected\n ?? untracked/\n+!! untracked/ignored\n EOF\n \n-test_expect_failure 'status untracked directory with --ignored' '\n+test_expect_success 'status untracked directory with --ignored' '\n \techo \"ignored\" >.gitignore &&\n \tmkdir untracked &&\n \t: >untracked/ignored &&\n@@ -20,7 +21,7 @@ test_expect_failure 'status untracked directory with --ignored' '\n \ttest_cmp expected actual\n '\n \n-test_expect_failure 'same with gitignore starting with BOM' '\n+test_expect_success 'same with gitignore starting with BOM' '\n \tprintf \"\\357\\273\\277ignored\\n\" >.gitignore &&\n \tmkdir -p untracked &&\n \t: >untracked/ignored &&\n-- \n2.12.2\n\n"},{"id":"319012","messageId":"22e44a7d-0e9a-3e49-14c7-d69d6c6d733a@web.de","threadId":"45826","inReplyTo":"20170505104611.17845-2-sxlijin@gmail.com","subject":"Re: [PATCH v2 1/9] t7300: skip untracked dirs containing ignored files","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2017-05-07T19:12:02Z","receivedAt":"2017-05-07T21:53:29Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 2017-05-05 12:46, Samuel Lijin wrote:\n> If git sees a directory which contains only untracked and ignored\n> files, clean -d should not remove that directory. It was recently\n> discovered that this is *not* true of git clean -d, and it's possible\n> that this has never worked correctly; this test and its accompanying\n> patch series aims to fix that.\n> \n> Signed-off-by: Samuel Lijin \n> ---\n> t/t7300-clean.sh | 10 ++++++++++\n> 1 file changed, 10 insertions(+)\n> \n> diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh\n> index b89fd2a6a..252c75b40 100755\n> --- a/t/t7300-clean.sh\n> +++ b/t/t7300-clean.sh\n> @@ -653,4 +653,14 @@ test_expect_success 'git clean -d respects pathspecs (pathspec is prefix of dir)\n> \ttest_path_is_dir foobar\n> '\n> \n> +test_expect_failure 'git clean -d skips untracked dirs containing ignored files' '\n> +\techo /foo/bar >.gitignore &&\n> +\trm -rf foo &&\n> +\tmkdir -p foo &&\nMinor remark:\nDo we need the -p here, or can it be dropped?\n\n> +\ttouch foo/bar &&\n> +\tgit clean -df &&\n> +\ttest_path_is_file foo/bar &&\n> +\ttest_path_is_dir foo\n> +'\n> +\n> test_done\n> \n\n"},{"id":"319046","messageId":"xmqqh90w9do7.fsf@gitster.mtv.corp.google.com","threadId":"45826","inReplyTo":"20170505104611.17845-1-sxlijin@gmail.com","subject":"Re: [PATCH v2 0/9] Keep git clean -d from inadvertently removing ignored files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-08T04:26:48Z","receivedAt":"2017-05-08T04:26:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Samuel Lijin writes:\n\n> Addresses the issues raised by Stefan and Junio (thanks for your\n> feedback) about not using C99-style comments and keeping tests\n> working on every commit to prevent breaking git bisect. (About the\n> latter one: is it necessary to prevent compiler warnings, in\n> addition to compiler errors? Because if so I should probably\n> squash some of the commits together.)\n\nSome of us build with -Werror, so yes. If by \"squashing\" you mean\n\"instead of piling a fix on top of a broken patch, I need to do\nthings right from the beginning\", then yes, please do so, not just\nfor compiler warnings but for all forms of changes.\n\n> Note that this introduces a breaking change in the behavior of git\n> status: when invoked with --ignored, git status will now return\n> ignored files in an untracked directory, whereas previously it\n> would not.\n\nWhat do you mean by a \"breaking change\"? Is it just \"a new bug\"?\nOr \"the current behaviour is logically broken, but people and\nscripts might have relied on that odd behaviour and fixing it this\nlate in the game would break their expectations\"? \n\n> It's possible that there are standard practices that I might have\n> missed, so if there is anything along those lines, I'd appreciate\n> you letting me know. (As an aside, about the git bisect thing: is\n> there a script somewhere that people use to test patch series\n> before sending them out?)\n\nI hear that people use variations of\n\n git rebase -x \"make test\"\n\non their topic.\n"},{"id":"319048","messageId":"xmqqd1bk9dc3.fsf@gitster.mtv.corp.google.com","threadId":"45826","inReplyTo":"20170505104611.17845-3-sxlijin@gmail.com","subject":"Re: [PATCH v2 2/9] t7061: expect failure where expected behavior will change","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-08T04:34:04Z","receivedAt":"2017-05-08T04:34:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Samuel Lijin writes:\n\n> This changes tests for `status --ignored` from test_expect_success to\n> test_expect_failure in preparation for a change in its expected behavior\n> (namely, that ignored files in untracked dirs will be reported).\n>\n> Signed-off-by: Samuel Lijin \n> ---\n\nThis is an odd way to do this. If we stop applying your patches at\nthis step, these tests will still see output from \"status --ignored\"\nthat is expected by them, so there is no expect_failure here.\n\nIf we decide that the current output from \"status --ignored\" is\nWRONG, and your series to fix \"clean -d\" FIXES \"status --ignored\" as\na side effect, then having a step to describe a desired behaviour in\nthe new world order in the test like this patch does makes sense,\nbut if that is what is going on, then not just flipping \"success\" to\n\"failure\", the patch would be changing the expected output as well,\ni.e. by adding the ignored files in untracked directories in the\nexpected output. Obviously the code at this point after applying\nonly patches 1 & 2 will not produce such an output, so marking the\ntest that expects output based on the new world order as \"expect\nfailure\" would make sense. Then your future commit that FIXES\n\"status --ignored\" output would flip _failure to _success.\n\nIt is unclear to me if the new behaviour of \"status --ignored\" is a\nbugfix, or a new bug, though.\n\n\n> t/t7061-wtstatus-ignore.sh | 4 ++--\n> 1 file changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/t/t7061-wtstatus-ignore.sh b/t/t7061-wtstatus-ignore.sh\n> index cdc0747bf..dc3be92a2 100755\n> --- a/t/t7061-wtstatus-ignore.sh\n> +++ b/t/t7061-wtstatus-ignore.sh\n> @@ -11,7 +11,7 @@ cat >expected <<\\EOF\n> ?? untracked/\n> EOF\n> \n> -test_expect_success 'status untracked directory with --ignored' '\n> +test_expect_failure 'status untracked directory with --ignored' '\n> \techo \"ignored\" >.gitignore &&\n> \tmkdir untracked &&\n> \t: >untracked/ignored &&\n> @@ -20,7 +20,7 @@ test_expect_success 'status untracked directory with --ignored' '\n> \ttest_cmp expected actual\n> '\n> \n> -test_expect_success 'same with gitignore starting with BOM' '\n> +test_expect_failure 'same with gitignore starting with BOM' '\n> \tprintf \"\\357\\273\\277ignored\\n\" >.gitignore &&\n> \tmkdir -p untracked &&\n> \t: >untracked/ignored &&\n"},{"id":"319049","messageId":"xmqq8tm89d6y.fsf@gitster.mtv.corp.google.com","threadId":"45826","inReplyTo":"20170505104611.17845-7-sxlijin@gmail.com","subject":"Re: [PATCH v2 6/9] dir: change linkage of cmp_name() and check_contains()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-08T04:37:09Z","receivedAt":"2017-05-08T04:37:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Samuel Lijin writes:\n\n> diff --git a/dir.h b/dir.h\n> index bf23a470a..1ddd8b611 100644\n> --- a/dir.h\n> +++ b/dir.h\n> @@ -326,6 +326,9 @@ static inline int dir_path_match(const struct dir_entry *ent,\n> \t\t\t has_trailing_dir);\n> }\n> \n> +int cmp_name(const void *p1, const void *p2);\n> +int check_contains(const struct dir_entry *out, const struct dir_entry *in);\n> +\n\nBoth of these would have been perfectly sensible names when they\nwere private to dir.c, but are way too generic to live in the global\nnamespace. When a person who works on Git internal hears a name\n\"check_contains()\", I am sure that the first guess of what the\nfunction does is to see if a commit is a descendant of another\ncommit.\n\n"},{"id":"319062","messageId":"xmqqy3u77t5c.fsf@gitster.mtv.corp.google.com","threadId":"45826","inReplyTo":"20170505104611.17845-10-sxlijin@gmail.com","subject":"Re: [PATCH v2 9/9] t7061: expect ignored files in untracked dirs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-08T06:35:27Z","receivedAt":"2017-05-08T06:35:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Samuel Lijin writes:\n\n> We now expect `status --ignored` to list ignored files even if they are\n> in an untracked directory.\n>\n> Signed-off-by: Samuel Lijin \n> ---\n> t/t7061-wtstatus-ignore.sh | 5 +++--\n> 1 file changed, 3 insertions(+), 2 deletions(-)\n>\n> diff --git a/t/t7061-wtstatus-ignore.sh b/t/t7061-wtstatus-ignore.sh\n> index dc3be92a2..fc6013ba3 100755\n> --- a/t/t7061-wtstatus-ignore.sh\n> +++ b/t/t7061-wtstatus-ignore.sh\n> @@ -9,9 +9,10 @@ cat >expected <<\\EOF\n> ?? actual\n> ?? expected\n> ?? untracked/\n> +!! untracked/ignored\n> EOF\n\nIn my comment on an earlier step (2/9, I think), I said it is\nunclear if the change in behaviour to \"status --ignored\" is a new\nbug you are introducing, or is a fix that happens as a side-effect.\n\nI may be misunderstanding what the test that uses this expected\noutput is trying to see, but to me, it seems that:\n\n --ignored::\n Show ignored files as well.\n\nis clear enough that \"status --ignored\", regardless of the\n\"--unracked\" settings, should show the ignored file even when the\ndirectory it happens to be in does not have any file that is\nregisterd in the index, and the expected output updated by this\npatch looks to me the one that we _should_ be expecting. IOW, the\nchange in behaviour looks like a bugfix to me. And if that is\nindeed the case, the above change should be in the earlier patch\nthat flips \"expect_success\" to \"expect_failure\". The \"expected\"\nfile is prepared to expect the \"correct\" output before the code is\nupdated to produce one (i.e. the test update declares that the\ncurrent behaviour is broken), and then with changes in a later step\n(i.e. somewhere before 7/9) the code starts to produce the \"correct\"\noutput at which point in that same patch you flip expect_failure into\nexpect_success.\n\nThe log message of eb8c5b87 (\"git-status: Test --ignored behavior\",\n2012-12-30) says otherwise, though. I am undecided, if I agree with\nthe design decision described by the first two bullet points:\n\n commit eb8c5b872ef144add4ac89f85bcddc974ac7114d\n Author: Antoine Pelisse \n Date: Sun Dec 30 15:39:01 2012 +0100\n\n git-status: Test --ignored behavior\n\n Test all possible use-cases of git-status \"--ignored\" with the\n \"--untracked-files\" option with values \"normal\" and \"all\":\n\n - An untracked directory is listed as untracked if it has a mix of\n untracked and ignored files in it. With -uall, ignored/untracked\n files are listed as ignored/untracked.\n\n - An untracked directory with only ignored files is listed as\n ignored. With -uall, all files in the directory are listed.\n\n - An ignored directory is listed as ignored. With -uall, all files\n in the directory are listed as ignored.\n\n - An ignored and committed directory is listed as ignored if it has\n untracked files. With -uall, all untracked files in the\n directory are listed as ignored.\n\n Signed-off-by: Antoine Pelisse \n Signed-off-by: Junio C Hamano \n\n t/t7061-wtstatus-ignore.sh | 146 +++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 146 insertions(+)\n\nWhat do others think?\n\nBy the way, I wonder what the performance impact of this change\nwould be. Do we end up needing to scan more parts of the\nfilesystem, when we already know that a directory does not have any\ntracked paths?\n\n> -test_expect_failure 'status untracked directory with --ignored' '\n> +test_expect_success 'status untracked directory with --ignored' '\n> \techo \"ignored\" >.gitignore &&\n> \tmkdir untracked &&\n> \t: >untracked/ignored &&\n> @@ -20,7 +21,7 @@ test_expect_failure 'status untracked directory with --ignored' '\n> \ttest_cmp expected actual\n> '\n> \n> -test_expect_failure 'same with gitignore starting with BOM' '\n> +test_expect_success 'same with gitignore starting with BOM' '\n> \tprintf \"\\357\\273\\277ignored\\n\" >.gitignore &&\n> \tmkdir -p untracked &&\n> \t: >untracked/ignored &&\n"},{"id":"319117","messageId":"CAJZjrdVGm9j_yja+sfW7K5CPdHqXKxGBK7WWqmxNpowEYZ_2gA@mail.gmail.com","threadId":"45826","inReplyTo":"22e44a7d-0e9a-3e49-14c7-d69d6c6d733a@web.de","subject":"Re: [PATCH v2 1/9] t7300: skip untracked dirs containing ignored files","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-08T21:24:17Z","receivedAt":"2017-05-08T21:25:03Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"On Sun, May 7, 2017 at 2:12 PM, Torsten Bögershausen wrote:\n> On 2017-05-05 12:46, Samuel Lijin wrote:\n>> If git sees a directory which contains only untracked and ignored\n>> files, clean -d should not remove that directory. It was recently\n>> discovered that this is *not* true of git clean -d, and it's possible\n>> that this has never worked correctly; this test and its accompanying\n>> patch series aims to fix that.\n>>\n>> Signed-off-by: Samuel Lijin \n>> ---\n>> t/t7300-clean.sh | 10 ++++++++++\n>> 1 file changed, 10 insertions(+)\n>>\n>> diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh\n>> index b89fd2a6a..252c75b40 100755\n>> --- a/t/t7300-clean.sh\n>> +++ b/t/t7300-clean.sh\n>> @@ -653,4 +653,14 @@ test_expect_success 'git clean -d respects pathspecs (pathspec is prefix of dir)\n>> test_path_is_dir foobar\n>> '\n>>\n>> +test_expect_failure 'git clean -d skips untracked dirs containing ignored files' '\n>> + echo /foo/bar >.gitignore &&\n>> + rm -rf foo &&\n>> + mkdir -p foo &&\n> Minor remark:\n> Do we need the -p here, or can it be dropped?\n\nYeah, the -p isn't needed here - will change when I reroll.\n\n>> + touch foo/bar &&\n>> + git clean -df &&\n>> + test_path_is_file foo/bar &&\n>> + test_path_is_dir foo\n>> +'\n>> +\n>> test_done\n>>\n>\n"},{"id":"319123","messageId":"CAJZjrdVTRdTOiHXdVzVY3CLoi7KMbGozg8rDvrmJOJADMXRFuw@mail.gmail.com","threadId":"45826","inReplyTo":"xmqqh90w9do7.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 0/9] Keep git clean -d from inadvertently removing ignored files","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-08T21:37:48Z","receivedAt":"2017-05-08T21:38:34Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"On Sun, May 7, 2017 at 11:26 PM, Junio C Hamano wrote:\n> Samuel Lijin writes:\n>\n>> Addresses the issues raised by Stefan and Junio (thanks for your\n>> feedback) about not using C99-style comments and keeping tests\n>> working on every commit to prevent breaking git bisect. (About the\n>> latter one: is it necessary to prevent compiler warnings, in\n>> addition to compiler errors? Because if so I should probably\n>> squash some of the commits together.)\n>\n> Some of us build with -Werror, so yes. If by \"squashing\" you mean\n> \"instead of piling a fix on top of a broken patch, I need to do\n> things right from the beginning\", then yes, please do so, not just\n> for compiler warnings but for all forms of changes.\n\nGot it - will keep this in mind when I reroll the patch series.\n\n>> Note that this introduces a breaking change in the behavior of git\n>> status: when invoked with --ignored, git status will now return\n>> ignored files in an untracked directory, whereas previously it\n>> would not.\n>\n> What do you mean by a \"breaking change\"? Is it just \"a new bug\"?\n> Or \"the current behaviour is logically broken, but people and\n> scripts might have relied on that odd behaviour and fixing it this\n> late in the game would break their expectations\"?\n\nThe latter, as I believe you noticed in your reply about patch 9/9.\n\nWhat happens right now is that because (1) directories containing only\nuntracked and ignored files are considered \"untracked\" and (2)\nread_directory_recursive() skips over untracked directories, even with\nDIR_SHOW_IGNORED_TOO set. As a result, `status --ignored` never lists\nignored files if they're in an \"untracked\" directory (and this is the\ncurrently defined behavior per t7061).\n\nBy teaching read_directory_recursive() to recurse into untracked\ndirectories in search of ignored files when DIR_SHOW_IGNORED_TOO is\nset, though, `status --ignored` now learns to report the existence of\nthese ignored files, whereas previously it did not.\n\n>> It's possible that there are standard practices that I might have\n>> missed, so if there is anything along those lines, I'd appreciate\n>> you letting me know. (As an aside, about the git bisect thing: is\n>> there a script somewhere that people use to test patch series\n>> before sending them out?)\n>\n> I hear that people use variations of\n>\n> git rebase -x \"make test\"\n>\n> on their topic.\n\nAha - thanks.\n"},{"id":"319129","messageId":"xmqqh90u7two.fsf@gitster.mtv.corp.google.com","threadId":"45826","inReplyTo":"CAJZjrdVTRdTOiHXdVzVY3CLoi7KMbGozg8rDvrmJOJADMXRFuw@mail.gmail.com","subject":"Re: [PATCH v2 0/9] Keep git clean -d from inadvertently removing ignored files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-09T00:31:19Z","receivedAt":"2017-05-09T00:31:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Samuel Lijin writes:\n\n> What happens right now is that because (1) directories containing only\n> untracked and ignored files are considered \"untracked\" and (2)\n> read_directory_recursive() skips over untracked directories, even with\n> DIR_SHOW_IGNORED_TOO set. As a result, `status --ignored` never lists\n> ignored files if they're in an \"untracked\" directory (and this is the\n> currently defined behavior per t7061).\n>\n> By teaching read_directory_recursive() to recurse into untracked\n> directories in search of ignored files when DIR_SHOW_IGNORED_TOO is\n> set, though, `status --ignored` now learns to report the existence of\n> these ignored files, whereas previously it did not.\n\nOK, if you are revisiting the design decision made by eb8c5b87\n(\"git-status: Test --ignored behavior\", 2012-12-30), we should\nclearly document it in the log message of the commit that does so in\na way similar to how eb8c5b87 described the behaviour it desired.\n\nThanks.\n"},{"id":"319897","messageId":"20170516073423.25762-1-sxlijin@gmail.com","threadId":"45826","inReplyTo":"20170505104611.17845-1-sxlijin@gmail.com","subject":"[PATCH v3 0/8] Fix clean -d and status --ignored","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-16T07:34:15Z","receivedAt":"2017-05-16T07:36:17Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"Restructures the patch series so that everything works properly.\n\nI also noticed while redoing it that I didn't properly fix the clean -d bug\nbefore; while it wouldn't remove a directory with only untracked and ignored\nfiles, it would miss the untracked contents of such a directory. That's now been\nfixed.\n\nSamuel Lijin (8):\n t7300: clean -d should skip dirs with ignored files\n t7061: status --ignored should search untracked dirs\n dir: recurse into untracked dirs for ignored files\n dir: hide untracked contents of untracked dirs\n dir: expose cmp_name() and check_contains()\n clean: teach clean -d to skip dirs containing ignored files\n t7300: clean -d now skips untracked dirs containing ignored files\n t7061: status --ignored now searches untracked dirs\n\n builtin/clean.c | 32 ++++++++++++++++++++--\n dir.c | 67 +++++++++++++++++++++++++++++++++++++++++++---\n dir.h | 6 ++++-\n t/t7061-wtstatus-ignore.sh | 1 +\n t/t7300-clean.sh | 11 ++++++++\n 5 files changed, 110 insertions(+), 7 deletions(-)\n\n-- \n2.12.2\n\n"},{"id":"319898","messageId":"20170516073423.25762-2-sxlijin@gmail.com","threadId":"45826","inReplyTo":"20170516073423.25762-1-sxlijin@gmail.com","subject":"[PATCH v3 1/8] t7300: clean -d should skip dirs with ignored files","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-16T07:34:16Z","receivedAt":"2017-05-16T07:36:19Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"If git sees a directory which contains only untracked and ignored\nfiles, clean -d should not remove that directory. It was recently\ndiscovered that this is *not* true of git clean -d, and it's possible\nthat this has never worked correctly; this test and its accompanying\npatch series aims to fix that.\n\nSigned-off-by: Samuel Lijin \n---\n t/t7300-clean.sh | 11 +++++++++++\n 1 file changed, 11 insertions(+)\n\ndiff --git a/t/t7300-clean.sh b/t/t7300-clean.sh\nindex b89fd2a6a..c14c98e56 100755\n--- a/t/t7300-clean.sh\n+++ b/t/t7300-clean.sh\n@@ -653,4 +653,15 @@ test_expect_success 'git clean -d respects pathspecs (pathspec is prefix of dir)\n \ttest_path_is_dir foobar\n '\n \n+test_expect_failure 'git clean -d skips untracked dirs containing ignored files' '\n+\techo /foo/bar >.gitignore &&\n+\trm -rf foo &&\n+\tmkdir foo &&\n+\ttouch foo/bar foo/baz &&\n+\tgit clean -df &&\n+\ttest_path_is_dir foo &&\n+\ttest_path_is_file foo/bar &&\n+\ttest_path_is_missing foo/baz\n+'\n+\n test_done\n-- \n2.12.2\n\n"},{"id":"319899","messageId":"20170516073423.25762-4-sxlijin@gmail.com","threadId":"45826","inReplyTo":"20170516073423.25762-1-sxlijin@gmail.com","subject":"[PATCH v3 3/8] dir: recurse into untracked dirs for ignored files","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-16T07:34:18Z","receivedAt":"2017-05-16T07:36:22Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"We consider directories containing only untracked and ignored files to\nbe themselves untracked, which in the usual case means we don't have to\nsearch these directories. This is problematic when we want to collect\nignored files with DIR_SHOW_IGNORED_TOO, though, so we teach\nread_directory_recursive() to recurse into untracked directories to find\nthe ignored files they contain when DIR_SHOW_IGNORED_TOO is set. This\nhas the side effect of also collecting all untracked files in untracked\ndirectories as well.\n\nSigned-off-by: Samuel Lijin \n---\n dir.c | 7 ++++++-\n 1 file changed, 6 insertions(+), 1 deletion(-)\n\ndiff --git a/dir.c b/dir.c\nindex f451bfa48..6bd0350e9 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -1784,7 +1784,12 @@ static enum path_treatment read_directory_recursive(struct dir_struct *dir,\n \t\t\tdir_state = state;\n \n \t\t/* recurse into subdir if instructed by treat_path */\n-\t\tif (state == path_recurse) {\n+\t\tif ((state == path_recurse) ||\n+\t\t\t\t((get_dtype(cdir.de, path.buf, path.len) == DT_DIR) &&\n+\t\t\t\t (state == path_untracked) &&\n+\t\t\t\t (dir->flags & DIR_SHOW_IGNORED_TOO))\n+\t\t\t\t)\n+\t\t{\n \t\t\tstruct untracked_cache_dir *ud;\n \t\t\tud = lookup_untracked(dir->untracked, untracked,\n \t\t\t\t\t path.buf + baselen,\n-- \n2.12.2\n\n"},{"id":"319900","messageId":"20170516073423.25762-3-sxlijin@gmail.com","threadId":"45826","inReplyTo":"20170516073423.25762-1-sxlijin@gmail.com","subject":"[PATCH v3 2/8] t7061: status --ignored should search untracked dirs","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-16T07:34:17Z","receivedAt":"2017-05-16T07:36:23Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"Per eb8c5b87, `status --ignored` by design does not list ignored files\nif they are in a directory which contains only ignored and untracked\nfiles (which is itself considered to be untracked) without `-uall`. This\ndoes not make sense for `--ignored`, which claims to \"Show ignored files\nas well.\"\n\nThus we revisit eb8c5b87 and decide that for such directories, `status\n--ignored` will list the directory as untracked *and* list all ignored\nfiles within said directory even without `-uall`.\n\nSigned-off-by: Samuel Lijin \n---\n t/t7061-wtstatus-ignore.sh | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t7061-wtstatus-ignore.sh b/t/t7061-wtstatus-ignore.sh\nindex cdc0747bf..15e7592b6 100755\n--- a/t/t7061-wtstatus-ignore.sh\n+++ b/t/t7061-wtstatus-ignore.sh\n@@ -9,9 +9,10 @@ cat >expected <<\\EOF\n ?? actual\n ?? expected\n ?? untracked/\n+!! untracked/ignored\n EOF\n \n-test_expect_success 'status untracked directory with --ignored' '\n+test_expect_failure 'status untracked directory with --ignored' '\n \techo \"ignored\" >.gitignore &&\n \tmkdir untracked &&\n \t: >untracked/ignored &&\n@@ -20,7 +21,7 @@ test_expect_success 'status untracked directory with --ignored' '\n \ttest_cmp expected actual\n '\n \n-test_expect_success 'same with gitignore starting with BOM' '\n+test_expect_failure 'same with gitignore starting with BOM' '\n \tprintf \"\\357\\273\\277ignored\\n\" >.gitignore &&\n \tmkdir -p untracked &&\n \t: >untracked/ignored &&\n-- \n2.12.2\n\n"},{"id":"319901","messageId":"20170516073423.25762-5-sxlijin@gmail.com","threadId":"45826","inReplyTo":"20170516073423.25762-1-sxlijin@gmail.com","subject":"[PATCH v3 4/8] dir: hide untracked contents of untracked dirs","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-16T07:34:19Z","receivedAt":"2017-05-16T07:36:25Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"When we taught read_directory_recursive() to recurse into untracked\ndirectories in search of ignored files given DIR_SHOW_IGNORED_TOO, that\nhad the side effect of teaching it to collect the untracked contents of\nuntracked directories. It doesn't always make sense to return these,\nthough (we do need them for `clean -d`), so we introduce a flag\n(DIR_KEEP_UNTRACKED_CONTENTS) to control whether or not read_directory()\nstrips dir->entries of the untracked contents of untracked dirs.\n\nWe also introduce check_contains() to check if one dir_entry corresponds\nto a path which contains the path corresponding to another dir_entry.\n\nSigned-off-by: Samuel Lijin \n---\n dir.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++\n dir.h | 3 ++-\n 2 files changed, 56 insertions(+), 1 deletion(-)\n\ndiff --git a/dir.c b/dir.c\nindex 6bd0350e9..214a148ee 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -1852,6 +1852,14 @@ static int cmp_name(const void *p1, const void *p2)\n \treturn name_compare(e1->name, e1->len, e2->name, e2->len);\n }\n \n+/* check if *out lexically contains *in */\n+static int check_contains(const struct dir_entry *out, const struct dir_entry *in)\n+{\n+\treturn (out->len < in->len) &&\n+\t\t\t(out->name[out->len - 1] == '/') &&\n+\t\t\t!memcmp(out->name, in->name, out->len);\n+}\n+\n static int treat_leading_path(struct dir_struct *dir,\n \t\t\t const char *path, int len,\n \t\t\t const struct pathspec *pathspec)\n@@ -2067,6 +2075,52 @@ int read_directory(struct dir_struct *dir, const char *path,\n \t\tread_directory_recursive(dir, path, len, untracked, 0, pathspec);\n \tQSORT(dir->entries, dir->nr, cmp_name);\n \tQSORT(dir->ignored, dir->ignored_nr, cmp_name);\n+\n+\t// if DIR_SHOW_IGNORED_TOO, read_directory_recursive() will also pick\n+\t// up untracked contents of untracked dirs; by default we discard these,\n+\t// but given DIR_KEEP_UNTRACKED_CONTENTS we do not\n+\tif ((dir->flags & DIR_SHOW_IGNORED_TOO)\n+\t\t && !(dir->flags & DIR_KEEP_UNTRACKED_CONTENTS)) {\n+\t\tint i, j, nr_removed = 0;\n+\n+\t\t// remove from dir->entries untracked contents of untracked dirs\n+\t\tfor (i = 0; i < dir->nr; i++) {\n+\t\t\tif (!dir->entries[i])\n+\t\t\t\tcontinue;\n+\n+\t\t\tfor (j = i + 1; j < dir->nr; j++) {\n+\t\t\t\tif (!dir->entries[j])\n+\t\t\t\t\tcontinue;\n+\t\t\t\tif (check_contains(dir->entries[i], dir->entries[j])) {\n+\t\t\t\t\tnr_removed++;\n+\t\t\t\t\tfree(dir->entries[j]);\n+\t\t\t\t\tdir->entries[j] = NULL;\n+\t\t\t\t}\n+\t\t\t\telse {\n+\t\t\t\t\tbreak;\n+\t\t\t\t}\n+\t\t\t}\n+\t\t}\n+\n+\t\t// strip dir->entries of NULLs\n+\t\tif (nr_removed) {\n+\t\t\tfor (i = 0;;) {\n+\t\t\t\twhile (i < dir->nr && dir->entries[i])\n+\t\t\t\t\ti++;\n+\t\t\t\tif (i == dir->nr)\n+\t\t\t\t\tbreak;\n+\t\t\t\tj = i;\n+\t\t\t\twhile (j < dir->nr && !dir->entries[j])\n+\t\t\t\t\tj++;\n+\t\t\t\tif (j == dir->nr)\n+\t\t\t\t\tbreak;\n+\t\t\t\tdir->entries[i] = dir->entries[j];\n+\t\t\t\tdir->entries[j] = NULL;\n+\t\t\t}\n+\t\t\tdir->nr -= nr_removed;\n+\t\t}\n+\t}\n+\n \tif (dir->untracked) {\n \t\tstatic struct trace_key trace_untracked_stats = TRACE_KEY_INIT(UNTRACKED_STATS);\n \t\ttrace_printf_key(&trace_untracked_stats,\ndiff --git a/dir.h b/dir.h\nindex bf23a470a..650e54bdf 100644\n--- a/dir.h\n+++ b/dir.h\n@@ -151,7 +151,8 @@ struct dir_struct {\n \t\tDIR_NO_GITLINKS = 1<<3,\n \t\tDIR_COLLECT_IGNORED = 1<<4,\n \t\tDIR_SHOW_IGNORED_TOO = 1<<5,\n-\t\tDIR_COLLECT_KILLED_ONLY = 1<<6\n+\t\tDIR_COLLECT_KILLED_ONLY = 1<<6,\n+\t\tDIR_KEEP_UNTRACKED_CONTENTS = 1<<7\n \t} flags;\n \tstruct dir_entry **entries;\n \tstruct dir_entry **ignored;\n-- \n2.12.2\n\n"},{"id":"319902","messageId":"20170516073423.25762-6-sxlijin@gmail.com","threadId":"45826","inReplyTo":"20170516073423.25762-1-sxlijin@gmail.com","subject":"[PATCH v3 5/8] dir: expose cmp_name() and check_contains()","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-16T07:34:20Z","receivedAt":"2017-05-16T07:36:26Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"We want to use cmp_name() and check_contains() (which both compare\n`struct dir_entry`s, the former in terms of the sort order, the latter\nin terms of whether one lexically contains another) outside of dir.c,\nso we have to (1) change their linkage and (2) rename them as\nappropriate for the global namespace. The second is achieved by\nrenaming cmp_name() to cmp_dir_entry() and check_contains() to\ncheck_dir_entry_contains().\n\nSigned-off-by: Samuel Lijin \n---\n dir.c | 10 +++++-----\n dir.h | 3 +++\n 2 files changed, 8 insertions(+), 5 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex 214a148ee..1bf2d20e7 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -1844,7 +1844,7 @@ static enum path_treatment read_directory_recursive(struct dir_struct *dir,\n \treturn dir_state;\n }\n \n-static int cmp_name(const void *p1, const void *p2)\n+int cmp_dir_entry(const void *p1, const void *p2)\n {\n \tconst struct dir_entry *e1 = *(const struct dir_entry **)p1;\n \tconst struct dir_entry *e2 = *(const struct dir_entry **)p2;\n@@ -1853,7 +1853,7 @@ static int cmp_name(const void *p1, const void *p2)\n }\n \n /* check if *out lexically contains *in */\n-static int check_contains(const struct dir_entry *out, const struct dir_entry *in)\n+int check_dir_entry_contains(const struct dir_entry *out, const struct dir_entry *in)\n {\n \treturn (out->len < in->len) &&\n \t\t\t(out->name[out->len - 1] == '/') &&\n@@ -2073,8 +2073,8 @@ int read_directory(struct dir_struct *dir, const char *path,\n \t\tdir->untracked = NULL;\n \tif (!len || treat_leading_path(dir, path, len, pathspec))\n \t\tread_directory_recursive(dir, path, len, untracked, 0, pathspec);\n-\tQSORT(dir->entries, dir->nr, cmp_name);\n-\tQSORT(dir->ignored, dir->ignored_nr, cmp_name);\n+\tQSORT(dir->entries, dir->nr, cmp_dir_entry);\n+\tQSORT(dir->ignored, dir->ignored_nr, cmp_dir_entry);\n \n \t// if DIR_SHOW_IGNORED_TOO, read_directory_recursive() will also pick\n \t// up untracked contents of untracked dirs; by default we discard these,\n@@ -2091,7 +2091,7 @@ int read_directory(struct dir_struct *dir, const char *path,\n \t\t\tfor (j = i + 1; j < dir->nr; j++) {\n \t\t\t\tif (!dir->entries[j])\n \t\t\t\t\tcontinue;\n-\t\t\t\tif (check_contains(dir->entries[i], dir->entries[j])) {\n+\t\t\t\tif (check_dir_entry_contains(dir->entries[i], dir->entries[j])) {\n \t\t\t\t\tnr_removed++;\n \t\t\t\t\tfree(dir->entries[j]);\n \t\t\t\t\tdir->entries[j] = NULL;\ndiff --git a/dir.h b/dir.h\nindex 650e54bdf..edb5fda58 100644\n--- a/dir.h\n+++ b/dir.h\n@@ -327,6 +327,9 @@ static inline int dir_path_match(const struct dir_entry *ent,\n \t\t\t has_trailing_dir);\n }\n \n+int cmp_dir_entry(const void *p1, const void *p2);\n+int check_dir_entry_contains(const struct dir_entry *out, const struct dir_entry *in);\n+\n void untracked_cache_invalidate_path(struct index_state *, const char *);\n void untracked_cache_remove_from_index(struct index_state *, const char *);\n void untracked_cache_add_to_index(struct index_state *, const char *);\n-- \n2.12.2\n\n"},{"id":"319903","messageId":"20170516073423.25762-7-sxlijin@gmail.com","threadId":"45826","inReplyTo":"20170516073423.25762-1-sxlijin@gmail.com","subject":"[PATCH v3 6/8] clean: teach clean -d to skip dirs containing ignored files","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-16T07:34:21Z","receivedAt":"2017-05-16T07:36:29Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"There is an implicit assumption that a directory containing only\nuntracked and ignored files should itself be considered untracked. This\nmakes sense in use cases where we're asking if a directory should be\nadded to the git database, but not when we're asking if a directory can\nbe safely removed from the working tree; as a result, clean -d would\nassume that an \"untracked\" directory containing ignored files could be\ndeleted.\n\nTo get around this, we teach clean -d to collect ignored files and skip\nover so-called \"untracked\" directories if they contain any ignored\nfiles (while still removing the untracked contents of such dirs).\n\nSigned-off-by: Samuel Lijin \n---\n builtin/clean.c | 32 ++++++++++++++++++++++++++++++--\n 1 file changed, 30 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/clean.c b/builtin/clean.c\nindex d861f836a..25f3efce5 100644\n--- a/builtin/clean.c\n+++ b/builtin/clean.c\n@@ -859,7 +859,7 @@ static void interactive_main_loop(void)\n \n int cmd_clean(int argc, const char **argv, const char *prefix)\n {\n-\tint i, res;\n+\tint i, j, k, res;\n \tint dry_run = 0, remove_directories = 0, quiet = 0, ignored = 0;\n \tint ignored_only = 0, config_set = 0, errors = 0, gone = 1;\n \tint rm_flags = REMOVE_DIR_KEEP_NESTED_GIT;\n@@ -911,6 +911,9 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \t\t\t\t \" refusing to clean\"));\n \t}\n \n+\tif (remove_directories)\n+\t\tdir.flags |= DIR_SHOW_IGNORED_TOO | DIR_KEEP_UNTRACKED_CONTENTS;\n+\n \tif (force > 1)\n \t\trm_flags = 0;\n \n@@ -932,7 +935,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \n \tfill_directory(&dir, &pathspec);\n \n-\tfor (i = 0; i < dir.nr; i++) {\n+\tfor (k = i = 0; i < dir.nr; i++) {\n \t\tstruct dir_entry *ent = dir.entries[i];\n \t\tint matches = 0;\n \t\tstruct stat st;\n@@ -954,10 +957,35 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \t\t matches != MATCHED_EXACTLY)\n \t\t\tcontinue;\n \n+\t\t// skip any dir.entries which contains a dir.ignored\n+\t\tfor (; k < dir.ignored_nr; k++) {\n+\t\t\tif (cmp_dir_entry(&dir.entries[i],\n+\t\t\t\t\t\t&dir.ignored[k]) < 0)\n+\t\t\t\tbreak;\n+\t\t}\n+\t\tif ((k < dir.ignored_nr) &&\n+\t\t\t\tcheck_dir_entry_contains(dir.entries[i], dir.ignored[k])) {\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\t// current entry does not contain any ignored files\n \t\trel = relative_path(ent->name, prefix, &buf);\n \t\tstring_list_append(&del_list, rel);\n+\n+\t\t// skip untracked contents of an untracked dir\n+\t\tfor (j = i + 1;\n+\t\t\t j < dir.nr &&\n+\t\t\t check_dir_entry_contains(dir.entries[i], dir.entries[j]);\n+\t\t\t j++);\n+\t\ti = j - 1;\n \t}\n \n+\tfor (i = 0; i < dir.nr; i++)\n+\t\tfree(dir.entries[i]);\n+\n+\tfor (i = 0; i < dir.ignored_nr; i++)\n+\t\tfree(dir.ignored[i]);\n+\n \tif (interactive && del_list.nr > 0)\n \t\tinteractive_main_loop();\n \n-- \n2.12.2\n\n"},{"id":"319904","messageId":"20170516073423.25762-8-sxlijin@gmail.com","threadId":"45826","inReplyTo":"20170516073423.25762-1-sxlijin@gmail.com","subject":"[PATCH v3 7/8] t7300: clean -d now skips untracked dirs containing ignored files","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-16T07:34:22Z","receivedAt":"2017-05-16T07:36:32Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"Signed-off-by: Samuel Lijin \n---\n t/t7300-clean.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t7300-clean.sh b/t/t7300-clean.sh\nindex c14c98e56..b65d4719c 100755\n--- a/t/t7300-clean.sh\n+++ b/t/t7300-clean.sh\n@@ -653,7 +653,7 @@ test_expect_success 'git clean -d respects pathspecs (pathspec is prefix of dir)\n \ttest_path_is_dir foobar\n '\n \n-test_expect_failure 'git clean -d skips untracked dirs containing ignored files' '\n+test_expect_success 'git clean -d skips untracked dirs containing ignored files' '\n \techo /foo/bar >.gitignore &&\n \trm -rf foo &&\n \tmkdir foo &&\n-- \n2.12.2\n\n"},{"id":"319905","messageId":"20170516073423.25762-9-sxlijin@gmail.com","threadId":"45826","inReplyTo":"20170516073423.25762-1-sxlijin@gmail.com","subject":"[PATCH v3 8/8] t7061: status --ignored now searches untracked dirs","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-16T07:34:23Z","receivedAt":"2017-05-16T07:36:34Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"Signed-off-by: Samuel Lijin \n---\n t/t7061-wtstatus-ignore.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t7061-wtstatus-ignore.sh b/t/t7061-wtstatus-ignore.sh\nindex 15e7592b6..fc6013ba3 100755\n--- a/t/t7061-wtstatus-ignore.sh\n+++ b/t/t7061-wtstatus-ignore.sh\n@@ -12,7 +12,7 @@ cat >expected <<\\EOF\n !! untracked/ignored\n EOF\n \n-test_expect_failure 'status untracked directory with --ignored' '\n+test_expect_success 'status untracked directory with --ignored' '\n \techo \"ignored\" >.gitignore &&\n \tmkdir untracked &&\n \t: >untracked/ignored &&\n@@ -21,7 +21,7 @@ test_expect_failure 'status untracked directory with --ignored' '\n \ttest_cmp expected actual\n '\n \n-test_expect_failure 'same with gitignore starting with BOM' '\n+test_expect_success 'same with gitignore starting with BOM' '\n \tprintf \"\\357\\273\\277ignored\\n\" >.gitignore &&\n \tmkdir -p untracked &&\n \t: >untracked/ignored &&\n-- \n2.12.2\n\n"},{"id":"320024","messageId":"xmqqy3twvw66.fsf@gitster.mtv.corp.google.com","threadId":"45826","inReplyTo":"20170516073423.25762-4-sxlijin@gmail.com","subject":"Re: [PATCH v3 3/8] dir: recurse into untracked dirs for ignored files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-17T06:23:29Z","receivedAt":"2017-05-17T06:23:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Samuel Lijin writes:\n\n> We consider directories containing only untracked and ignored files to\n> be themselves untracked, which in the usual case means we don't have to\n> search these directories. This is problematic when we want to collect\n> ignored files with DIR_SHOW_IGNORED_TOO, though, so we teach\n> read_directory_recursive() to recurse into untracked directories to find\n> the ignored files they contain when DIR_SHOW_IGNORED_TOO is set. This\n> has the side effect of also collecting all untracked files in untracked\n> directories as well.\n>\n> Signed-off-by: Samuel Lijin \n> ---\n> dir.c | 7 ++++++-\n> 1 file changed, 6 insertions(+), 1 deletion(-)\n>\n> diff --git a/dir.c b/dir.c\n> index f451bfa48..6bd0350e9 100644\n> --- a/dir.c\n> +++ b/dir.c\n> @@ -1784,7 +1784,12 @@ static enum path_treatment read_directory_recursive(struct dir_struct *dir,\n> \t\t\tdir_state = state;\n> \n> \t\t/* recurse into subdir if instructed by treat_path */\n> -\t\tif (state == path_recurse) {\n> +\t\tif ((state == path_recurse) ||\n> +\t\t\t\t((get_dtype(cdir.de, path.buf, path.len) == DT_DIR) &&\n\nI see a conditional in treat_path() that is prepared to deal with a\nNULL in cdir.de; do we know cdir.de is always non-NULL at this point\nin the code, or is get_dtype() prepared to see NULL as its first\nparameter?\n\n\t... goes and looks ...\n\nYes, it falls back to the usual lstat() dance in such a case, so\nwe'd be OK here.\n\n> +\t\t\t\t (state == path_untracked) &&\n> +\t\t\t\t (dir->flags & DIR_SHOW_IGNORED_TOO))\n\nIf we are told to SHOW_IGNORED_TOO, we'd recurse into an untracked\nthing if it is a directory. No other behaviour change.\n\nIsn't get_dtype() potentially slower than other two checks if it\ntriggers? I am wondering if these three conditions in &&-chain\nshould be reordered to call get_dtype() the last, i.e.\n\n if ((state == path_recurse) ||\n ((dir->flags & DIR_SHOW_IGNORED_TOO)) &&\n (state == path_untracked) &&\n (get_dtype(cdir.de, path.buf, path.len) == DT_DIR)) {\n\n> +\t\t\t\t)\n> +\t\t{\n\nIt may be just a style, but these new lines are indented overly too\ndeep. We don't use a lone \"{\" on a line to open a block, either.\n\n> \t\t\tstruct untracked_cache_dir *ud;\n> \t\t\tud = lookup_untracked(dir->untracked, untracked,\n> \t\t\t\t\t path.buf + baselen,\n"},{"id":"320027","messageId":"xmqqo9usvv1m.fsf@gitster.mtv.corp.google.com","threadId":"45826","inReplyTo":"20170516073423.25762-5-sxlijin@gmail.com","subject":"Re: [PATCH v3 4/8] dir: hide untracked contents of untracked dirs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-17T06:47:49Z","receivedAt":"2017-05-17T06:47:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Samuel Lijin writes:\n\n> When we taught read_directory_recursive() to recurse into untracked\n> directories in search of ignored files given DIR_SHOW_IGNORED_TOO, that\n> had the side effect of teaching it to collect the untracked contents of\n> untracked directories. It doesn't always make sense to return these,\n> though (we do need them for `clean -d`), so we introduce a flag\n> (DIR_KEEP_UNTRACKED_CONTENTS) to control whether or not read_directory()\n> strips dir->entries of the untracked contents of untracked dirs.\n>\n> We also introduce check_contains() to check if one dir_entry corresponds\n> to a path which contains the path corresponding to another dir_entry.\n>\n> Signed-off-by: Samuel Lijin \n> ---\n> dir.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++\n> dir.h | 3 ++-\n> 2 files changed, 56 insertions(+), 1 deletion(-)\n>\n> diff --git a/dir.c b/dir.c\n> index 6bd0350e9..214a148ee 100644\n> --- a/dir.c\n> +++ b/dir.c\n> @@ -1852,6 +1852,14 @@ static int cmp_name(const void *p1, const void *p2)\n> \treturn name_compare(e1->name, e1->len, e2->name, e2->len);\n> }\n> \n> +/* check if *out lexically contains *in */\n> +static int check_contains(const struct dir_entry *out, const struct dir_entry *in)\n> +{\n> +\treturn (out->len < in->len) &&\n> +\t\t\t(out->name[out->len - 1] == '/') &&\n> +\t\t\t!memcmp(out->name, in->name, out->len);\n> +}\n\nOK, treat_one_path() and treat_pah_fast() both ensure that a path to\na directory is terminated with '/' before calling dir_add_name() and\ndir_add_ignored(), so we know a dir_entry \"out\" that is a directory\nmust end with '/'. Good.\n\nThe second and third line being overly indented is a bit\ndistracting, though.\n\n> static int treat_leading_path(struct dir_struct *dir,\n> \t\t\t const char *path, int len,\n> \t\t\t const struct pathspec *pathspec)\n> @@ -2067,6 +2075,52 @@ int read_directory(struct dir_struct *dir, const char *path,\n> \t\tread_directory_recursive(dir, path, len, untracked, 0, pathspec);\n> \tQSORT(dir->entries, dir->nr, cmp_name);\n> \tQSORT(dir->ignored, dir->ignored_nr, cmp_name);\n> +\n> +\t// if DIR_SHOW_IGNORED_TOO, read_directory_recursive() will also pick\n> +\t// up untracked contents of untracked dirs; by default we discard these,\n> +\t// but given DIR_KEEP_UNTRACKED_CONTENTS we do not\n\n\t/*\n\t * Our multi-line comments are formatted like this \n\t * example. No C++/C99 // comments, outside of\n\t * borrowed code and platform specific compat/ code,\n\t * please.\n\t */\n\n> +\tif ((dir->flags & DIR_SHOW_IGNORED_TOO)\n> +\t\t && !(dir->flags & DIR_KEEP_UNTRACKED_CONTENTS)) {\n\nBoth having && at the end and && at the beginning are valid C, but\nplease stick to one style in a single file.\n\n> +\t\tint i, j, nr_removed = 0;\n> +\n> +\t\t// remove from dir->entries untracked contents of untracked dirs\n\n\t/* And our single-liner comments look like this */\n\n> +\t\tfor (i = 0; i < dir->nr; i++) {\n> +\t\t\tif (!dir->entries[i])\n> +\t\t\t\tcontinue;\n> +\n> +\t\t\tfor (j = i + 1; j < dir->nr; j++) {\n> +\t\t\t\tif (!dir->entries[j])\n> +\t\t\t\t\tcontinue;\n> +\t\t\t\tif (check_contains(dir->entries[i], dir->entries[j])) {\n> +\t\t\t\t\tnr_removed++;\n> +\t\t\t\t\tfree(dir->entries[j]);\n> +\t\t\t\t\tdir->entries[j] = NULL;\n> +\t\t\t\t}\n> +\t\t\t\telse {\n> +\t\t\t\t\tbreak;\n> +\t\t\t\t}\n> +\t\t\t}\n> +\t\t}\n\nThis loop is O(n^2). I wonder if we can do better, especially we\nknow dir->entries[] is sorted already.\n\nWell, because it is sorted, if A/, A/B, and A/B/C are all untracked,\nthe first round that scans for A/ will nuke both A/B and A/B/C, so\nwe won't have to scan looking for entries inside A/B, which is a bit\nof consolation ;-)\n\n> +\t\t\tfor (i = 0;;) {\n> +\t\t\t\twhile (i < dir->nr && dir->entries[i])\n> +\t\t\t\t\ti++;\n> +\t\t\t\tif (i == dir->nr)\n> +\t\t\t\t\tbreak;\n> +\t\t\t\tj = i;\n> +\t\t\t\twhile (j < dir->nr && !dir->entries[j])\n> +\t\t\t\t\tj++;\n> +\t\t\t\tif (j == dir->nr)\n> +\t\t\t\t\tbreak;\n> +\t\t\t\tdir->entries[i] = dir->entries[j];\n> +\t\t\t\tdir->entries[j] = NULL;\n> +\t\t\t}\n> +\t\t\tdir->nr -= nr_removed;\n\nThis looks like an overly complicated way to scan an array and skip\nNULLs. Are you doing an equivalent of this loop, or am I missing\nsomething subtle?\n\n\tfor (src = dst = 0; src < nr; src++)\n\t\tif (array[src])\n\t\t\tarray[dst++] = src;\n\tnr = dst;\n"},{"id":"320028","messageId":"CAJZjrdXTWsOX6-=4XJwa0EwOUieWw7xpK6i9hfOaxpAAjqqz-g@mail.gmail.com","threadId":"45826","inReplyTo":"xmqqy3twvw66.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v3 3/8] dir: recurse into untracked dirs for ignored files","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-17T07:02:07Z","receivedAt":"2017-05-17T07:03:30Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"On Wed, May 17, 2017 at 2:23 AM, Junio C Hamano wrote:\n> Samuel Lijin writes:\n>\n>> We consider directories containing only untracked and ignored files to\n>> be themselves untracked, which in the usual case means we don't have to\n>> search these directories. This is problematic when we want to collect\n>> ignored files with DIR_SHOW_IGNORED_TOO, though, so we teach\n>> read_directory_recursive() to recurse into untracked directories to find\n>> the ignored files they contain when DIR_SHOW_IGNORED_TOO is set. This\n>> has the side effect of also collecting all untracked files in untracked\n>> directories as well.\n>>\n>> Signed-off-by: Samuel Lijin \n>> ---\n>> dir.c | 7 ++++++-\n>> 1 file changed, 6 insertions(+), 1 deletion(-)\n>>\n>> diff --git a/dir.c b/dir.c\n>> index f451bfa48..6bd0350e9 100644\n>> --- a/dir.c\n>> +++ b/dir.c\n>> @@ -1784,7 +1784,12 @@ static enum path_treatment read_directory_recursive(struct dir_struct *dir,\n>> dir_state = state;\n>>\n>> /* recurse into subdir if instructed by treat_path */\n>> - if (state == path_recurse) {\n>> + if ((state == path_recurse) ||\n>> + ((get_dtype(cdir.de, path.buf, path.len) == DT_DIR) &&\n>\n> I see a conditional in treat_path() that is prepared to deal with a\n> NULL in cdir.de; do we know cdir.de is always non-NULL at this point\n> in the code, or is get_dtype() prepared to see NULL as its first\n> parameter?\n>\n> ... goes and looks ...\n>\n> Yes, it falls back to the usual lstat() dance in such a case, so\n> we'd be OK here.\n>\n>> + (state == path_untracked) &&\n>> + (dir->flags & DIR_SHOW_IGNORED_TOO))\n>\n> If we are told to SHOW_IGNORED_TOO, we'd recurse into an untracked\n> thing if it is a directory. No other behaviour change.\n>\n> Isn't get_dtype() potentially slower than other two checks if it\n> triggers? I am wondering if these three conditions in &&-chain\n> should be reordered to call get_dtype() the last, i.e.\n>\n> if ((state == path_recurse) ||\n> ((dir->flags & DIR_SHOW_IGNORED_TOO)) &&\n> (state == path_untracked) &&\n> (get_dtype(cdir.de, path.buf, path.len) == DT_DIR)) {\n\nAh, I didn't realize that. I figured that get_dtype() was just a\nhelper to invoke the appropriate macros, but if it's actually mildly\nexpensive I'll take your reorder.\n\n>> + )\n>> + {\n>\n> It may be just a style, but these new lines are indented overly too\n> deep. We don't use a lone \"{\" on a line to open a block, either.\n>\n>> struct untracked_cache_dir *ud;\n>> ud = lookup_untracked(dir->untracked, untracked,\n>> path.buf + baselen,\n"},{"id":"320030","messageId":"CAJZjrdXVzYZ_77Eod6oq-CB+tn5wdF4B3nWzN5zQvjduL9Lnfw@mail.gmail.com","threadId":"45826","inReplyTo":"xmqqo9usvv1m.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v3 4/8] dir: hide untracked contents of untracked dirs","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-17T07:32:09Z","receivedAt":"2017-05-17T07:33:01Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"On Wed, May 17, 2017 at 2:47 AM, Junio C Hamano wrote:\n> Samuel Lijin writes:\n>\n>> When we taught read_directory_recursive() to recurse into untracked\n>> directories in search of ignored files given DIR_SHOW_IGNORED_TOO, that\n>> had the side effect of teaching it to collect the untracked contents of\n>> untracked directories. It doesn't always make sense to return these,\n>> though (we do need them for `clean -d`), so we introduce a flag\n>> (DIR_KEEP_UNTRACKED_CONTENTS) to control whether or not read_directory()\n>> strips dir->entries of the untracked contents of untracked dirs.\n>>\n>> We also introduce check_contains() to check if one dir_entry corresponds\n>> to a path which contains the path corresponding to another dir_entry.\n>>\n>> Signed-off-by: Samuel Lijin \n>> ---\n>> dir.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++\n>> dir.h | 3 ++-\n>> 2 files changed, 56 insertions(+), 1 deletion(-)\n>>\n>> diff --git a/dir.c b/dir.c\n>> index 6bd0350e9..214a148ee 100644\n>> --- a/dir.c\n>> +++ b/dir.c\n>> @@ -1852,6 +1852,14 @@ static int cmp_name(const void *p1, const void *p2)\n>> return name_compare(e1->name, e1->len, e2->name, e2->len);\n>> }\n>>\n>> +/* check if *out lexically contains *in */\n>> +static int check_contains(const struct dir_entry *out, const struct dir_entry *in)\n>> +{\n>> + return (out->len < in->len) &&\n>> + (out->name[out->len - 1] == '/') &&\n>> + !memcmp(out->name, in->name, out->len);\n>> +}\n>\n> OK, treat_one_path() and treat_pah_fast() both ensure that a path to\n> a directory is terminated with '/' before calling dir_add_name() and\n> dir_add_ignored(), so we know a dir_entry \"out\" that is a directory\n> must end with '/'. Good.\n>\n> The second and third line being overly indented is a bit\n> distracting, though.\n>\n>> static int treat_leading_path(struct dir_struct *dir,\n>> const char *path, int len,\n>> const struct pathspec *pathspec)\n>> @@ -2067,6 +2075,52 @@ int read_directory(struct dir_struct *dir, const char *path,\n>> read_directory_recursive(dir, path, len, untracked, 0, pathspec);\n>> QSORT(dir->entries, dir->nr, cmp_name);\n>> QSORT(dir->ignored, dir->ignored_nr, cmp_name);\n>> +\n>> + // if DIR_SHOW_IGNORED_TOO, read_directory_recursive() will also pick\n>> + // up untracked contents of untracked dirs; by default we discard these,\n>> + // but given DIR_KEEP_UNTRACKED_CONTENTS we do not\n>\n> /*\n> * Our multi-line comments are formatted like this\n> * example. No C++/C99 // comments, outside of\n> * borrowed code and platform specific compat/ code,\n> * please.\n> */\n\nGahhhh, I keep forgetting about this, sorry. (There has to be a way to\ntell my compiler to catch this, right? It's pretty embarrassing to get\ncalled out for this twice...)\n\n>> + if ((dir->flags & DIR_SHOW_IGNORED_TOO)\n>> + && !(dir->flags & DIR_KEEP_UNTRACKED_CONTENTS)) {\n>\n> Both having && at the end and && at the beginning are valid C, but\n> please stick to one style in a single file.\n\nGot it.\n\n>> + int i, j, nr_removed = 0;\n>> +\n>> + // remove from dir->entries untracked contents of untracked dirs\n>\n> /* And our single-liner comments look like this */\n>\n>> + for (i = 0; i < dir->nr; i++) {\n>> + if (!dir->entries[i])\n>> + continue;\n>> +\n>> + for (j = i + 1; j < dir->nr; j++) {\n>> + if (!dir->entries[j])\n>> + continue;\n>> + if (check_contains(dir->entries[i], dir->entries[j])) {\n>> + nr_removed++;\n>> + free(dir->entries[j]);\n>> + dir->entries[j] = NULL;\n>> + }\n>> + else {\n>> + break;\n>> + }\n>> + }\n>> + }\n>\n> This loop is O(n^2). I wonder if we can do better, especially we\n> know dir->entries[] is sorted already.\n\nNow that I think about it, dropping an `i = j - 1` into the inner loop\nright before the break should work:\n\n+ else {\n+ i = j - 1;\n+ break;\n+ }\n\n> Well, because it is sorted, if A/, A/B, and A/B/C are all untracked,\n> the first round that scans for A/ will nuke both A/B and A/B/C, so\n> we won't have to scan looking for entries inside A/B, which is a bit\n> of consolation ;-)\n>\n>> + for (i = 0;;) {\n>> + while (i < dir->nr && dir->entries[i])\n>> + i++;\n>> + if (i == dir->nr)\n>> + break;\n>> + j = i;\n>> + while (j < dir->nr && !dir->entries[j])\n>> + j++;\n>> + if (j == dir->nr)\n>> + break;\n>> + dir->entries[i] = dir->entries[j];\n>> + dir->entries[j] = NULL;\n>> + }\n>> + dir->nr -= nr_removed;\n>\n> This looks like an overly complicated way to scan an array and skip\n> NULLs. Are you doing an equivalent of this loop, or am I missing\n> something subtle?\n>\n> for (src = dst = 0; src < nr; src++)\n> if (array[src])\n> array[dst++] = src;\n> nr = dst;\n\nNope, that's pretty much it. Just me overthinking the problem.\n"},{"id":"320031","messageId":"xmqqfug3x01p.fsf@gitster.mtv.corp.google.com","threadId":"45826","inReplyTo":"CAJZjrdXVzYZ_77Eod6oq-CB+tn5wdF4B3nWzN5zQvjduL9Lnfw@mail.gmail.com","subject":"Re: [PATCH v3 4/8] dir: hide untracked contents of untracked dirs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-17T10:14:26Z","receivedAt":"2017-05-17T10:14:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Samuel Lijin writes:\n\n>> /* And our single-liner comments look like this */\n>>\n>>> + for (i = 0; i < dir->nr; i++) {\n>>> + if (!dir->entries[i])\n>>> + continue;\n>>> +\n>>> + for (j = i + 1; j < dir->nr; j++) {\n>>> + if (!dir->entries[j])\n>>> + continue;\n>>> + if (check_contains(dir->entries[i], dir->entries[j])) {\n>>> + nr_removed++;\n>>> + free(dir->entries[j]);\n>>> + dir->entries[j] = NULL;\n>>> + }\n>>> + else {\n>>> + break;\n>>> + }\n>>> + }\n>>> + }\n>>\n>> This loop is O(n^2). I wonder if we can do better, especially we\n>> know dir->entries[] is sorted already.\n>\n> Now that I think about it, dropping an `i = j - 1` into the inner loop\n> right before the break should work:\n\nActually, I think you should be able to do this in a single loop and\nwrite in such a way that it is clearly O(N), perhaps like:\n\n\tfor (src = dst = 0; src < nr; src++) {\n\t\tif (!dst ||\n\t\t !is_a_directory(array[dst - 1]) ||\n\t\t !contains(array[dst - 1], array[src])) {\n\t\t\tarray[dst++] = array[src];\n\t\t} else {\n\t /* \n\t * Otherwise, src is inside (dst-1), so \n\t\t\t * we just can discard it.\n\t\t\t */\n\t\t\tfree(array[src]);\n\t\t\tarray[src] = NULL; /* not needed */\n\t\t}\n\t}\n\tnr = dst;\n\nThat is, dst points at the next place to save the final elements of\nthe array, and dst-1 is the last one that is known to be the final\nset. src points at the element we are inspecting.\n\nIf dst-1 does not exist (i.e. we are looking at the first element),\nif dst-1 is not a directory, or if dst-1 does not contain the src,\nthen we need to keep the src in the final set. Otherwise we discard.\n"},{"id":"320125","messageId":"xmqq4lwix58s.fsf@gitster.mtv.corp.google.com","threadId":"45826","inReplyTo":"20170516073423.25762-7-sxlijin@gmail.com","subject":"Re: [PATCH v3 6/8] clean: teach clean -d to skip dirs containing ignored files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-18T02:34:27Z","receivedAt":"2017-05-18T02:37:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Samuel Lijin writes:\n\n> @@ -932,7 +935,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n> \n> \tfill_directory(&dir, &pathspec);\n> \n> -\tfor (i = 0; i < dir.nr; i++) {\n> +\tfor (k = i = 0; i < dir.nr; i++) {\n> \t\tstruct dir_entry *ent = dir.entries[i];\n> \t\tint matches = 0;\n> \t\tstruct stat st;\n> @@ -954,10 +957,35 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n> \t\t matches != MATCHED_EXACTLY)\n> \t\t\tcontinue;\n> \n> +\t\t// skip any dir.entries which contains a dir.ignored\n> +\t\tfor (; k < dir.ignored_nr; k++) {\n> +\t\t\tif (cmp_dir_entry(&dir.entries[i],\n> +\t\t\t\t\t\t&dir.ignored[k]) < 0)\n> +\t\t\t\tbreak;\n\nIt is a bit unfortunate that we do not use the short-hand \"ent\" we\nestablished for dir.entries[i] at the beginning of this loop here.\n\n> +\t\t}\n> +\t\tif ((k < dir.ignored_nr) &&\n> +\t\t\t\tcheck_dir_entry_contains(dir.entries[i], dir.ignored[k])) {\n> +\t\t\tcontinue;\n> +\t\t}\n\nThe above logic is not needed when dir.entries[i] is a directory,\nright?\n\n> +\n> +\t\t// current entry does not contain any ignored files\n\n... or the entry may not have been a directory in the first place.\n\n> \t\trel = relative_path(ent->name, prefix, &buf);\n> \t\tstring_list_append(&del_list, rel);\n> +\n> +\t\t// skip untracked contents of an untracked dir\n> +\t\tfor (j = i + 1;\n> +\t\t\t j < dir.nr &&\n> +\t\t\t check_dir_entry_contains(dir.entries[i], dir.entries[j]);\n> +\t\t\t j++);\n> +\t\ti = j - 1;\n> \t}\n>\n> +\tfor (i = 0; i < dir.nr; i++)\n> +\t\tfree(dir.entries[i]);\n> +\n> +\tfor (i = 0; i < dir.ignored_nr; i++)\n> +\t\tfree(dir.ignored[i]);\n> +\n\nThe above logic may not be incorrect per-se, but I have this\nsuspicion that the resulting code may become cleaner and easier to\nunderstand if we did it in a new separate loop we call immediately\nafter fill_directory() returns. Or it could even be a call to a\nhelper that \"post processes\" the \"dir\" thing that was polupated by\nfill_directory() that removes elements in the entries[] array that\ncontains any element in ignored[] array.\n\n> \tif (interactive && del_list.nr > 0)\n> \t\tinteractive_main_loop();\n\nThanks.\n"},{"id":"320126","messageId":"xmqqzieavqgt.fsf@gitster.mtv.corp.google.com","threadId":"45826","inReplyTo":"20170516073423.25762-8-sxlijin@gmail.com","subject":"Re: [PATCH v3 7/8] t7300: clean -d now skips untracked dirs containing ignored files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-18T02:38:58Z","receivedAt":"2017-05-18T02:39:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This and 8/8 needs to be part of the code change that fixes them.\nOtherwise, running tests after applying only 1-6/8 would give you:\n\n Test Summary Report\n -------------------\n t7061-wtstatus-ignore.sh (Wstat: 0 Tests: 21 Failed: 0)\n TODO passed: 1-2\n t7300-clean.sh (Wstat: 0 Tests: 45 Failed: 0)\n TODO passed: 45\n Files=2, Tests=66,...\n\nor\n\n $ sh t7061-wtstatus-ignore.sh\n ok 1 - status untracke... # TODO known breakage vanished\n ...\n # 2 known breakage(s) vanished; please update test(s)\n # passed all remaining 19 test(s)\n 1..21\n\nwith the \"please update\" painted in red.\n\n \n"},{"id":"320127","messageId":"xmqqshk2vq43.fsf@gitster.mtv.corp.google.com","threadId":"45826","inReplyTo":"20170516073423.25762-9-sxlijin@gmail.com","subject":"Re: [PATCH v3 8/8] t7061: status --ignored now searches untracked dirs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-18T02:46:36Z","receivedAt":"2017-05-18T02:49:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Samuel Lijin writes:\n\n> Signed-off-by: Samuel Lijin \n> ---\n\nThis was fixed at \"hide untracked\" patch, so squash these changes in\nto that commit and add something like\n\n This fixes known breakages in t7061 documented earlier in the series.\n\nat the end of the log message of that one. The same for 7/8, which\nwas fixed in \"teach clean -d to skip dirs\" patch.\n\nThanks.\n\n\n> t/t7061-wtstatus-ignore.sh | 4 ++--\n> 1 file changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/t/t7061-wtstatus-ignore.sh b/t/t7061-wtstatus-ignore.sh\n> index 15e7592b6..fc6013ba3 100755\n> --- a/t/t7061-wtstatus-ignore.sh\n> +++ b/t/t7061-wtstatus-ignore.sh\n> @@ -12,7 +12,7 @@ cat >expected <<\\EOF\n> !! untracked/ignored\n> EOF\n> \n> -test_expect_failure 'status untracked directory with --ignored' '\n> +test_expect_success 'status untracked directory with --ignored' '\n> \techo \"ignored\" >.gitignore &&\n> \tmkdir untracked &&\n> \t: >untracked/ignored &&\n> @@ -21,7 +21,7 @@ test_expect_failure 'status untracked directory with --ignored' '\n> \ttest_cmp expected actual\n> '\n> \n> -test_expect_failure 'same with gitignore starting with BOM' '\n> +test_expect_success 'same with gitignore starting with BOM' '\n> \tprintf \"\\357\\273\\277ignored\\n\" >.gitignore &&\n> \tmkdir -p untracked &&\n> \t: >untracked/ignored &&\n"},{"id":"320141","messageId":"CAJZjrdUPChKF=3DvQ01-CusaxYoUWJTpsrPBbnnqnmh3MQ9eYg@mail.gmail.com","threadId":"45826","inReplyTo":"xmqq4lwix58s.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v3 6/8] clean: teach clean -d to skip dirs containing ignored files","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-18T06:32:15Z","receivedAt":"2017-05-18T06:33:13Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"On Wed, May 17, 2017 at 10:34 PM, Junio C Hamano wrote:\n> Samuel Lijin writes:\n>\n>> @@ -932,7 +935,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n>>\n>> fill_directory(&dir, &pathspec);\n>>\n>> - for (i = 0; i < dir.nr; i++) {\n>> + for (k = i = 0; i < dir.nr; i++) {\n>> struct dir_entry *ent = dir.entries[i];\n>> int matches = 0;\n>> struct stat st;\n>> @@ -954,10 +957,35 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n>> matches != MATCHED_EXACTLY)\n>> continue;\n>>\n>> + // skip any dir.entries which contains a dir.ignored\n>> + for (; k < dir.ignored_nr; k++) {\n>> + if (cmp_dir_entry(&dir.entries[i],\n>> + &dir.ignored[k]) < 0)\n>> + break;\n>\n> It is a bit unfortunate that we do not use the short-hand \"ent\" we\n> established for dir.entries[i] at the beginning of this loop here.\n>\n>> + }\n>> + if ((k < dir.ignored_nr) &&\n>> + check_dir_entry_contains(dir.entries[i], dir.ignored[k])) {\n>> + continue;\n>> + }\n>\n> The above logic is not needed when dir.entries[i] is a directory,\n> right?\n\nAu contraire - this logic only matters when dir.entries[i] is a directory.\n\n>> +\n>> + // current entry does not contain any ignored files\n>\n> ... or the entry may not have been a directory in the first place.\n\nIf it's not a directory, it can't contain ignored files ;)\n\n>> rel = relative_path(ent->name, prefix, &buf);\n>> string_list_append(&del_list, rel);\n>> +\n>> + // skip untracked contents of an untracked dir\n>> + for (j = i + 1;\n>> + j < dir.nr &&\n>> + check_dir_entry_contains(dir.entries[i], dir.entries[j]);\n>> + j++);\n>> + i = j - 1;\n>> }\n>>\n>> + for (i = 0; i < dir.nr; i++)\n>> + free(dir.entries[i]);\n>> +\n>> + for (i = 0; i < dir.ignored_nr; i++)\n>> + free(dir.ignored[i]);\n>> +\n>\n> The above logic may not be incorrect per-se, but I have this\n> suspicion that the resulting code may become cleaner and easier to\n> understand if we did it in a new separate loop we call immediately\n> after fill_directory() returns. Or it could even be a call to a\n> helper that \"post processes\" the \"dir\" thing that was polupated by\n> fill_directory() that removes elements in the entries[] array that\n> contains any element in ignored[] array.\n\nWill try it out.\n\n>> if (interactive && del_list.nr > 0)\n>> interactive_main_loop();\n>\n> Thanks.\n"},{"id":"320142","messageId":"20170518082154.28643-1-sxlijin@gmail.com","threadId":"45826","inReplyTo":"20170516073423.25762-1-sxlijin@gmail.com","subject":"[PATCH v4 0/6] Fix clean -d and status --ignored","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-18T08:21:48Z","receivedAt":"2017-05-18T08:22:18Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"Incorporates all of Junio's feedback on v4:\n\n* squashes test expect_failure -> expect_success flips into the commits that\n fix them\n* the O(n^2) pruning loop added for DIR_KEEP_UNTRACKED_CONTENTS is now O(n)\n* the logic in cmd_clean() that keeps the contents of an untracked dir from the\n del_list is now hoisted into a separate loop\n\nAlso includes the following:\n\n* expands the test I add in t7300 to check clean -d, for better coverage of the\n pruning logic in cmd_clean() (the logic that tells clean -d to not remove a/b\n and a/c if it's already removing a/)\n* documents DIR_KEEP_UNTRACKED_CONTENTS in the corresponding technical API doc\n\nSamuel Lijin (6):\n t7300: clean -d should skip dirs with ignored files\n t7061: status --ignored should search untracked dirs\n dir: recurse into untracked dirs for ignored files\n dir: hide untracked contents of untracked dirs\n dir: expose cmp_name() and check_contains()\n clean: teach clean -d to skip dirs containing ignored files\n\n Documentation/technical/api-directory-listing.txt | 6 ++++\n builtin/clean.c | 38 +++++++++++++++++++-\n dir.c | 42 ++++++++++++++++++++---\n dir.h | 6 +++-\n t/t7061-wtstatus-ignore.sh | 1 +\n t/t7300-clean.sh | 16 +++++++++\n 6 files changed, 103 insertions(+), 6 deletions(-)\n\n-- \n2.13.0\n\n"},{"id":"320143","messageId":"20170518082154.28643-2-sxlijin@gmail.com","threadId":"45826","inReplyTo":"20170518082154.28643-1-sxlijin@gmail.com","subject":"[PATCH v4 1/6] t7300: clean -d should skip dirs with ignored files","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-18T08:21:49Z","receivedAt":"2017-05-18T08:22:22Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"If git sees a directory which contains only untracked and ignored\nfiles, clean -d should not remove that directory. It was recently\ndiscovered that this is *not* true of git clean -d, and it's possible\nthat this has never worked correctly; this test and its accompanying\npatch series aims to fix that.\n\nSigned-off-by: Samuel Lijin \n---\n t/t7300-clean.sh | 16 ++++++++++++++++\n 1 file changed, 16 insertions(+)\n\ndiff --git a/t/t7300-clean.sh b/t/t7300-clean.sh\nindex b89fd2a6a..3a2d709c2 100755\n--- a/t/t7300-clean.sh\n+++ b/t/t7300-clean.sh\n@@ -653,4 +653,20 @@ test_expect_success 'git clean -d respects pathspecs (pathspec is prefix of dir)\n \ttest_path_is_dir foobar\n '\n \n+test_expect_failure 'git clean -d skips untracked dirs containing ignored files' '\n+\techo /foo/bar >.gitignore &&\n+\techo ignoreme >>.gitignore &&\n+\trm -rf foo &&\n+\tmkdir -p foo/a/aa/aaa foo/b/bb/bbb &&\n+\ttouch foo/bar foo/baz foo/a/aa/ignoreme foo/b/ignoreme foo/b/bb/1 foo/b/bb/2 &&\n+\tgit clean -df &&\n+\ttest_path_is_dir foo &&\n+\ttest_path_is_file foo/bar &&\n+\ttest_path_is_missing foo/baz &&\n+\ttest_path_is_file foo/a/aa/ignoreme &&\n+\ttest_path_is_missing foo/a/aa/aaa &&\n+\ttest_path_is_file foo/b/ignoreme &&\n+\ttest_path_is_missing foo/b/bb\n+'\n+\n test_done\n-- \n2.13.0\n\n"},{"id":"320144","messageId":"20170518082154.28643-6-sxlijin@gmail.com","threadId":"45826","inReplyTo":"20170518082154.28643-1-sxlijin@gmail.com","subject":"[PATCH v4 5/6] dir: expose cmp_name() and check_contains()","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-18T08:21:53Z","receivedAt":"2017-05-18T08:22:23Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"We want to use cmp_name() and check_contains() (which both compare\n`struct dir_entry`s, the former in terms of the sort order, the latter\nin terms of whether one lexically contains another) outside of dir.c,\nso we have to (1) change their linkage and (2) rename them as\nappropriate for the global namespace. The second is achieved by\nrenaming cmp_name() to cmp_dir_entry() and check_contains() to\ncheck_dir_entry_contains().\n\nSigned-off-by: Samuel Lijin \n---\n dir.c | 11 ++++++-----\n dir.h | 3 +++\n 2 files changed, 9 insertions(+), 5 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex 4b0a99323..2c520f17a 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -1842,7 +1842,7 @@ static enum path_treatment read_directory_recursive(struct dir_struct *dir,\n \treturn dir_state;\n }\n \n-static int cmp_name(const void *p1, const void *p2)\n+int cmp_dir_entry(const void *p1, const void *p2)\n {\n \tconst struct dir_entry *e1 = *(const struct dir_entry **)p1;\n \tconst struct dir_entry *e2 = *(const struct dir_entry **)p2;\n@@ -1851,7 +1851,7 @@ static int cmp_name(const void *p1, const void *p2)\n }\n \n /* check if *out lexically strictly contains *in */\n-static int check_contains(const struct dir_entry *out, const struct dir_entry *in)\n+int check_dir_entry_contains(const struct dir_entry *out, const struct dir_entry *in)\n {\n \treturn (out->len < in->len) &&\n \t\t(out->name[out->len - 1] == '/') &&\n@@ -2071,8 +2071,8 @@ int read_directory(struct dir_struct *dir, const char *path,\n \t\tdir->untracked = NULL;\n \tif (!len || treat_leading_path(dir, path, len, pathspec))\n \t\tread_directory_recursive(dir, path, len, untracked, 0, pathspec);\n-\tQSORT(dir->entries, dir->nr, cmp_name);\n-\tQSORT(dir->ignored, dir->ignored_nr, cmp_name);\n+\tQSORT(dir->entries, dir->nr, cmp_dir_entry);\n+\tQSORT(dir->ignored, dir->ignored_nr, cmp_dir_entry);\n \n \t/* if DIR_SHOW_IGNORED_TOO, read_directory_recursive() will also pick\n \t * up untracked contents of untracked dirs; by default we discard these,\n@@ -2084,7 +2084,8 @@ int read_directory(struct dir_struct *dir, const char *path,\n \n \t\t/* remove from dir->entries untracked contents of untracked dirs */\n \t\tfor (i = j = 0; j < dir->nr; j++) {\n-\t\t\tif (i && check_contains(dir->entries[i - 1], dir->entries[j])) {\n+\t\t\tif (i &&\n+\t\t\t check_dir_entry_contains(dir->entries[i - 1], dir->entries[j])) {\n \t\t\t\tfree(dir->entries[j]);\n \t\t\t\tdir->entries[j] = NULL;\n \t\t\t} else {\ndiff --git a/dir.h b/dir.h\nindex 650e54bdf..edb5fda58 100644\n--- a/dir.h\n+++ b/dir.h\n@@ -327,6 +327,9 @@ static inline int dir_path_match(const struct dir_entry *ent,\n \t\t\t has_trailing_dir);\n }\n \n+int cmp_dir_entry(const void *p1, const void *p2);\n+int check_dir_entry_contains(const struct dir_entry *out, const struct dir_entry *in);\n+\n void untracked_cache_invalidate_path(struct index_state *, const char *);\n void untracked_cache_remove_from_index(struct index_state *, const char *);\n void untracked_cache_add_to_index(struct index_state *, const char *);\n-- \n2.13.0\n\n"},{"id":"320145","messageId":"20170518082154.28643-7-sxlijin@gmail.com","threadId":"45826","inReplyTo":"20170518082154.28643-1-sxlijin@gmail.com","subject":"[PATCH v4 6/6] clean: teach clean -d to skip dirs containing ignored files","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-18T08:21:54Z","receivedAt":"2017-05-18T08:22:26Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"There is an implicit assumption that a directory containing only\nuntracked and ignored files should itself be considered untracked. This\nmakes sense in use cases where we're asking if a directory should be\nadded to the git database, but not when we're asking if a directory can\nbe safely removed from the working tree; as a result, clean -d would\nassume that an \"untracked\" directory containing ignored files could be\ndeleted.\n\nTo get around this, we teach clean -d to collect ignored files and skip\nover so-called \"untracked\" directories if they contain any ignored\nfiles (while still removing the untracked contents of such dirs).\n\nThis also fixes the known breakage in t7300 since clean -d now skips\nuntracked directories containing ignored files.\n\nSigned-off-by: Samuel Lijin \n---\n builtin/clean.c | 38 +++++++++++++++++++++++++++++++++++++-\n t/t7300-clean.sh | 2 +-\n 2 files changed, 38 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/clean.c b/builtin/clean.c\nindex d861f836a..0d490d76e 100644\n--- a/builtin/clean.c\n+++ b/builtin/clean.c\n@@ -859,7 +859,7 @@ static void interactive_main_loop(void)\n \n int cmd_clean(int argc, const char **argv, const char *prefix)\n {\n-\tint i, res;\n+\tint i, j, res;\n \tint dry_run = 0, remove_directories = 0, quiet = 0, ignored = 0;\n \tint ignored_only = 0, config_set = 0, errors = 0, gone = 1;\n \tint rm_flags = REMOVE_DIR_KEEP_NESTED_GIT;\n@@ -911,6 +911,9 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \t\t\t\t \" refusing to clean\"));\n \t}\n \n+\tif (remove_directories)\n+\t\tdir.flags |= DIR_SHOW_IGNORED_TOO | DIR_KEEP_UNTRACKED_CONTENTS;\n+\n \tif (force > 1)\n \t\trm_flags = 0;\n \n@@ -932,12 +935,39 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \n \tfill_directory(&dir, &pathspec);\n \n+\tfor (j = i = 0; i < dir.nr;) {\n+\t\tfor (;\n+\t\t j < dir.ignored_nr &&\n+\t\t 0 <= cmp_dir_entry(&dir.entries[i], &dir.ignored[j]);\n+\t\t j++);\n+\n+\t\tif ((j < dir.ignored_nr) &&\n+\t\t\t\tcheck_dir_entry_contains(dir.entries[i], dir.ignored[j])) {\n+\t\t\t/* skip any dir.entries which contains a dir.ignored */\n+\t\t\tfree(dir.entries[i]);\n+\t\t\tdir.entries[i++] = NULL;\n+\t\t} else {\n+\t\t\t/* prune the contents of a dir.entries which will be removed */\n+\t\t\tstruct dir_entry *ent = dir.entries[i++];\n+\t\t\tfor (;\n+\t\t\t i < dir.nr &&\n+\t\t\t check_dir_entry_contains(ent, dir.entries[i]);\n+\t\t\t i++) {\n+\t\t\t\tfree(dir.entries[i]);\n+\t\t\t\tdir.entries[i] = NULL;\n+\t\t\t}\n+\t\t}\n+\t}\n+\n \tfor (i = 0; i < dir.nr; i++) {\n \t\tstruct dir_entry *ent = dir.entries[i];\n \t\tint matches = 0;\n \t\tstruct stat st;\n \t\tconst char *rel;\n \n+\t\tif (!ent)\n+\t\t\tcontinue;\n+\n \t\tif (!cache_name_is_other(ent->name, ent->len))\n \t\t\tcontinue;\n \n@@ -958,6 +988,12 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \t\tstring_list_append(&del_list, rel);\n \t}\n \n+\tfor (i = 0; i < dir.nr; i++)\n+\t\tfree(dir.entries[i]);\n+\n+\tfor (i = 0; i < dir.ignored_nr; i++)\n+\t\tfree(dir.ignored[i]);\n+\n \tif (interactive && del_list.nr > 0)\n \t\tinteractive_main_loop();\n \ndiff --git a/t/t7300-clean.sh b/t/t7300-clean.sh\nindex 3a2d709c2..7b36954d6 100755\n--- a/t/t7300-clean.sh\n+++ b/t/t7300-clean.sh\n@@ -653,7 +653,7 @@ test_expect_success 'git clean -d respects pathspecs (pathspec is prefix of dir)\n \ttest_path_is_dir foobar\n '\n \n-test_expect_failure 'git clean -d skips untracked dirs containing ignored files' '\n+test_expect_success 'git clean -d skips untracked dirs containing ignored files' '\n \techo /foo/bar >.gitignore &&\n \techo ignoreme >>.gitignore &&\n \trm -rf foo &&\n-- \n2.13.0\n\n"},{"id":"320146","messageId":"20170518082154.28643-4-sxlijin@gmail.com","threadId":"45826","inReplyTo":"20170518082154.28643-1-sxlijin@gmail.com","subject":"[PATCH v4 3/6] dir: recurse into untracked dirs for ignored files","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-18T08:21:51Z","receivedAt":"2017-05-18T08:22:38Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"We consider directories containing only untracked and ignored files to\nbe themselves untracked, which in the usual case means we don't have to\nsearch these directories. This is problematic when we want to collect\nignored files with DIR_SHOW_IGNORED_TOO, though, so we teach\nread_directory_recursive() to recurse into untracked directories to find\nthe ignored files they contain when DIR_SHOW_IGNORED_TOO is set. This\nhas the side effect of also collecting all untracked files in untracked\ndirectories as well.\n\nSigned-off-by: Samuel Lijin \n---\n dir.c | 5 ++++-\n 1 file changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/dir.c b/dir.c\nindex f451bfa48..68cf6e47c 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -1784,7 +1784,10 @@ static enum path_treatment read_directory_recursive(struct dir_struct *dir,\n \t\t\tdir_state = state;\n \n \t\t/* recurse into subdir if instructed by treat_path */\n-\t\tif (state == path_recurse) {\n+\t\tif ((state == path_recurse) ||\n+\t\t\t((state == path_untracked) &&\n+\t\t\t (dir->flags & DIR_SHOW_IGNORED_TOO) &&\n+\t\t\t (get_dtype(cdir.de, path.buf, path.len) == DT_DIR))) {\n \t\t\tstruct untracked_cache_dir *ud;\n \t\t\tud = lookup_untracked(dir->untracked, untracked,\n \t\t\t\t\t path.buf + baselen,\n-- \n2.13.0\n\n"},{"id":"320147","messageId":"20170518082154.28643-3-sxlijin@gmail.com","threadId":"45826","inReplyTo":"20170518082154.28643-1-sxlijin@gmail.com","subject":"[PATCH v4 2/6] t7061: status --ignored should search untracked dirs","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-18T08:21:50Z","receivedAt":"2017-05-18T08:22:39Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"Per eb8c5b87, `status --ignored` by design does not list ignored files\nif they are in a directory which contains only ignored and untracked\nfiles (which is itself considered to be untracked) without `-uall`. This\ndoes not make sense for `--ignored`, which claims to \"Show ignored files\nas well.\"\n\nThus we revisit eb8c5b87 and decide that for such directories, `status\n--ignored` will list the directory as untracked *and* list all ignored\nfiles within said directory even without `-uall`.\n\nSigned-off-by: Samuel Lijin \n---\n t/t7061-wtstatus-ignore.sh | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t7061-wtstatus-ignore.sh b/t/t7061-wtstatus-ignore.sh\nindex cdc0747bf..15e7592b6 100755\n--- a/t/t7061-wtstatus-ignore.sh\n+++ b/t/t7061-wtstatus-ignore.sh\n@@ -9,9 +9,10 @@ cat >expected <<\\EOF\n ?? actual\n ?? expected\n ?? untracked/\n+!! untracked/ignored\n EOF\n \n-test_expect_success 'status untracked directory with --ignored' '\n+test_expect_failure 'status untracked directory with --ignored' '\n \techo \"ignored\" >.gitignore &&\n \tmkdir untracked &&\n \t: >untracked/ignored &&\n@@ -20,7 +21,7 @@ test_expect_success 'status untracked directory with --ignored' '\n \ttest_cmp expected actual\n '\n \n-test_expect_success 'same with gitignore starting with BOM' '\n+test_expect_failure 'same with gitignore starting with BOM' '\n \tprintf \"\\357\\273\\277ignored\\n\" >.gitignore &&\n \tmkdir -p untracked &&\n \t: >untracked/ignored &&\n-- \n2.13.0\n\n"},{"id":"320148","messageId":"20170518082154.28643-5-sxlijin@gmail.com","threadId":"45826","inReplyTo":"20170518082154.28643-1-sxlijin@gmail.com","subject":"[PATCH v4 4/6] dir: hide untracked contents of untracked dirs","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-18T08:21:52Z","receivedAt":"2017-05-18T08:22:40Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"When we taught read_directory_recursive() to recurse into untracked\ndirectories in search of ignored files given DIR_SHOW_IGNORED_TOO, that\nhad the side effect of teaching it to collect the untracked contents of\nuntracked directories. It doesn't always make sense to return these,\nthough (we do need them for `clean -d`), so we introduce a flag\n(DIR_KEEP_UNTRACKED_CONTENTS) to control whether or not read_directory()\nstrips dir->entries of the untracked contents of untracked dirs.\n\nWe also introduce check_contains() to check if one dir_entry corresponds\nto a path which contains the path corresponding to another dir_entry.\n\nThis also fixes known breakages in t7061, since status --ignored now\nsearches untracked directories for ignored files.\n\nSigned-off-by: Samuel Lijin \n---\n Documentation/technical/api-directory-listing.txt | 6 +++++\n dir.c | 30 +++++++++++++++++++++++\n dir.h | 3 ++-\n t/t7061-wtstatus-ignore.sh | 4 +--\n 4 files changed, 40 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/technical/api-directory-listing.txt b/Documentation/technical/api-directory-listing.txt\nindex 7f8e78d91..6c77b4920 100644\n--- a/Documentation/technical/api-directory-listing.txt\n+++ b/Documentation/technical/api-directory-listing.txt\n@@ -33,6 +33,12 @@ The notable options are:\n \tSimilar to `DIR_SHOW_IGNORED`, but return ignored files in `ignored[]`\n \tin addition to untracked files in `entries[]`.\n \n+`DIR_KEEP_UNTRACKED_CONTENTS`:::\n+\n+\tOnly has meaning if `DIR_SHOW_IGNORED_TOO` is also set; if this is set, the\n+\tuntracked contents of untracked directories are also returned in\n+\t`entries[]`.\n+\n `DIR_COLLECT_IGNORED`:::\n \n \tSpecial mode for git-add. Return ignored files in `ignored[]` and\ndiff --git a/dir.c b/dir.c\nindex 68cf6e47c..4b0a99323 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -1850,6 +1850,14 @@ static int cmp_name(const void *p1, const void *p2)\n \treturn name_compare(e1->name, e1->len, e2->name, e2->len);\n }\n \n+/* check if *out lexically strictly contains *in */\n+static int check_contains(const struct dir_entry *out, const struct dir_entry *in)\n+{\n+\treturn (out->len < in->len) &&\n+\t\t(out->name[out->len - 1] == '/') &&\n+\t\t!memcmp(out->name, in->name, out->len);\n+}\n+\n static int treat_leading_path(struct dir_struct *dir,\n \t\t\t const char *path, int len,\n \t\t\t const struct pathspec *pathspec)\n@@ -2065,6 +2073,28 @@ int read_directory(struct dir_struct *dir, const char *path,\n \t\tread_directory_recursive(dir, path, len, untracked, 0, pathspec);\n \tQSORT(dir->entries, dir->nr, cmp_name);\n \tQSORT(dir->ignored, dir->ignored_nr, cmp_name);\n+\n+\t/* if DIR_SHOW_IGNORED_TOO, read_directory_recursive() will also pick\n+\t * up untracked contents of untracked dirs; by default we discard these,\n+\t * but given DIR_KEEP_UNTRACKED_CONTENTS we do not\n+\t */\n+\tif ((dir->flags & DIR_SHOW_IGNORED_TOO) &&\n+\t\t !(dir->flags & DIR_KEEP_UNTRACKED_CONTENTS)) {\n+\t\tint i, j;\n+\n+\t\t/* remove from dir->entries untracked contents of untracked dirs */\n+\t\tfor (i = j = 0; j < dir->nr; j++) {\n+\t\t\tif (i && check_contains(dir->entries[i - 1], dir->entries[j])) {\n+\t\t\t\tfree(dir->entries[j]);\n+\t\t\t\tdir->entries[j] = NULL;\n+\t\t\t} else {\n+\t\t\t\tdir->entries[i++] = dir->entries[j];\n+\t\t\t}\n+\t\t}\n+\n+\t\tdir->nr = i;\n+\t}\n+\n \tif (dir->untracked) {\n \t\tstatic struct trace_key trace_untracked_stats = TRACE_KEY_INIT(UNTRACKED_STATS);\n \t\ttrace_printf_key(&trace_untracked_stats,\ndiff --git a/dir.h b/dir.h\nindex bf23a470a..650e54bdf 100644\n--- a/dir.h\n+++ b/dir.h\n@@ -151,7 +151,8 @@ struct dir_struct {\n \t\tDIR_NO_GITLINKS = 1<<3,\n \t\tDIR_COLLECT_IGNORED = 1<<4,\n \t\tDIR_SHOW_IGNORED_TOO = 1<<5,\n-\t\tDIR_COLLECT_KILLED_ONLY = 1<<6\n+\t\tDIR_COLLECT_KILLED_ONLY = 1<<6,\n+\t\tDIR_KEEP_UNTRACKED_CONTENTS = 1<<7\n \t} flags;\n \tstruct dir_entry **entries;\n \tstruct dir_entry **ignored;\ndiff --git a/t/t7061-wtstatus-ignore.sh b/t/t7061-wtstatus-ignore.sh\nindex 15e7592b6..fc6013ba3 100755\n--- a/t/t7061-wtstatus-ignore.sh\n+++ b/t/t7061-wtstatus-ignore.sh\n@@ -12,7 +12,7 @@ cat >expected <<\\EOF\n !! untracked/ignored\n EOF\n \n-test_expect_failure 'status untracked directory with --ignored' '\n+test_expect_success 'status untracked directory with --ignored' '\n \techo \"ignored\" >.gitignore &&\n \tmkdir untracked &&\n \t: >untracked/ignored &&\n@@ -21,7 +21,7 @@ test_expect_failure 'status untracked directory with --ignored' '\n \ttest_cmp expected actual\n '\n \n-test_expect_failure 'same with gitignore starting with BOM' '\n+test_expect_success 'same with gitignore starting with BOM' '\n \tprintf \"\\357\\273\\277ignored\\n\" >.gitignore &&\n \tmkdir -p untracked &&\n \t: >untracked/ignored &&\n-- \n2.13.0\n\n"},{"id":"320383","messageId":"xmqq60gtraik.fsf@gitster.mtv.corp.google.com","threadId":"45826","inReplyTo":"20170518082154.28643-6-sxlijin@gmail.com","subject":"Re: [PATCH v4 5/6] dir: expose cmp_name() and check_contains()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-22T00:38:27Z","receivedAt":"2017-05-22T00:38:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Samuel Lijin writes:\n\n> We want to use cmp_name() and check_contains() (which both compare\n> `struct dir_entry`s, the former in terms of the sort order, the latter\n> in terms of whether one lexically contains another) outside of dir.c,\n> so we have to (1) change their linkage and (2) rename them as\n> appropriate for the global namespace. The second is achieved by\n> renaming cmp_name() to cmp_dir_entry() and check_contains() to\n> check_dir_entry_contains().\n>\n> Signed-off-by: Samuel Lijin \n> ---\n> dir.c | 11 ++++++-----\n> dir.h | 3 +++\n> 2 files changed, 9 insertions(+), 5 deletions(-)\n\nUp to this point in the series all looked sensible. I haven't\nlooked the last one carefully to form an opinion yet.\n\nThanks.\n"},{"id":"320392","messageId":"xmqq37bxpos8.fsf@gitster.mtv.corp.google.com","threadId":"45826","inReplyTo":"20170518082154.28643-5-sxlijin@gmail.com","subject":"Re: [PATCH v4 4/6] dir: hide untracked contents of untracked dirs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-22T03:13:11Z","receivedAt":"2017-05-22T03:13:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Samuel Lijin writes:\n\n> +\n> +\t/* if DIR_SHOW_IGNORED_TOO, read_directory_recursive() will also pick\n> +\t * up untracked contents of untracked dirs; by default we discard these,\n> +\t * but given DIR_KEEP_UNTRACKED_CONTENTS we do not\n> +\t */\n\nNo need to resend only to fix this, as I'll fix it up while queuing,\nbut for the next time, \n\n /*\n * Our multi-line comments have their opening slash-aster\n * and closing aster-slash on their own lines, like this.\n */\n\n"},{"id":"320394","messageId":"xmqqtw4do5tf.fsf@gitster.mtv.corp.google.com","threadId":"45826","inReplyTo":"20170518082154.28643-7-sxlijin@gmail.com","subject":"Re: [PATCH v4 6/6] clean: teach clean -d to skip dirs containing ignored files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-22T04:48:12Z","receivedAt":"2017-05-22T04:48:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Samuel Lijin writes:\n\n> +\tfor (j = i = 0; i < dir.nr;) {\n> +\t\tfor (;\n> +\t\t j < dir.ignored_nr &&\n> +\t\t 0 <= cmp_dir_entry(&dir.entries[i], &dir.ignored[j]);\n> +\t\t j++);\n> +\n> +\t\tif ((j < dir.ignored_nr) &&\n> +\t\t\t\tcheck_dir_entry_contains(dir.entries[i], dir.ignored[j])) {\n> +\t\t\t/* skip any dir.entries which contains a dir.ignored */\n> +\t\t\tfree(dir.entries[i]);\n> +\t\t\tdir.entries[i++] = NULL;\n> +\t\t} else {\n> +\t\t\t/* prune the contents of a dir.entries which will be removed */\n> +\t\t\tstruct dir_entry *ent = dir.entries[i++];\n> +\t\t\tfor (;\n> +\t\t\t i < dir.nr &&\n> +\t\t\t check_dir_entry_contains(ent, dir.entries[i]);\n> +\t\t\t i++) {\n> +\t\t\t\tfree(dir.entries[i]);\n> +\t\t\t\tdir.entries[i] = NULL;\n> +\t\t\t}\n> +\t\t}\n> +\t}\n\nThe second loop in the else clause is a bit tricky, and the comment\n\"which will be removed\" is not all that helpful to explain why the\nloop is there.\n\nBut I think the code is correct. Here is how I understood it.\n\n While looking at dir.entries[i], the code noticed that nothing\n in that directory is ignored. But entries in dir.entries[] that\n come later may be contained in dir.entries[i] and we just want\n to show the top-level untracked one (e.g. \"a/\" and \"a/b/\" were\n in entries[], there is nothing in \"a/\", so naturally there is\n nothing in \"a/b/\", but we do not want to bother showing\n both---showing \"a/\" alone saying \"the entire a/ is untracked\" is\n what we want).\n\nWe may want to have a test to ensure \"a/b/\" is indeed omitted in\nsuch a situation from the output, though.\n\nBy the way, instead of putting NULL, it may be easier to follow if\nyou used two pointers, src and dst, into dir.entries[], just like\nyou did in your latest version of [PATCH 4/6]. That way, you do not\nhave to change anything in the later loop that walks over elements\nin the dir.entries[] array. It would also help the logic easier to\nfollow if the above loop were its own helper function.\n\nPutting them all together, here is what I came up with that can be\nsquashed into your patch. I am undecided myself if this is easier\nto follow than your version, but it seems to pass your test ;-)\n\nThanks.\n\n builtin/clean.c | 70 ++++++++++++++++++++++++++++++++++-----------------------\n 1 file changed, 42 insertions(+), 28 deletions(-)\n\ndiff --git a/builtin/clean.c b/builtin/clean.c\nindex dd3308a447..c8712e7ac8 100644\n--- a/builtin/clean.c\n+++ b/builtin/clean.c\n@@ -851,9 +851,49 @@ static void interactive_main_loop(void)\n \t}\n }\n \n+static void simplify_untracked(struct dir_struct *dir)\n+{\n+\tint src, dst, ign;\n+\n+\tfor (src = dst = ign = 0; src < dir->nr; src++) {\n+\t\t/*\n+\t\t * Skip entries in ignored[] that cannot be inside\n+\t\t * entries[src]\n+\t\t */\n+\t\twhile (ign < dir->ignored_nr &&\n+\t\t 0 <= cmp_dir_entry(&dir->entries[src], &dir->ignored[ign]))\n+\t\t\tign++;\n+\n+\t\tif (dir->ignored_nr <= ign ||\n+\t\t !check_dir_entry_contains(dir->entries[src], dir->ignored[ign])) {\n+\t\t\t/*\n+\t\t\t * entries[src] does not contain an ignored\n+\t\t\t * path -- we need to keep it. But we do not\n+\t\t\t * want to show entries[] that are contained\n+\t\t\t * in entries[src].\n+\t\t\t */\n+\t\t\tstruct dir_entry *ent = dir->entries[src++];\n+\t\t\tdir->entries[dst++] = ent;\n+\t\t\twhile (src < dir->nr &&\n+\t\t\t check_dir_entry_contains(ent, dir->entries[src])) {\n+\t\t\t\tfree(dir->entries[src++]);\n+\t\t\t}\n+\t\t\t/* compensate for the outer loop's loop control */\n+\t\t\tsrc--;\n+\t\t} else {\n+\t\t\t/*\n+\t\t\t * entries[src] contains an ignored path --\n+\t\t\t * drop it.\n+\t\t\t */\n+\t\t\tfree(dir->entries[src]);\n+\t\t}\n+\t}\n+\tdir->nr = dst;\n+}\n+\n int cmd_clean(int argc, const char **argv, const char *prefix)\n {\n-\tint i, j, res;\n+\tint i, res;\n \tint dry_run = 0, remove_directories = 0, quiet = 0, ignored = 0;\n \tint ignored_only = 0, config_set = 0, errors = 0, gone = 1;\n \tint rm_flags = REMOVE_DIR_KEEP_NESTED_GIT;\n@@ -928,30 +968,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \t\t prefix, argv);\n \n \tfill_directory(&dir, &pathspec);\n-\n-\tfor (j = i = 0; i < dir.nr;) {\n-\t\tfor (;\n-\t\t j < dir.ignored_nr &&\n-\t\t 0 <= cmp_dir_entry(&dir.entries[i], &dir.ignored[j]);\n-\t\t j++);\n-\n-\t\tif ((j < dir.ignored_nr) &&\n-\t\t\t\tcheck_dir_entry_contains(dir.entries[i], dir.ignored[j])) {\n-\t\t\t/* skip any dir.entries which contains a dir.ignored */\n-\t\t\tfree(dir.entries[i]);\n-\t\t\tdir.entries[i++] = NULL;\n-\t\t} else {\n-\t\t\t/* prune the contents of a dir.entries which will be removed */\n-\t\t\tstruct dir_entry *ent = dir.entries[i++];\n-\t\t\tfor (;\n-\t\t\t i < dir.nr &&\n-\t\t\t check_dir_entry_contains(ent, dir.entries[i]);\n-\t\t\t i++) {\n-\t\t\t\tfree(dir.entries[i]);\n-\t\t\t\tdir.entries[i] = NULL;\n-\t\t\t}\n-\t\t}\n-\t}\n+\tsimplify_untracked(&dir);\n \n \tfor (i = 0; i < dir.nr; i++) {\n \t\tstruct dir_entry *ent = dir.entries[i];\n@@ -959,9 +976,6 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \t\tstruct stat st;\n \t\tconst char *rel;\n \n-\t\tif (!ent)\n-\t\t\tcontinue;\n-\n \t\tif (!cache_name_is_other(ent->name, ent->len))\n \t\t\tcontinue;\n \n"},{"id":"320395","messageId":"CAJZjrdX9BnuxY3tmpswG+yEdDm1+AR8rc5wKGZyVCMp-jP218A@mail.gmail.com","threadId":"45826","inReplyTo":"xmqqtw4do5tf.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v4 6/6] clean: teach clean -d to skip dirs containing ignored files","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-22T05:58:53Z","receivedAt":"2017-05-22T05:59:40Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"On Mon, May 22, 2017 at 12:48 AM, Junio C Hamano wrote:\n> Samuel Lijin writes:\n>\n>> + for (j = i = 0; i < dir.nr;) {\n>> + for (;\n>> + j < dir.ignored_nr &&\n>> + 0 <= cmp_dir_entry(&dir.entries[i], &dir.ignored[j]);\n>> + j++);\n>> +\n>> + if ((j < dir.ignored_nr) &&\n>> + check_dir_entry_contains(dir.entries[i], dir.ignored[j])) {\n>> + /* skip any dir.entries which contains a dir.ignored */\n>> + free(dir.entries[i]);\n>> + dir.entries[i++] = NULL;\n>> + } else {\n>> + /* prune the contents of a dir.entries which will be removed */\n>> + struct dir_entry *ent = dir.entries[i++];\n>> + for (;\n>> + i < dir.nr &&\n>> + check_dir_entry_contains(ent, dir.entries[i]);\n>> + i++) {\n>> + free(dir.entries[i]);\n>> + dir.entries[i] = NULL;\n>> + }\n>> + }\n>> + }\n>\n> The second loop in the else clause is a bit tricky, and the comment\n> \"which will be removed\" is not all that helpful to explain why the\n> loop is there.\n>\n> But I think the code is correct. Here is how I understood it.\n>\n> While looking at dir.entries[i], the code noticed that nothing\n> in that directory is ignored. But entries in dir.entries[] that\n> come later may be contained in dir.entries[i] and we just want\n> to show the top-level untracked one (e.g. \"a/\" and \"a/b/\" were\n> in entries[], there is nothing in \"a/\", so naturally there is\n> nothing in \"a/b/\", but we do not want to bother showing\n> both---showing \"a/\" alone saying \"the entire a/ is untracked\" is\n> what we want).\n\nYep, that's the gist of it.\n\nMore specifically: the contents of untracked dirs have to be picked up\nso that clean -d can distinguish between purely untracked dirs and\nuntracked dirs which also contain ignored files. In the case of a\npurely untracked dir, clean -d can remove the dir itself, and should\njust skip over all the dir's contents; in the case of a mixed\nuntracked dir, clean -d should not remove the dir itself, just the\nuntracked contents.\n\n> We may want to have a test to ensure \"a/b/\" is indeed omitted in\n> such a situation from the output, though.\n\nIs there a reason to ensure specifically a/b/ is tested, or are the\ncurrent tests, which do cover the a/b (i.e. where a/b is a file, not\nwhere a/b/ is a dir) case, insufficient for some reason?\n\n> By the way, instead of putting NULL, it may be easier to follow if\n> you used two pointers, src and dst, into dir.entries[], just like\n> you did in your latest version of [PATCH 4/6]. That way, you do not\n> have to change anything in the later loop that walks over elements\n> in the dir.entries[] array. It would also help the logic easier to\n> follow if the above loop were its own helper function.\n\nAgreed on the helper function. On the src-dst thing: I considered it,\nbut I figured another O(n) set of array moves was unnecessary. I guess\nthis is one of those cases where premature optimization doesn't make\nsense?\n\n> Putting them all together, here is what I came up with that can be\n> squashed into your patch. I am undecided myself if this is easier\n> to follow than your version, but it seems to pass your test ;-)\n>\n> Thanks.\n>\n> builtin/clean.c | 70 ++++++++++++++++++++++++++++++++++-----------------------\n> 1 file changed, 42 insertions(+), 28 deletions(-)\n>\n> diff --git a/builtin/clean.c b/builtin/clean.c\n> index dd3308a447..c8712e7ac8 100644\n> --- a/builtin/clean.c\n> +++ b/builtin/clean.c\n> @@ -851,9 +851,49 @@ static void interactive_main_loop(void)\n> }\n> }\n>\n> +static void simplify_untracked(struct dir_struct *dir)\n> +{\n> + int src, dst, ign;\n> +\n> + for (src = dst = ign = 0; src < dir->nr; src++) {\n> + /*\n> + * Skip entries in ignored[] that cannot be inside\n> + * entries[src]\n> + */\n> + while (ign < dir->ignored_nr &&\n> + 0 <= cmp_dir_entry(&dir->entries[src], &dir->ignored[ign]))\n> + ign++;\n> +\n> + if (dir->ignored_nr <= ign ||\n> + !check_dir_entry_contains(dir->entries[src], dir->ignored[ign])) {\n> + /*\n> + * entries[src] does not contain an ignored\n> + * path -- we need to keep it. But we do not\n> + * want to show entries[] that are contained\n> + * in entries[src].\n> + */\n> + struct dir_entry *ent = dir->entries[src++];\n> + dir->entries[dst++] = ent;\n> + while (src < dir->nr &&\n> + check_dir_entry_contains(ent, dir->entries[src])) {\n> + free(dir->entries[src++]);\n> + }\n> + /* compensate for the outer loop's loop control */\n> + src--;\n> + } else {\n> + /*\n> + * entries[src] contains an ignored path --\n> + * drop it.\n> + */\n> + free(dir->entries[src]);\n> + }\n> + }\n> + dir->nr = dst;\n> +}\n> +\n> int cmd_clean(int argc, const char **argv, const char *prefix)\n> {\n> - int i, j, res;\n> + int i, res;\n> int dry_run = 0, remove_directories = 0, quiet = 0, ignored = 0;\n> int ignored_only = 0, config_set = 0, errors = 0, gone = 1;\n> int rm_flags = REMOVE_DIR_KEEP_NESTED_GIT;\n> @@ -928,30 +968,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n> prefix, argv);\n>\n> fill_directory(&dir, &pathspec);\n> -\n> - for (j = i = 0; i < dir.nr;) {\n> - for (;\n> - j < dir.ignored_nr &&\n> - 0 <= cmp_dir_entry(&dir.entries[i], &dir.ignored[j]);\n> - j++);\n> -\n> - if ((j < dir.ignored_nr) &&\n> - check_dir_entry_contains(dir.entries[i], dir.ignored[j])) {\n> - /* skip any dir.entries which contains a dir.ignored */\n> - free(dir.entries[i]);\n> - dir.entries[i++] = NULL;\n> - } else {\n> - /* prune the contents of a dir.entries which will be removed */\n> - struct dir_entry *ent = dir.entries[i++];\n> - for (;\n> - i < dir.nr &&\n> - check_dir_entry_contains(ent, dir.entries[i]);\n> - i++) {\n> - free(dir.entries[i]);\n> - dir.entries[i] = NULL;\n> - }\n> - }\n> - }\n> + simplify_untracked(&dir);\n>\n> for (i = 0; i < dir.nr; i++) {\n> struct dir_entry *ent = dir.entries[i];\n> @@ -959,9 +976,6 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n> struct stat st;\n> const char *rel;\n>\n> - if (!ent)\n> - continue;\n> -\n> if (!cache_name_is_other(ent->name, ent->len))\n> continue;\n>\n"},{"id":"320397","messageId":"xmqqk259o1o2.fsf@gitster.mtv.corp.google.com","threadId":"45826","inReplyTo":"CAJZjrdX9BnuxY3tmpswG+yEdDm1+AR8rc5wKGZyVCMp-jP218A@mail.gmail.com","subject":"Re: [PATCH v4 6/6] clean: teach clean -d to skip dirs containing ignored files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-22T06:17:49Z","receivedAt":"2017-05-22T06:17:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Samuel Lijin writes:\n\n>> By the way, instead of putting NULL, it may be easier to follow if\n>> you used two pointers, src and dst, into dir.entries[], just like\n>> you did in your latest version of [PATCH 4/6]. That way, you do not\n>> have to change anything in the later loop that walks over elements\n>> in the dir.entries[] array. It would also help the logic easier to\n>> follow if the above loop were its own helper function.\n>\n> Agreed on the helper function. On the src-dst thing: I considered it,\n> but I figured another O(n) set of array moves was unnecessary. I guess\n> this is one of those cases where premature optimization doesn't make\n> sense?\n\nI actually did not mean to give the variables more descriptive names\nand preserve the original 'main loop' (namely, not adding the \"skip\nif NULL\" which would never happen in normal case where \"-d\" is not\nused without \"-x\") as \"optimization\", whether it is premature or\nnot. My suggestions were purely from \"wouldn't the resulting code\neasier to follow and understand, leading to fewer bugs in the\nfuture?\" point of view.\n\nAs I said, I am undecided if the result is easier to follow than\nyour version ;-)\n"},{"id":"320512","messageId":"CAJZjrdVeGy6mgsjuHL+O29xys8z90J8aKXdZ5XqiNraNZ9pQfg@mail.gmail.com","threadId":"45826","inReplyTo":"xmqqk259o1o2.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v4 6/6] clean: teach clean -d to skip dirs containing ignored files","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-23T02:43:34Z","receivedAt":"2017-05-23T02:44:20Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"On Mon, May 22, 2017 at 2:17 AM, Junio C Hamano wrote:\n> Samuel Lijin writes:\n>\n>>> By the way, instead of putting NULL, it may be easier to follow if\n>>> you used two pointers, src and dst, into dir.entries[], just like\n>>> you did in your latest version of [PATCH 4/6]. That way, you do not\n>>> have to change anything in the later loop that walks over elements\n>>> in the dir.entries[] array. It would also help the logic easier to\n>>> follow if the above loop were its own helper function.\n>>\n>> Agreed on the helper function. On the src-dst thing: I considered it,\n>> but I figured another O(n) set of array moves was unnecessary. I guess\n>> this is one of those cases where premature optimization doesn't make\n>> sense?\n>\n> I actually did not mean to give the variables more descriptive names\n> and preserve the original 'main loop' (namely, not adding the \"skip\n> if NULL\" which would never happen in normal case where \"-d\" is not\n> used without \"-x\") as \"optimization\", whether it is premature or\n> not. My suggestions were purely from \"wouldn't the resulting code\n> easier to follow and understand, leading to fewer bugs in the\n> future?\" point of view.\n>\n> As I said, I am undecided if the result is easier to follow than\n> your version ;-)\n\nI think I'll defer to your patch: I do agree that your version is\neasier to follow and understand. Should I reroll just this patch and\nits commit message, or would you prefer to handle that in the queuing\nyourself?\n"},{"id":"320530","messageId":"xmqq1srgko52.fsf@gitster.mtv.corp.google.com","threadId":"45826","inReplyTo":"CAJZjrdVeGy6mgsjuHL+O29xys8z90J8aKXdZ5XqiNraNZ9pQfg@mail.gmail.com","subject":"Re: [PATCH v4 6/6] clean: teach clean -d to skip dirs containing ignored files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-23T07:50:33Z","receivedAt":"2017-05-23T07:50:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Samuel Lijin writes:\n\n>> As I said, I am undecided if the result is easier to follow than\n>> your version ;-)\n>\n> I think I'll defer to your patch: I do agree that your version is\n> easier to follow and understand. Should I reroll just this patch and\n> its commit message, or would you prefer to handle that in the queuing\n> yourself?\n\nI was going thru the entries in \"What's cooking\" draft and was\nwondering what the final outcome would be. A v5 that I can just\napply without thinking would probably be easier for me (I can tend\nto other topics while waiting your final version).\n\nThanks.\n"},{"id":"320537","messageId":"20170523091829.1746-1-sxlijin@gmail.com","threadId":"45826","inReplyTo":"20170518082154.28643-1-sxlijin@gmail.com","subject":"[PATCH v5 0/6] Fix clean -d and status --ignored","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-23T09:18:23Z","receivedAt":"2017-05-23T09:18:42Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"Incorporates latest round of feedback from Junio about how to best structure\nthe changes to cmd_clean() for maintainability.\n\nSamuel Lijin (6):\n t7300: clean -d should skip dirs with ignored files\n t7061: status --ignored should search untracked dirs\n dir: recurse into untracked dirs for ignored files\n dir: hide untracked contents of untracked dirs\n dir: expose cmp_name() and check_contains()\n clean: teach clean -d to preserve ignored paths\n\n Documentation/technical/api-directory-listing.txt | 6 ++++\n builtin/clean.c | 31 ++++++++++++++++\n dir.c | 43 ++++++++++++++++++++---\n dir.h | 6 +++-\n t/t7061-wtstatus-ignore.sh | 1 +\n t/t7300-clean.sh | 16 +++++++++\n 6 files changed, 98 insertions(+), 5 deletions(-)\n\n-- \n2.13.0\n\n"},{"id":"320538","messageId":"20170523091829.1746-2-sxlijin@gmail.com","threadId":"45826","inReplyTo":"20170523091829.1746-1-sxlijin@gmail.com","subject":"[PATCH v5 1/6] t7300: clean -d should skip dirs with ignored files","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-23T09:18:24Z","receivedAt":"2017-05-23T09:18:45Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"If git sees a directory which contains only untracked and ignored\nfiles, clean -d should not remove that directory. It was recently\ndiscovered that this is *not* true of git clean -d, and it's possible\nthat this has never worked correctly; this test and its accompanying\npatch series aims to fix that.\n\nSigned-off-by: Samuel Lijin \n---\n t/t7300-clean.sh | 16 ++++++++++++++++\n 1 file changed, 16 insertions(+)\n\ndiff --git a/t/t7300-clean.sh b/t/t7300-clean.sh\nindex b89fd2a6a..3a2d709c2 100755\n--- a/t/t7300-clean.sh\n+++ b/t/t7300-clean.sh\n@@ -653,4 +653,20 @@ test_expect_success 'git clean -d respects pathspecs (pathspec is prefix of dir)\n \ttest_path_is_dir foobar\n '\n \n+test_expect_failure 'git clean -d skips untracked dirs containing ignored files' '\n+\techo /foo/bar >.gitignore &&\n+\techo ignoreme >>.gitignore &&\n+\trm -rf foo &&\n+\tmkdir -p foo/a/aa/aaa foo/b/bb/bbb &&\n+\ttouch foo/bar foo/baz foo/a/aa/ignoreme foo/b/ignoreme foo/b/bb/1 foo/b/bb/2 &&\n+\tgit clean -df &&\n+\ttest_path_is_dir foo &&\n+\ttest_path_is_file foo/bar &&\n+\ttest_path_is_missing foo/baz &&\n+\ttest_path_is_file foo/a/aa/ignoreme &&\n+\ttest_path_is_missing foo/a/aa/aaa &&\n+\ttest_path_is_file foo/b/ignoreme &&\n+\ttest_path_is_missing foo/b/bb\n+'\n+\n test_done\n-- \n2.13.0\n\n"},{"id":"320539","messageId":"20170523091829.1746-3-sxlijin@gmail.com","threadId":"45826","inReplyTo":"20170523091829.1746-1-sxlijin@gmail.com","subject":"[PATCH v5 2/6] t7061: status --ignored should search untracked dirs","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-23T09:18:25Z","receivedAt":"2017-05-23T09:18:46Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"Per eb8c5b87, `status --ignored` by design does not list ignored files\nif they are in a directory which contains only ignored and untracked\nfiles (which is itself considered to be untracked) without `-uall`. This\ndoes not make sense for `--ignored`, which claims to \"Show ignored files\nas well.\"\n\nThus we revisit eb8c5b87 and decide that for such directories, `status\n--ignored` will list the directory as untracked *and* list all ignored\nfiles within said directory even without `-uall`.\n\nSigned-off-by: Samuel Lijin \n---\n t/t7061-wtstatus-ignore.sh | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t7061-wtstatus-ignore.sh b/t/t7061-wtstatus-ignore.sh\nindex cdc0747bf..15e7592b6 100755\n--- a/t/t7061-wtstatus-ignore.sh\n+++ b/t/t7061-wtstatus-ignore.sh\n@@ -9,9 +9,10 @@ cat >expected <<\\EOF\n ?? actual\n ?? expected\n ?? untracked/\n+!! untracked/ignored\n EOF\n \n-test_expect_success 'status untracked directory with --ignored' '\n+test_expect_failure 'status untracked directory with --ignored' '\n \techo \"ignored\" >.gitignore &&\n \tmkdir untracked &&\n \t: >untracked/ignored &&\n@@ -20,7 +21,7 @@ test_expect_success 'status untracked directory with --ignored' '\n \ttest_cmp expected actual\n '\n \n-test_expect_success 'same with gitignore starting with BOM' '\n+test_expect_failure 'same with gitignore starting with BOM' '\n \tprintf \"\\357\\273\\277ignored\\n\" >.gitignore &&\n \tmkdir -p untracked &&\n \t: >untracked/ignored &&\n-- \n2.13.0\n\n"},{"id":"320540","messageId":"20170523091829.1746-4-sxlijin@gmail.com","threadId":"45826","inReplyTo":"20170523091829.1746-1-sxlijin@gmail.com","subject":"[PATCH v5 3/6] dir: recurse into untracked dirs for ignored files","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-23T09:18:26Z","receivedAt":"2017-05-23T09:18:49Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"We consider directories containing only untracked and ignored files to\nbe themselves untracked, which in the usual case means we don't have to\nsearch these directories. This is problematic when we want to collect\nignored files with DIR_SHOW_IGNORED_TOO, though, so we teach\nread_directory_recursive() to recurse into untracked directories to find\nthe ignored files they contain when DIR_SHOW_IGNORED_TOO is set. This\nhas the side effect of also collecting all untracked files in untracked\ndirectories as well.\n\nSigned-off-by: Samuel Lijin \n---\n dir.c | 5 ++++-\n 1 file changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/dir.c b/dir.c\nindex f451bfa48..68cf6e47c 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -1784,7 +1784,10 @@ static enum path_treatment read_directory_recursive(struct dir_struct *dir,\n \t\t\tdir_state = state;\n \n \t\t/* recurse into subdir if instructed by treat_path */\n-\t\tif (state == path_recurse) {\n+\t\tif ((state == path_recurse) ||\n+\t\t\t((state == path_untracked) &&\n+\t\t\t (dir->flags & DIR_SHOW_IGNORED_TOO) &&\n+\t\t\t (get_dtype(cdir.de, path.buf, path.len) == DT_DIR))) {\n \t\t\tstruct untracked_cache_dir *ud;\n \t\t\tud = lookup_untracked(dir->untracked, untracked,\n \t\t\t\t\t path.buf + baselen,\n-- \n2.13.0\n\n"},{"id":"320541","messageId":"20170523091829.1746-6-sxlijin@gmail.com","threadId":"45826","inReplyTo":"20170523091829.1746-1-sxlijin@gmail.com","subject":"[PATCH v5 5/6] dir: expose cmp_name() and check_contains()","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-23T09:18:28Z","receivedAt":"2017-05-23T09:18:55Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"We want to use cmp_name() and check_contains() (which both compare\n`struct dir_entry`s, the former in terms of the sort order, the latter\nin terms of whether one lexically contains another) outside of dir.c,\nso we have to (1) change their linkage and (2) rename them as\nappropriate for the global namespace. The second is achieved by\nrenaming cmp_name() to cmp_dir_entry() and check_contains() to\ncheck_dir_entry_contains().\n\nSigned-off-by: Samuel Lijin \n---\n dir.c | 11 ++++++-----\n dir.h | 3 +++\n 2 files changed, 9 insertions(+), 5 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex ba5eadeda..aae6d00b4 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -1842,7 +1842,7 @@ static enum path_treatment read_directory_recursive(struct dir_struct *dir,\n \treturn dir_state;\n }\n \n-static int cmp_name(const void *p1, const void *p2)\n+int cmp_dir_entry(const void *p1, const void *p2)\n {\n \tconst struct dir_entry *e1 = *(const struct dir_entry **)p1;\n \tconst struct dir_entry *e2 = *(const struct dir_entry **)p2;\n@@ -1851,7 +1851,7 @@ static int cmp_name(const void *p1, const void *p2)\n }\n \n /* check if *out lexically strictly contains *in */\n-static int check_contains(const struct dir_entry *out, const struct dir_entry *in)\n+int check_dir_entry_contains(const struct dir_entry *out, const struct dir_entry *in)\n {\n \treturn (out->len < in->len) &&\n \t\t(out->name[out->len - 1] == '/') &&\n@@ -2071,8 +2071,8 @@ int read_directory(struct dir_struct *dir, const char *path,\n \t\tdir->untracked = NULL;\n \tif (!len || treat_leading_path(dir, path, len, pathspec))\n \t\tread_directory_recursive(dir, path, len, untracked, 0, pathspec);\n-\tQSORT(dir->entries, dir->nr, cmp_name);\n-\tQSORT(dir->ignored, dir->ignored_nr, cmp_name);\n+\tQSORT(dir->entries, dir->nr, cmp_dir_entry);\n+\tQSORT(dir->ignored, dir->ignored_nr, cmp_dir_entry);\n \n \t/* \n \t * if DIR_SHOW_IGNORED_TOO, read_directory_recursive() will also pick\n@@ -2085,7 +2085,8 @@ int read_directory(struct dir_struct *dir, const char *path,\n \n \t\t/* remove from dir->entries untracked contents of untracked dirs */\n \t\tfor (i = j = 0; j < dir->nr; j++) {\n-\t\t\tif (i && check_contains(dir->entries[i - 1], dir->entries[j])) {\n+\t\t\tif (i &&\n+\t\t\t check_dir_entry_contains(dir->entries[i - 1], dir->entries[j])) {\n \t\t\t\tfree(dir->entries[j]);\n \t\t\t\tdir->entries[j] = NULL;\n \t\t\t} else {\ndiff --git a/dir.h b/dir.h\nindex 650e54bdf..edb5fda58 100644\n--- a/dir.h\n+++ b/dir.h\n@@ -327,6 +327,9 @@ static inline int dir_path_match(const struct dir_entry *ent,\n \t\t\t has_trailing_dir);\n }\n \n+int cmp_dir_entry(const void *p1, const void *p2);\n+int check_dir_entry_contains(const struct dir_entry *out, const struct dir_entry *in);\n+\n void untracked_cache_invalidate_path(struct index_state *, const char *);\n void untracked_cache_remove_from_index(struct index_state *, const char *);\n void untracked_cache_add_to_index(struct index_state *, const char *);\n-- \n2.13.0\n\n"},{"id":"320542","messageId":"20170523091829.1746-5-sxlijin@gmail.com","threadId":"45826","inReplyTo":"20170523091829.1746-1-sxlijin@gmail.com","subject":"[PATCH v5 4/6] dir: hide untracked contents of untracked dirs","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-23T09:18:27Z","receivedAt":"2017-05-23T09:18:57Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"When we taught read_directory_recursive() to recurse into untracked\ndirectories in search of ignored files given DIR_SHOW_IGNORED_TOO, that\nhad the side effect of teaching it to collect the untracked contents of\nuntracked directories. It doesn't always make sense to return these,\nthough (we do need them for `clean -d`), so we introduce a flag\n(DIR_KEEP_UNTRACKED_CONTENTS) to control whether or not read_directory()\nstrips dir->entries of the untracked contents of untracked dirs.\n\nWe also introduce check_contains() to check if one dir_entry corresponds\nto a path which contains the path corresponding to another dir_entry.\n\nThis also fixes known breakages in t7061, since status --ignored now\nsearches untracked directories for ignored files.\n\nSigned-off-by: Samuel Lijin \n---\n Documentation/technical/api-directory-listing.txt | 6 +++++\n dir.c | 31 +++++++++++++++++++++++\n dir.h | 3 ++-\n t/t7061-wtstatus-ignore.sh | 4 +--\n 4 files changed, 41 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/technical/api-directory-listing.txt b/Documentation/technical/api-directory-listing.txt\nindex 7f8e78d91..6c77b4920 100644\n--- a/Documentation/technical/api-directory-listing.txt\n+++ b/Documentation/technical/api-directory-listing.txt\n@@ -33,6 +33,12 @@ The notable options are:\n \tSimilar to `DIR_SHOW_IGNORED`, but return ignored files in `ignored[]`\n \tin addition to untracked files in `entries[]`.\n \n+`DIR_KEEP_UNTRACKED_CONTENTS`:::\n+\n+\tOnly has meaning if `DIR_SHOW_IGNORED_TOO` is also set; if this is set, the\n+\tuntracked contents of untracked directories are also returned in\n+\t`entries[]`.\n+\n `DIR_COLLECT_IGNORED`:::\n \n \tSpecial mode for git-add. Return ignored files in `ignored[]` and\ndiff --git a/dir.c b/dir.c\nindex 68cf6e47c..ba5eadeda 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -1850,6 +1850,14 @@ static int cmp_name(const void *p1, const void *p2)\n \treturn name_compare(e1->name, e1->len, e2->name, e2->len);\n }\n \n+/* check if *out lexically strictly contains *in */\n+static int check_contains(const struct dir_entry *out, const struct dir_entry *in)\n+{\n+\treturn (out->len < in->len) &&\n+\t\t(out->name[out->len - 1] == '/') &&\n+\t\t!memcmp(out->name, in->name, out->len);\n+}\n+\n static int treat_leading_path(struct dir_struct *dir,\n \t\t\t const char *path, int len,\n \t\t\t const struct pathspec *pathspec)\n@@ -2065,6 +2073,29 @@ int read_directory(struct dir_struct *dir, const char *path,\n \t\tread_directory_recursive(dir, path, len, untracked, 0, pathspec);\n \tQSORT(dir->entries, dir->nr, cmp_name);\n \tQSORT(dir->ignored, dir->ignored_nr, cmp_name);\n+\n+\t/* \n+\t * if DIR_SHOW_IGNORED_TOO, read_directory_recursive() will also pick\n+\t * up untracked contents of untracked dirs; by default we discard these,\n+\t * but given DIR_KEEP_UNTRACKED_CONTENTS we do not\n+\t */\n+\tif ((dir->flags & DIR_SHOW_IGNORED_TOO) &&\n+\t\t !(dir->flags & DIR_KEEP_UNTRACKED_CONTENTS)) {\n+\t\tint i, j;\n+\n+\t\t/* remove from dir->entries untracked contents of untracked dirs */\n+\t\tfor (i = j = 0; j < dir->nr; j++) {\n+\t\t\tif (i && check_contains(dir->entries[i - 1], dir->entries[j])) {\n+\t\t\t\tfree(dir->entries[j]);\n+\t\t\t\tdir->entries[j] = NULL;\n+\t\t\t} else {\n+\t\t\t\tdir->entries[i++] = dir->entries[j];\n+\t\t\t}\n+\t\t}\n+\n+\t\tdir->nr = i;\n+\t}\n+\n \tif (dir->untracked) {\n \t\tstatic struct trace_key trace_untracked_stats = TRACE_KEY_INIT(UNTRACKED_STATS);\n \t\ttrace_printf_key(&trace_untracked_stats,\ndiff --git a/dir.h b/dir.h\nindex bf23a470a..650e54bdf 100644\n--- a/dir.h\n+++ b/dir.h\n@@ -151,7 +151,8 @@ struct dir_struct {\n \t\tDIR_NO_GITLINKS = 1<<3,\n \t\tDIR_COLLECT_IGNORED = 1<<4,\n \t\tDIR_SHOW_IGNORED_TOO = 1<<5,\n-\t\tDIR_COLLECT_KILLED_ONLY = 1<<6\n+\t\tDIR_COLLECT_KILLED_ONLY = 1<<6,\n+\t\tDIR_KEEP_UNTRACKED_CONTENTS = 1<<7\n \t} flags;\n \tstruct dir_entry **entries;\n \tstruct dir_entry **ignored;\ndiff --git a/t/t7061-wtstatus-ignore.sh b/t/t7061-wtstatus-ignore.sh\nindex 15e7592b6..fc6013ba3 100755\n--- a/t/t7061-wtstatus-ignore.sh\n+++ b/t/t7061-wtstatus-ignore.sh\n@@ -12,7 +12,7 @@ cat >expected <<\\EOF\n !! untracked/ignored\n EOF\n \n-test_expect_failure 'status untracked directory with --ignored' '\n+test_expect_success 'status untracked directory with --ignored' '\n \techo \"ignored\" >.gitignore &&\n \tmkdir untracked &&\n \t: >untracked/ignored &&\n@@ -21,7 +21,7 @@ test_expect_failure 'status untracked directory with --ignored' '\n \ttest_cmp expected actual\n '\n \n-test_expect_failure 'same with gitignore starting with BOM' '\n+test_expect_success 'same with gitignore starting with BOM' '\n \tprintf \"\\357\\273\\277ignored\\n\" >.gitignore &&\n \tmkdir -p untracked &&\n \t: >untracked/ignored &&\n-- \n2.13.0\n\n"},{"id":"320543","messageId":"20170523091829.1746-7-sxlijin@gmail.com","threadId":"45826","inReplyTo":"20170523091829.1746-1-sxlijin@gmail.com","subject":"[PATCH v5 6/6] clean: teach clean -d to preserve ignored paths","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-23T09:18:29Z","receivedAt":"2017-05-23T09:19:02Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"There is an implicit assumption that a directory containing only\nuntracked and ignored paths should itself be considered untracked. This\nmakes sense in use cases where we're asking if a directory should be\nadded to the git database, but not when we're asking if a directory can\nbe safely removed from the working tree; as a result, clean -d would\nassume that an \"untracked\" directory containing ignored paths could be\ndeleted, even though doing so would also remove the ignored paths.\n\nTo get around this, we teach clean -d to collect ignored paths and skip\nan untracked directory if it contained an ignored path, instead just\nremoving the untracked contents thereof. To achieve this, cmd_clean()\nhas to collect all untracked contents of untracked directories, in\naddition to all ignored paths, to determine which untracked dirs must be\nskipped (because they contain ignored paths) and which ones should *not*\nbe skipped.\n\nFor this purpose, correct_untracked_entries() is introduced to prune a\ngiven dir_struct of untracked entries containing ignored paths and those\nuntracked entries encompassed by the untracked entries which are not\npruned away.\n\nThis also fixes the known breakage in t7300, since clean -d now skips\nuntracked directories containing ignored paths.\n\nSigned-off-by: Samuel Lijin \n---\n builtin/clean.c | 31 +++++++++++++++++++++++++++++++\n t/t7300-clean.sh | 2 +-\n 2 files changed, 32 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/clean.c b/builtin/clean.c\nindex d861f836a..45dbdcd18 100644\n--- a/builtin/clean.c\n+++ b/builtin/clean.c\n@@ -857,6 +857,36 @@ static void interactive_main_loop(void)\n \t}\n }\n \n+static void correct_untracked_entries(struct dir_struct *dir)\n+{\n+\tint src, dst, ign;\n+\n+\tfor (src = dst = ign = 0; src < dir->nr; src++) {\n+\t\t/* skip paths in ignored[] that cannot be inside entries[src] */\n+\t\twhile (ign < dir->ignored_nr &&\n+\t\t 0 <= cmp_dir_entry(&dir->entries[src], &dir->ignored[ign]))\n+\t\t\tign++;\n+\n+\t\tif (ign < dir->ignored_nr &&\n+\t\t check_dir_entry_contains(dir->entries[src], dir->ignored[ign])) {\n+\t\t\t/* entries[src] contains an ignored path, so we drop it */\n+\t\t\tfree(dir->entries[src]);\n+\t\t} else {\n+\t\t\t/* entries[src] does not contain an ignored path, so we keep it */\n+\t\t\tdir->entries[dst++] = dir->entries[src++];\n+\n+\t\t\t/* then discard paths in entries[] contained inside entries[src] */\n+\t\t\twhile (src < dir->nr &&\n+\t\t\t check_dir_entry_contains(ent, dir->entries[src]))\n+\t\t\t\tfree(dir->entries[src++]);\n+\n+\t\t\t/* compensate for the outer loop's loop control */\n+\t\t\tsrc--;\n+\t\t}\n+\t}\n+\tdir->nr = dst;\n+}\n+\n int cmd_clean(int argc, const char **argv, const char *prefix)\n {\n \tint i, res;\n@@ -931,6 +961,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \t\t prefix, argv);\n \n \tfill_directory(&dir, &pathspec);\n+\tcorrect_untracked_entries(&dir);\n \n \tfor (i = 0; i < dir.nr; i++) {\n \t\tstruct dir_entry *ent = dir.entries[i];\ndiff --git a/t/t7300-clean.sh b/t/t7300-clean.sh\nindex 3a2d709c2..7b36954d6 100755\n--- a/t/t7300-clean.sh\n+++ b/t/t7300-clean.sh\n@@ -653,7 +653,7 @@ test_expect_success 'git clean -d respects pathspecs (pathspec is prefix of dir)\n \ttest_path_is_dir foobar\n '\n \n-test_expect_failure 'git clean -d skips untracked dirs containing ignored files' '\n+test_expect_success 'git clean -d skips untracked dirs containing ignored files' '\n \techo /foo/bar >.gitignore &&\n \techo ignoreme >>.gitignore &&\n \trm -rf foo &&\n-- \n2.13.0\n\n"},{"id":"320556","messageId":"xmqqshjvka5x.fsf@gitster.mtv.corp.google.com","threadId":"45826","inReplyTo":"20170523091829.1746-7-sxlijin@gmail.com","subject":"Re: [PATCH v5 6/6] clean: teach clean -d to preserve ignored paths","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-23T12:52:26Z","receivedAt":"2017-05-23T12:52:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Samuel Lijin writes:\n\n> @@ -931,6 +961,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n> \t\t prefix, argv);\n> \n> \tfill_directory(&dir, &pathspec);\n> +\tcorrect_untracked_entries(&dir);\n> \n> \tfor (i = 0; i < dir.nr; i++) {\n> \t\tstruct dir_entry *ent = dir.entries[i];\n\nYou used to set SHOW_IGNORED_TOO and KEEP_UNTRACKED_CONTENTS in\ndir.flags early in the function, and then free dir.entries[] and\ndir.ignored[] after we are done. They are gone in this version.\n\nIntended?\n\n> diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh\n> index 3a2d709c2..7b36954d6 100755\n> --- a/t/t7300-clean.sh\n> +++ b/t/t7300-clean.sh\n> @@ -653,7 +653,7 @@ test_expect_success 'git clean -d respects pathspecs (pathspec is prefix of dir)\n> \ttest_path_is_dir foobar\n> '\n> \n> -test_expect_failure 'git clean -d skips untracked dirs containing ignored files' '\n> +test_expect_success 'git clean -d skips untracked dirs containing ignored files' '\n> \techo /foo/bar >.gitignore &&\n> \techo ignoreme >>.gitignore &&\n> \trm -rf foo &&\n"},{"id":"320574","messageId":"CAJZjrdWgthKzACE6=tpYOHwXr-zGO2SyX=W6Hbnxw7kk1cQn0Q@mail.gmail.com","threadId":"45826","inReplyTo":"xmqqshjvka5x.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v5 6/6] clean: teach clean -d to preserve ignored paths","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-23T19:16:59Z","receivedAt":"2017-05-23T19:17:45Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"On Tue, May 23, 2017 at 8:52 AM, Junio C Hamano wrote:\n> Samuel Lijin writes:\n>\n>> @@ -931,6 +961,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n>> prefix, argv);\n>>\n>> fill_directory(&dir, &pathspec);\n>> + correct_untracked_entries(&dir);\n>>\n>> for (i = 0; i < dir.nr; i++) {\n>> struct dir_entry *ent = dir.entries[i];\n>\n> You used to set SHOW_IGNORED_TOO and KEEP_UNTRACKED_CONTENTS in\n> dir.flags early in the function, and then free dir.entries[] and\n> dir.ignored[] after we are done. They are gone in this version.\n>\n> Intended?\n\nEmbarrassingly, no. Rerolling.\n\n>> diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh\n>> index 3a2d709c2..7b36954d6 100755\n>> --- a/t/t7300-clean.sh\n>> +++ b/t/t7300-clean.sh\n>> @@ -653,7 +653,7 @@ test_expect_success 'git clean -d respects pathspecs (pathspec is prefix of dir)\n>> test_path_is_dir foobar\n>> '\n>>\n>> -test_expect_failure 'git clean -d skips untracked dirs containing ignored files' '\n>> +test_expect_success 'git clean -d skips untracked dirs containing ignored files' '\n>> echo /foo/bar >.gitignore &&\n>> echo ignoreme >>.gitignore &&\n>> rm -rf foo &&\n"},{"id":"320583","messageId":"20170523100937.8752-1-sxlijin@gmail.com","threadId":"45826","inReplyTo":"20170523091829.1746-1-sxlijin@gmail.com","subject":"[PATCH v6 0/6] Fix clean -d and status --ignored","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-23T10:09:31Z","receivedAt":"2017-05-23T19:32:02Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"Messed up on 6/6 in v5, forgot to include changes from earlier versions (karma\nfor not running tests before I send-email'd the patch series).\n\nSamuel Lijin (6):\n t7300: clean -d should skip dirs with ignored files\n t7061: status --ignored should search untracked dirs\n dir: recurse into untracked dirs for ignored files\n dir: hide untracked contents of untracked dirs\n dir: expose cmp_name() and check_contains()\n clean: teach clean -d to preserve ignored paths\n\n Documentation/technical/api-directory-listing.txt | 6 ++++\n builtin/clean.c | 42 ++++++++++++++++++++++\n dir.c | 43 ++++++++++++++++++++---\n dir.h | 6 +++-\n t/t7061-wtstatus-ignore.sh | 1 +\n t/t7300-clean.sh | 16 +++++++++\n 6 files changed, 109 insertions(+), 5 deletions(-)\n\n-- \n2.13.0\n\n"},{"id":"320584","messageId":"20170523100937.8752-2-sxlijin@gmail.com","threadId":"45826","inReplyTo":"20170523100937.8752-1-sxlijin@gmail.com","subject":"[PATCH v6 1/6] t7300: clean -d should skip dirs with ignored files","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-23T10:09:32Z","receivedAt":"2017-05-23T19:32:08Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"If git sees a directory which contains only untracked and ignored\nfiles, clean -d should not remove that directory. It was recently\ndiscovered that this is *not* true of git clean -d, and it's possible\nthat this has never worked correctly; this test and its accompanying\npatch series aims to fix that.\n\nSigned-off-by: Samuel Lijin \n---\n t/t7300-clean.sh | 16 ++++++++++++++++\n 1 file changed, 16 insertions(+)\n\ndiff --git a/t/t7300-clean.sh b/t/t7300-clean.sh\nindex b89fd2a6a..3a2d709c2 100755\n--- a/t/t7300-clean.sh\n+++ b/t/t7300-clean.sh\n@@ -653,4 +653,20 @@ test_expect_success 'git clean -d respects pathspecs (pathspec is prefix of dir)\n \ttest_path_is_dir foobar\n '\n \n+test_expect_failure 'git clean -d skips untracked dirs containing ignored files' '\n+\techo /foo/bar >.gitignore &&\n+\techo ignoreme >>.gitignore &&\n+\trm -rf foo &&\n+\tmkdir -p foo/a/aa/aaa foo/b/bb/bbb &&\n+\ttouch foo/bar foo/baz foo/a/aa/ignoreme foo/b/ignoreme foo/b/bb/1 foo/b/bb/2 &&\n+\tgit clean -df &&\n+\ttest_path_is_dir foo &&\n+\ttest_path_is_file foo/bar &&\n+\ttest_path_is_missing foo/baz &&\n+\ttest_path_is_file foo/a/aa/ignoreme &&\n+\ttest_path_is_missing foo/a/aa/aaa &&\n+\ttest_path_is_file foo/b/ignoreme &&\n+\ttest_path_is_missing foo/b/bb\n+'\n+\n test_done\n-- \n2.13.0\n\n"},{"id":"320585","messageId":"20170523100937.8752-3-sxlijin@gmail.com","threadId":"45826","inReplyTo":"20170523100937.8752-1-sxlijin@gmail.com","subject":"[PATCH v6 2/6] t7061: status --ignored should search untracked dirs","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-23T10:09:33Z","receivedAt":"2017-05-23T19:32:11Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"Per eb8c5b87, `status --ignored` by design does not list ignored files\nif they are in a directory which contains only ignored and untracked\nfiles (which is itself considered to be untracked) without `-uall`. This\ndoes not make sense for `--ignored`, which claims to \"Show ignored files\nas well.\"\n\nThus we revisit eb8c5b87 and decide that for such directories, `status\n--ignored` will list the directory as untracked *and* list all ignored\nfiles within said directory even without `-uall`.\n\nSigned-off-by: Samuel Lijin \n---\n t/t7061-wtstatus-ignore.sh | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t7061-wtstatus-ignore.sh b/t/t7061-wtstatus-ignore.sh\nindex cdc0747bf..15e7592b6 100755\n--- a/t/t7061-wtstatus-ignore.sh\n+++ b/t/t7061-wtstatus-ignore.sh\n@@ -9,9 +9,10 @@ cat >expected <<\\EOF\n ?? actual\n ?? expected\n ?? untracked/\n+!! untracked/ignored\n EOF\n \n-test_expect_success 'status untracked directory with --ignored' '\n+test_expect_failure 'status untracked directory with --ignored' '\n \techo \"ignored\" >.gitignore &&\n \tmkdir untracked &&\n \t: >untracked/ignored &&\n@@ -20,7 +21,7 @@ test_expect_success 'status untracked directory with --ignored' '\n \ttest_cmp expected actual\n '\n \n-test_expect_success 'same with gitignore starting with BOM' '\n+test_expect_failure 'same with gitignore starting with BOM' '\n \tprintf \"\\357\\273\\277ignored\\n\" >.gitignore &&\n \tmkdir -p untracked &&\n \t: >untracked/ignored &&\n-- \n2.13.0\n\n"},{"id":"320586","messageId":"20170523100937.8752-4-sxlijin@gmail.com","threadId":"45826","inReplyTo":"20170523100937.8752-1-sxlijin@gmail.com","subject":"[PATCH v6 3/6] dir: recurse into untracked dirs for ignored files","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-23T10:09:34Z","receivedAt":"2017-05-23T19:32:13Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"We consider directories containing only untracked and ignored files to\nbe themselves untracked, which in the usual case means we don't have to\nsearch these directories. This is problematic when we want to collect\nignored files with DIR_SHOW_IGNORED_TOO, though, so we teach\nread_directory_recursive() to recurse into untracked directories to find\nthe ignored files they contain when DIR_SHOW_IGNORED_TOO is set. This\nhas the side effect of also collecting all untracked files in untracked\ndirectories as well.\n\nSigned-off-by: Samuel Lijin \n---\n dir.c | 5 ++++-\n 1 file changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/dir.c b/dir.c\nindex f451bfa48..68cf6e47c 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -1784,7 +1784,10 @@ static enum path_treatment read_directory_recursive(struct dir_struct *dir,\n \t\t\tdir_state = state;\n \n \t\t/* recurse into subdir if instructed by treat_path */\n-\t\tif (state == path_recurse) {\n+\t\tif ((state == path_recurse) ||\n+\t\t\t((state == path_untracked) &&\n+\t\t\t (dir->flags & DIR_SHOW_IGNORED_TOO) &&\n+\t\t\t (get_dtype(cdir.de, path.buf, path.len) == DT_DIR))) {\n \t\t\tstruct untracked_cache_dir *ud;\n \t\t\tud = lookup_untracked(dir->untracked, untracked,\n \t\t\t\t\t path.buf + baselen,\n-- \n2.13.0\n\n"},{"id":"320587","messageId":"20170523100937.8752-6-sxlijin@gmail.com","threadId":"45826","inReplyTo":"20170523100937.8752-1-sxlijin@gmail.com","subject":"[PATCH v6 5/6] dir: expose cmp_name() and check_contains()","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-23T10:09:36Z","receivedAt":"2017-05-23T19:32:18Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"We want to use cmp_name() and check_contains() (which both compare\n`struct dir_entry`s, the former in terms of the sort order, the latter\nin terms of whether one lexically contains another) outside of dir.c,\nso we have to (1) change their linkage and (2) rename them as\nappropriate for the global namespace. The second is achieved by\nrenaming cmp_name() to cmp_dir_entry() and check_contains() to\ncheck_dir_entry_contains().\n\nSigned-off-by: Samuel Lijin \n---\n dir.c | 11 ++++++-----\n dir.h | 3 +++\n 2 files changed, 9 insertions(+), 5 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex ba5eadeda..aae6d00b4 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -1842,7 +1842,7 @@ static enum path_treatment read_directory_recursive(struct dir_struct *dir,\n \treturn dir_state;\n }\n \n-static int cmp_name(const void *p1, const void *p2)\n+int cmp_dir_entry(const void *p1, const void *p2)\n {\n \tconst struct dir_entry *e1 = *(const struct dir_entry **)p1;\n \tconst struct dir_entry *e2 = *(const struct dir_entry **)p2;\n@@ -1851,7 +1851,7 @@ static int cmp_name(const void *p1, const void *p2)\n }\n \n /* check if *out lexically strictly contains *in */\n-static int check_contains(const struct dir_entry *out, const struct dir_entry *in)\n+int check_dir_entry_contains(const struct dir_entry *out, const struct dir_entry *in)\n {\n \treturn (out->len < in->len) &&\n \t\t(out->name[out->len - 1] == '/') &&\n@@ -2071,8 +2071,8 @@ int read_directory(struct dir_struct *dir, const char *path,\n \t\tdir->untracked = NULL;\n \tif (!len || treat_leading_path(dir, path, len, pathspec))\n \t\tread_directory_recursive(dir, path, len, untracked, 0, pathspec);\n-\tQSORT(dir->entries, dir->nr, cmp_name);\n-\tQSORT(dir->ignored, dir->ignored_nr, cmp_name);\n+\tQSORT(dir->entries, dir->nr, cmp_dir_entry);\n+\tQSORT(dir->ignored, dir->ignored_nr, cmp_dir_entry);\n \n \t/* \n \t * if DIR_SHOW_IGNORED_TOO, read_directory_recursive() will also pick\n@@ -2085,7 +2085,8 @@ int read_directory(struct dir_struct *dir, const char *path,\n \n \t\t/* remove from dir->entries untracked contents of untracked dirs */\n \t\tfor (i = j = 0; j < dir->nr; j++) {\n-\t\t\tif (i && check_contains(dir->entries[i - 1], dir->entries[j])) {\n+\t\t\tif (i &&\n+\t\t\t check_dir_entry_contains(dir->entries[i - 1], dir->entries[j])) {\n \t\t\t\tfree(dir->entries[j]);\n \t\t\t\tdir->entries[j] = NULL;\n \t\t\t} else {\ndiff --git a/dir.h b/dir.h\nindex 650e54bdf..edb5fda58 100644\n--- a/dir.h\n+++ b/dir.h\n@@ -327,6 +327,9 @@ static inline int dir_path_match(const struct dir_entry *ent,\n \t\t\t has_trailing_dir);\n }\n \n+int cmp_dir_entry(const void *p1, const void *p2);\n+int check_dir_entry_contains(const struct dir_entry *out, const struct dir_entry *in);\n+\n void untracked_cache_invalidate_path(struct index_state *, const char *);\n void untracked_cache_remove_from_index(struct index_state *, const char *);\n void untracked_cache_add_to_index(struct index_state *, const char *);\n-- \n2.13.0\n\n"},{"id":"320588","messageId":"20170523100937.8752-5-sxlijin@gmail.com","threadId":"45826","inReplyTo":"20170523100937.8752-1-sxlijin@gmail.com","subject":"[PATCH v6 4/6] dir: hide untracked contents of untracked dirs","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-23T10:09:35Z","receivedAt":"2017-05-23T19:32:19Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"When we taught read_directory_recursive() to recurse into untracked\ndirectories in search of ignored files given DIR_SHOW_IGNORED_TOO, that\nhad the side effect of teaching it to collect the untracked contents of\nuntracked directories. It doesn't always make sense to return these,\nthough (we do need them for `clean -d`), so we introduce a flag\n(DIR_KEEP_UNTRACKED_CONTENTS) to control whether or not read_directory()\nstrips dir->entries of the untracked contents of untracked dirs.\n\nWe also introduce check_contains() to check if one dir_entry corresponds\nto a path which contains the path corresponding to another dir_entry.\n\nThis also fixes known breakages in t7061, since status --ignored now\nsearches untracked directories for ignored files.\n\nSigned-off-by: Samuel Lijin \n---\n Documentation/technical/api-directory-listing.txt | 6 +++++\n dir.c | 31 +++++++++++++++++++++++\n dir.h | 3 ++-\n t/t7061-wtstatus-ignore.sh | 4 +--\n 4 files changed, 41 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/technical/api-directory-listing.txt b/Documentation/technical/api-directory-listing.txt\nindex 7f8e78d91..6c77b4920 100644\n--- a/Documentation/technical/api-directory-listing.txt\n+++ b/Documentation/technical/api-directory-listing.txt\n@@ -33,6 +33,12 @@ The notable options are:\n \tSimilar to `DIR_SHOW_IGNORED`, but return ignored files in `ignored[]`\n \tin addition to untracked files in `entries[]`.\n \n+`DIR_KEEP_UNTRACKED_CONTENTS`:::\n+\n+\tOnly has meaning if `DIR_SHOW_IGNORED_TOO` is also set; if this is set, the\n+\tuntracked contents of untracked directories are also returned in\n+\t`entries[]`.\n+\n `DIR_COLLECT_IGNORED`:::\n \n \tSpecial mode for git-add. Return ignored files in `ignored[]` and\ndiff --git a/dir.c b/dir.c\nindex 68cf6e47c..ba5eadeda 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -1850,6 +1850,14 @@ static int cmp_name(const void *p1, const void *p2)\n \treturn name_compare(e1->name, e1->len, e2->name, e2->len);\n }\n \n+/* check if *out lexically strictly contains *in */\n+static int check_contains(const struct dir_entry *out, const struct dir_entry *in)\n+{\n+\treturn (out->len < in->len) &&\n+\t\t(out->name[out->len - 1] == '/') &&\n+\t\t!memcmp(out->name, in->name, out->len);\n+}\n+\n static int treat_leading_path(struct dir_struct *dir,\n \t\t\t const char *path, int len,\n \t\t\t const struct pathspec *pathspec)\n@@ -2065,6 +2073,29 @@ int read_directory(struct dir_struct *dir, const char *path,\n \t\tread_directory_recursive(dir, path, len, untracked, 0, pathspec);\n \tQSORT(dir->entries, dir->nr, cmp_name);\n \tQSORT(dir->ignored, dir->ignored_nr, cmp_name);\n+\n+\t/* \n+\t * if DIR_SHOW_IGNORED_TOO, read_directory_recursive() will also pick\n+\t * up untracked contents of untracked dirs; by default we discard these,\n+\t * but given DIR_KEEP_UNTRACKED_CONTENTS we do not\n+\t */\n+\tif ((dir->flags & DIR_SHOW_IGNORED_TOO) &&\n+\t\t !(dir->flags & DIR_KEEP_UNTRACKED_CONTENTS)) {\n+\t\tint i, j;\n+\n+\t\t/* remove from dir->entries untracked contents of untracked dirs */\n+\t\tfor (i = j = 0; j < dir->nr; j++) {\n+\t\t\tif (i && check_contains(dir->entries[i - 1], dir->entries[j])) {\n+\t\t\t\tfree(dir->entries[j]);\n+\t\t\t\tdir->entries[j] = NULL;\n+\t\t\t} else {\n+\t\t\t\tdir->entries[i++] = dir->entries[j];\n+\t\t\t}\n+\t\t}\n+\n+\t\tdir->nr = i;\n+\t}\n+\n \tif (dir->untracked) {\n \t\tstatic struct trace_key trace_untracked_stats = TRACE_KEY_INIT(UNTRACKED_STATS);\n \t\ttrace_printf_key(&trace_untracked_stats,\ndiff --git a/dir.h b/dir.h\nindex bf23a470a..650e54bdf 100644\n--- a/dir.h\n+++ b/dir.h\n@@ -151,7 +151,8 @@ struct dir_struct {\n \t\tDIR_NO_GITLINKS = 1<<3,\n \t\tDIR_COLLECT_IGNORED = 1<<4,\n \t\tDIR_SHOW_IGNORED_TOO = 1<<5,\n-\t\tDIR_COLLECT_KILLED_ONLY = 1<<6\n+\t\tDIR_COLLECT_KILLED_ONLY = 1<<6,\n+\t\tDIR_KEEP_UNTRACKED_CONTENTS = 1<<7\n \t} flags;\n \tstruct dir_entry **entries;\n \tstruct dir_entry **ignored;\ndiff --git a/t/t7061-wtstatus-ignore.sh b/t/t7061-wtstatus-ignore.sh\nindex 15e7592b6..fc6013ba3 100755\n--- a/t/t7061-wtstatus-ignore.sh\n+++ b/t/t7061-wtstatus-ignore.sh\n@@ -12,7 +12,7 @@ cat >expected <<\\EOF\n !! untracked/ignored\n EOF\n \n-test_expect_failure 'status untracked directory with --ignored' '\n+test_expect_success 'status untracked directory with --ignored' '\n \techo \"ignored\" >.gitignore &&\n \tmkdir untracked &&\n \t: >untracked/ignored &&\n@@ -21,7 +21,7 @@ test_expect_failure 'status untracked directory with --ignored' '\n \ttest_cmp expected actual\n '\n \n-test_expect_failure 'same with gitignore starting with BOM' '\n+test_expect_success 'same with gitignore starting with BOM' '\n \tprintf \"\\357\\273\\277ignored\\n\" >.gitignore &&\n \tmkdir -p untracked &&\n \t: >untracked/ignored &&\n-- \n2.13.0\n\n"},{"id":"320589","messageId":"20170523100937.8752-7-sxlijin@gmail.com","threadId":"45826","inReplyTo":"20170523100937.8752-1-sxlijin@gmail.com","subject":"[PATCH v6 6/6] clean: teach clean -d to preserve ignored paths","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-23T10:09:37Z","receivedAt":"2017-05-23T19:32:22Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"There is an implicit assumption that a directory containing only\nuntracked and ignored paths should itself be considered untracked. This\nmakes sense in use cases where we're asking if a directory should be\nadded to the git database, but not when we're asking if a directory can\nbe safely removed from the working tree; as a result, clean -d would\nassume that an \"untracked\" directory containing ignored paths could be\ndeleted, even though doing so would also remove the ignored paths.\n\nTo get around this, we teach clean -d to collect ignored paths and skip\nan untracked directory if it contained an ignored path, instead just\nremoving the untracked contents thereof. To achieve this, cmd_clean()\nhas to collect all untracked contents of untracked directories, in\naddition to all ignored paths, to determine which untracked dirs must be\nskipped (because they contain ignored paths) and which ones should *not*\nbe skipped.\n\nFor this purpose, correct_untracked_entries() is introduced to prune a\ngiven dir_struct of untracked entries containing ignored paths and those\nuntracked entries encompassed by the untracked entries which are not\npruned away.\n\nA memory leak is also fixed in cmd_clean().\n\nThis also fixes the known breakage in t7300, since clean -d now skips\nuntracked directories containing ignored paths.\n\nSigned-off-by: Samuel Lijin \n---\n builtin/clean.c | 42 ++++++++++++++++++++++++++++++++++++++++++\n t/t7300-clean.sh | 2 +-\n 2 files changed, 43 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/clean.c b/builtin/clean.c\nindex d861f836a..937eb17b6 100644\n--- a/builtin/clean.c\n+++ b/builtin/clean.c\n@@ -857,6 +857,38 @@ static void interactive_main_loop(void)\n \t}\n }\n \n+static void correct_untracked_entries(struct dir_struct *dir)\n+{\n+\tint src, dst, ign;\n+\n+\tfor (src = dst = ign = 0; src < dir->nr; src++) {\n+\t\t/* skip paths in ignored[] that cannot be inside entries[src] */\n+\t\twhile (ign < dir->ignored_nr &&\n+\t\t 0 <= cmp_dir_entry(&dir->entries[src], &dir->ignored[ign]))\n+\t\t\tign++;\n+\n+\t\tif (ign < dir->ignored_nr &&\n+\t\t check_dir_entry_contains(dir->entries[src], dir->ignored[ign])) {\n+\t\t\t/* entries[src] contains an ignored path, so we drop it */\n+\t\t\tfree(dir->entries[src]);\n+\t\t} else {\n+\t\t\tstruct dir_entry *ent = dir->entries[src++];\n+\n+\t\t\t/* entries[src] does not contain an ignored path, so we keep it */\n+\t\t\tdir->entries[dst++] = ent;\n+\n+\t\t\t/* then discard paths in entries[] contained inside entries[src] */\n+\t\t\twhile (src < dir->nr &&\n+\t\t\t check_dir_entry_contains(ent, dir->entries[src]))\n+\t\t\t\tfree(dir->entries[src++]);\n+\n+\t\t\t/* compensate for the outer loop's loop control */\n+\t\t\tsrc--;\n+\t\t}\n+\t}\n+\tdir->nr = dst;\n+}\n+\n int cmd_clean(int argc, const char **argv, const char *prefix)\n {\n \tint i, res;\n@@ -916,6 +948,9 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \n \tdir.flags |= DIR_SHOW_OTHER_DIRECTORIES;\n \n+\tif (remove_directories)\n+\t\tdir.flags |= DIR_SHOW_IGNORED_TOO | DIR_KEEP_UNTRACKED_CONTENTS;\n+\n \tif (read_cache() < 0)\n \t\tdie(_(\"index file corrupt\"));\n \n@@ -931,6 +966,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \t\t prefix, argv);\n \n \tfill_directory(&dir, &pathspec);\n+\tcorrect_untracked_entries(&dir);\n \n \tfor (i = 0; i < dir.nr; i++) {\n \t\tstruct dir_entry *ent = dir.entries[i];\n@@ -958,6 +994,12 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \t\tstring_list_append(&del_list, rel);\n \t}\n \n+\tfor (i = 0; i < dir.nr; i++)\n+\t\tfree(dir.entries[i]);\n+\n+\tfor (i = 0; i < dir.ignored_nr; i++)\n+\t\tfree(dir.ignored[i]);\n+\n \tif (interactive && del_list.nr > 0)\n \t\tinteractive_main_loop();\n \ndiff --git a/t/t7300-clean.sh b/t/t7300-clean.sh\nindex 3a2d709c2..7b36954d6 100755\n--- a/t/t7300-clean.sh\n+++ b/t/t7300-clean.sh\n@@ -653,7 +653,7 @@ test_expect_success 'git clean -d respects pathspecs (pathspec is prefix of dir)\n \ttest_path_is_dir foobar\n '\n \n-test_expect_failure 'git clean -d skips untracked dirs containing ignored files' '\n+test_expect_success 'git clean -d skips untracked dirs containing ignored files' '\n \techo /foo/bar >.gitignore &&\n \techo ignoreme >>.gitignore &&\n \trm -rf foo &&\n-- \n2.13.0\n\n"},{"id":"320601","messageId":"xmqqefvfjj64.fsf@gitster.mtv.corp.google.com","threadId":"45826","inReplyTo":"20170523100937.8752-7-sxlijin@gmail.com","subject":"Re: [PATCH v6 6/6] clean: teach clean -d to preserve ignored paths","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-23T22:35:31Z","receivedAt":"2017-05-23T22:35:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Samuel Lijin writes:\n\n> There is an implicit assumption that a directory containing only\n> untracked and ignored paths should itself be considered untracked. This\n> makes sense in use cases where we're asking if a directory should be\n> added to the git database, but not when we're asking if a directory can\n> be safely removed from the working tree; as a result, clean -d would\n> assume that an \"untracked\" directory containing ignored paths could be\n> deleted, even though doing so would also remove the ignored paths.\n>\n> To get around this, we teach clean -d to collect ignored paths and skip\n> an untracked directory if it contained an ignored path, instead just\n> removing the untracked contents thereof. To achieve this, cmd_clean()\n> has to collect all untracked contents of untracked directories, in\n> addition to all ignored paths, to determine which untracked dirs must be\n> skipped (because they contain ignored paths) and which ones should *not*\n> be skipped.\n>\n> For this purpose, correct_untracked_entries() is introduced to prune a\n> given dir_struct of untracked entries containing ignored paths and those\n> untracked entries encompassed by the untracked entries which are not\n> pruned away.\n>\n> A memory leak is also fixed in cmd_clean().\n>\n> This also fixes the known breakage in t7300, since clean -d now skips\n> untracked directories containing ignored paths.\n\nNicely explained. Will replace the previous 6/6 and my squash on\ntop that were queued on 'pu'.\n\nThanks.\n"},{"id":"320614","messageId":"517a9182-0366-d51a-b7e5-f9085df5b4f9@web.de","threadId":"45826","inReplyTo":"20170523100937.8752-7-sxlijin@gmail.com","subject":"Re: [PATCH v6 6/6] clean: teach clean -d to preserve ignored paths","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2017-05-24T04:14:44Z","receivedAt":"2017-05-24T04:13:42Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"\n> diff --git a/builtin/clean.c b/builtin/clean.c\n> index d861f836a..937eb17b6 100644\n> --- a/builtin/clean.c\n> +++ b/builtin/clean.c\n> @@ -857,6 +857,38 @@ static void interactive_main_loop(void)\n> \t}\n> }\n> \n> +static void correct_untracked_entries(struct dir_struct *dir)\n> +{\n> +\tint src, dst, ign;\n> +\n> +\tfor (src = dst = ign = 0; src < dir->nr; src++) {\n> +\t\t/* skip paths in ignored[] that cannot be inside entries[src] */\n> +\t\twhile (ign < dir->ignored_nr &&\n> +\t\t 0 <= cmp_dir_entry(&dir->entries[src], &dir->ignored[ign]))\n> +\t\t\tign++;\n> +\n> +\t\tif (ign < dir->ignored_nr &&\n> +\t\t check_dir_entry_contains(dir->entries[src], dir->ignored[ign])) {\n> +\t\t\t/* entries[src] contains an ignored path, so we drop it */\n> +\t\t\tfree(dir->entries[src]);\n> +\t\t} else {\n> +\t\t\tstruct dir_entry *ent = dir->entries[src++];\n> +\n> +\t\t\t/* entries[src] does not contain an ignored path, so we keep it */\n> +\t\t\tdir->entries[dst++] = ent;\n> +\n> +\t\t\t/* then discard paths in entries[] contained inside entries[src] */\n> +\t\t\twhile (src < dir->nr &&\n> +\t\t\t check_dir_entry_contains(ent, dir->entries[src]))\n> +\t\t\t\tfree(dir->entries[src++]);\n> +\n> +\t\t\t/* compensate for the outer loop's loop control */\n> +\t\t\tsrc--;\n> +\t\t}\n> +\t}\n> +\tdir->nr = dst;\n> +}\n> +\n\nLooking what the function does, would\ndrop_or_keep_untracked_entries()\nmake more sense ?\n\n\n"},{"id":"320714","messageId":"CAJZjrdXZkxjHt3XFFhS2BDWwwW8Gmz7frfbUzxgBBwcFYBZe3Q@mail.gmail.com","threadId":"45826","inReplyTo":"517a9182-0366-d51a-b7e5-f9085df5b4f9@web.de","subject":"Re: [PATCH v6 6/6] clean: teach clean -d to preserve ignored paths","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-25T15:36:35Z","receivedAt":"2017-05-25T15:37:21Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"On Wed, May 24, 2017 at 12:14 AM, Torsten Bögershausen wrote:\n>\n>> diff --git a/builtin/clean.c b/builtin/clean.c\n>> index d861f836a..937eb17b6 100644\n>> --- a/builtin/clean.c\n>> +++ b/builtin/clean.c\n>> @@ -857,6 +857,38 @@ static void interactive_main_loop(void)\n>> }\n>> }\n>> +static void correct_untracked_entries(struct dir_struct *dir)\n>\n> Looking what the function does, would\n> drop_or_keep_untracked_entries()\n> make more sense ?\n\nTo me, drop_or_keep_ implies that they're either all dropped or all\nkept, nothing in between, which is why I went with correct_, to\nindicate that the set of untracked entries in the dir_struct prior to\ncalling the method needs to be corrected.\n"},{"id":"320779","messageId":"xmqqh908ctrr.fsf@gitster.mtv.corp.google.com","threadId":"45826","inReplyTo":"CAJZjrdXZkxjHt3XFFhS2BDWwwW8Gmz7frfbUzxgBBwcFYBZe3Q@mail.gmail.com","subject":"Re: [PATCH v6 6/6] clean: teach clean -d to preserve ignored paths","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-26T01:05:12Z","receivedAt":"2017-05-26T01:05:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Samuel Lijin writes:\n\n> On Wed, May 24, 2017 at 12:14 AM, Torsten Bögershausen wrote:\n>>\n>>> diff --git a/builtin/clean.c b/builtin/clean.c\n>>> index d861f836a..937eb17b6 100644\n>>> --- a/builtin/clean.c\n>>> +++ b/builtin/clean.c\n>>> @@ -857,6 +857,38 @@ static void interactive_main_loop(void)\n>>> }\n>>> }\n>>> +static void correct_untracked_entries(struct dir_struct *dir)\n>>\n>> Looking what the function does, would\n>> drop_or_keep_untracked_entries()\n>> make more sense ?\n>\n> To me, drop_or_keep_ implies that they're either all dropped or all\n> kept, nothing in between, which is why I went with correct_, to\n> indicate that the set of untracked entries in the dir_struct prior to\n> calling the method needs to be corrected.\n\nNeither is a particularly good name, but if I have to pick, I'd say\nwe should keep yours.\n\ndrop-or-keep may indicate some are dropped while others are kept but\nit does not say what the function is for. correct is better in the\nsense that the readers can guess that there is something wrong in\n\"dir\" at the point and needs correcting by calling the helper, but\nstill does not convey what exactly is wrong. How the wrong-ness is\ncorrected does not have to be explained in the name (i.e. I am\nsaying that drop-or-keep does not give us much useful information),\nbut I wish that the name of the helper hinted what kind of wrongness\nis there to be corrected to the readers.\n\n\n\n"}]}