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

Junio C Hamano [off-list ref] writes:

Junio C Hamano [off-list ref] writes:

quoted

Matthieu Moy [off-list ref] writes:

quoted

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.

Confirmed.

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.

Yes, this is the problem, with git_log_output_encoding as you guessed.

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

Not that much in the case of git_log_output_encoding, but other uses
of the same pattern may exist.

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

[...]

 - 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.

I think it's reasonable to do both. Having both git_config() and
command-line parsing write to the same variable is fragile and should
be avoided IMHO, but OTOH, arbitrary calls to
git_config(git_default_config) may break other things, so ...

   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.

I just keep two global variables instead of two, and implement a
straightforward accessor. Command-line option parsing already used to
write to a global variable, so it doesn't change much.

New patch serie follows,

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/