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

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 18, 2016, 18:00 UTC
Message-ID: <xmqqa8lvr7ix.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <CACsJy8A2FKn-8nWtK4QPMHDCDYvTZBrQs1RVMApnuejXQis19g@mail.gmail.com>

Duy Nguyen <pclouds@gmail.com> writes:

Show 7 quoted lines

> v2.7.3 differs in the way "!" is handled. It does extend reversion
> outside dir1/dir2/show, back to dir1/dir2. While 2.8+1+2 recognizes
> and follows the "/*" and "!dir1/dir2/" pair.
>
> The way I interpreted the rules above, though, may be because I'm just
> trying to defend the current code. Junio, your call on whether to
> revert this whole patch series.

It seems that the more you dig into piling changes on this topic there is somebody else who comes back with yet another corner case.

At this point in the release cycle, I am inclined to say that we should revert the whole thing from 2.8-rc. Even with that plan, we probably need an extra rc as the reverting a topic this late in the cycle is something I feel uncomfortable with.

What is the ultimate goal of nd/exclusion-regression-fix topic over the behaviour before 2.7.0 and after 2.7.1 (there was a regression in 2.7.0 that was reverted in 2.7.1)? From the cover letter of the series:

    Take one was bad and reverted in commit 8c72236. Take two provides a
    more complete solution to the pair of rules

      exclude/this
      !exclude/this/except/this

    3/4 should do a better job at stopping regressions in take 1. 4/4
    provides the solution. I think I have tested (and wrote tests) for all
    the cases I can imagine.

"solution to the pair of rules" hints there are some problem in the pair of rules, without stating what it is trying to solve.

Isn't the root cause of the issue that treat_one_path() when deciding if it is worth descending into a directory check if the directory itself is excluded and returns path_excluded, even if some paths inside it may have a countermanding ! entries that would match them? What if we change that part smarter and allow it to descend?