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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Dec 09, 2020 at 11:10:04AM -0500, Jeff King wrote:
> On Wed, Dec 09, 2020 at 12:52:26PM +0100, Patrick Steinhardt wrote:
> 
> > Co-authored-by: Jeff King <peff@xxxxxxxx>
> > Signed-off-by: Patrick Steinhardt <ps@xxxxxx>
> 
> In case we want it, this is also:
> 
>   Signed-off-by: Jeff King <peff@xxxxxxxx>
> 
> > +--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 this is just weirdly worded in the `-c` case, which I mostly
copied. We _do_ keep the empty string, which effectively means that `git
config --type=bool` will return `false`.

Or do you mean that we should allow `--config-env=foo.bar=`?

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

Makes sense.

Patrick

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

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux