Re: [PATCH 2/2] config: add include directive

From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 2/2] config: add include directive
Date: Tue, 7 Feb 2012 13:36:44 -0500	[thread overview]
Message-ID: <20120207183644.GB32367@sigill.intra.peff.net> (raw)
In-Reply-To: <7v62fj60ya.fsf@alter.siamese.dyndns.org>

On Mon, Feb 06, 2012 at 02:39:41PM -0800, Junio C Hamano wrote:

> > +Includes
> > +~~~~~~~~
> > +
> > +You can include one config file from another by setting the special
> > +`include.path` variable to the name of the file to be included. The
> > +included file is expanded immediately, as if its contents had been
> > +found at the location of the include directive. If the value of the
> > +`include.path` variable is a relative path, the path is considered to be
> > +relative to the configuration file in which the include directive was
> > +found. See below for examples.
> 
> If the file referenced by this directive does not exist, what should
> happen?  Should it be signalled as an error?  Should it stop the whole
> calling process with die()?

I silently ignore it. My thinking was that you might want to have
something like:

  [include]
          path = .gitconfig-local

in a stock .gitconfig that you ship to all of your machines[1]. Then
machines that need it can put things in .gitconfig-local, and those that
don't can just ignore it.

It is a tradeoff, of course, in that typos will be silently ignored. For
this use case, you could also just create an empty .gitconfig-local on
machines that don't have anything to put there.

[1] Actually, a similar use might be a ~/.gitconfig that is shared by a
    mounted home directory (e.g., via NFS) NFS, and a ~/.gitconfig-$HOST
    that is specific to each machine. The current code doesn't expand
    environment variables (nor tildes), but perhaps it should.

> I think "die() when we are honoring the include, ignore when we are not"
> would be a good way to handle this, as it allows us to catch mistakes
> while allowing the user to fix broken configuration files using "git
> config --unset include.path", but I may be overlooking something.

The writing path does not use the include callback wrapper at all; so
include.path can be manipulated just as any other variable, and the
value is not treated specially.

-Peff