Re*: [RFC PATCH 0/2] upload-pack.c: limit allowed filter choices
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 18, 2020, 18:26 UTC
- Message-ID
- <xmqqtv2lfrk7.fsf_-_@gitster.c.googlers.com>
- In-Reply-To
- <20200318101825.GB1227946@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 10 quoted lines
>> - 'uploadpack.filter.<filter>.allow' -> specifying whether or not each >> filter kind is allowed or not. (Originally this was given as 'git >> config uploadpack.filter=blob:none.allow true', but this '=' is >> ambiguous to configuration given over '-c', which itself uses an '=' >> to separate keys from values.) > > One thing that's a little ugly here is the embedded dot in the > subsection (i.e., "filter.<filter>"). It makes it look like a four-level > key, but really there is no such thing in Git. But everything else we > tried was even uglier.
I think this gives us the best arrangement by upfront forcing all the configuration handers for "<subcommand>.*.<token>" namespace, current and future, to use "<group-prefix>" before the unbounded set of user-specifiable values that affects the <subcommand> (which is "uploadpack").
So far, the configuration variables that needs to be grouped by unbounded set of user-specifiable values we supported happened to have only one sensible such set for each <subcommand>, so we could get away without such <group-prefix> and it was perfectly OK to have, say "guitool.<name>.cmd".
Syntactically, the convention to always end such <group-prefix> with a dot "." may look unusual, or once readers' eyes get used to them, may look natural. One tiny sad thing about it is that it cannot be mechanically enforced, but that is minor.
> We could do "uploadpackfilter.allow" and "uploadpackfilter.<filter>.allow", > but that's both ugly _and_ separates these options from the rest of > uploadpack.*.
There is an existing instance of a configuration that affects <subcommand> that uses a different word after <subcommand>, which is credentialCache.ignoreSIGHUP, and I tend to agree that it is ugly.
By the way, I noticed the following while I was studying the current practice, so before I forget...
-- >8 -- Subject: [PATCH] separate tar.* config to its own source file
Even though there is only one configuration variable in the namespace, it is not quite right to have tar.umask described among the variables for tag.* namespace.
Signed-off-by: Junio C Hamano <gitster@pobox.com> --- Documentation/config.txt | 2 ++ Documentation/config/tag.txt | 7 ------- Documentation/config/tar.txt | 6 ++++++ 3 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt index 08b13ba72b..2450589a0e 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -447,6 +447,8 @@ include::config/submodule.txt[] include::config/tag.txt[] +include::config/tar.txt[] + include::config/trace2.txt[] include::config/transfer.txt[] diff --git a/Documentation/config/tag.txt b/Documentation/config/tag.txt index 6d9110d84c..5062a057ff 100644 --- a/Documentation/config/tag.txt +++ b/Documentation/config/tag.txt @@ -15,10 +15,3 @@ tag.gpgSign:: convenient to use an agent to avoid typing your gpg passphrase several times. Note that this option doesn't affect tag signing behavior enabled by "-u <keyid>" or "--local-user=<keyid>" options. - -tar.umask:: - This variable can be used to restrict the permission bits of - tar archive entries. The default is 0002, which turns off the - world write bit. The special value "user" indicates that the - archiving user's umask will be used instead. See umask(2) and - linkgit:git-archive[1]. diff --git a/Documentation/config/tar.txt b/Documentation/config/tar.txt new file mode 100644 index 0000000000..de8ff48ea9 --- /dev/null +++ b/Documentation/config/tar.txt @@ -0,0 +1,6 @@ +tar.umask:: + This variable can be used to restrict the permission bits of + tar archive entries. The default is 0002, which turns off the + world write bit. The special value "user" indicates that the + archiving user's umask will be used instead. See umask(2) and + linkgit:git-archive[1].