git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: Git 2.7.0 gitignore behaviour regression

From
Duy Nguyen <pclouds@gmail.com>
Date
Jan 8, 2016, 09:02 UTC
Message-ID
<20160108090231.GA9609@lanh>
In-Reply-To
<20160108024125.GD265296@vauxhall.crustytoothpaste.net>
On Fri, Jan 08, 2016 at 02:41:25AM +0000, brian m. carlson wrote:
Show 45 quoted lines
> 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
> > <sandals@crustytoothpaste.net> 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
Previous: brian m. carlson
Message 13 of 13 in “Git 2.7.0 gitignore behaviour regression”
  1. Mike McQuaidJan 5, 2016
  2. Jeff KingJan 5, 2016
  3. Duy NguyenJan 6, 2016
  4. Mike McQuaidJan 6, 2016
  5. Duy NguyenJan 6, 2016
  6. Junio C HamanoJan 6, 2016
  7. Jeff KingJan 7, 2016
  8. Duy NguyenJan 7, 2016
  9. Carlos Martín NietoJan 8, 2016
  10. brian m. carlsonJan 7, 2016
  11. Duy NguyenJan 8, 2016
  12. brian m. carlsonJan 8, 2016
  13. Duy NguyenJan 8, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.