Re: [PATCH v4 2/6] config: add new way to pass config via `--config-env`

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> On Wed, Dec 09, 2020 at 12:52:26PM +0100, Patrick Steinhardt wrote:
>
>> Co-authored-by: Jeff King <peff@peff.net>
>> Signed-off-by: Patrick Steinhardt <ps@pks.im>
>
> In case we want it, this is also:
>
>   Signed-off-by: Jeff King <peff@peff.net>
>
>> +--config-env=<name>=<envvar>::
>> +	Pass a configuration parameter to the command. The <envvar>
>> +	given will be replaced with the contents of the environment
>> +	variable of that name. In contrast to `-c`, an envvar must
>> +	always be given and exist in the environment. Passing an
>> +	environment variable with empty value will set <name> to the
>> +	empty string which `git config --type=bool` will convert to
>> +	`false`.
>
> I agree with Ævar that we probably should keep an empty variable as the
> empty string. I think some options use an empty string to clear a list
> (e.g., push.pushOption), and I'm not sure how they'd react to a bool
> instead. It would be nice to also have a way to do the implicit-bool
> thing, but I don't think it's strictly necessary (it's always correct to
> put the string "true" into the variable instead).
>
> I think we should also document that <envvar> can't contain an "=" sign.
> Of course using strrchr() here doesn't help much with just this patch,
> because we flatten the string before stuffing it into
> $GIT_CONFIG_PARAMETERS, so the reading side would mis-parse it.
>
> But here's a fix for that. I built it on top of your whole series, since
> you touched some of the related functions, but it could easily be
> rebased onto just this part.

Hmph, so 

 (1) Patrick's 1 & 2 are about adding --config-env,

 (2) These three patches can come on top to make it more robust to
     pass key=value with GIT_CONFIG_PARAMETERS (including what is
     added via the --config-env=name=envvar), and

 (3) The remainder of Patrick's 6-patch series is to further add the
     pairs of environment variables to pass keys and values?

I am still not sure if we want the last part, but whether we take
(1) or (3) or neither or both, (2) sounds like a good thing to do.
And (2) would not shine without (1).  In the traditional use of -c,
we do not know which = from the end user separates key and value,
but when (1) places a = to separate the <name> and the value in the
environment variable, we know where that = is and can quote
accordingly.

But these three patches are done on top of (1) and (3), at least for
now.

The above is my understanding of the state of these patches.  Am I
getting it right?

Thanks.

>   [1/3]: quote: make sq_dequote_step() a public function
>   [2/3]: config: parse more robust format in GIT_CONFIG_PARAMETERS
>   [3/3]: config: store "git -c" variables using more robust format
>
>  config.c          | 118 +++++++++++++++++++++++++++++++++++++---------
>  quote.c           |  15 ++++--
>  quote.h           |  18 ++++++-
>  t/t1300-config.sh |  60 +++++++++++++++++++++++
>  4 files changed, 183 insertions(+), 28 deletions(-)
>
> -Peff