Subject: Re: Git 2.7.0 gitignore behaviour regression

From: Duy Nguyen <[email-masked]>
Date: Fri, 8 Jan 2016 16:02:31 +0700

On Fri, Jan 08, 2016 at 02:41:25AM +0000, brian m. carlson wrote:
> On Fri, Jan 08, 2016 at 07:38:58AM +0700, Duy Nguyen wrote:
> > On Fri, Jan 8, 2016 at 6:44 AM, brian m. carlson
> > <[email-masked]> wrote:
> > > I think there's still a bug in the code here.  If you do
> > >
> > >   git init
> > >   mkdir -p base/a/
> > >   printf 'base/a/\n!base/a/b.txt\n' >.gitignore
> > 
> > Here we have the ignore rule "base/a/", but gitignore.txt, section
> > NOTES mentions this
> > 
> >  - The rules to exclude the parent directory must not end with a
> >    trailing slash.
> 
> The text here says, "To re-include files or directories when their
> parent directory is excluded, the following conditions must be met".  In
> other words, the text implies that it's required for re-inclusion to
> work, not exclusion.
> 
> > >   git add .gitignore
> > >   git commit -m 'Add .gitignore'
> > >   >base/a/b.txt
> > >   git add base/a/b.txt
> > >   git commit -m 'Add base/a/b.txt'
> > >   >base/a/c.txt
> > >   git status --porcelain
> > >
> > > git status outputs base/a/c.txt as unknown, when it should be ignored.
> > > We saw this in a repository at $DAYJOB.
> > 
> > If I delete that trailing slash, c.txt is ignored. So it's known
> > limitation. I think we can make trailing slash case work too, but if I
> > remember correctly it would involve a lot more changes, so I didn't do
> > it (there are other conditions to follow anyway to make it work).
> 
> The case I'm seeing is that b.txt was already checked into the
> repository before being re-added, and c.txt was not.  So it didn't
> affect us that b.txt was ignored (as it was already in the repo), but
> c.txt not being ignored broke a whole bunch of scripts that checked that
> the repository was clean, simply because we upgraded Git.
> 
> I think regardless of whether b.txt is re-included, c.txt should be
> ignored.  If it isn't possible to re-include b.txt, that's fine, since
> that isn't a regression, but ignored files should remain ignored.

Thanks for clarification. I looked at this the wrong way. I agree it
is a regression.

The following should fix it. It looks correct (and does fix your test
case), but I will have to look harder over the weekend before sending
a proper patch.

-- 8< --
diff --git a/dir.c b/dir.c
index d2a8f06..7934e87 100644
--- a/dir.c
+++ b/dir.c
@@ -1008,6 +1008,7 @@ static struct exclude *last_exclude_matching_from_list(const char *pathname,
 	if (exc &&
 	    !(exc->flags & EXC_FLAG_NEGATIVE) &&
 	    !(exc->flags & EXC_FLAG_NODIR) &&
+	    !(exc->flags & EXC_FLAG_MUSTBEDIR) &&
 	    matched_negative_path)
 		exc = NULL;
 	return exc;
-- 8< --
--
Duy