On Mon, Nov 16 2020, Junio C Hamano wrote: > Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes: > >> On Fri, Nov 13 2020, Patrick Steinhardt wrote: >> >>> While not document, it is currently possible to specify config entries >> >> "While not documented..." >> >>> + 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.
