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

[Durham Goode <durham@fb.com>]

On 3/17/16 10:40 PM, Duy Nguyen wrote:
> On Fri, Mar 18, 2016 at 11:51 AM, Durham Goode <durham@fb.com> wrote:
>> On 3/17/16 4:49 PM, Junio C Hamano wrote:
>>> Thanks for these 5 patches, two of which need to be discarded ;-).
>>> I think you can pick either one of 1/2, pick the one that says
>>> "non-NULL" (as opposed to "something") in the log message for 2/2.
>>>
>>> Durham, does it fix your issues if you apply the 1/2 and 2/2 (but
>>> not 3/2) on top of 2.8-rc?
>>>
>>> Duy, how comfortable are you with the idea of including this two in
>>> 2.8 final?  We have long passed the final -rc, and while it is
>>> probably OK to prolong the cycle and do another -rc, we cannot keep
>>> going like "oops, there is another thing discovered by somebod new"
>>> forever.
>>>
>>> Thanks.
>> Patches 1+2 fix the repro steps in the report, yes.  But I've found another
>> case that produces different results in 2.8 than in 2.7:
>>
>> Given a repo with files:
>>
>> dir1/dir2/show/file
>> dir1/dir2/hide/file
>>
>> and a sparse-checkout of
>>
>> /*
>> /dir1/dir2/show
>> !/dir1/dir2/
> I would say this is "undefined behavior" patterns. The intention of
> "!" pattern is to revert a subset of a matched pattern, e.g.
> !/dir1/dir2/show/something. Combining lines 2 and 3 together,
> "!dir1/dir2/" is not only supposed to revert dir1/dir2/show entirely,
> but extend the reversion outside of it. At least to me that's not
> intended.
After looking at it further, I agree with you.  The new behavior is as 
justifiable as the old. In addition, my description of this secondary 
issue was wrong (I said it left the files around, when in reality it 
resulted in an empty working copy, so git read-tree failed and left the 
files).  So we can ignore that case. Thanks.
>
> Skipping that tricky pair, the pairs "/*" and "!/dir1/dir2/" means
> "exclude everything except dir1/dir2" (in .gitignore sense) or
> "include everything except dir1/dir2" in sparse checkout sense. Which
> results in empty worktree. 1+2 trips when the trailing slash in the
> last rule exists and includes both files in show/hide. Patch 3/2 fixes
> the tripping and exclude both. If the last rule is "!/dir1/dir2" then
> 1+2 results in empty worktree as well.
>
I'm not sure I fully understand.  Here's what I'm seeing, with patch 1, 
2, and 3 applied:  with patterns "/*', "!/dir1/dir2", "/dir1/dir2/show", 
I see the contents of dir1/dir2/show/ (good). If I add a trailing slash 
to the last pattern (so it becomes "/dir1/dir2/show/"), it now results 
in an empty working copy. That seems funky, given that the last rule is 
to include that directory. Am I misunderstanding the trailing slash?

We're pretty far beyond my ability to understand ignore patterns now.  
The patches seem to make the behavior better though, so thanks for that.