Subject: Re: [PATCH 5/7] enforce `xfuncname` precedence over `funcname`

From: Junio C Hamano <[email-masked]>
Date: Thu, 24 Jul 2014 10:09:15 -0700

Matthieu Moy <[email-masked]> writes:

> Junio C Hamano <[email-masked]> writes:
>
>> in my ~/.gitconfig and then for a particular project I wanted to use
>> a custom pattern in its .git/config but it was sufficient to define
>> the pattern without using extended regexp, I would be tempted to say
>>
>> 	diff.foo.funcname "Wine$"
>
> The point is: to do that, you need to know that funcname exists, hence
> read archives of the doc before 2008 (it disapeared in
> 45d9414fa5599b41578625961b53e18a9b9148c7) or read the code. And if I
> read correctly the commit message for the commit, funcname is not
> portable so you should use xfuncname anyway.
>
> The real reason for this change is to let the code work with the new
> git_config, but I think it's reasonable not to bother with the original
> behavior for a feature deprecated so long ago.

We may be able to explain funcname vs xfuncname away, but the
problem is much deeper, and it is not just you and Tanay, but
anybody who thought that it is a natural and logical conclusion
to emulate git_config() by iterating over in-core hashtable X-<

We have assumed that it is OK for git_config() to call its callbacks
with any random order of keys as long as the values for the same key
are given in the same order as the original "read from file every
time" implementation would have called.  The assumption overlooked
the possibility that values given to two different keys can
semantically interact.  $foo.funcname and $foo.xfuncname being two
different ways to define the same piece of information is merely one
of the many ways such interactions can happen.  For example, by
assuming that orders of callbacks across keys does not matter, we
would be breaking scripts that expected to be able to deal with
something like this:

	[character]
        	name = Snoopy
                color = white black

                name = Hobbes
                color = yellow white black

For *our* own data, we can force them to be spelled this way instead:

	[character "Snoopy"]
        	color = white black
	[character "Hobbes"]
        	color = yellow white black

but that misses the point, as our config file format and its
semantics are not only about .git/config but the system is designed
and promoted to be a way for third-party applications to store their
own data as well.

Because we do not condone using libgit.a as a library to link into
third-party applications, git_config() at the C level may not have
to read a random third-party data and call its callback in the
predictable way, but this shows us that "git config -l" interface
must return the data in the same order as we have done always by
either using the _raw interface to return things uncached, or
teaching by git_config() to also be predicatable.

> That said, I think the commit message should be improved (recall the
> drawbacks of funcname).

In any case, I agree that the log message and the subject is wrong.
It is not that we want to give xfuncname precedence over funcname;
such a behaviour is undesirable if both were in wide use.  It is
papering over the fallout that comes from the loss of ordering
information, which is imposed by the data structure chosen to
represent a config_set.

I haven't formed a firm opinion whether preserving the order is
necessary at git_config() iteration level yet.  If the only in-core
user that will be broken by not doing so is xfuncname vs funcname,
it may not be too bad, but it will constrain us in the future, which
is a lot bigger problem.  We will be forbidden from correcting a UI
mistake by using the approach we took to transtion "funcname" over
to "xfuncname" (i.e. giving users "funcname" and allowing the
platform BRE parser that may or may not allow the users to write
backslashed ERE is spreading possible incompatibility among the
participants of a project; explicitly stating that the value is ERE
has fewer compatibility issues), while still supporting "funcname"
and have them interact in a sane way.