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

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 4, 2016, 19:12 UTC
Message-ID: <xmqqbn6uys4o.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <xmqq4mcm17b4.fsf@gitster.mtv.corp.google.com>

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

Show 24 quoted lines

> Duy Nguyen <pclouds@gmail.com> writes:
>
>> On Fri, Mar 4, 2016 at 6:56 PM, Kevin Daudt <me@ikke.info> wrote:
>>> Verified that it's different in 2.7.0, but 2.7.2 gives expected output.
>>
>> Thanks. 2.7.1 reverts the faulty commit from 2.7.0 that generated two
>> other regression reports before this one. I guess it's all good then
>> (except for the people still on 2.7.0)
>
> Are we good at 2.8.0-rc0, too?  Somehow I had an impression that we
> queued "another attempt to do it differently" or something.
>
>  ... goes and looks ...
>
>     $ rungit maint status -suall
>     ?? baz/quux/corge/wibble.txt
>     ?? baz/quux/grault.txt
>     ?? foo/bar.txt
>     $ rungit master status -suall
>     ?? baz/quux/corge/wibble.txt
>     ?? baz/quux/grault.txt
>     ?? baz/waldo.txt
>     ?? foo/bar.txt
>     ?? foo/garply.txt

It seems to bisect down to this one between maint..master:

commit a60ea8fb66945a886ea53fd3f41e61cc5fb3201e
Author: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Date:   Mon Feb 15 16:03:36 2016 +0700

    dir.c: fix match_pathname()
    
    Given the pattern "1/2/3/4" and the path "1/2/3/4/f", the pattern
    prefix is "1/2/3/4". We will compare and remove the prefix from both
    pattern and path and come to this code
    
        /*
         * If the whole pattern did not have a wildcard,
         * then our prefix match is all we need; we
         * do not need to call fnmatch at all.
         */
        if (!patternlen && !namelen)
                return 1;
    
    where patternlen is zero (full pattern consumed) and the remaining
    path in "name" is "/f". We fail to realize it's matched in this case
    and fall back to fnmatch(), which also fails to catch it. Fix it.
    
    Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>

But I do not think this change alone is the culprit; the change itself does make sense in the context of the series.

At this point, we have two choices. Either we revert the merge of the whole series:

    commit 5e57f9c3dfe7dd44a1b56bb5b3327d7a1356ec7c
    Merge: e79112d d589a67
    Author: Junio C Hamano <gitster@pobox.com>
    Date:   Wed Feb 24 13:25:59 2016 -0800

        Merge branch 'nd/exclusion-regression-fix'

        Another try to add support to the ignore mechanism that lets you
        say "this is excluded" and then later say "oh, no, this part (that
        is a subset of the previous part) is not excluded".

        * nd/exclusion-regression-fix:
          dir.c: don't exclude whole dir prematurely
          dir.c: support marking some patterns already matched
          dir.c: support tracing exclude
          dir.c: fix match_pathname()

to go back to the 2.7.2 behaviour, or add a follow-on change to the nd/exclusion-regression-fix topic to fix what it wanted to fix without breaking Charles's use case. I am inclined to go for the former (unless the follow-on fix is really trivial), but I haven't dug into the codebase myself yet, so...