On Mon, Mar 01, 2010 at 12:26:37AM -0800, Junio C Hamano wrote:
> - "git add ignored-pattern.o", i.e. the pathspec exactly matches but is
> ignored by the traversal.
>
> For the former, the function immediately errored out. The latter were
> queued in the dir structure and later used to give an error message with
> "use -f if you really mean it" advice.
>
> e96980e (builtin-add: simplify (and increase accuracy of) exclude
> handling, 2007-06-12) somehow lost the latter. This adds the logic back,
> but with a bit of twist, as the code after e96980e uses collect_ignored
> option that does half the necessary work during the traversal.
I'm not sure that logic is accurate. The "increase accuracy" part of
e96980e was about the fact that COLLECT_IGNORED _knows_ something is
ignored, but builtin-add's prune_directory has to just _guess_ about it.
And I think your new code may have similar problems. Look at the
failing test in t0050 (the one you tweaked in patch 1/3). Before this
patch, it silently fails to add. Which is a bug, of course, but not this
one. But after this patch, we say "Oh, CamelCase was excluded, use -f,
etc". But that's not true. We are just making a guess after the fact
about why it wasn't included.
Now I am slightly hesitant to use that as an example, because it clearly
was also broken _before_ your patch. So it may be that the behavior it
exhibits (a pathspec not showing up as used, but the file exists and was
_not_ ignored) shouldn't ever happen for other cases. I'm not sure,
which makes me a little nervous about the change. But I would be willing
to accept "it seems to work, so let's go with it for now and see if
anybody complains" if you want to try that.
But also:
> + /* Existing file? We must have ignored it */
> + if (file_exists(match)) {
> + int len = strlen(match);
> + int i;
> + for (i = 0; i < dir->ignored_nr; i++)
> + if (dir->ignored[i]->len == len &&
> + !memcmp(dir->ignored[i]->name, match, len))
> + break;
> + if (dir->ignored_nr <= i)
> + dir_add_ignored(dir, match, strlen(match));
> + continue;
> + }
> + die("pathspec '%s' did not match any files", match);
Is that memcmp right? If I have something like:
echo subdir >.gitignore
git add 'sub*'
shouldn't it also say "Sorry, subdir [or sub*] was ignored"? But it
doesn't actually get added to the exclude list, and we get "pathspec did
not match any files", which is not quite true.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html