list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: "brian m. carlson" <>
To: Jeff King <>
Cc: "Junio C Hamano" <>,
	"Ævar Arnfjörð Bjarmason" <>,
	"Patrick Steinhardt" <>,
Subject: Re: [PATCH 2/2] config: allow specifying config entries via envvar pairs
Date: Wed, 18 Nov 2020 02:25:32 +0000	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

[-- Attachment #1: Type: text/plain, Size: 3887 bytes --]

On 2020-11-18 at 01:59:07, Jeff King wrote:
> I think we mostly already have that tooling.
>       git rev-parse --sq-quote \
> \
>         'section.key=with spaces' \
>         "or.even=embedded 'quotes'" |
>       sed 's/^ //'; # yuck
>     )
>   $ git config --list --show-scope | grep ^command
>   command
>   command	section.key=with spaces
>   command	or.even=embedded 'quotes'
> The "yuck" there is because --sq-quote insists on putting a space at the
> front. Our parser should probably ignore that, but right now it's quite
> picky.

I feel that this approach leaves a few things to be desired, yes.

> Though I suppose:
>   - do you mean ENV_VAR to literally look up an environment variable?
>     That solves Patrick's original problem of not wanting to put
>     sensitive values onto the command line. But it's much more annoying
>     to use if you _don't_ already have the value in an environment
>     variable. I'm not sure if that original problem matters here, as a
>     program that does a lot of this would probably implement the quoting
>     itself.

Yeah, I did mean that, or various options to either read a literal value
or from the environment.  Your proposal, while it obviously works,
doesn't solve Patrick's problem.

>   - obviously it is doubling down on the shell-quoting as a public
>     thing, and part of your point is that we would leave it opaque.
>     I'm OK with that aspect (even if it ends up as an alias for
>     --sq-quote for now). I'm not entirely sure people aren't using this
>     in the wild already, though. Saying "it was undocumented" may give
>     us a leg to stand on if we change the format, but it will still be
>     annoying to people we break.

I've been telling people for some time that Git doesn't have a way to do
this, so I'm comfortable sticking with that line until we have a
documented way to do it.  I knew we had an internal environment variable
(because I was curious and looked), but I knew from just looking at it
that it was internal.

> Yes, I think concatenating uri_encode(key) + "=" + uri_encode(value)
> would be easier to reason about, and solves the ambiguity problem. If we
> are willing to break from the existing format, it's probably a
> reasonable direction.

We'll have to deal with embedded NULs, but other than that it seems
robust and easy to reason about.  I like the proposal below better,

> I wondered at first how this is different from:
>   git -c a.b.c=value foo
> but I guess it is meant to do the same environment-lookup? We could
> probably add:
>   git --env-config a.b.c=ENV_VAR foo
> to have Git internally stick $ENV_VAR into $GIT_CONFIG_PARAMETERS. That
> avoids all of the rev-parse rigamarole, keeps the format of the
> environment variable opaque, and solves Patrick's problem of putting the
> value onto the command-line.

Sure, that could be an option.  It's the simplest, and we already know
how to handle config this way.  People will be able to figure out how to
use it pretty easily.

> It doesn't solve the ambiguity with "=" in the subsection, but IMHO that
> is not all that important a problem.

I'm fine with saying that we don't support environment variable names
with equals signs and just going with that.  It solves the ambiguity
problem and I'm not sure that they could usefully work on Unix anyway.

Moreover, people usually use the unrestricted data in the second chunk
for URLs, which shouldn't have unquoted equals signs.  You could
technically name other second fields that way, but why would you when
you could just not?

So I'm not too worried about it either way.
brian m. carlson (he/him or they/them)
Houston, Texas, US

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

  reply	other threads:[~2020-11-18  2:28 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-13 12:16 [PATCH 0/2] " Patrick Steinhardt
2020-11-13 12:16 ` [PATCH 1/2] config: extract function to parse config pairs Patrick Steinhardt
2020-11-13 12:16 ` [PATCH 2/2] config: allow specifying config entries via envvar pairs Patrick Steinhardt
2020-11-13 13:04   ` Ævar Arnfjörð Bjarmason
2020-11-16 19:39     ` Junio C Hamano
2020-11-17  2:34       ` Jeff King
2020-11-17  6:37         ` Patrick Steinhardt
2020-11-17  7:01           ` Jeff King
2020-11-17 14:22         ` Ævar Arnfjörð Bjarmason
2020-11-17 23:57           ` Jeff King
2020-11-18 13:44             ` Ævar Arnfjörð Bjarmason
2020-11-18  0:50         ` brian m. carlson
2020-11-18  1:59           ` Jeff King
2020-11-18  2:25             ` brian m. carlson [this message]
2020-11-18  7:04               ` Patrick Steinhardt
2020-11-19  2:11                 ` brian m. carlson
2020-11-19  6:37                   ` Patrick Steinhardt
2020-11-18  5:44           ` Junio C Hamano
2020-11-17  6:28       ` Patrick Steinhardt
2020-11-17  7:06         ` Junio C Hamano
2020-11-18 13:49           ` Ævar Arnfjörð Bjarmason
2020-11-18 13:56             ` Patrick Steinhardt
2020-11-18 16:01             ` Junio C Hamano
2020-11-17 14:03       ` Ævar Arnfjörð Bjarmason
2020-11-13 16:37   ` Philip Oakley
2020-11-17  6:40     ` Patrick Steinhardt
2020-11-13 13:11 ` [PATCH 0/2] " Æ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:

  List information:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \ \
    --subject='Re: [PATCH 2/2] config: allow specifying config entries via envvar pairs' \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Code repositories for project(s) associated with this inbox:

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).