From: Matthieu Moy <Matthieu.Moy@grenoble-inp•fr>
To: Junio C Hamano <gitster@pobox•com>
Cc: "Štěpán Němec" <stepnem@gmail•com>,
git@vger•kernel.org, "Petr Onderka" <gsvick@gmail•com>
Subject: Re: [PATCH v3?] Add global and system-wide gitattributes
Date: Tue, 31 Aug 2010 00:55:53 +0200 [thread overview]
Message-ID: <vpqhbibbthi.fsf@bauges.imag.fr> (raw)
In-Reply-To: <7vbp8jerfq.fsf@alter.siamese.dyndns.org> (Junio C. Hamano's message of "Mon\, 30 Aug 2010 14\:11\:53 -0700")
Junio C Hamano <gitster@pobox•com> writes:
> Junio C Hamano <gitster@pobox•com> writes:
>
>> 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.
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/
next prev parent reply other threads:[~2010-08-30 22:56 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-08-11 1:04 [PATCH/RFC] Add global and system-wide gitattributes Petr Onderka
2010-08-11 9:20 ` Henrik Grubbström
2010-08-11 10:50 ` Petr Onderka
2010-08-11 12:31 ` Matthieu Moy
2010-08-11 22:19 ` Junio C Hamano
2010-08-16 16:51 ` Petr Onderka
2010-08-16 16:56 ` [PATCH v2] " Petr Onderka
2010-08-25 9:55 ` Štěpán Němec
2010-08-28 17:33 ` Matthieu Moy
2010-08-30 5:34 ` Junio C Hamano
2010-08-30 9:09 ` Štěpán Němec
2010-08-28 18:35 ` Matthieu Moy
2010-08-28 18:41 ` [PATCH] core.attributesfile: a fix, a simplification, and a test Matthieu Moy
2010-08-29 10:32 ` [PATCH v3?] Add global and system-wide gitattributes Štěpán Němec
2010-08-30 8:04 ` Junio C Hamano
2010-08-30 8:26 ` Matthieu Moy
2010-08-30 20:47 ` Junio C Hamano
2010-08-30 21:11 ` Junio C Hamano
2010-08-30 22:55 ` Matthieu Moy [this message]
2010-08-30 23:15 ` [PATCH 1/3 v2] tests: factor HOME=$(pwd) in test-lib.sh Matthieu Moy
2010-08-31 7:42 ` Ævar Arnfjörð Bjarmason
2010-09-01 7:56 ` Ævar Arnfjörð Bjarmason
2010-09-01 15:24 ` Junio C Hamano
2010-09-01 15:40 ` Ævar Arnfjörð Bjarmason
2010-09-01 16:57 ` Matthieu Moy
2010-08-30 23:15 ` [PATCH 2/3] don't write to git_log_output_encoding outside git_config() Matthieu Moy
2010-09-02 8:56 ` Matthieu Moy
2010-09-02 15:49 ` Junio C Hamano
2010-08-30 23:15 ` [PATCH 3/3 v4] Add global and system-wide gitattributes Matthieu Moy
2010-08-31 22:41 ` Matthieu Moy
2010-08-31 22:42 ` [PATCH] " Matthieu Moy
2010-08-31 23:56 ` [PATCH 3/3 v4] " Junio C Hamano
2010-08-30 9:50 ` [PATCH] tests: factor HOME=$(pwd) in test-lib.sh Matthieu Moy
2010-08-30 10:22 ` Ævar Arnfjörð Bjarmason
2010-08-30 10:54 ` Matthieu Moy
2010-08-30 11:08 ` Ævar Arnfjörð Bjarmason
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=vpqhbibbthi.fsf@bauges.imag.fr \
--to=matthieu.moy@grenoble-inp$(echo .)fr \
--cc=git@vger$(echo .)kernel.org \
--cc=gitster@pobox$(echo .)com \
--cc=gsvick@gmail$(echo .)com \
--cc=stepnem@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox