On Wed, Nov 18, 2020 at 02:49:39PM +0100, Ævar Arnfjörð Bjarmason wrote: > > On Tue, Nov 17 2020, Junio C Hamano wrote: > > > Patrick Steinhardt <ps@xxxxxx> writes: > > > >>> 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?) > >> > >> You only have to unset `GIT_CONFIG_KEY_1` as the parser will stop > >> iterating on the first missing key. More generally, if you set `n` keys, > >> it's sufficient to unset key `n+1`. > > > > Yes, but those who want to set N keys would likely to be content > > with setting 1..N and happily forget unsetting N+1, and that is > > where "how would one cleanse the environment to give a clean slate?" > > comes from. > > Not as an argument from whataboutism, but just to note a bug/existing > prior art: > > Nobody in this thread has mentioned GIT_PUSH_OPTION_* which works pretty > much like Patrick's suggestion, and it looks like --local-env-vars > misses those: > > $ GIT_PUSH_OPTION_0=foo GIT_PUSH_OPTION_COUNT=20 git rev-parse --local-env-vars | grep GIT_PUSH > $ > > I haven't tested this, but I expect there's a bug where a push hook > itself does a local push to another repo and that repo has a hook, that > the push options are erroneously carried forward to the sub-process. > > That might also be a feature, depending on your point of view. I didn't actually know about it, thanks for pointing it out! If we're going to use the same `_COUNT` approach, then I think the issues which were discussed would mostly go away. No gaps needed, no requirement to unset `$n+1`. Any properly behaving user would know to set it as otherwise the written code/script cannot work. Also, git-rev-parse(1) wouldn't have to dynamically adjust based on whether a previously existing gap was filled with new keys or not. I'd be happy to pursue that road, but I'll wait for some feedback first. Patrick
Attachment:
signature.asc
Description: PGP signature
