Re: Change in .gitignore handling: intended or bug?

From: Duy Nguyen <pclouds@gmail.com>
Date: Mar 8, 2016, 10:19 UTC
Message-ID: <CACsJy8C5r2f76p3oq5oX_1P5Vqt9qd7TAafuKxJ=Y8baELbJog@mail.gmail.com>
In-Reply-To: <xmqq8u1tmr6l.fsf@gitster.mtv.corp.google.com>

On Tue, Mar 8, 2016 at 1:14 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 20 quoted lines

> Junio C Hamano <gitster@pobox.com> writes:
>
>> We need documentation update to settle this one before 2.8 final
>> ships, as we seem to be seeing more and more end-user confusion on
>> the list.  I tried to come up with a trimmed-down example, which is
>> shown below, but I suspect that the code is not exactly working the
>> way it is described in that (1) dir/file1 is ignored and (3)
>> !dir/file3 entry makes difference.
>>
>> Where did my example go wrong?
>>
>> FYI, if I prefix '/' to all the .gitignore entries in the example, i.e.
>> making it
>>
>>     *
>>     !/dir
>>     /dir/file2
>>     !/dir/file3
>>
>> instead, then dir/file1 and dir/file3 do get shown as unignored.

Arghhh.. bug!!!

The difference between "dir" and "/dir" is, the former is basename matching while the latter is pathname matching. When we check dir/file1 after we enter "dir", we do try to check rule "!/dir" (or "!dir") before rule "*". In the pathname matching case, it works thanks to a60ea8f.

In the basename matching case (i.e. rule "!dir"), the code does not do the right thing. It blindly checks the base name of dir/file1, which is "file1", against the pattern "dir". The right thing to do is check the "dir" in "dir/file1" part against pattern "dir". Failing that, we fall back to pattern "*" and excludes dir/file1 as well.

Show 15 quoted lines

>> If it is documented somewhere, then I can update the example and
>> declare victory (but then the text that accompanies the example
>> still needs to remind the readers why the leading '/' matters.
>
> I also found that having an extra slash at the end of the
> "everything underneath 'dir' directory is included", i.e.
>
>      *
>      !/dir/
>      /dir/file2
>      !/dir/file3
>
> breaks it.  dir/file1 is ignored, dir/file3 isn't but the latter is
> only because there is an explicit !/dir/file3.  This is contrary to
> this rule in the documentation:

This is pretty much the same. But instead basename matching unable to deal with nested rules, the trailing slash means "check if it is a directory". Again, the code tries to check if "dir/file1" is a directory instead of "dir".

I haven't dug back in history, but my impression is it has been so probably from the beginning of "!" introduction. This has nothing to do with nd/exclusion-regression-fix, which is more about

    dir
    !dir/file

than

    !dir
    dir/file

Although the first patch in that series happens to fix the pathname matching case in this example.

I don't know. It seems to me I should fix this anyway, by making both "NODIR" and "MUSTBEDIR" code work well inside a directory. That should fix it. I hope I don't have to turn dir.c up side down for that. Last time I looked, it wasn't easy, which led to hack/avoid it with 57534ee and that was eventually reverted.

-- 
Duy