Re: [PATCH 2/2] config: allow specifying config entries via envvar pairs

On Mon, Nov 16 2020, Junio C Hamano wrote:

Ævar Arnfjörð Bjarmason [off-list ref] writes:

quoted

On Fri, Nov 13 2020, Patrick Steinhardt wrote:

quoted

While not document, it is currently possible to specify config entries

"While not documented..."

quoted

+		strbuf_addf(&envvar, "GIT_CONFIG_KEY_%d", i);
+		if ((key = getenv(envvar.buf)) == NULL)
+			break;

The convention in git.git is to avoid explicit NULL checks. So maybe
something like this, which also avoids the assignment inside an "if"

    key = getenv(envvar.buf);
    if (!key)
        break;

All good suggestions, but...

"While not documented" yes, for sure, but we do not document it for
a good reason---it is a pure implementation detail between Git
process that runs another one as its internal implementation detail.

*nod* I didn't mean it should be treated as some API, just "it happens
 to work". I do agree with Jeff downthread that it would be nice to have
 it explicitly supported.

I especially do not think we want to read from unbounded number of
GIT_CONFIG_KEY_<N> variables like this patch does.  How would a
script cleanse its environment to protect itself from stray such
environment variable pair?  Count up from 1 to forever?  Run "env"
and grep for "GIT_CONFIG_KEY_[0-9]*=" (the answer is No.  What if
some environment variables have newline in its values?)

Purely on an implementation note, if we went that route we could provide
something based on compat/unsetenv.c (or environ iteration in general)
that would loop over the env, but I agree it would be better to make
GIT_CONFIG_PARAMETERS nice.