Re: Expanding Includes in .gitignore

From: Jeff King <peff@peff.net>
Date: Oct 27, 2016, 20:55 UTC
Message-ID: <20161027205508.vqw44zlbnqpj2cvd@sigill.intra.peff.net>
In-Reply-To: <b20b458c-440d-df09-d2c7-e510ac20492c@pelly.co>

On Fri, Oct 28, 2016 at 09:28:23AM +1300, Aaron Pelly wrote:

Show 10 quoted lines

> >   - we parse possibly-hostile .gitignore files from cloned repositories.
> >     What happens when I include ask to include /etc/passwd? Probably
> >     nothing, but there are setups where it might matter (e.g., something
> >     like Travis that auto-builds untrusted repositories, and you could
> >     potentially leak the contents of files via error messages). It's
> >     nice to avoid the issue entirely.
> 
> I understand the issue.
> 
> It's not obvious to me how using a .d solves this problem though.

It doesn't by itself. But we are worried only about tracked .gitignore files (recall that even repo-level files in $GIT_DIR/info are generated fresh by the clone process, and don't come from the remote). If we apply the feature only to core.excludeFile and $GIT_DIR/info/exclude, those are already under the user's control.

It's true that we could make a similar exception for an "include" feature, and respect include directives only in those "safe" files. Somehow that seems more confusing to me, though, than doing adding the feature at the file level, as it introduces slightly varying syntax between the locations.

Show 5 quoted lines

> > Whereas letting any of the user- or repo-level exclude files be a
> > directory, and simply reading all of the files inside, seems simple and
> > obvious.
> 
> Apart from backwards compatibility, unless there's something I'm missing.

I'm not sure what you mean. If we make:

  mkdir .git/info/exclude
  echo whatever >.git/info/exclude/local

work, I don't think we have to care about backwards compatibility. That was nonsensical before, and never did anything useful (so somebody might have done it, but we can assume anybody relying on it not to read the contents is crazy).

Show 10 quoted lines

> > It
> > can't handle all cases (some items in "00foo" want precedence over "01bar"
> > and vice versa), but I don't think there's an easy solution. That's a
> > good sign that one or more of the files should be broken up.
> 
> I've been burned by this myself by packages interfering with each other
> in /etc/sysctl.d
> 
> Could we put this down to caveat emptor? I think this sorting should be
> intuitive to most people these days, and simple to document and comprehend.

Yeah, I think caveat emptor is fine there. Sorry if I implied otherwise.

-Peff