| Message ID | xmqqtv2lfrk7.fsf_-_@gitster.c.googlers.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Re*: [RFC PATCH 0/2] upload-pack.c: limit allowed filter choices | expand |
On Wed, Mar 18, 2020 at 11:26:00AM -0700, Junio C Hamano wrote: > > 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". Yeah. We have often just split those out into a separate hierarchy from <subcommand> E.g., tar.<format>.command, which is really feeding the git-archive command. We could do that here, too, but I wasn't sure of a good name (this really is upload-pack specific, though I guess in theory other commands could grow a need to look at or restrict "remote object filters"). > 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. The biggest downside to implying a 4-level key is that the case-sensitivity rules may be different. I.e., you can say: UploadPack.filter.blob:none.Allow but not: UploadPack.Filter.blob:none.Allow Since "filter" is part of the subsection, it's case sensitive. We could match it case-insensitively in upload_pack_config(), but it would crop up in other laces (e.g., "git config --unset" would still care). > > 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. I don't think that's what's going on here. It affects only the credential-cache subcommand, but we avoid hyphens in our key names. So it really is the subcommand; it's just that the name is a superset of another command name. :) > 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. Yeah, this is definitely an improvement. But I was surprised that tar.<format>.* wasn't covered here. It is documented in git-archive. Probably worth moving or duplicating it in git-config. -Peff
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].