Re: [PATCH 2/2] config: allow specifying config entries via envvar pairs

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Nov 18, 2020, 13:49 UTC
Message-ID: <87d00arczw.fsf@evledraar.gmail.com>
In-Reply-To: <xmqq4klo7992.fsf@gitster.c.googlers.com>

On Tue, Nov 17 2020, Junio C Hamano wrote:

Show 17 quoted lines

> Patrick Steinhardt <ps@pks.im> writes:
>
>>> I especially do not think we want to read from unbounded number of
>>> GIT_CONFIG_KEY_<N> variables like this patch does.  How would a
>>> script cleanse its environment to protect itself from stray such
>>> environment variable pair?  Count up from 1 to forever?  Run "env"
>>> and grep for "GIT_CONFIG_KEY_[0-9]*=" (the answer is No.  What if
>>> some environment variables have newline in its values?)
>>
>> You only have to unset `GIT_CONFIG_KEY_1` as the parser will stop
>> iterating on the first missing key. More generally, if you set `n` keys,
>> it's sufficient to unset key `n+1`.
>
> Yes, but those who want to set N keys would likely to be content
> with setting 1..N and happily forget unsetting N+1, and that is
> where "how would one cleanse the environment to give a clean slate?"
> comes from.

Not as an argument from whataboutism, but just to note a bug/existing prior art:

Nobody in this thread has mentioned GIT_PUSH_OPTION_* which works pretty much like Patrick's suggestion, and it looks like --local-env-vars misses those:

    $ GIT_PUSH_OPTION_0=foo GIT_PUSH_OPTION_COUNT=20 git rev-parse --local-env-vars | grep GIT_PUSH
    $

I haven't tested this, but I expect there's a bug where a push hook itself does a local push to another repo and that repo has a hook, that the push options are erroneously carried forward to the sub-process.

That might also be a feature, depending on your point of view.