{"thread":{"id":"22722","subject":"'git add' regression in git-1.7?","startedAt":"2010-02-19T04:30:54Z","lastAt":"2010-03-15T02:02:53Z","messageCount":17,"participants":["SungHyun Nam","Avery Pennarun","Jeff King","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"135034","messageId":"hll45t$50o$1@ger.gmane.org","threadId":"22722","inReplyTo":null,"subject":"'git add' regression in git-1.7?","fromName":"SungHyun Nam","fromEmail":"goweol@gmail.com","sentAt":"2010-02-19T04:30:54Z","receivedAt":"2010-02-19T04:30:54Z","isPatch":false,"sender":{"key":"goweol@gmail.com","avatar":null},"body":"Hello,\n\n'git add' does NOT add files in ignored path.\n\nWhen the .gitignore file contains:\n     tmp/\nIf I do:\n     git add tmp/test.txt\nNothing happens.\n\nIf I removed 'tmp/' from the .gitignore, git add works\nfine.\n\nBecause I have backup copies of GIT, I tested serveral versions.\nThe version below works as expected.\n     git version 1.6.6.243.gff6d2\n\nAnd below does NOT work.\n     git version 1.7.0.rc1.7.gc0da5\n     git version 1.7.0.31.g1df487\n\nThanks,\nnamsh\n"},{"id":"135035","messageId":"32541b131002182042p610fce4ex96efbffea9afe2ed@mail.gmail.com","threadId":"22722","inReplyTo":"hll45t$50o$1@ger.gmane.org","subject":"Re: 'git add' regression in git-1.7?","fromName":"Avery Pennarun","fromEmail":"apenwarr@gmail.com","sentAt":"2010-02-19T04:42:48Z","receivedAt":"2010-02-19T04:42:48Z","isPatch":false,"sender":{"key":"apenwarr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/20592?v=4"},"body":"On Thu, Feb 18, 2010 at 11:30 PM, SungHyun Nam <goweol@gmail.com> wrote:\n> 'git add' does NOT add files in ignored path.\n>\n> When the .gitignore file contains:\n>    tmp/\n> If I do:\n>    git add tmp/test.txt\n> Nothing happens.\n\nTry using:\n     git add -f tmp/test.txt\n\nAvery\n"},{"id":"135036","messageId":"hll65c$87a$1@ger.gmane.org","threadId":"22722","inReplyTo":"32541b131002182042p610fce4ex96efbffea9afe2ed@mail.gmail.com","subject":"Re: 'git add' regression in git-1.7?","fromName":"SungHyun Nam","fromEmail":"goweol@gmail.com","sentAt":"2010-02-19T05:04:46Z","receivedAt":"2010-02-19T05:04:46Z","isPatch":false,"sender":{"key":"goweol@gmail.com","avatar":null},"body":"Avery Pennarun wrote:\n> On Thu, Feb 18, 2010 at 11:30 PM, SungHyun Nam<goweol@gmail.com>  wrote:\n>> 'git add' does NOT add files in ignored path.\n>>\n>> When the .gitignore file contains:\n>>     tmp/\n>> If I do:\n>>     git add tmp/test.txt\n>> Nothing happens.\n>\n> Try using:\n>       git add -f tmp/test.txt\n\nThanks, it works.\n\nWell, before sending the previous email, I checked the\nRelNotes-1.7.*.txt,  and could not find such a change by searching\n'git add'.  So, I thought it's a regression.\n\nRegards,\nnamsh\n"},{"id":"135037","messageId":"32541b131002182115t5501d0d1u19367a4d8e7627e4@mail.gmail.com","threadId":"22722","inReplyTo":"hll65c$87a$1@ger.gmane.org","subject":"Re: 'git add' regression in git-1.7?","fromName":"Avery Pennarun","fromEmail":"apenwarr@gmail.com","sentAt":"2010-02-19T05:15:02Z","receivedAt":"2010-02-19T05:15:02Z","isPatch":false,"sender":{"key":"apenwarr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/20592?v=4"},"body":"On Fri, Feb 19, 2010 at 12:04 AM, SungHyun Nam <goweol@gmail.com> wrote:\n> Well, before sending the previous email, I checked the\n> RelNotes-1.7.*.txt,  and could not find such a change by searching\n> 'git add'.  So, I thought it's a regression.\n\nAs far as I know, git add has refused to add ignored files for as long\nas I can remember.  Maybe there was briefly a bug in this behaviour\nthat was later fixed...\n\nIf you use 'git bisect' on the git repo, you could probably discover\nwhat happened, in case you're interested.\n\nAvery\n"},{"id":"135038","messageId":"20100219053431.GB22645@coredump.intra.peff.net","threadId":"22722","inReplyTo":"32541b131002182115t5501d0d1u19367a4d8e7627e4@mail.gmail.com","subject":"Re: 'git add' regression in git-1.7?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-02-19T05:34:31Z","receivedAt":"2010-02-19T05:34:31Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 19, 2010 at 12:15:02AM -0500, Avery Pennarun wrote:\n\n> On Fri, Feb 19, 2010 at 12:04 AM, SungHyun Nam <goweol@gmail.com> wrote:\n> > Well, before sending the previous email, I checked the\n> > RelNotes-1.7.*.txt,  and could not find such a change by searching\n> > 'git add'.  So, I thought it's a regression.\n> \n> As far as I know, git add has refused to add ignored files for as long\n> as I can remember.  Maybe there was briefly a bug in this behaviour\n> that was later fixed...\n> \n> If you use 'git bisect' on the git repo, you could probably discover\n> what happened, in case you're interested.\n\nIt was intentional. Try 48ffef9 (ls-files: fix overeager pathspec\noptimization, 2010-01-08).\n\n-Peff\n"},{"id":"135041","messageId":"20100219060249.GD22645@coredump.intra.peff.net","threadId":"22722","inReplyTo":"20100219053431.GB22645@coredump.intra.peff.net","subject":"Re: 'git add' regression in git-1.7?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-02-19T06:02:49Z","receivedAt":"2010-02-19T06:02:49Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 19, 2010 at 12:34:31AM -0500, Jeff King wrote:\n\n> > On Fri, Feb 19, 2010 at 12:04 AM, SungHyun Nam <goweol@gmail.com> wrote:\n> > > Well, before sending the previous email, I checked the\n> > > RelNotes-1.7.*.txt,  and could not find such a change by searching\n> > > 'git add'.  So, I thought it's a regression.\n> > \n> > As far as I know, git add has refused to add ignored files for as long\n> > as I can remember.  Maybe there was briefly a bug in this behaviour\n> > that was later fixed...\n> > \n> > If you use 'git bisect' on the git repo, you could probably discover\n> > what happened, in case you're interested.\n> \n> It was intentional. Try 48ffef9 (ls-files: fix overeager pathspec\n> optimization, 2010-01-08).\n\nBut this is a little disturbing still:\n\n  $ git init\n  $ mkdir dir\n  $ touch dir/sub\n  $ touch root\n  $ echo dir >.gitignore\n  $ echo root >>.gitignore\n\n  $ git add root\n  The following paths are ignored by one of your .gitignore files:\n  root\n  Use -f if you really want to add them.\n  fatal: no files added\n  $ echo $?\n  128\n\n  $ git add dir\n  The following paths are ignored by one of your .gitignore files:\n  dir\n  Use -f if you really want to add them.\n  fatal: no files added\n  $ echo $?\n  128\n\n  $ git add dir/sub\n  $ echo $?\n  0\n\nbut we didn't actually add the file.\n\n-Peff\n"},{"id":"135063","messageId":"20100219082445.GB13691@coredump.intra.peff.net","threadId":"22722","inReplyTo":"20100219060249.GD22645@coredump.intra.peff.net","subject":"Re: 'git add' regression in git-1.7?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-02-19T08:24:45Z","receivedAt":"2010-02-19T08:24:45Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 19, 2010 at 01:02:49AM -0500, Jeff King wrote:\n\n> But this is a little disturbing still:\n> \n>   $ git init\n>   $ mkdir dir\n>   $ touch dir/sub\n>   $ touch root\n>   $ echo dir >.gitignore\n>   $ echo root >>.gitignore\n> \n>   $ git add root\n>   The following paths are ignored by one of your .gitignore files:\n>   root\n>   Use -f if you really want to add them.\n>   fatal: no files added\n>   $ echo $?\n>   128\n> \n>   $ git add dir\n>   The following paths are ignored by one of your .gitignore files:\n>   dir\n>   Use -f if you really want to add them.\n>   fatal: no files added\n>   $ echo $?\n>   128\n> \n>   $ git add dir/sub\n>   $ echo $?\n>   0\n> \n> but we didn't actually add the file.\n\nJunio,\n\nThis seems to be caused by dir.c:treat_one_path. In the first few lines:\n\n        int exclude = excluded(dir, path, &dtype);\n        if (exclude && (dir->flags & DIR_COLLECT_IGNORED)\n            && in_pathspec(path, *len, simplify))\n                dir_add_ignored(dir, path, *len);\n\nwe see that the prefix \"dir\" is excluded, but it is not in our pathspec\n(\"dir/sub\"), so we do not add it to the ignored list.\n\nThis is related to your recent 48ffef9 (ls-files: fix overeager pathspec\noptimization, 2010-01-08), as before then we actually didn't consider\n\"dir/sub\" to be ignored at all.  The in_pathspec check did not originate\nthere; it's from my e96980e (builtin-add: simplify (and increase\naccuracy of) exclude handling, 2007-06-12). But it is definitely still\nnecessary.\n\nI'm not sure of the right way to fix this. We can drop further down into\nthe directory hierarchy when doing COLLECT_IGNORED and look for actual\nfiles, but that may have a negative performance impact. Perhaps we can\ngo further only if we are a prefix of a pathspec. Or maybe there is some\nway to be more clever.\n\nI dunno. I'm out of ideas for the evening, and since you looked at this\nnot too long ago, I thought you might have some insight.\n\n-Peff\n"},{"id":"135918","messageId":"7vhbp0ls26.fsf@alter.siamese.dyndns.org","threadId":"22722","inReplyTo":"20100219082445.GB13691@coredump.intra.peff.net","subject":"Re: 'git add' regression in git-1.7?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-03-01T02:00:17Z","receivedAt":"2010-03-01T02:00:17Z","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> I'm not sure of the right way to fix this. We can drop further down into\n> the directory hierarchy when doing COLLECT_IGNORED and look for actual\n> files, but that may have a negative performance impact.\n\nWouldn't that have negative correctness impact?  I don't see an obvious\nway out, other than perhaps checking the set of pathspecs twice.  One\nthing that might help is to carry the seen[] array a bit longer so that we\ndo not have to lose sight of what paths we were given but didn't match.\n"},{"id":"135920","messageId":"7v8wachgek.fsf_-_@alter.siamese.dyndns.org","threadId":"22722","inReplyTo":"7vhbp0ls26.fsf@alter.siamese.dyndns.org","subject":"[PATCH] add: fail \"git add ignored-dir/file\" without -f","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-03-01T03:25:39Z","receivedAt":"2010-03-01T03:25:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"git add <pathspec>\" usually fails the request and gives an advice message\nto use the -f option when <pathspec> exactly names an existing path in the\nwork tree.  However, we didn't issue the warning nor fail the request when\nthe <pathspec> matches an existing path but it is ignored because a higher\nlevel directory is ignored as a whole.  In such a case, we do not even\ndescend into it (and we shouldn't).\n\nThis catches such a case and issues some warning.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\n * This is still provisional and I am not fully happy with it; it seems to\n   pass the tests.  The error message is based on the full directory name\n   we are culling, not on the actual pathspec the user gave us, as we do\n   not have access to it.\n\n dir.c                  |   15 +++++-----\n t/t2204-add-ignored.sh |   70 ++++++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 78 insertions(+), 7 deletions(-)\n create mode 100755 t/t2204-add-ignored.sh\n\ndiff --git a/dir.c b/dir.c\nindex 00d698d..af4ba92 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -413,13 +413,10 @@ static struct dir_entry *dir_add_name(struct dir_struct *dir, const char *pathna\n \treturn dir->entries[dir->nr++] = dir_entry_new(pathname, len);\n }\n \n-static struct dir_entry *dir_add_ignored(struct dir_struct *dir, const char *pathname, int len)\n+static void dir_add_ignored(struct dir_struct *dir, const char *pathname, int len)\n {\n-\tif (!cache_name_is_other(pathname, len))\n-\t\treturn NULL;\n-\n \tALLOC_GROW(dir->ignored, dir->ignored_nr+1, dir->ignored_alloc);\n-\treturn dir->ignored[dir->ignored_nr++] = dir_entry_new(pathname, len);\n+\tdir->ignored[dir->ignored_nr++] = dir_entry_new(pathname, len);\n }\n \n enum exist_status {\n@@ -638,7 +635,8 @@ static enum path_treatment treat_one_path(struct dir_struct *dir,\n {\n \tint exclude = excluded(dir, path, &dtype);\n \tif (exclude && (dir->flags & DIR_COLLECT_IGNORED)\n-\t    && in_pathspec(path, *len, simplify))\n+\t    && in_pathspec(path, *len, simplify)\n+\t    && cache_name_is_other(path, *len))\n \t\tdir_add_ignored(dir, path, *len);\n \n \t/*\n@@ -841,8 +839,11 @@ static int treat_leading_path(struct dir_struct *dir,\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\t\t   DT_DIR, NULL) == path_ignored) {\n+\t\t\tif (dir->flags & DIR_COLLECT_IGNORED)\n+\t\t\t\tdir_add_ignored(dir, pathbuf, baselen);\n \t\t\treturn 0; /* do not recurse into it */\n+\t\t}\n \t\tif (len <= baselen)\n \t\t\treturn 1; /* finished checking */\n \t}\ndiff --git a/t/t2204-add-ignored.sh b/t/t2204-add-ignored.sh\nnew file mode 100755\nindex 0000000..c1ce12b\n--- /dev/null\n+++ b/t/t2204-add-ignored.sh\n@@ -0,0 +1,70 @@\n+#!/bin/sh\n+\n+test_description='giving ignored paths to git add'\n+\n+. ./test-lib.sh\n+\n+test_expect_success setup '\n+\tmkdir sub dir dir/sub &&\n+\techo sub >.gitignore &&\n+\techo ign >>.gitignore &&\n+\tfor p in . sub dir dir/sub\n+\tdo\n+\t\t>\"$p/ign\" &&\n+\t\t>\"$p/file\" || exit 1\n+\tdone\n+'\n+\n+for i in file dir/file dir 'd*'\n+do\n+\ttest_expect_success \"no complaints for unignored $i\" '\n+\t\trm -f .git/index &&\n+\t\tgit add \"$i\" &&\n+\t\tgit ls-files \"$i\" >out &&\n+\t\ttest -s out\n+\t'\n+done\n+\n+for i in ign dir/ign dir/sub dir/sub/*ign sub/file sub sub/*\n+do\n+\ttest_expect_success \"complaints for ignored $i\" '\n+\t\trm -f .git/index &&\n+\t\ttest_must_fail git add \"$i\" 2>err &&\n+\t\tgit ls-files \"$i\" >out &&\n+\t\t! test -s out &&\n+\t\tgrep -e \"Use -f if\" err &&\n+\t\tcat err\n+\t'\n+done\n+\n+for i in sub sub/*\n+do\n+\ttest_expect_success \"complaints for ignored $i in dir\" '\n+\t\trm -f .git/index &&\n+\t\t(\n+\t\t\tcd dir &&\n+\t\t\ttest_must_fail git add \"$i\" 2>err &&\n+\t\t\tgit ls-files \"$i\" >out &&\n+\t\t\t! test -s out &&\n+\t\t\tgrep -e \"Use -f if\" err &&\n+\t\t\tcat err\n+\t\t)\n+\t'\n+done\n+\n+for i in ign file\n+do\n+\ttest_expect_success \"complaints for ignored $i in sub\" '\n+\t\trm -f .git/index &&\n+\t\t(\n+\t\t\tcd sub &&\n+\t\t\ttest_must_fail git add \"$i\" 2>err &&\n+\t\t\tgit ls-files \"$i\" >out &&\n+\t\t\t! test -s out &&\n+\t\t\tgrep -e \"Use -f if\" err &&\n+\t\t\tcat err\n+\t\t)\n+\t'\n+done\n+\n+test_done\n-- \n1.7.0.1.241.g6604f\n"},{"id":"135928","messageId":"7veik45ty9.fsf@alter.siamese.dyndns.org","threadId":"22722","inReplyTo":"7v8wachgek.fsf_-_@alter.siamese.dyndns.org","subject":"[PATCH 1/3] t0050: mark non-working test as such","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-03-01T08:26:06Z","receivedAt":"2010-03-01T08:26:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The test is to prepare an empty file \"camelcase\" in the index, remove\nand replace it with another file \"CamelCase\" with \"1\" as its contents\nin the working tree, and add it to the index, in a repository configured\nto be case insensitive.\n\nHowever, the test actually checked ls-files knows about a pathname that\nmatches \"camelcase\" case insensitively.  It didn't check if the added\ncontents actually was the updated one.\n\nMark the test as non-working.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\n * This comes from 8a19aaa (t0050: Add test for case insensitive add,\n   2008-05-11); back then the code did add both camelcase and CamelCase,\n   and somewhere after that we once fixed it, but 1e5f764 (builtin-add.c:\n   optimize -A option and \"git add .\", 2008-07-22) broke it in a different\n   way (namely, it stopped adding the updated contents); I am not going to\n   have time to debug it further anytime soon (hint hint)...\n\n t/t0050-filesystem.sh |    8 ++++++--\n 1 files changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t0050-filesystem.sh b/t/t0050-filesystem.sh\nindex 89282cc..41df6bc 100755\n--- a/t/t0050-filesystem.sh\n+++ b/t/t0050-filesystem.sh\n@@ -108,13 +108,17 @@ $test_case 'merge (case change)' '\n \n '\n \n-$test_case 'add (with different case)' '\n+\n+\n+test_expect_failure 'add (with different case)' '\n \n \tgit reset --hard initial &&\n \trm camelcase &&\n \techo 1 >CamelCase &&\n \tgit add CamelCase &&\n-\ttest $(git ls-files | grep -i camelcase | wc -l) = 1\n+\tcamel=$(git ls-files | grep -i camelcase) &&\n+\ttest $(echo \"$camel\" | wc -l) = 1 &&\n+\ttest \"z$(git cat-file blob :$camel)\" = z1\n \n '\n \n-- \n1.7.0.1.241.g6604f\n"},{"id":"136474","messageId":"20100309223729.GA25265@sigill.intra.peff.net","threadId":"22722","inReplyTo":"7vhbp0ls26.fsf@alter.siamese.dyndns.org","subject":"Re: 'git add' regression in git-1.7?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-03-09T22:37:30Z","receivedAt":"2010-03-09T22:37:30Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Feb 28, 2010 at 06:00:17PM -0800, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > I'm not sure of the right way to fix this. We can drop further down into\n> > the directory hierarchy when doing COLLECT_IGNORED and look for actual\n> > files, but that may have a negative performance impact.\n> \n> Wouldn't that have negative correctness impact?  I don't see an obvious\n> way out, other than perhaps checking the set of pathspecs twice.  One\n> thing that might help is to carry the seen[] array a bit longer so that we\n> do not have to lose sight of what paths we were given but didn't match.\n\nSorry for the very late reply. Day-job has kept me busy.\n\nNo, we would still be correct if we recurse into the ignored directory\n_only_ to collect the ignored bits (so we don't even bother if\nCOLLECT_IGNORED isn't set). But what I don't like is that you take a\nperformance hit, because in most cases you won't ever care what's inside\nthose directories. You need to recurse only when:\n\n  - you actually care about all files. git-add does. git-status does not\n    (unless you explicitly told it to show directories). So that would\n    probably need a flag passed to fill_directory.\n\n  - you have a pathspec that means the contents of the directory might\n    be interesting. Right now we check in_pathspec in treat_one_path.\n    But I think we would need to recognize that \"subdir/file\" is\n    means \"subdir\" is in our pathspec (and that \"sub*\" means the same\n    thing).\n\nYour solution does something similar after the fact, but I am not 100%\nsatisfied with it. I'll respond separately to that patch.\n\n-Peff\n"},{"id":"136476","messageId":"20100309230931.GC25265@sigill.intra.peff.net","threadId":"22722","inReplyTo":"20100309223729.GA25265@sigill.intra.peff.net","subject":"Re: 'git add' regression in git-1.7?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-03-09T23:09:31Z","receivedAt":"2010-03-09T23:09:31Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 09, 2010 at 05:37:30PM -0500, Jeff King wrote:\n\n> No, we would still be correct if we recurse into the ignored directory\n> _only_ to collect the ignored bits (so we don't even bother if\n> COLLECT_IGNORED isn't set). But what I don't like is that you take a\n> performance hit, because in most cases you won't ever care what's inside\n> those directories. You need to recurse only when:\n> \n>   - you actually care about all files. git-add does. git-status does not\n>     (unless you explicitly told it to show directories). So that would\n>     probably need a flag passed to fill_directory.\n> \n>   - you have a pathspec that means the contents of the directory might\n>     be interesting. Right now we check in_pathspec in treat_one_path.\n>     But I think we would need to recognize that \"subdir/file\" is\n>     means \"subdir\" is in our pathspec (and that \"sub*\" means the same\n>     thing).\n\nActually, if we accept that the message simply mentions the excluded\npath, i.e.:\n\n  $ git add subdir/file\n  The following paths are ignored by one of your .gitignore files:\n  subdir\n  Use -f if you really want to add them.\n\nthen we don't really need to recurse. We just need to fix in_pathspec to\nflag files that are _relevant_ to a pathspec.\n\nAnd something like this seems to fix the OP's problem:\n\ndiff --git a/dir.c b/dir.c\nindex 00d698d..5091bfd 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -554,13 +554,17 @@ static int simplify_away(const char *path, int pathlen, const struct path_simpli\n \treturn 0;\n }\n \n-static int in_pathspec(const char *path, int len, const struct path_simplify *simplify)\n+static int relevant_pathspec(const char *path, int len, const struct path_simplify *simplify)\n {\n \tif (simplify) {\n \t\tfor (; simplify->path; simplify++) {\n \t\t\tif (len == simplify->len\n \t\t\t    && !memcmp(path, simplify->path, len))\n \t\t\t\treturn 1;\n+\t\t\tif (len < simplify->len\n+\t\t\t    && simplify->path[len] == '/'\n+\t\t\t    && !memcmp(path, simplify->path, len))\n+\t\t\t\treturn 1;\n \t\t}\n \t}\n \treturn 0;\n@@ -638,7 +642,7 @@ static enum path_treatment treat_one_path(struct dir_struct *dir,\n {\n \tint exclude = excluded(dir, path, &dtype);\n \tif (exclude && (dir->flags & DIR_COLLECT_IGNORED)\n-\t    && in_pathspec(path, *len, simplify))\n+\t    && relevant_pathspec(path, *len, simplify))\n \t\tdir_add_ignored(dir, path, *len);\n \n \t/*\n\nWhich is similar to your fix, but hoisted into the ignore-collection\nphase. Like the original code and your patch, it suffers from using a\nstraight memcmp. I think it should actually be checking the pathspec\nexpansion to catch things like 'sub*/file' being relevant to 'subdir'.\n\n-Peff\n"},{"id":"136498","messageId":"7veijsmza0.fsf@alter.siamese.dyndns.org","threadId":"22722","inReplyTo":"20100309230931.GC25265@sigill.intra.peff.net","subject":"Re: 'git add' regression in git-1.7?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-03-10T07:06:15Z","receivedAt":"2010-03-10T07:06:15Z","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> Actually, if we accept that the message simply mentions the excluded\n> path, i.e.:\n>\n>   $ git add subdir/file\n>   The following paths are ignored by one of your .gitignore files:\n>   subdir\n>   Use -f if you really want to add them.\n>\n> then we don't really need to recurse. We just need to fix in_pathspec to\n> flag files that are _relevant_ to a pathspec.\n\nClever and to the point.\n\n> And something like this seems to fix the OP's problem:\n> ...\n> Which is similar to your fix, but hoisted into the ignore-collection\n> phase. Like the original code and your patch, it suffers from using a\n> straight memcmp. I think it should actually be checking the pathspec\n> expansion to catch things like 'sub*/file' being relevant to 'subdir'.\n\nYeah.  Care to roll a patch to replace 13bb0ce (builtin-add: fix exclude\nhandling, 2010-02-28)?  We probably should build the glob matching on top\nof your version instead.\n"},{"id":"136560","messageId":"20100311071543.GA8750@sigill.intra.peff.net","threadId":"22722","inReplyTo":"7veijsmza0.fsf@alter.siamese.dyndns.org","subject":"Re: 'git add' regression in git-1.7?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-03-11T07:15:43Z","receivedAt":"2010-03-11T07:15:43Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 09, 2010 at 11:06:15PM -0800, Junio C Hamano wrote:\n\n> > And something like this seems to fix the OP's problem:\n> > ...\n> > Which is similar to your fix, but hoisted into the ignore-collection\n> > phase. Like the original code and your patch, it suffers from using a\n> > straight memcmp. I think it should actually be checking the pathspec\n> > expansion to catch things like 'sub*/file' being relevant to 'subdir'.\n> \n> Yeah.  Care to roll a patch to replace 13bb0ce (builtin-add: fix exclude\n> handling, 2010-02-28)?  We probably should build the glob matching on top\n> of your version instead.\n\nPatch is below. It's based on your 13bb0ce^, and assumes 13bb0c3 itself\nwill be reverted. However, doesn't your patch 2/3 that adds t2204 break\nbisectability? The fix doesn't come until 3/3. It should\ntest_expect_failure, or it should come after.\n\nI thought about globbing for a minute. I don't think the change in dir.c\nwould be too hard, but it will expose another corner case for add. If we\nhave _anything_ in dir->ignored, add currently complains. But if I do\nsomething like:\n\n  $ touch bar baz\n  $ echo baz >.gitignore\n  $ git add 'b*'\n\nright now it adds 'bar' and silently ignores 'baz', because\nCOLLECT_IGNORED fails to realize that 'baz' is interesting to us. If we\nfix the COLLECT_IGNORED bug with globbing, it will start to complain\nthat 'baz' was ignored.\n\nI think we could get around it by switching git-add to complain about\nignored files _only_ if there is a pathspec that is not \"seen\". If\neverything was seen, then we know that even if there are ignored paths,\nthey were all part of pathspecs that at least partially matched.\nHowever, it is still a bit unsatisfying; in the case of failure, we\ndon't know which of the ignored files came from which pathspec. So we\nwill print the whole list of ignored paths, even though some of them may\nnot have been responsible for the error.\n\nAnother option is to declare the current behavior wrong. Letting the\nshell glob produces different results for obvious reasons:\n\n  $ git add b* ;# will fail, because we see individual pathspecs\n\nbut perhaps that is the \"feature\" of letting add glob itself. Personally\nI have never asked \"git add\" to glob on my behalf, so I don't know why\npeople would do it.\n\nAnyway, here's the patch.\n\n-- >8 --\nSubject: [PATCH] dir: fix COLLECT_IGNORED on excluded prefixes\n\nAs we walk the directory tree, if we see an ignored path, we\nwant to add it to the ignored list only if it matches any\npathspec that we were given. We used to check for the\npathspec to appear explicitly. E.g., if we see \"subdir/file\"\nand it is excluded, we check to see if we have \"subdir/file\"\nin our pathspec.\n\nHowever, this interacts badly with the optimization to avoid\nrecursing into ignored subdirectories. If \"subdir\" as a\nwhole is ignored, then we never recurse, and consider only\nwhether \"subdir\" itself is in our pathspec.  It would not\nmatch a pathspec of \"subdir/file\" explicitly, even though it\nis the reason that subdir/file would be excluded.\n\nThis manifests itself to the user as \"git add subdir/file\"\nfailing to correctly note that the pathspec was ignored.\n\nThis patch extends the in_pathspec logic to include prefix\ndirectory case.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n dir.c |   20 ++++++++++++++++++--\n 1 files changed, 18 insertions(+), 2 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex 00d698d..14ac91a 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -554,13 +554,29 @@ static int simplify_away(const char *path, int pathlen, const struct path_simpli\n \treturn 0;\n }\n \n-static int in_pathspec(const char *path, int len, const struct path_simplify *simplify)\n+/*\n+ * This function tells us whether an excluded path matches a\n+ * list of \"interesting\" pathspecs. That is, whether a path matched\n+ * by any of the pathspecs could possibly be ignored by excluding\n+ * the specified path. This can happen if:\n+ *\n+ *   1. the path is mentioned explicitly in the pathspec\n+ *\n+ *   2. the path is a directory prefix of some element in the\n+ *      pathspec\n+ */\n+static int exclude_matches_pathspec(const char *path, int len,\n+\t\tconst struct path_simplify *simplify)\n {\n \tif (simplify) {\n \t\tfor (; simplify->path; simplify++) {\n \t\t\tif (len == simplify->len\n \t\t\t    && !memcmp(path, simplify->path, len))\n \t\t\t\treturn 1;\n+\t\t\tif (len < simplify->len\n+\t\t\t    && simplify->path[len] == '/'\n+\t\t\t    && !memcmp(path, simplify->path, len))\n+\t\t\t\treturn 1;\n \t\t}\n \t}\n \treturn 0;\n@@ -638,7 +654,7 @@ static enum path_treatment treat_one_path(struct dir_struct *dir,\n {\n \tint exclude = excluded(dir, path, &dtype);\n \tif (exclude && (dir->flags & DIR_COLLECT_IGNORED)\n-\t    && in_pathspec(path, *len, simplify))\n+\t    && exclude_matches_pathspec(path, *len, simplify))\n \t\tdir_add_ignored(dir, path, *len);\n \n \t/*\n-- \n1.7.0.2.393.g3eb23\n"},{"id":"136749","messageId":"7veijns96t.fsf@alter.siamese.dyndns.org","threadId":"22722","inReplyTo":"20100311071543.GA8750@sigill.intra.peff.net","subject":"Re: 'git add' regression in git-1.7?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-03-14T06:34:34Z","receivedAt":"2010-03-14T06:34:34Z","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> Another option is to declare the current behavior wrong. Letting the\n> shell glob produces different results for obvious reasons:\n>\n>   $ git add b* ;# will fail, because we see individual pathspecs\n>\n> but perhaps that is the \"feature\" of letting add glob itself. Personally\n> I have never asked \"git add\" to glob on my behalf, so I don't know why\n> people would do it.\n\nI know of one reason:\n\n    $ git add '*.[ch]'\n\nwill add a/b.c and c/d/f.h for you.\n"},{"id":"136775","messageId":"20100314204459.GA31564@coredump.intra.peff.net","threadId":"22722","inReplyTo":"7veijns96t.fsf@alter.siamese.dyndns.org","subject":"Re: 'git add' regression in git-1.7?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-03-14T20:44:59Z","receivedAt":"2010-03-14T20:44:59Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Mar 13, 2010 at 10:34:34PM -0800, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > Another option is to declare the current behavior wrong. Letting the\n> > shell glob produces different results for obvious reasons:\n> >\n> >   $ git add b* ;# will fail, because we see individual pathspecs\n> >\n> > but perhaps that is the \"feature\" of letting add glob itself. Personally\n> > I have never asked \"git add\" to glob on my behalf, so I don't know why\n> > people would do it.\n> \n> I know of one reason:\n> \n>     $ git add '*.[ch]'\n> \n> will add a/b.c and c/d/f.h for you.\n\nHrm, that makes handling globs with ignores a bit trickier. If I have:\n\n  $ touch root.c\n  $ mkdir subdir && touch subdir/file.c\n  $ echo subdir >.gitignore\n  $ git add '*.[ch]'\n\nwhat should happen? I would say it should probably not generate an\nerror, but just add 'root.c'.\n\nIn which case, I think we perhaps actively _don't_ want to complain\nabout ignored globs at all. If they match nothing except excluded files,\nwe will already complain that the pathspec was useless. And if they do\nmatch some other files, we will silently except. The only \"downside\"\nversus handling globs in the ignore code is that for the no-matches case\nwe say \"pathspec did not match\" and not \"it _could_ have matched, but\nthese files were ignored, and you need -f to add them\".\n\nBut I don't think that latter message even makes sense for a glob. If\nyou show me just the glob, then it isn't helpful; I don't know which\nignored files matched the glob. And if you show me the list of files, it\nis likely to contain unhelpful cruft like \"build/generated.c\". So I\nwon't just repeat my command with \"-f\" anyway; I will find the ignored\nfile I was interested in adding and specify it explicitly.\n\nSo if that reasoning is sound, I think we want to just leave git-add's\nbehavior as it is currently (with my patch from earlier in this thread\napplied, of course). You get different error messages for \"git add *.c\"\nand \"git add '*.c'\", but that is only natural. You also get different\n_behavior_, and that is intentional.\n\nOther callers of COLLECT_IGNORED could conceivably want different\nglobbing behavior, but right now git-add is the only caller. So it's\ncertainly not worth caring about at this point.\n\n-Peff\n"},{"id":"136786","messageId":"7vljdul4tu.fsf@alter.siamese.dyndns.org","threadId":"22722","inReplyTo":"20100314204459.GA31564@coredump.intra.peff.net","subject":"Re: 'git add' regression in git-1.7?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-03-15T02:02:53Z","receivedAt":"2010-03-15T02:02:53Z","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> In which case, I think we perhaps actively _don't_ want to complain\n> about ignored globs at all.\n> ...\n> So if that reasoning is sound, I think we want to just leave git-add's\n> behavior as it is currently (with my patch from earlier in this thread\n> applied, of course). You get different error messages for \"git add *.c\"\n> and \"git add '*.c'\", but that is only natural. You also get different\n> _behavior_, and that is intentional.\n\nYeah, I think that makes sense.\n"}]}