Re: [PATCH] dir.c: fix dir re-inclusion rules with "NODIR" and "MUSTBEDIR"

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 9, 2016, 17:55 UTC
Message-ID: <xmqqoaanilia.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <1457521693-26141-1-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> Side note, it's probably a bad idea to use basename matching on a
> negative rule (ie. no slashes in the pattern) because what the pattern
> says is "re-include _any_ directory named 'dir'", not just the directory
> "dir" at right below this directory.

I am not sure I agree with this comment. When we say '.depend/' in our .gitignore file, we do want to ignore everything in the directory with that name anywhere in that tree. Perhaps '!include/' followed by '*' may be a good way to pick only the header files that are found in any directory with that name anywhere in the tree in a similar fashion. It certainly is a disappointing comment if made by somebody who wants to make 'dir' and '!dir' behave in a more similar way, I'd have to say.

Show 11 quoted lines

> In the MUSTBEDIR case, the patterns look like this
>
>     *          # exclude dir, dir/file1 and dir/file2..
>     !dir/      #  ..except that dir and everything inside is re-included..
>     dir/file2  # ..except (again!) that dir/file2 is excluded
>                # ..which means dir/file1 stays included
>
> Again, we're ok at the toplevel, then we enter "dir" and test
> "dir/file1". The MUSTBEDIR code tests if the _full_ path (ie. "dir/file1")
> is a directory. We want it to test the "dir" part from "dir/file1"
> instead.

That certainly sounds like a breakage worth fixing.

> In both cases, we want to test if the pattern matches a parent
> directory. match_dir_component() is for this purpose.

I do not follow. I would understand "In all cases, we want to behave as if we are testing the _full_ path against the pattern", though.

IOW, "dir/file1" matches '*' (because 'file1' and '*' matches), '!dir/' (because the pattern is "everything under dir/ directory), and nothing else in the pattern list, and the last match wins---which happens to be negative ignore, hence dir/file1 is not ignored.

> We do want to be careful not to get back too far. Given the file
> foo/bar/.gitignore, we should only check as far back as foo/bar because
> this .gitignore file can never match outside that directory, which is
> "toplevel" in the above paragraphs, to begin with.

Yes. But isn't that what exclude_stack mechanism already gives us? That is, when you are not looking at a path inside foo/bar/, entries stored in foo/bar/.gitignore will not participate the determination of the fate of the path.

I am not sure why you have to say this.  Puzzled...