Re: [PATCH v3?] Add global and system-wide gitattributes

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 30, 2010, 21:11 UTC
Message-ID: <7vbp8jerfq.fsf@alter.siamese.dyndns.org>
In-Reply-To: <7voccjesk7.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

Show 10 quoted lines

> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>
>> I don't understand why this breaks the test. It seems blame
>> --encoding=UTF-8 relies on the fact that the i18n section of the
>> configuration is not loaded.
>
> That's interesting; I haven't traced the codepath involved, but I do not
> think "configuration is not loaded" is the issue. "Reading either before
> the main codepath is ready, or more likely overwriting/destroying what the
> main codepath has read it by re-reading the configuration" may be.

I think that hunch is correct. A typical way we default to hardcoded value, overridable by configuration file, and then further use command line to override that, is for the main codepath to do the following in this order:

 - call git_config(git_appropriate_config); this changes the variables
   (with possibly hardcoded default) defined in environment.c;

 - parse command line options and override the variable;

 - use the variable at runtime.

This obviously relies on the main codepath having _total_ control of the calls made to git_config(). If you call git_config(git_default_config) when an attribute is asked for for the first time (which would be way after all of the above happened) behind the main codepath's back, you will of course break things. In the "blame" case, aren't you stomping on git_log_output_encoding?

The correct solution would be twofold, but the latter is rather painful:

 - The call from the bootstrap_attr_stack should use a callback that reads
   only the attribute file location configuration and _nothing else_.

   Also I do not think the parsing of this configuration variable needs to
   be in git_default_config() to begin with, if you are reading it from
   the bootstrap codepath on demand anyway, and not relying on the main
   codepath of any particular program to be calling on git_config().

 - The way programs (this is not limited to blame and other rev-list
   machinery users) implement the "use configured values but let command
   line override them" need to be changed.
   
   One possibility is to copy the values determined by reading the config
   and the command line to their own variables, so that later random call
   to git_config() won't stomp on the actual values to be used.  This is
   painful as environment.c variables are _meant_ to be easily usable as
   global variables and copying them away (which means they now need to be
   passed around throughout the callchain in the various APIs) defeats
   the whole point of having them.