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

From: Jeff King <peff@peff.net>
Date: Dec 10, 2020, 00:09 UTC
Message-ID: <X9FnU5Jj2XPl8BFk@coredump.intra.peff.net>
In-Reply-To: <xmqqy2i6zg1j.fsf@gitster.c.googlers.com>

On Wed, Dec 09, 2020 at 04:00:08PM -0800, Junio C Hamano wrote:

Show 16 quoted lines

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

Yep, exactly.

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

More or less. I did use the config_parse_pair() helper from patch 4, and there's some textual dependency on his patch 5 (but it would be easy to rebase).

Show 7 quoted lines

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

Exactly. Without (1), then (2) is not nearly as exciting. We could also implement a non-ambiguous version of "-c", like:

  git --config key.with=equals.foo some-value ...

which would also benefit from (2). But I think Patrick's main goal was to get secret values off the command-line, so he'd want either (1) or (3) for that.

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

Yes. I'd be happy to rebase them if we're not going to do the GIT_CONFIG_{KEY,VALUE}_<n> parts.

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

Yep.

-Peff