Re: Possible bug in git-completion.sh

[Junio C Hamano <gitster@pobox.com>]

Junio C Hamano <gitster@pobox.com> writes:

> Jeff King <peff@peff.net> writes:
>
>> Well, yes, if you wanted to compare apples to apples. But actually, my
>> point in showing "status" at all was to note that Michael's statement
>> that they would be the same is wrong.
>>
>> But just looking at the ls-files output, do you not agree that there is
>> a bug?
>
> If I agreed, I wouldn't have suggested _you_ to cd up and use pathspec,
> but instead would have suggested to patch ls-files to make it do so for
> you.

Nah, I should have checked the code.

The implementation of ls-files does cd up and uses pathspec, so the intent
is to apply higher level gitignore.

It however feeds paths from the already ignored directories, which _is_
the real bug.

For example, if you have

    .gitignore (ignores t/)
    t/
    t/junk

in your work tree, it will read .gitignore at the top level, and
eventually end up feeding "t/junk" to dir.c::excluded_1(), which does:

		for (i = el->nr - 1; 0 <= i; i--) {
			struct exclude *x = el->excludes[i];
			const char *exclude = x->pattern;
			int to_exclude = x->to_exclude;

			if (x->flags & EXC_FLAG_MUSTBEDIR) {
				if (*dtype == DT_UNKNOWN)
					*dtype = get_dtype(NULL, pathname, pathlen);
				if (*dtype != DT_DIR)
					continue;
			}
			...

where *x has "'t/' that must be directory".  but the path "t/junk" does
not match with "t/" and is not excluded by this rule; this part notices
"t/junk" is not a directory, and continues without giving the later part a
chance to intervene.  Of course, the later part also is not prepared to do
a "leading path" match, as the function is not meant to be fed entries
from ignored "t/" directory in the first place.

I think a proper fix should be in dir.c::read_directory() where it calls
read_directory_recursive() without first checking if the directory itself
should be ignored.  read_directory_recursive() itself has a logic to see
if the dirent it found is worth recursing into and a similar logic should
be in the toplevel caller.