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

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

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.

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.