From: Patrick Steinhardt <email@example.com> To: "Ævar Arnfjörð Bjarmason" <firstname.lastname@example.org> Cc: Junio C Hamano <email@example.com>, firstname.lastname@example.org Subject: Re: [PATCH 2/2] config: allow specifying config entries via envvar pairs Date: Wed, 18 Nov 2020 14:56:50 +0100 [thread overview] Message-ID: <X7UoIjflsJe7xOx8@ncase> (raw) In-Reply-To: <email@example.com> [-- Attachment #1: Type: text/plain, Size: 2261 bytes --] 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 <firstname.lastname@example.org> 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 #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --]
next prev parent reply other threads:[~2020-11-18 13:58 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 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 [this message] 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 List information: http://vger.kernel.org/majordomo-info.html * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=X7UoIjflsJe7xOx8@ncase \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --subject='Re: [PATCH 2/2] config: allow specifying config entries via envvar pairs' \ /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
Code repositories for project(s) associated with this inbox: https://80x24.org/mirrors/git.git 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).