{"thread":{"id":"22131","subject":"Possible bug in git-completion.sh","startedAt":"2010-01-08T15:17:07Z","lastAt":"2010-01-12T16:33:11Z","messageCount":28,"participants":["Jon Schewe","Michael J Gruber","Jeff King","Junio C Hamano","Linus Torvalds"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"131095","messageId":"4B474C73.8080100@mtu.net","threadId":"22131","inReplyTo":null,"subject":"Possible bug in git-completion.sh","fromName":"Jon Schewe","fromEmail":"jpschewe@mtu.net","sentAt":"2010-01-08T15:17:07Z","receivedAt":"2010-01-08T15:17:07Z","isPatch":false,"sender":{"key":"jpschewe@mtu.net","avatar":null},"body":"If I create a directory \"build\" at the top of my git repository and then\nadd it to .gitignore, git behaves as expected and ignores the build\ndirectory when checking status. Now git-completion.sh has some issues. I\nhave GIT_PS1_SHOWUNTRACKEDFILES to \"1\", so that I will be notified when\nthere are untracked files in my working directory. When I'm in the\ntop-level directory my prompt looks like expected, no '%'. However if I\nchange to the build directory I get a '%', even though git status shows\nno untracked files. I see that git-completion.sh is using git ls-files\nto check this and that function does indeed show output when in my build\ndirectory. So the question here: Is git-completion.sh using ls-files\nimproperly or is ls-files behaving improperly?\n\n-- \nJon Schewe | http://mtu.net/~jpschewe\nIf you see an attachment named signature.asc, this is my digital\nsignature. See http://www.gnupg.org for more information.\n\nFor I am convinced that neither death nor life, neither angels nor\ndemons, neither the present nor the future, nor any powers,\nneither height nor depth, nor anything else in all creation, will\nbe able to separate us from the love of God that is in Christ\nJesus our Lord. - Romans 8:38-39\n"},{"id":"131097","messageId":"4B4751EA.8060707@drmicha.warpmail.net","threadId":"22131","inReplyTo":"4B474C73.8080100@mtu.net","subject":"Re: Possible bug in git-completion.sh","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2010-01-08T15:40:26Z","receivedAt":"2010-01-08T15:40:26Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Jon Schewe venit, vidit, dixit 08.01.2010 16:17:\n> If I create a directory \"build\" at the top of my git repository and then\n> add it to .gitignore, git behaves as expected and ignores the build\n> directory when checking status. Now git-completion.sh has some issues. I\n> have GIT_PS1_SHOWUNTRACKEDFILES to \"1\", so that I will be notified when\n> there are untracked files in my working directory. When I'm in the\n> top-level directory my prompt looks like expected, no '%'. However if I\n> change to the build directory I get a '%', even though git status shows\n> no untracked files. I see that git-completion.sh is using git ls-files\n> to check this and that function does indeed show output when in my build\n> directory. So the question here: Is git-completion.sh using ls-files\n> improperly or is ls-files behaving improperly?\n> \n\nNeither, but: output between status and ls-files is inconsistent. More\nspecifically, different commands behave differently with respect to the\ntreatment of subdirs. ls-files assumes \".\" implicitly, status does not.\n\"git status .\" should give you the same behavior is \"git ls-files\" in\nthis regard.\n\nMichael\n"},{"id":"131105","messageId":"20100108162404.GA5799@coredump.intra.peff.net","threadId":"22131","inReplyTo":"4B4751EA.8060707@drmicha.warpmail.net","subject":"Re: Possible bug in git-completion.sh","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-01-08T16:24:04Z","receivedAt":"2010-01-08T16:24:04Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jan 08, 2010 at 04:40:26PM +0100, Michael J Gruber wrote:\n\n> Jon Schewe venit, vidit, dixit 08.01.2010 16:17:\n> > If I create a directory \"build\" at the top of my git repository and then\n> > add it to .gitignore, git behaves as expected and ignores the build\n> > directory when checking status. Now git-completion.sh has some issues. I\n> > have GIT_PS1_SHOWUNTRACKEDFILES to \"1\", so that I will be notified when\n> > there are untracked files in my working directory. When I'm in the\n> > top-level directory my prompt looks like expected, no '%'. However if I\n> > change to the build directory I get a '%', even though git status shows\n> > no untracked files. I see that git-completion.sh is using git ls-files\n> > to check this and that function does indeed show output when in my build\n> > directory. So the question here: Is git-completion.sh using ls-files\n> > improperly or is ls-files behaving improperly?\n> > \n> \n> Neither, but: output between status and ls-files is inconsistent. More\n> specifically, different commands behave differently with respect to the\n> treatment of subdirs. ls-files assumes \".\" implicitly, status does not.\n> \"git status .\" should give you the same behavior is \"git ls-files\" in\n> this regard.\n\nIt doesn't, and I think there is a bug in ls-files.\n\nTry this:\n\n  git init\n  touch base-cruft\n  mkdir subdir\n  touch subdir/cruft\n  echo subdir >.gitignore\n  git status ;# shows gitignore and base-cruft\n  git ls-files -o --exclude-standard ;# ditto\n  cd subdir\n  git status . ;# shows nothing, since everything in subdir is ignored\n  git ls-files -o --exclude-standard ;# BUG: shows cruft\n\nYes, ls-files operates in the subdirectory, which means it should not\nshow cruft from the root (and it does not). But it should respect\n.gitignore directives going all the way back to the root, and it\ndoesn't.\n\n-Peff\n"},{"id":"131107","messageId":"7vr5q05z74.fsf@alter.siamese.dyndns.org","threadId":"22131","inReplyTo":"20100108162404.GA5799@coredump.intra.peff.net","subject":"Re: Possible bug in git-completion.sh","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-01-08T16:38:39Z","receivedAt":"2010-01-08T16:38:39Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> Try this:\n>\n>   git init\n>   touch base-cruft\n>   mkdir subdir\n>   touch subdir/cruft\n>   echo subdir >.gitignore\n>   git status ;# shows gitignore and base-cruft\n>   git ls-files -o --exclude-standard ;# ditto\n>   cd subdir\n>   git status . ;# shows nothing, since everything in subdir is ignored\n>   git ls-files -o --exclude-standard ;# BUG: shows cruft\n>\n> Yes, ls-files operates in the subdirectory, which means it should not\n> show cruft from the root (and it does not). But it should respect\n> .gitignore directives going all the way back to the root, and it\n> doesn't.\n\nShouldn't the ls-files be run from the root with subdir/ as pathspec, if\nyou wanted to do apples-to-apples comparison between it and status?\n"},{"id":"131108","messageId":"20100108164132.GA6171@coredump.intra.peff.net","threadId":"22131","inReplyTo":"7vr5q05z74.fsf@alter.siamese.dyndns.org","subject":"Re: Possible bug in git-completion.sh","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-01-08T16:41:32Z","receivedAt":"2010-01-08T16:41:32Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jan 08, 2010 at 08:38:39AM -0800, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > Try this:\n> >\n> >   git init\n> >   touch base-cruft\n> >   mkdir subdir\n> >   touch subdir/cruft\n> >   echo subdir >.gitignore\n> >   git status ;# shows gitignore and base-cruft\n> >   git ls-files -o --exclude-standard ;# ditto\n> >   cd subdir\n> >   git status . ;# shows nothing, since everything in subdir is ignored\n> >   git ls-files -o --exclude-standard ;# BUG: shows cruft\n> >\n> > Yes, ls-files operates in the subdirectory, which means it should not\n> > show cruft from the root (and it does not). But it should respect\n> > .gitignore directives going all the way back to the root, and it\n> > doesn't.\n> \n> Shouldn't the ls-files be run from the root with subdir/ as pathspec, if\n> you wanted to do apples-to-apples comparison between it and status?\n\nWell, yes, if you wanted to compare apples to apples. But actually, my\npoint in showing \"status\" at all was to note that Michael's statement\nthat they would be the same is wrong.\n\nBut just looking at the ls-files output, do you not agree that there is\na bug? Respecting gitignore means respecting _all_ gitignore files in\nthe repository, not just the subset that you happen to be in.\n\n-Peff\n"},{"id":"131109","messageId":"7vskag1r5o.fsf@alter.siamese.dyndns.org","threadId":"22131","inReplyTo":"20100108164132.GA6171@coredump.intra.peff.net","subject":"Re: Possible bug in git-completion.sh","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-01-08T16:45:55Z","receivedAt":"2010-01-08T16:45:55Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> Well, yes, if you wanted to compare apples to apples. But actually, my\n> point in showing \"status\" at all was to note that Michael's statement\n> that they would be the same is wrong.\n>\n> But just looking at the ls-files output, do you not agree that there is\n> a bug?\n\nIf I agreed, I wouldn't have suggested _you_ to cd up and use pathspec,\nbut instead would have suggested to patch ls-files to make it do so for\nyou.\n\nYou can see it as a feature that you can use to check what would happen\nif you stopped ignoring the directory from the higher level.  With a patch\nto always cd-up and use pathspec, that will become impossible.\n\nMaybe nobody needs such a feature (I don't), in which case we can declare\nit as a bug.  But I wasn't ready to do so myself when I wrote the message\nyou are responding to, and I still am not.\n"},{"id":"131110","messageId":"7vaawosfg9.fsf@alter.siamese.dyndns.org","threadId":"22131","inReplyTo":"7vskag1r5o.fsf@alter.siamese.dyndns.org","subject":"Re: Possible bug in git-completion.sh","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-01-08T16:56:38Z","receivedAt":"2010-01-08T16:56:38Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> You can see it as a feature that you can use to check what would happen\n> if you stopped ignoring the directory from the higher level.  With a patch\n> to always cd-up and use pathspec, that will become impossible.\n\nI hate to say this, but I have a feeling that status might be what needs\nto be fixed instead.  It isn't hard to imagine a use case where you don't\nwant to be bothered by crufts in lower level directories when you are\nlooking at the bigger picture (e.g. at the top of the hierarchy) but when\nyou go to individual subdirectory you would want to see them.\n"},{"id":"131112","messageId":"20100108172134.GA6344@coredump.intra.peff.net","threadId":"22131","inReplyTo":"7vskag1r5o.fsf@alter.siamese.dyndns.org","subject":"Re: Possible bug in git-completion.sh","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-01-08T17:21:34Z","receivedAt":"2010-01-08T17:21:34Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jan 08, 2010 at 08:45:55AM -0800, Junio C Hamano wrote:\n\n> > But just looking at the ls-files output, do you not agree that there is\n> > a bug?\n> \n> If I agreed, I wouldn't have suggested _you_ to cd up and use pathspec,\n> but instead would have suggested to patch ls-files to make it do so for\n> you.\n\nAh, I missed the subtlety there.\n\n> You can see it as a feature that you can use to check what would happen\n> if you stopped ignoring the directory from the higher level.  With a patch\n> to always cd-up and use pathspec, that will become impossible.\n> \n> Maybe nobody needs such a feature (I don't), in which case we can declare\n> it as a bug.  But I wasn't ready to do so myself when I wrote the message\n> you are responding to, and I still am not.\n\nThat feature seems somewhat insane to me. If I wanted to know how things\nwould look without gitignore, I would not have said --exclude-standard.\n\nHowever, I was wrong before that it ignores .gitignore. It doesn't. If\nyou put \"cruft\" instead of \"subdir\" into gitignore in my previous\nexample, it is correctly ignored. So it is sort of a \"half-use\ngitignore\", which you cannot accomplish any other way.\n\nI still think it's a bit crazy to have as the default behavior. But at\nleast it's constrained to a plumbing command, which scripts can work\naround to get what they want. With the current behavior, that means the\nbash prompt code should be doing \"git rev-parse --show-cdup --show-prefix\"\nand moving to the toplevel.\n\n-Peff\n"},{"id":"131113","messageId":"20100108172445.GB6344@coredump.intra.peff.net","threadId":"22131","inReplyTo":"7vaawosfg9.fsf@alter.siamese.dyndns.org","subject":"Re: Possible bug in git-completion.sh","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-01-08T17:24:45Z","receivedAt":"2010-01-08T17:24:45Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jan 08, 2010 at 08:56:38AM -0800, Junio C Hamano wrote:\n\n> Junio C Hamano <gitster@pobox.com> writes:\n> \n> > You can see it as a feature that you can use to check what would happen\n> > if you stopped ignoring the directory from the higher level.  With a patch\n> > to always cd-up and use pathspec, that will become impossible.\n> \n> I hate to say this, but I have a feeling that status might be what needs\n> to be fixed instead.  It isn't hard to imagine a use case where you don't\n> want to be bothered by crufts in lower level directories when you are\n> looking at the bigger picture (e.g. at the top of the hierarchy) but when\n> you go to individual subdirectory you would want to see them.\n\nFor people who have status.relativepaths off, status is still a global\ncommand. So I would hope that behavior would come out only when doing\n\"git status .\", or at the very least be disabled if status.relativepaths\nis set.\n\nPersonally, I think having ignored files show up as untracked based on\nheuristics like cwd is just going to end up confusing people.\n\n-Peff\n"},{"id":"131114","messageId":"7v8wc8jw3k.fsf@alter.siamese.dyndns.org","threadId":"22131","inReplyTo":"7vskag1r5o.fsf@alter.siamese.dyndns.org","subject":"Re: Possible bug in git-completion.sh","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-01-08T18:21:51Z","receivedAt":"2010-01-08T18:21:51Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Jeff King <peff@peff.net> writes:\n>\n>> Well, yes, if you wanted to compare apples to apples. But actually, my\n>> point in showing \"status\" at all was to note that Michael's statement\n>> that they would be the same is wrong.\n>>\n>> But just looking at the ls-files output, do you not agree that there is\n>> a bug?\n>\n> If I agreed, I wouldn't have suggested _you_ to cd up and use pathspec,\n> but instead would have suggested to patch ls-files to make it do so for\n> you.\n\nNah, I should have checked the code.\n\nThe implementation of ls-files does cd up and uses pathspec, so the intent\nis to apply higher level gitignore.\n\nIt however feeds paths from the already ignored directories, which _is_\nthe real bug.\n\nFor example, if you have\n\n    .gitignore (ignores t/)\n    t/\n    t/junk\n\nin your work tree, it will read .gitignore at the top level, and\neventually end up feeding \"t/junk\" to dir.c::excluded_1(), which does:\n\n\t\tfor (i = el->nr - 1; 0 <= i; i--) {\n\t\t\tstruct exclude *x = el->excludes[i];\n\t\t\tconst char *exclude = x->pattern;\n\t\t\tint to_exclude = x->to_exclude;\n\n\t\t\tif (x->flags & EXC_FLAG_MUSTBEDIR) {\n\t\t\t\tif (*dtype == DT_UNKNOWN)\n\t\t\t\t\t*dtype = get_dtype(NULL, pathname, pathlen);\n\t\t\t\tif (*dtype != DT_DIR)\n\t\t\t\t\tcontinue;\n\t\t\t}\n\t\t\t...\n\nwhere *x has \"'t/' that must be directory\".  but the path \"t/junk\" does\nnot match with \"t/\" and is not excluded by this rule; this part notices\n\"t/junk\" is not a directory, and continues without giving the later part a\nchance to intervene.  Of course, the later part also is not prepared to do\na \"leading path\" match, as the function is not meant to be fed entries\nfrom ignored \"t/\" directory in the first place.\n\nI think a proper fix should be in dir.c::read_directory() where it calls\nread_directory_recursive() without first checking if the directory itself\nshould be ignored.  read_directory_recursive() itself has a logic to see\nif the dirent it found is worth recursing into and a similar logic should\nbe in the toplevel caller.\n"},{"id":"131119","messageId":"7vvdfcfjxo.fsf@alter.siamese.dyndns.org","threadId":"22131","inReplyTo":"7v8wc8jw3k.fsf@alter.siamese.dyndns.org","subject":"Re: Possible bug in git-completion.sh","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-01-08T19:58:11Z","receivedAt":"2010-01-08T19:58:11Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Nah, I should have checked the code.\n>\n> The implementation of ls-files does cd up and uses pathspec, so the intent\n> is to apply higher level gitignore.\n> \n> It however feeds paths from the already ignored directories, which _is_\n> the real bug.\n> ...\n> I think a proper fix should be in dir.c::read_directory() where it calls\n> read_directory_recursive() without first checking if the directory itself\n> should be ignored.  read_directory_recursive() itself has a logic to see\n> if the dirent it found is worth recursing into and a similar logic should\n> be in the toplevel caller.\n\nActually doing less in fill_directory() turned out to be a simpler\nsolution.\n\nbuiltin_add() cares about the return value from fill_directory() and\nperforms prune_directory() optimization magic, and we may want to change\nit not to do so as well, but I didn't want to worry about too many things\nat once, so this version still runs the \"common_prefix()\" that forgets to\ntake .gitignore at higher levels (or a $GIT_DIR/info/exclude entry that\nignores the common prefix directory of given pathspecs) into account.\n\nAnother possibility is to fix common_prefix() and make it walk the path it\nreturns one level at a time from the top, making sure they are not\nignored, and that would probably be a better fix, but at least this patch\nwill give you a starting point and tests to check the result against.\n\ndiff --git a/dir.c b/dir.c\nindex d0999ba..56d3f60 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -53,7 +53,6 @@ static int common_prefix(const char **pathspec)\n \n int fill_directory(struct dir_struct *dir, const char **pathspec)\n {\n-\tconst char *path;\n \tint len;\n \n \t/*\n@@ -61,13 +60,8 @@ int fill_directory(struct dir_struct *dir, const char **pathspec)\n \t * use that to optimize the directory walk\n \t */\n \tlen = common_prefix(pathspec);\n-\tpath = \"\";\n-\n-\tif (len)\n-\t\tpath = xmemdupz(*pathspec, len);\n-\n \t/* Read the directory and prune it */\n-\tread_directory(dir, path, len, pathspec);\n+\tread_directory(dir, \"\", 0, pathspec);\n \treturn len;\n }\n \ndiff --git a/t/t3001-ls-files-others-exclude.sh b/t/t3001-ls-files-others-exclude.sh\nindex c65bca8..17d1764 100755\n--- a/t/t3001-ls-files-others-exclude.sh\n+++ b/t/t3001-ls-files-others-exclude.sh\n@@ -153,4 +153,42 @@ test_expect_success 'negated exclude matches can override previous ones' '\n \tgrep \"^a.1\" output\n '\n \n+test_expect_success 'subdirectory ignore (setup)' '\n+\tmkdir -p top/l1/l2 &&\n+\t(\n+\t\tcd top &&\n+\t\tgit init &&\n+\t\techo /.gitignore >.gitignore &&\n+\t\techo l1 >>.gitignore &&\n+\t\techo l2 >l1/.gitignore\n+\t)\n+'\n+\n+test_expect_success 'subdirectory ignore (toplevel)' '\n+\t(\n+\t\tcd top &&\n+\t\tgit ls-files -o --exclude-standard\n+\t) >actual &&\n+\t>expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'subdirectory ignore (l1/l2)' '\n+\t(\n+\t\tcd top/l1/l2 &&\n+\t\tgit ls-files -o --exclude-standard\n+\t) >actual &&\n+\t>expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'subdirectory ignore (l1)' '\n+\t(\n+\t\tcd top/l1 &&\n+\t\tgit ls-files -o --exclude-standard\n+\t) >actual &&\n+\t>expect &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n"},{"id":"131141","messageId":"7veim0w68q.fsf_-_@alter.siamese.dyndns.org","threadId":"22131","inReplyTo":"7vvdfcfjxo.fsf@alter.siamese.dyndns.org","subject":"[PATCH] ls-files: fix overeager pathspec optimization","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-01-08T23:01:57Z","receivedAt":"2010-01-08T23:01:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Given pathspecs that share a common prefix, ls-files optimized its call\ninto recursive directory reader by starting at the common prefix\ndirectory.\n\nIf you have a directory \"t\" with an untracked file \"t/junk\" in it, but the\ntop-level .gitignore file told us to ignore \"t/\", this resulted in an\nunexpected behaviour:\n\n    $ git ls-files -o --exclude-standard\n    $ cd t && git ls-files -o --exclude-standard\n    junk\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\nJunio C Hamano <gitster@pobox.com> writes:\n\n>> I think a proper fix should be in dir.c::read_directory() where it calls\n>> read_directory_recursive() without first checking if the directory itself\n>> should be ignored.  read_directory_recursive() itself has a logic to see\n>> if the dirent it found is worth recursing into and a similar logic should\n>> be in the toplevel caller.\n\nSo here it is.\n\n dir.c                              |   37 ++++++++++++++++++++++++++++++++++\n t/t3001-ls-files-others-exclude.sh |   39 ++++++++++++++++++++++++++++++++++++\n 2 files changed, 76 insertions(+), 0 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex f9ac454..9316eb6 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -787,6 +787,40 @@ static void free_simplify(struct path_simplify *simplify)\n \tfree(simplify);\n }\n \n+static int has_leading_ignored_dir(struct dir_struct *dir,\n+\t\t\t\t   const char *path_, int len,\n+\t\t\t\t   const struct path_simplify *simplify)\n+{\n+\tint at, dtype = DT_DIR;\n+\tchar path[PATH_MAX];\n+\n+\tat = 0;\n+\tmemcpy(path, path_, len);\n+\twhile (1) {\n+\t\tchar *cp;\n+\t\tpath[at] = '\\0';\n+\t\t/*\n+\t\t * NOTE! NOTE! NOTE!: we might want to actually lstat(2)\n+\t\t * path[] to make sure it is a directory.\n+\t\t */\n+\t\tif (excluded(dir, path, &dtype))\n+\t\t\treturn 1;\n+\t\tif (at < len) {\n+\t\t\tpath[at] = path_[at];\n+\t\t\tif (at < len && path[at] == '/')\n+\t\t\t\tat++;\n+\t\t}\n+\t\tif (len <= at)\n+\t\t\tbreak;\n+\t\tcp = memchr(path + at, '/', len - at);\n+\t\tif (!cp)\n+\t\t\tat = len;\n+\t\telse\n+\t\t\tat = cp - path;\n+\t}\n+\treturn 0;\n+}\n+\n int read_directory(struct dir_struct *dir, const char *path, int len, const char **pathspec)\n {\n \tstruct path_simplify *simplify;\n@@ -795,6 +829,9 @@ int read_directory(struct dir_struct *dir, const char *path, int len, const char\n \t\treturn dir->nr;\n \n \tsimplify = create_simplify(pathspec);\n+\tif (has_leading_ignored_dir(dir, path, len, simplify))\n+\t\treturn dir->nr;\n+\n \tread_directory_recursive(dir, path, len, 0, simplify);\n \tfree_simplify(simplify);\n \tqsort(dir->entries, dir->nr, sizeof(struct dir_entry *), cmp_name);\ndiff --git a/t/t3001-ls-files-others-exclude.sh b/t/t3001-ls-files-others-exclude.sh\nindex c65bca8..9e71260 100755\n--- a/t/t3001-ls-files-others-exclude.sh\n+++ b/t/t3001-ls-files-others-exclude.sh\n@@ -153,4 +153,43 @@ test_expect_success 'negated exclude matches can override previous ones' '\n \tgrep \"^a.1\" output\n '\n \n+test_expect_success 'subdirectory ignore (setup)' '\n+\tmkdir -p top/l1/l2 &&\n+\t(\n+\t\tcd top &&\n+\t\tgit init &&\n+\t\techo /.gitignore >.gitignore &&\n+\t\techo l1 >>.gitignore &&\n+\t\techo l2 >l1/.gitignore &&\n+\t\t>l1/l2/l1\n+\t)\n+'\n+\n+test_expect_success 'subdirectory ignore (toplevel)' '\n+\t(\n+\t\tcd top &&\n+\t\tgit ls-files -o --exclude-standard\n+\t) >actual &&\n+\t>expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'subdirectory ignore (l1/l2)' '\n+\t(\n+\t\tcd top/l1/l2 &&\n+\t\tgit ls-files -o --exclude-standard\n+\t) >actual &&\n+\t>expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'subdirectory ignore (l1)' '\n+\t(\n+\t\tcd top/l1 &&\n+\t\tgit ls-files -o --exclude-standard\n+\t) >actual &&\n+\t>expect &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n1.6.6.209.g52296.dirty\n"},{"id":"131148","messageId":"alpine.LFD.2.00.1001081520240.7821@localhost.localdomain","threadId":"22131","inReplyTo":"7veim0w68q.fsf_-_@alter.siamese.dyndns.org","subject":"Re: [PATCH] ls-files: fix overeager pathspec optimization","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2010-01-08T23:24:13Z","receivedAt":"2010-01-08T23:24:13Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Fri, 8 Jan 2010, Junio C Hamano wrote:\n>\n> Given pathspecs that share a common prefix, ls-files optimized its call\n> into recursive directory reader by starting at the common prefix\n> directory.\n> \n> If you have a directory \"t\" with an untracked file \"t/junk\" in it, but the\n> top-level .gitignore file told us to ignore \"t/\", this resulted in an\n> unexpected behaviour:\n\nOk, I'm not sure how \"unexpected\" this is, since arguably you are \noverriding the ignore file by _being_ in that directory (the same way \nindex contents override ignore files), but I could go either way on that.\n\nYour patch looks fine, although I think you did this in a very odd way.\n\n> +\tat = 0;\n> +\tmemcpy(path, path_, len);\n> +\twhile (1) {\n> +\t\tchar *cp;\n> +\t\tpath[at] = '\\0';\n> +\t\t/*\n> +\t\t * NOTE! NOTE! NOTE!: we might want to actually lstat(2)\n> +\t\t * path[] to make sure it is a directory.\n> +\t\t */\n> +\t\tif (excluded(dir, path, &dtype))\n> +\t\t\treturn 1;\n\nThe above starts by testing the empty string, and then after that test it \ngoes on to the next directory component. That is just _odd_.\n\nWouldn't it be more natural to write the loop the other way around, ie \n_first_ look up the next directory component, and _then_ do the exclude \nprocessing for thoose components? \n\nOr is there some subtle reason I'm missing for actually checking the empty \nname?\n\n\t\t\t\tLinus\n"},{"id":"131149","messageId":"7vwrzsp413.fsf@alter.siamese.dyndns.org","threadId":"22131","inReplyTo":"alpine.LFD.2.00.1001081520240.7821@localhost.localdomain","subject":"Re: [PATCH] ls-files: fix overeager pathspec optimization","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-01-08T23:31:36Z","receivedAt":"2010-01-08T23:31:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@linux-foundation.org> writes:\n\n> On Fri, 8 Jan 2010, Junio C Hamano wrote:\n>>\n>> Given pathspecs that share a common prefix, ls-files optimized its call\n>> into recursive directory reader by starting at the common prefix\n>> directory.\n>> \n>> If you have a directory \"t\" with an untracked file \"t/junk\" in it, but the\n>> top-level .gitignore file told us to ignore \"t/\", this resulted in an\n>> unexpected behaviour:\n>\n> Ok, I'm not sure how \"unexpected\" this is, since arguably you are \n> overriding the ignore file by _being_ in that directory (the same way \n> index contents override ignore files), but I could go either way on that.\n>\n> Your patch looks fine, although I think you did this in a very odd way.\n>\n>> +\tat = 0;\n>> +\tmemcpy(path, path_, len);\n>> +\twhile (1) {\n>> +\t\tchar *cp;\n>> +\t\tpath[at] = '\\0';\n>> +\t\t/*\n>> +\t\t * NOTE! NOTE! NOTE!: we might want to actually lstat(2)\n>> +\t\t * path[] to make sure it is a directory.\n>> +\t\t */\n>> +\t\tif (excluded(dir, path, &dtype))\n>> +\t\t\treturn 1;\n>\n> The above starts by testing the empty string, and then after that test it \n> goes on to the next directory component. That is just _odd_.\n>\n> Wouldn't it be more natural to write the loop the other way around, ie \n> _first_ look up the next directory component, and _then_ do the exclude \n> processing for thoose components? \n>\n> Or is there some subtle reason I'm missing for actually checking the empty \n> name?\n\nNo, just being paranoid in case somebody managed to .gitignore the\ntop-level of the working tree ;-)\n"},{"id":"131150","messageId":"7v3a2gnnv2.fsf@alter.siamese.dyndns.org","threadId":"22131","inReplyTo":"alpine.LFD.2.00.1001081520240.7821@localhost.localdomain","subject":"Re: [PATCH] ls-files: fix overeager pathspec optimization","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-01-09T00:06:09Z","receivedAt":"2010-01-09T00:06:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@linux-foundation.org> writes:\n\n> On Fri, 8 Jan 2010, Junio C Hamano wrote:\n>>\n>> Given pathspecs that share a common prefix, ls-files optimized its call\n>> into recursive directory reader by starting at the common prefix\n>> directory.\n>> \n>> If you have a directory \"t\" with an untracked file \"t/junk\" in it, but the\n>> top-level .gitignore file told us to ignore \"t/\", this resulted in an\n>> unexpected behaviour:\n>\n> Ok, I'm not sure how \"unexpected\" this is, since arguably you are \n> overriding the ignore file by _being_ in that directory (the same way \n> index contents override ignore files), but I could go either way on that.\n>\n> Your patch looks fine, although I think you did this in a very odd way.\n\nActually, there is some funny interaction with \"git add\" I ran out of time\nto figure out what the right fix should be:\n\n    $ git init new\n    $ cd new\n    $ mkdir t\n    $ >kuzu\n    $ >t/gomi\n    $ echo t >.gitignore\n    $ echo kuzu >>.gitignore\n    $ git add kuzu\n    The following paths are ignored...\n    kuzu\n    Use -f if you really want to add them.\n    $ git add t/gomi\n\nSince \"git add\" uses the exact same codepath to find the untracked files\nthat match the pathspecs, I expected the \"fix\" will make it complain about\n\"t/gomi\" being ignored.  Not so.\n\nActually, it does start ignoring t/gomi (the index does not have t/gomi\nafter the above sequence with the patch), but lack of the error message\nmakes it a rather unfortunate regression---it works as specified in the\nsense that ignored paths are not added to the index unless --forced, but\nit does so without telling the user about it.\n"},{"id":"131151","messageId":"alpine.LFD.2.00.1001081619570.7821@localhost.localdomain","threadId":"22131","inReplyTo":"7v3a2gnnv2.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] ls-files: fix overeager pathspec optimization","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2010-01-09T00:24:21Z","receivedAt":"2010-01-09T00:24:21Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Fri, 8 Jan 2010, Junio C Hamano wrote:\n> \n> Since \"git add\" uses the exact same codepath to find the untracked files\n> that match the pathspecs, I expected the \"fix\" will make it complain about\n> \"t/gomi\" being ignored.  Not so.\n\nNo. Since 't' is ignored, it won't even do a readdir() in there.\n\nAnd since it didn't do a readdir() in there, it won't find any files in \nthere to add to the ignored list.\n\nAnd that's how the \"git add\" logic for \"apparently ignored\" files works: \nit looks at whether the list of ignored files is empty or not.\n\n> Actually, it does start ignoring t/gomi (the index does not have t/gomi\n> after the above sequence with the patch), but lack of the error message\n> makes it a rather unfortunate regression---it works as specified in the\n> sense that ignored paths are not added to the index unless --forced, but\n> it does so without telling the user about it.\n\nI have this memory that _used_ to have a per-filename flag in \"git add\" \nthat checked if that particular filename component was used or not. But \nnow it just looks at 'dir->ignored_nr' and 'dir->ignored[]'.\n\n[ Digging back in history.. Yes: commit e96980ef ]\n\n\t\tLinus\n"},{"id":"131154","messageId":"7v8wc8kshh.fsf@alter.siamese.dyndns.org","threadId":"22131","inReplyTo":"alpine.LFD.2.00.1001081619570.7821@localhost.localdomain","subject":"Re: [PATCH] ls-files: fix overeager pathspec optimization","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-01-09T00:54:34Z","receivedAt":"2010-01-09T00:54:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@linux-foundation.org> writes:\n\n> I have this memory that _used_ to have a per-filename flag in \"git add\" \n> that checked if that particular filename component was used or not. But \n> now it just looks at 'dir->ignored_nr' and 'dir->ignored[]'.\n\nYes, and the previous patch wasn't adding what is ignored to the array, so\nhere is a re-roll to fix that in addition to the fix to \"should the loop\nstart from checking an empty path?\" issue you noticed.\n\nBut I am starting to wonder if we might be better off restructuring\nread_directory_recursive().  Currently it assumes that the path it was\ngiven _must_ be of interest (i.e. not ignored) and runs excluded() on\nsubdirectories it finds to make that same decision before recursing into\nthem or skipping them.  It might make more sense if it first checked if\nthe path given by the caller should be ignored and act accordingly. If it\nis to be ignored, perhaps it will simply return without doing anything\n(normal case), or it will return but adds the path to ignored[]\n(DIR_COLLECT_IGNORED case), or it will recurse into itself but tell it\nthat everything it finds is to be ignored.  I dunno...\n\n-- >8 --\nSubject: [PATCH (v3)] ls-files: fix overeager pathspec optimization\n\nGiven pathspecs that share a common prefix, ls-files optimized its call\ninto recursive directory reader by starting at the common prefix\ndirectory.\n\nIf you have a directory \"t\" with an untracked file \"t/junk\" in it, but the\ntop-level .gitignore file told us to ignore \"t/\", this resulted in:\n\n    $ git ls-files -o --exclude-standard\n    $ git ls-files -o --exclude-standard t/\n    t/junk\n    $ git ls-files -o --exclude-standard t/junk\n    t/junk\n    $ cd t && git ls-files -o --exclude-standard\n    junk\n\nWe could argue that you are overriding the ignore file by giving a\npatchspec that matches or being in that directory, but it is somewhat\nunexpected.  Worse yet, these behave differently:\n\n    $ git ls-files -o --exclude-standard t/ .\n    $ git ls-files -o --exclude-standard t/\n    t/junk\n\nThis patch changes the optimization so that it notices when the common\nprefix directory that it starts reading from is an ignored one.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n dir.c                              |   36 +++++++++++++++++++++++++++++++++\n t/t3001-ls-files-others-exclude.sh |   39 ++++++++++++++++++++++++++++++++++++\n 2 files changed, 75 insertions(+), 0 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex d0999ba..e8be909 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -778,6 +778,39 @@ static void free_simplify(struct path_simplify *simplify)\n \tfree(simplify);\n }\n \n+static int has_leading_ignored_dir(struct dir_struct *dir,\n+\t\t\t\t   const char *path_, int len,\n+\t\t\t\t   const struct path_simplify *simplify)\n+{\n+\tint dtype = DT_DIR;\n+\tchar path[PATH_MAX], *cp = path;\n+\n+\tmemcpy(path, path_, len);\n+\twhile (1) {\n+\t\tchar *next = memchr(cp, '/', path + len - cp);\n+\t\tint exclude;\n+\n+\t\t/*\n+\t\t * NOTE! NOTE! NOTE!: we might want to actually lstat(2)\n+\t\t * path[] to make sure it is a directory.\n+\t\t */\n+\t\tif (next)\n+\t\t\t*next = '\\0';\n+\t\texclude = excluded(dir, path, &dtype);\n+\t\tif (next)\n+\t\t\t*next = '/';\n+\t\tif (exclude) {\n+\t\t\tif (dir->flags & DIR_COLLECT_IGNORED)\n+\t\t\t\tdir_add_ignored(dir, path, len);\n+\t\t\treturn 1;\n+\t\t}\n+\t\tif (!next)\n+\t\t\tbreak;\n+\t\tcp = next + 1;\n+\t}\n+\treturn 0;\n+}\n+\n int read_directory(struct dir_struct *dir, const char *path, int len, const char **pathspec)\n {\n \tstruct path_simplify *simplify;\n@@ -786,6 +819,9 @@ int read_directory(struct dir_struct *dir, const char *path, int len, const char\n \t\treturn dir->nr;\n \n \tsimplify = create_simplify(pathspec);\n+\tif (has_leading_ignored_dir(dir, path, len, simplify))\n+\t\treturn dir->nr;\n+\n \tread_directory_recursive(dir, path, len, 0, simplify);\n \tfree_simplify(simplify);\n \tqsort(dir->entries, dir->nr, sizeof(struct dir_entry *), cmp_name);\ndiff --git a/t/t3001-ls-files-others-exclude.sh b/t/t3001-ls-files-others-exclude.sh\nindex c65bca8..9e71260 100755\n--- a/t/t3001-ls-files-others-exclude.sh\n+++ b/t/t3001-ls-files-others-exclude.sh\n@@ -153,4 +153,43 @@ test_expect_success 'negated exclude matches can override previous ones' '\n \tgrep \"^a.1\" output\n '\n \n+test_expect_success 'subdirectory ignore (setup)' '\n+\tmkdir -p top/l1/l2 &&\n+\t(\n+\t\tcd top &&\n+\t\tgit init &&\n+\t\techo /.gitignore >.gitignore &&\n+\t\techo l1 >>.gitignore &&\n+\t\techo l2 >l1/.gitignore &&\n+\t\t>l1/l2/l1\n+\t)\n+'\n+\n+test_expect_success 'subdirectory ignore (toplevel)' '\n+\t(\n+\t\tcd top &&\n+\t\tgit ls-files -o --exclude-standard\n+\t) >actual &&\n+\t>expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'subdirectory ignore (l1/l2)' '\n+\t(\n+\t\tcd top/l1/l2 &&\n+\t\tgit ls-files -o --exclude-standard\n+\t) >actual &&\n+\t>expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'subdirectory ignore (l1)' '\n+\t(\n+\t\tcd top/l1 &&\n+\t\tgit ls-files -o --exclude-standard\n+\t) >actual &&\n+\t>expect &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n1.6.6.209.g52296.dirty\n"},{"id":"131158","messageId":"alpine.LFD.2.00.1001081701570.7821@localhost.localdomain","threadId":"22131","inReplyTo":"7v8wc8kshh.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] ls-files: fix overeager pathspec optimization","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2010-01-09T01:07:46Z","receivedAt":"2010-01-09T01:07:46Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Fri, 8 Jan 2010, Junio C Hamano wrote:\n> \n> Yes, and the previous patch wasn't adding what is ignored to the array, so\n> here is a re-roll to fix that in addition to the fix to \"should the loop\n> start from checking an empty path?\" issue you noticed.\n\nAck. Looks ok to me, and I think it's a lot more obvious.\n\n> But I am starting to wonder if we might be better off restructuring\n> read_directory_recursive().  Currently it assumes that the path it was\n> given _must_ be of interest (i.e. not ignored) and runs excluded() on\n> subdirectories it finds to make that same decision before recursing into\n> them or skipping them.  It might make more sense if it first checked if\n> the path given by the caller should be ignored and act accordingly.\n\nHmm. I can't make myself care one way or the other, I have to admit.  I \nassume you mean basically taking the path and using the first component of \nit _instead_ of doing a readdir() - and getting rid of the simplification \nup front?\n\nI agree that that should work. Would it be simpler and cleaner? Perhaps. \nI'd have to see both patches to be able to tell. I do admit that while I \nacked your patch, it sure ain't _pretty_ to do that special odd \n\"has_leading_ignored_dir()\" thing.\n\n\t\tLinus\n"},{"id":"131167","messageId":"20100109054240.GA13321@coredump.intra.peff.net","threadId":"22131","inReplyTo":"alpine.LFD.2.00.1001081701570.7821@localhost.localdomain","subject":"Re: [PATCH] ls-files: fix overeager pathspec optimization","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-01-09T05:42:40Z","receivedAt":"2010-01-09T05:42:40Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jan 08, 2010 at 05:07:46PM -0800, Linus Torvalds wrote:\n\n> > But I am starting to wonder if we might be better off restructuring\n> > read_directory_recursive().  Currently it assumes that the path it was\n> > given _must_ be of interest (i.e. not ignored) and runs excluded() on\n> > subdirectories it finds to make that same decision before recursing into\n> > them or skipping them.  It might make more sense if it first checked if\n> > the path given by the caller should be ignored and act accordingly.\n> \n> Hmm. I can't make myself care one way or the other, I have to admit.  I \n> assume you mean basically taking the path and using the first component of \n> it _instead_ of doing a readdir() - and getting rid of the simplification \n> up front?\n> \n> I agree that that should work. Would it be simpler and cleaner? Perhaps. \n> I'd have to see both patches to be able to tell. I do admit that while I \n> acked your patch, it sure ain't _pretty_ to do that special odd \n> \"has_leading_ignored_dir()\" thing.\n\nIt would look something like this:\n\ndiff --git a/dir.c b/dir.c\nindex 3a8d3e6..306d354 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -811,12 +811,19 @@ static void free_simplify(struct path_simplify *simplify)\n int read_directory(struct dir_struct *dir, const char *path, int len, const char **pathspec)\n {\n \tstruct path_simplify *simplify;\n+\tint d_type = DT_DIR;\n+\tint exclude;\n \n \tif (has_symlink_leading_path(path, len))\n \t\treturn dir->nr;\n \n \tsimplify = create_simplify(pathspec);\n-\tread_directory_recursive(dir, path, len, 0, simplify);\n+\texclude = excluded(dir, path, &d_type);\n+\tif (exclude && (dir->flags & DIR_COLLECT_IGNORED) &&\n+\t    in_pathspec(path, len, simplify))\n+\t\tdir_add_ignored(dir, path, len);\n+\tif (!exclude || (dir->flags & DIR_SHOW_IGNORED))\n+\t\tread_directory_recursive(dir, path, len, 0, simplify);\n \tfree_simplify(simplify);\n \tqsort(dir->entries, dir->nr, sizeof(struct dir_entry *), cmp_name);\n \tqsort(dir->ignored, dir->ignored_nr, sizeof(struct dir_entry *), cmp_name);\n\nBut unfortunately excluded() is not happy with the trailing slash on the\npath given to read_directory, so we also need on top:\n\ndiff --git a/dir.c b/dir.c\nindex 306d354..6045a84 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -813,12 +813,17 @@ int read_directory(struct dir_struct *dir, const char *path, int len, const char\n \tstruct path_simplify *simplify;\n \tint d_type = DT_DIR;\n \tint exclude;\n+\tchar *path_without_slash;\n \n \tif (has_symlink_leading_path(path, len))\n \t\treturn dir->nr;\n \n \tsimplify = create_simplify(pathspec);\n-\texclude = excluded(dir, path, &d_type);\n+\tpath_without_slash = xstrdup(path);\n+\tif (path_without_slash[strlen(path_without_slash)-1] == '/')\n+\t\tpath_without_slash[strlen(path_without_slash)-1] = '\\0';\n+\texclude = excluded(dir, path_without_slash, &d_type);\n+\tfree(path_without_slash);\n \tif (exclude && (dir->flags & DIR_COLLECT_IGNORED) &&\n \t    in_pathspec(path, len, simplify))\n \t\tdir_add_ignored(dir, path, len);\n\nAnd that does fix the case that triggered this whole discussion, but I\nhaven't tested thoroughly to make sure we are not adversely affecting\nother cases.\n\n-Peff\n"},{"id":"131171","messageId":"7veilzaguf.fsf@alter.siamese.dyndns.org","threadId":"22131","inReplyTo":"alpine.LFD.2.00.1001081701570.7821@localhost.localdomain","subject":"Re: [PATCH] ls-files: fix overeager pathspec optimization","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-01-09T07:16:08Z","receivedAt":"2010-01-09T07:16:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@linux-foundation.org> writes:\n\n> I'd have to see both patches to be able to tell. I do admit that while I\n> acked your patch, it sure ain't _pretty_ to do that special odd\n> \"has_leading_ignored_dir()\" thing.\n\nRevised patch (v4) series is coming shortly.\n"},{"id":"131172","messageId":"1263022535-12822-1-git-send-email-gitster@pobox.com","threadId":"22131","inReplyTo":"7veilzaguf.fsf@alter.siamese.dyndns.org","subject":"[PATCH 1/4] t3001: test ls-files -o ignored/dir","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-01-09T07:35:32Z","receivedAt":"2010-01-09T07:35:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"When you have \"t\" directory that is marked as ignored in the top-level\n.gitignore file (or $GIT_DIR/info/exclude), running\n\n    $ git ls-files -o --exclude-standard\n\nfrom the top-level correctly excludes files in \"t\" directory, but\nany of the following:\n\n    $ git ls-files -o --exclude-standard t/\n    $ cd t && git ls-files -o --exclude-standard\n\nwould show untracked files in that directory.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n t/t3001-ls-files-others-exclude.sh |   39 ++++++++++++++++++++++++++++++++++++\n 1 files changed, 39 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t3001-ls-files-others-exclude.sh b/t/t3001-ls-files-others-exclude.sh\nindex c65bca8..e3e4d71 100755\n--- a/t/t3001-ls-files-others-exclude.sh\n+++ b/t/t3001-ls-files-others-exclude.sh\n@@ -153,4 +153,43 @@ test_expect_success 'negated exclude matches can override previous ones' '\n \tgrep \"^a.1\" output\n '\n \n+test_expect_success 'subdirectory ignore (setup)' '\n+\tmkdir -p top/l1/l2 &&\n+\t(\n+\t\tcd top &&\n+\t\tgit init &&\n+\t\techo /.gitignore >.gitignore &&\n+\t\techo l1 >>.gitignore &&\n+\t\techo l2 >l1/.gitignore &&\n+\t\t>l1/l2/l1\n+\t)\n+'\n+\n+test_expect_success 'subdirectory ignore (toplevel)' '\n+\t(\n+\t\tcd top &&\n+\t\tgit ls-files -o --exclude-standard\n+\t) >actual &&\n+\t>expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'subdirectory ignore (l1/l2)' '\n+\t(\n+\t\tcd top/l1/l2 &&\n+\t\tgit ls-files -o --exclude-standard\n+\t) >actual &&\n+\t>expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_failure 'subdirectory ignore (l1)' '\n+\t(\n+\t\tcd top/l1 &&\n+\t\tgit ls-files -o --exclude-standard\n+\t) >actual &&\n+\t>expect &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n1.6.6.209.g52296.dirty\n"},{"id":"131173","messageId":"1263022535-12822-2-git-send-email-gitster@pobox.com","threadId":"22131","inReplyTo":"1263022535-12822-1-git-send-email-gitster@pobox.com","subject":"[PATCH 2/4] read_directory_recursive(): refactor handling of a single path into a separate function","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-01-09T07:35:33Z","receivedAt":"2010-01-09T07:35:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Primarily because I want to reuse it in a separate function later,\nbut this de-dents a huge function by one tabstop which by itself is\nan improvement as well.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n dir.c |  153 ++++++++++++++++++++++++++++++++++++++---------------------------\n 1 files changed, 90 insertions(+), 63 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex d0999ba..dec8365 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -625,6 +625,84 @@ static int get_dtype(struct dirent *de, const char *path, int len)\n \treturn dtype;\n }\n \n+enum path_treatment {\n+\tpath_ignored,\n+\tpath_handled,\n+\tpath_recurse,\n+};\n+\n+static enum path_treatment treat_path(struct dir_struct *dir,\n+\t\t\t\t      struct dirent *de,\n+\t\t\t\t      char *path, int path_max,\n+\t\t\t\t      int baselen,\n+\t\t\t\t      const struct path_simplify *simplify,\n+\t\t\t\t      int *len)\n+{\n+\tint dtype, exclude;\n+\n+\tif (is_dot_or_dotdot(de->d_name) || !strcmp(de->d_name, \".git\"))\n+\t\treturn path_ignored;\n+\t*len = strlen(de->d_name);\n+\t/* Ignore overly long pathnames! */\n+\tif (*len + baselen + 8 > path_max)\n+\t\treturn path_ignored;\n+\tmemcpy(path + baselen, de->d_name, *len + 1);\n+\t*len += baselen;\n+\tif (simplify_away(path, *len, simplify))\n+\t\treturn path_ignored;\n+\n+\tdtype = DTYPE(de);\n+\texclude = excluded(dir, path, &dtype);\n+\tif (exclude && (dir->flags & DIR_COLLECT_IGNORED)\n+\t    && in_pathspec(path, *len, simplify))\n+\t\tdir_add_ignored(dir, path, *len);\n+\n+\t/*\n+\t * Excluded? If we don't explicitly want to show\n+\t * ignored files, ignore it\n+\t */\n+\tif (exclude && !(dir->flags & DIR_SHOW_IGNORED))\n+\t\treturn path_ignored;\n+\n+\tif (dtype == DT_UNKNOWN)\n+\t\tdtype = get_dtype(de, path, *len);\n+\n+\t/*\n+\t * Do we want to see just the ignored files?\n+\t * We still need to recurse into directories,\n+\t * even if we don't ignore them, since the\n+\t * directory may contain files that we do..\n+\t */\n+\tif (!exclude && (dir->flags & DIR_SHOW_IGNORED)) {\n+\t\tif (dtype != DT_DIR)\n+\t\t\treturn path_ignored;\n+\t}\n+\n+\tswitch (dtype) {\n+\tdefault:\n+\t\treturn path_ignored;\n+\tcase DT_DIR:\n+\t\tmemcpy(path + *len, \"/\", 2);\n+\t\t(*len)++;\n+\t\tswitch (treat_directory(dir, path, *len, simplify)) {\n+\t\tcase show_directory:\n+\t\t\tif (exclude != !!(dir->flags\n+\t\t\t\t\t  & DIR_SHOW_IGNORED))\n+\t\t\t\treturn path_ignored;\n+\t\t\tbreak;\n+\t\tcase recurse_into_directory:\n+\t\t\treturn path_recurse;\n+\t\tcase ignore_directory:\n+\t\t\treturn path_ignored;\n+\t\t}\n+\t\tbreak;\n+\tcase DT_REG:\n+\tcase DT_LNK:\n+\t\tbreak;\n+\t}\n+\treturn path_handled;\n+}\n+\n /*\n  * Read a directory tree. We currently ignore anything but\n  * directories, regular files and symlinks. That's because git\n@@ -634,7 +712,10 @@ static int get_dtype(struct dirent *de, const char *path, int len)\n  * Also, we ignore the name \".git\" (even if it is not a directory).\n  * That likely will not change.\n  */\n-static int read_directory_recursive(struct dir_struct *dir, const char *base, int baselen, int check_only, const struct path_simplify *simplify)\n+static int read_directory_recursive(struct dir_struct *dir,\n+\t\t\t\t    const char *base, int baselen,\n+\t\t\t\t    int check_only,\n+\t\t\t\t    const struct path_simplify *simplify)\n {\n \tDIR *fdir = opendir(*base ? base : \".\");\n \tint contents = 0;\n@@ -645,70 +726,16 @@ static int read_directory_recursive(struct dir_struct *dir, const char *base, in\n \t\tmemcpy(path, base, baselen);\n \n \t\twhile ((de = readdir(fdir)) != NULL) {\n-\t\t\tint len, dtype;\n-\t\t\tint exclude;\n-\n-\t\t\tif (is_dot_or_dotdot(de->d_name) ||\n-\t\t\t     !strcmp(de->d_name, \".git\"))\n-\t\t\t\tcontinue;\n-\t\t\tlen = strlen(de->d_name);\n-\t\t\t/* Ignore overly long pathnames! */\n-\t\t\tif (len + baselen + 8 > sizeof(path))\n+\t\t\tint len;\n+\t\t\tswitch (treat_path(dir, de, path, sizeof(path),\n+\t\t\t\t\t   baselen, simplify, &len)) {\n+\t\t\tcase path_recurse:\n+\t\t\t\tcontents += read_directory_recursive\n+\t\t\t\t\t(dir, path, len, 0, simplify);\n \t\t\t\tcontinue;\n-\t\t\tmemcpy(path + baselen, de->d_name, len+1);\n-\t\t\tlen = baselen + len;\n-\t\t\tif (simplify_away(path, len, simplify))\n+\t\t\tcase path_ignored:\n \t\t\t\tcontinue;\n-\n-\t\t\tdtype = DTYPE(de);\n-\t\t\texclude = excluded(dir, path, &dtype);\n-\t\t\tif (exclude && (dir->flags & DIR_COLLECT_IGNORED)\n-\t\t\t    && in_pathspec(path, len, simplify))\n-\t\t\t\tdir_add_ignored(dir, path,len);\n-\n-\t\t\t/*\n-\t\t\t * Excluded? If we don't explicitly want to show\n-\t\t\t * ignored files, ignore it\n-\t\t\t */\n-\t\t\tif (exclude && !(dir->flags & DIR_SHOW_IGNORED))\n-\t\t\t\tcontinue;\n-\n-\t\t\tif (dtype == DT_UNKNOWN)\n-\t\t\t\tdtype = get_dtype(de, path, len);\n-\n-\t\t\t/*\n-\t\t\t * Do we want to see just the ignored files?\n-\t\t\t * We still need to recurse into directories,\n-\t\t\t * even if we don't ignore them, since the\n-\t\t\t * directory may contain files that we do..\n-\t\t\t */\n-\t\t\tif (!exclude && (dir->flags & DIR_SHOW_IGNORED)) {\n-\t\t\t\tif (dtype != DT_DIR)\n-\t\t\t\t\tcontinue;\n-\t\t\t}\n-\n-\t\t\tswitch (dtype) {\n-\t\t\tdefault:\n-\t\t\t\tcontinue;\n-\t\t\tcase DT_DIR:\n-\t\t\t\tmemcpy(path + len, \"/\", 2);\n-\t\t\t\tlen++;\n-\t\t\t\tswitch (treat_directory(dir, path, len, simplify)) {\n-\t\t\t\tcase show_directory:\n-\t\t\t\t\tif (exclude != !!(dir->flags\n-\t\t\t\t\t\t\t& DIR_SHOW_IGNORED))\n-\t\t\t\t\t\tcontinue;\n-\t\t\t\t\tbreak;\n-\t\t\t\tcase recurse_into_directory:\n-\t\t\t\t\tcontents += read_directory_recursive(dir,\n-\t\t\t\t\t\tpath, len, 0, simplify);\n-\t\t\t\t\tcontinue;\n-\t\t\t\tcase ignore_directory:\n-\t\t\t\t\tcontinue;\n-\t\t\t\t}\n-\t\t\t\tbreak;\n-\t\t\tcase DT_REG:\n-\t\t\tcase DT_LNK:\n+\t\t\tcase path_handled:\n \t\t\t\tbreak;\n \t\t\t}\n \t\t\tcontents++;\n-- \n1.6.6.209.g52296.dirty\n"},{"id":"131174","messageId":"1263022535-12822-3-git-send-email-gitster@pobox.com","threadId":"22131","inReplyTo":"1263022535-12822-1-git-send-email-gitster@pobox.com","subject":"[PATCH 3/4] read_directory(): further split treat_path()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-01-09T07:35:34Z","receivedAt":"2010-01-09T07:35:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The next caller I'll be adding won't have an access to struct dirent\nbecause it won't be reading from a directory stream.  Split the main\npart of the function further into a separate function to make it usable\nby a caller without passing a dirent as long as it knows what type is\nfeeding the function.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n dir.c |   50 +++++++++++++++++++++++++++++---------------------\n 1 files changed, 29 insertions(+), 21 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex dec8365..35cc89b 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -631,28 +631,12 @@ enum path_treatment {\n \tpath_recurse,\n };\n \n-static enum path_treatment treat_path(struct dir_struct *dir,\n-\t\t\t\t      struct dirent *de,\n-\t\t\t\t      char *path, int path_max,\n-\t\t\t\t      int baselen,\n-\t\t\t\t      const struct path_simplify *simplify,\n-\t\t\t\t      int *len)\n+static enum path_treatment treat_one_path(struct dir_struct *dir,\n+\t\t\t\t\t  char *path, int *len,\n+\t\t\t\t\t  const struct path_simplify *simplify,\n+\t\t\t\t\t  int dtype, struct dirent *de)\n {\n-\tint dtype, exclude;\n-\n-\tif (is_dot_or_dotdot(de->d_name) || !strcmp(de->d_name, \".git\"))\n-\t\treturn path_ignored;\n-\t*len = strlen(de->d_name);\n-\t/* Ignore overly long pathnames! */\n-\tif (*len + baselen + 8 > path_max)\n-\t\treturn path_ignored;\n-\tmemcpy(path + baselen, de->d_name, *len + 1);\n-\t*len += baselen;\n-\tif (simplify_away(path, *len, simplify))\n-\t\treturn path_ignored;\n-\n-\tdtype = DTYPE(de);\n-\texclude = excluded(dir, path, &dtype);\n+\tint exclude = excluded(dir, path, &dtype);\n \tif (exclude && (dir->flags & DIR_COLLECT_IGNORED)\n \t    && in_pathspec(path, *len, simplify))\n \t\tdir_add_ignored(dir, path, *len);\n@@ -703,6 +687,30 @@ static enum path_treatment treat_path(struct dir_struct *dir,\n \treturn path_handled;\n }\n \n+static enum path_treatment treat_path(struct dir_struct *dir,\n+\t\t\t\t      struct dirent *de,\n+\t\t\t\t      char *path, int path_max,\n+\t\t\t\t      int baselen,\n+\t\t\t\t      const struct path_simplify *simplify,\n+\t\t\t\t      int *len)\n+{\n+\tint dtype;\n+\n+\tif (is_dot_or_dotdot(de->d_name) || !strcmp(de->d_name, \".git\"))\n+\t\treturn path_ignored;\n+\t*len = strlen(de->d_name);\n+\t/* Ignore overly long pathnames! */\n+\tif (*len + baselen + 8 > path_max)\n+\t\treturn path_ignored;\n+\tmemcpy(path + baselen, de->d_name, *len + 1);\n+\t*len += baselen;\n+\tif (simplify_away(path, *len, simplify))\n+\t\treturn path_ignored;\n+\n+\tdtype = DTYPE(de);\n+\treturn treat_one_path(dir, path, len, simplify, dtype, de);\n+}\n+\n /*\n  * Read a directory tree. We currently ignore anything but\n  * directories, regular files and symlinks. That's because git\n-- \n1.6.6.209.g52296.dirty\n"},{"id":"131175","messageId":"1263022535-12822-4-git-send-email-gitster@pobox.com","threadId":"22131","inReplyTo":"1263022535-12822-1-git-send-email-gitster@pobox.com","subject":"[PATCH 4/4] ls-files: fix overeager pathspec optimization","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-01-09T07:35:35Z","receivedAt":"2010-01-09T07:35:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Given pathspecs that share a common prefix, ls-files optimized its call\ninto recursive directory reader by starting at the common prefix\ndirectory.\n\nIf you have a directory \"t\" with an untracked file \"t/junk\" in it, but the\ntop-level .gitignore file told us to ignore \"t/\", this resulted in:\n\n    $ git ls-files -o --exclude-standard\n    $ git ls-files -o --exclude-standard t/\n    t/junk\n    $ git ls-files -o --exclude-standard t/junk\n    t/junk\n    $ cd t && git ls-files -o --exclude-standard\n    junk\n\nWe could argue that you are overriding the ignore file by giving a\npatchspec that matches or being in that directory, but it is somewhat\nunexpected.  Worse yet, these behave differently:\n\n    $ git ls-files -o --exclude-standard t/ .\n    $ git ls-files -o --exclude-standard t/\n    t/junk\n\nThis patch changes the optimization so that it notices when the common\nprefix directory that it starts reading from is an ignored one.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n dir.c                              |   38 +++++++++++++++++++++++++++++++++++-\n t/t3001-ls-files-others-exclude.sh |    2 +-\n 2 files changed, 38 insertions(+), 2 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex 35cc89b..00d698d 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -813,6 +813,41 @@ static void free_simplify(struct path_simplify *simplify)\n \tfree(simplify);\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 path_simplify *simplify)\n+{\n+\tchar pathbuf[PATH_MAX];\n+\tint baselen, blen;\n+\tconst char *cp;\n+\n+\twhile (len && path[len - 1] == '/')\n+\t\tlen--;\n+\tif (!len)\n+\t\treturn 1;\n+\tbaselen = 0;\n+\twhile (1) {\n+\t\tcp = path + baselen + !!baselen;\n+\t\tcp = memchr(cp, '/', path + len - cp);\n+\t\tif (!cp)\n+\t\t\tbaselen = len;\n+\t\telse\n+\t\t\tbaselen = cp - path;\n+\t\tmemcpy(pathbuf, path, baselen);\n+\t\tpathbuf[baselen] = '\\0';\n+\t\tif (!is_directory(pathbuf))\n+\t\t\treturn 0;\n+\t\tif (simplify_away(pathbuf, baselen, simplify))\n+\t\t\treturn 0;\n+\t\tblen = baselen;\n+\t\tif (treat_one_path(dir, pathbuf, &blen, simplify,\n+\t\t\t\t   DT_DIR, NULL) == path_ignored)\n+\t\t\treturn 0; /* do not recurse into it */\n+\t\tif (len <= baselen)\n+\t\t\treturn 1; /* finished checking */\n+\t}\n+}\n+\n int read_directory(struct dir_struct *dir, const char *path, int len, const char **pathspec)\n {\n \tstruct path_simplify *simplify;\n@@ -821,7 +856,8 @@ int read_directory(struct dir_struct *dir, const char *path, int len, const char\n \t\treturn dir->nr;\n \n \tsimplify = create_simplify(pathspec);\n-\tread_directory_recursive(dir, path, len, 0, simplify);\n+\tif (!len || treat_leading_path(dir, path, len, simplify))\n+\t\tread_directory_recursive(dir, path, len, 0, simplify);\n \tfree_simplify(simplify);\n \tqsort(dir->entries, dir->nr, sizeof(struct dir_entry *), cmp_name);\n \tqsort(dir->ignored, dir->ignored_nr, sizeof(struct dir_entry *), cmp_name);\ndiff --git a/t/t3001-ls-files-others-exclude.sh b/t/t3001-ls-files-others-exclude.sh\nindex e3e4d71..9e71260 100755\n--- a/t/t3001-ls-files-others-exclude.sh\n+++ b/t/t3001-ls-files-others-exclude.sh\n@@ -183,7 +183,7 @@ test_expect_success 'subdirectory ignore (l1/l2)' '\n \ttest_cmp expect actual\n '\n \n-test_expect_failure 'subdirectory ignore (l1)' '\n+test_expect_success 'subdirectory ignore (l1)' '\n \t(\n \t\tcd top/l1 &&\n \t\tgit ls-files -o --exclude-standard\n-- \n1.6.6.209.g52296.dirty\n"},{"id":"131177","messageId":"7vfx6f3dm3.fsf@alter.siamese.dyndns.org","threadId":"22131","inReplyTo":"7veilzaguf.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] ls-files: fix overeager pathspec optimization","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-01-09T08:07:48Z","receivedAt":"2010-01-09T08:07:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Linus Torvalds <torvalds@linux-foundation.org> writes:\n>\n>> I'd have to see both patches to be able to tell. I do admit that while I\n>> acked your patch, it sure ain't _pretty_ to do that special odd\n>> \"has_leading_ignored_dir()\" thing.\n>\n> Revised patch (v4) series is coming shortly.\n\nHaving sent these patches, I am wondering if the simplest fix might be this\none-liner.\n\ndiff --git a/dir.c b/dir.c\nindex d0999ba..7fba335 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -788,3 +788,3 @@ int read_directory(struct dir_struct *dir, const char *path, int len, const char\n \tsimplify = create_simplify(pathspec);\n-\tread_directory_recursive(dir, path, len, 0, simplify);\n+\tread_directory_recursive(dir, \"\", 0, 0, simplify);\n \tfree_simplify(simplify);\n\nWhat my series does is to keep all the \"depending on dir->flags and what\nexcluded() says for path, decide to recurse, add it to dir->ignored[],\netc.\" logic and optimize only the readdir() loop, pretending as if it\nreturned only the entry on the \"common prefix\" path and nothing else, as\nwe know all other paths will be skipped by either simplified away or\nfiltered by in_pathspec() check.\n\nIf the directories we are reading are not humongous, maybe using this much\nsimpler patch might be preferrable (although it is completely untested).\n"},{"id":"131188","messageId":"alpine.LFD.2.00.1001091004380.7821@localhost.localdomain","threadId":"22131","inReplyTo":"7vfx6f3dm3.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] ls-files: fix overeager pathspec optimization","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2010-01-09T18:05:44Z","receivedAt":"2010-01-09T18:05:44Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sat, 9 Jan 2010, Junio C Hamano wrote:\n> \n> Having sent these patches, I am wondering if the simplest fix might be this\n> one-liner.\n> \n> diff --git a/dir.c b/dir.c\n> index d0999ba..7fba335 100644\n> --- a/dir.c\n> +++ b/dir.c\n> @@ -788,3 +788,3 @@ int read_directory(struct dir_struct *dir, const char *path, int len, const char\n>  \tsimplify = create_simplify(pathspec);\n> -\tread_directory_recursive(dir, path, len, 0, simplify);\n> +\tread_directory_recursive(dir, \"\", 0, 0, simplify);\n>  \tfree_simplify(simplify);\n\nThat one-liner doesn't work at all for me.\n\nLookie here:\n\n\t[torvalds@nehalem linux]$ touch drivers/char/hello.c\n\t[torvalds@nehalem linux]$ ~/git/git ls-files --exclude-standard -o drivers/char\n\t[torvalds@nehalem linux]$ git ls-files --exclude-standard -o drivers/char\n\tdrivers/char/hello.c\n\nwhere that ~/git/git is the version with the one-liner.\n\nIOW, it now ignores _everything_, because the dir and the path don't \nmatch.\n\n\t\tLinus\n"},{"id":"131209","messageId":"7vmy0mfp2o.fsf@alter.siamese.dyndns.org","threadId":"22131","inReplyTo":"alpine.LFD.2.00.1001091004380.7821@localhost.localdomain","subject":"Re: [PATCH] ls-files: fix overeager pathspec optimization","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-01-10T06:31:43Z","receivedAt":"2010-01-10T06:31:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@linux-foundation.org> writes:\n\n> On Sat, 9 Jan 2010, Junio C Hamano wrote:\n>> \n>> Having sent these patches, I am wondering if the simplest fix might be this\n>> one-liner.\n>> \n>> diff --git a/dir.c b/dir.c\n>> index d0999ba..7fba335 100644\n>> --- a/dir.c\n>> +++ b/dir.c\n>> @@ -788,3 +788,3 @@ int read_directory(struct dir_struct *dir, const char *path, int len, const char\n>>  \tsimplify = create_simplify(pathspec);\n>> -\tread_directory_recursive(dir, path, len, 0, simplify);\n>> +\tread_directory_recursive(dir, \"\", 0, 0, simplify);\n>>  \tfree_simplify(simplify);\n>\n> That one-liner doesn't work at all for me.\n> ...\n> IOW, it now ignores _everything_, because the dir and the path don't \n> match.\n\nHmph, you are right.  The real series is not equivalent to the one-liner\nat all.\n"},{"id":"131390","messageId":"20100112163311.GD25092@coredump.intra.peff.net","threadId":"22131","inReplyTo":"1263022535-12822-4-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 4/4] ls-files: fix overeager pathspec optimization","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-01-12T16:33:11Z","receivedAt":"2010-01-12T16:33:11Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jan 08, 2010 at 11:35:35PM -0800, Junio C Hamano wrote:\n\n> This patch changes the optimization so that it notices when the common\n> prefix directory that it starts reading from is an ignored one.\n\nHaving just produced the similar but more ugly and messy patch earlier\nin the thread, this series looks right to me.\n\n-Peff\n"}]}