All of lore.kernel.org
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: "brian m. carlson" <sandals@crustytoothpaste.net>,
	"Jeff King" <peff@peff.net>, "Junio C Hamano" <gitster@pobox.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	git@vger.kernel.org
Subject: Re: [PATCH 2/2] config: allow specifying config entries via envvar pairs
Date: Wed, 18 Nov 2020 08:04:30 +0100	[thread overview]
Message-ID: <X7THfjaP91+GV//V@ncase> (raw)
In-Reply-To: <20201118022532.GD389879@camp.crustytoothpaste.net>

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

On Wed, Nov 18, 2020 at 02:25:32AM +0000, brian m. carlson wrote:
> On 2020-11-18 at 01:59:07, Jeff King wrote:
> > 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,
> though.

This would be easy enough to handle and fixes all the problems I
currently have.

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

At first, this idea sounds quite interesting. But only until one
realizes that it's got the exact same problem which I'm trying to solve:
there's still a point in time where one can observe config values via
the command line, even though that moment now is a lot shorter compared
to running the "real" git command with those keys.

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

There's not only URLs though, but also paths in 'includeIf.*.path'.
Sure, it's unlikely that paths have an equals sign embedded. But I think
we should try to not paint ourselves into a corner.

This problem also would be nicely solved by URI-encoding both key and
value and then passing it via GIT_CONFIG_PARAMETER.

Patrick

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

  reply	other threads:[~2020-11-18  7:04 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-13 12:16 [PATCH 0/2] config: allow specifying config entries via envvar pairs 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
2020-11-18  7:04               ` Patrick Steinhardt [this message]
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:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

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

  git send-email \
    --in-reply-to=X7THfjaP91+GV//V@ncase \
    --to=ps@pks.im \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    --cc=sandals@crustytoothpaste.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.