Re: [PATCH/RFC] Hacky version of a glob() driven config include

From: Jakub Narebski <jnareb@gmail.com>
Date: May 7, 2010, 23:43 UTC
Message-ID: <201005080143.21172.jnareb@gmail.com>
In-Reply-To: <AANLkTinCaPrThtuQd7tUFxNNn9KUx9v3_PXnH_6C8yco@mail.gmail.com>

On Sat, 8 May 2010, Ævar Arnfjörð Bjarmason wrote:

> On Fri, May 7, 2010 at 20:46, Jakub Narebski <jnareb@gmail.com> wrote:
> > Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

Show 10 quoted lines

> > > Known bugs:
> > >
> > >   * Breaks the model of being able to *set* config values. That
> > >     doesn't work for the included files. Maybe not a bug.
> >
> > Errr... do I understand correctly that it simply means that you are
> > not able to set config values that came from included files, in
> > included files?
> >
> > This is quite serious limitation.

I was wrong there; this is not even a problem.

Show 10 quoted lines

> It is. And recap, you can now you can set Git's config in either
> places .git/config, ~/.gitconfig and $prefix/etc/gitconfig.
> 
> With inclusion this is a bit more complex. If my ~/.gitconfig includes
> a seekrt.key=foobar via an include in ~/.gitconfig/seekrt, what should
> `git config --global seekrt.key newkey` do? How about `git config
> --global seekrt.some_new_value blah`?
> 
> I think it's best to not try to get into that mess and just let the
> user manage included files manually, or with `git config --file`.

This is not a problem: while "git config --get foo.bar" would search through $GIT_DIR/config, ~/.gitconfig and /etc/gitconfig (and with your addition also included files), "git config foo.bar baz" would set foo.bar to baz always in per-repository config file (in absence of --global / --system / --file=<file> option).

So this would be simply an extension of existing situation.  Not a bug.

Show 9 quoted lines

> > >   * The whole bit with saving/restoring global state for config
> > >     inclusion is evil, but then again so is the global state.
> >
> > Why not encapsulate those global variables in a struct, passed to
> > appropriate functions, with a global variable holding an instance of
> > such struct (IIRC similarly to what is done for "the_index").
> 
> That's indeed the sane way to go. I'll do that (and look at
> the_index).

Note that variable might not be called the_index...

[...]

Show 14 quoted lines

> > > +cat > .git/config << EOF
> > > +[some]
> > > +     variable = blah
> > > +[voodoo]
> > > +     include = .git/more_config_*
> > > +EOF
> >
> > I don't like this syntax.
> 
> Me neither.
> 
> > First, it forces git-config to hide all 'include' keys.  I think
> > there might be some legitimate <section>.include config variables
> > (perhaps outside git-core); with this patch they are impossible.

Here I didn't notice that it has to be voodoo.include, and not any other fully qualified variable name, i.e. section name must be voodoo.

> It's only hiding the full 'voodoo.include' key currently, you can
> still have e.g. 'bleh.include'.

voodoo.include shows that black magic voodoo; include.file could be a bit better.

Show 7 quoted lines

> > I would propose
> >
> >  include .git/more_config_*
> >
> > if not for the fast that it would trip older git.  Perhaps
> >
> >  ## include ".git/more_config_*"

Or perhaps

  #include ".git/more_config_*"

:-)

Show 5 quoted lines

> 
> Probably not a good idea to mix up comments & configuration like
> that. Some (semi-broken) parsers of .gitconfig also use INI parsers to
> parse it, which breaks on # comments. Those are already broken, but it
> would be nice if a feature didn't require them.

BTW IIRC ini-files format (at least one of them) allows for ';'-prefixed line comments (comment must be on its own line); I wonder how it is with ini-like git config format.

But in some ini-formats definition we have that both the hash mark (#) and the semicolon (;) are comment characters.

Show 10 quoted lines

> 
> >  [include .git/more_config_*]
> 
> Syntax error on older Gits.
> 
> >  [include ".git/more_config_*"]
> 
> I like this one the best. It's also easy to modify the parser (so it
> doesn't think it's a section) to handle it. And it doesn't incur the
> confusion of looking like a normal configuration variable.

What I don't like with this proposal is that one could write

  [include ".git/more_config_*"]
  	foo = bar

Which is confusing.

But perhaps we can break backwards compatibility here.  I don't know...

  <include .git/more_config_*>
  [[.git/more_config_*]]
  {{.git/more_config_*}}
  [=.git/more_config_*=]
  [@.git/more_config_*]
  %include ".git/more_config_*"
  @INCLUDE = .git/more_config_*

-- 
Jakub Narebski
Poland