On Tue, Mar 17, 2020 at 02:39:05PM -0600, Taylor Blau wrote: > Hi Christian, > > Of course, I would be happy to send along our patches. They are included > in the series below, and correspond roughly to what we are running at > GitHub. (For us, there have been a few more clean-ups and additional > patches, but I squashed them into 2/2 below). > > The approach is roughly that we have: > > - 'uploadpack.filter.allow' -> specifying the default for unspecified > filter choices, itself defaulting to true in order to maintain > backwards compatibility, and > > - '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 we want to declare a real subsection for each filter and not just "uploadpack.filter.<filter>". That gives us room to expand to other config options besides "allow" later on if we need to. We don't want to claim "uploadpack.allow" and "uploadpack.<filter>.allow"; that's too generic. Likewise "filter.allow" is too generic. We could do "uploadpackfilter.allow" and "uploadpackfilter.<filter>.allow", but that's both ugly _and_ separates these options from the rest of uploadpack.*. We could use a character besides ".", which would reduce confusion. But what? Using colon is kind of ugly, because it's already syntactically significant in filter names, and you get: uploadpack.filter:blob:none.allow We tried equals, like: uploadpack.filter=blob:none.allow but there's an interesting side effect. Doing: git -c uploadpack.filter=blob:none.allow=true upload-pack ... doesn't work, because the "-c" parser ends the key at the first "=". As it should, because otherwise we'd get confused by an "=" in a value. This is a failing of the "-c" syntax; it can't represent values with "=". Fixing it would be awkward, and I've never seen it come up in practice outside of this (you _could_ have a branch with a funny name and try to do "git -c branch.my=funny=branch.remote=origin" or something, but the lack of bug reports suggests nobody is that masochistic). So...maybe the extra dot is the last bad thing? > I noted in the second patch that there is the unfortunate possibility of > encountering a SIGPIPE when trying to write the ERR sideband back to a > client who requested a non-supported filter. Peff and I have had some > discussion off-list about resurrecting SZEDZER's work which makes room > in the buffer by reading one packet back from the client when the server > encounters a SIGPIPE. It is for this reason that I am marking the series > as 'RFC'. For reference, the patch I was thinking of was this: https://lore.kernel.org/git/20190830121005.GI8571@xxxxxxxxxx/ -Peff
