Re: [PATCH] Add two core.sharedRepository options: group-readable and world-readable

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 12, 2008, 10:03 UTC
Message-ID: <7vprsvie4x.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <20080412091723.GB20443@atjola.homenet>

Björn Steinbrink <B.Steinbrink@gmx.de> writes:

Show 15 quoted lines

> On 2008.04.11 21:48:36 -0700, Junio C Hamano wrote:
>> ...
>> My question was about the "o=" part.  I did not see you dropping bits for
>> others in your patch.
>> 
>> And if your answer is "the user should have xx7 umask", that defeats the
>> whole point of your patch, as you are trying to dissociate the umask used
>> by the user for his usual task and enforce particular permission policy
>> for the repository.
>
> I don't think it defeats the purpose of the patch...
> ... From that point of view, I think that the patch is a natural
> enhancement, allowing to override the umask in a way that only grants
> additional read permissions for either the group, or the group and
> others. And that's exactly what Heikki was after.

That is exactly my point.

If a patch (cl)aims to improve things by overriding user's umask, why should it limit itself only in the loosening direction?

The patch adds a handful new keywords allowed in the config area; if we want to replace or enhance what Heikki's patch does later with a more general mechanism that allows both loosening and tightening user's umask, these keywords that represent "a set of umask modifications that were designed with only loosening in mind" can become maintenance burden, as we must keep them for backward compatibility.

They may look small, but these things add up. Even though incremental improvement is possible and it generally is a much better way to enhance the system, compared to an overengineered complexity that ends up being not used by anybody, it is my job to bring it up as a potential issue when there is a future enhancement possibilty (which may or may not be useful after all) that may become more cumbersome to implement if we do this half-step now.

For this reason, I am not impressed by a "this is a half-step and is better than the status quo" as a supporting argument. A supporting argument that says "Theoretically tightening could also be useful, but it is much less so, compared to loosening which Heikki implemented. The reason why tightening is not so useful is because of such-and-such. Hence, it is reasonable to implement only loosening." is much much more convincing.

Please fill in the "such-and-such" above, convince me and others, so that I can apply Heikki's patch.

I use quite a liberal umask myself for normal uses and a natural expectation for a patch to improve in this area from me is to allow applying a tighter-than-usual mask for a particular set of repositories that house my personal stuff. One practical workaround I can think of is to just manually limit the access by running chmod at the toplevel of the repository, so that _could_ be your "such-and-such". Although it feels somewhat hacky, I can live with it.