Re*: [RFC PATCH 0/2] upload-pack.c: limit allowed filter choices

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

>>   - '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].