Re: [PATCHv3] Fix backwards-incompatible handling of core.sharedRepository

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

Petr Baudis <pasky@suse.cz> writes:

> The 06cbe8550324e0fd2290839bf3b9a92aa53b70ab core.sharedRepository
> handling extension broke backwards compatibility; before, shared=1 meant
> that Git merely ensured the repository is group-writable, not that it's
> _only_ group-writable, which is the current behaviour. Thus, with umask 002,
> Git creates repository with /rw.rw.--./, this patch fixes it back to
> /rw.rw.r-./.

Is it just me who finds the above unreadable blob of black ink?

	06cbe85 (Make core.sharedRepository more generic, 2008-04-16)
	broke the traditional setting core.sharedRepository to true,
        which was to make the repository group writable.

	The call to adjust_shared_perm() should only loosen the
	permission.  If the user has umask 002 (or 022) that allow others
	to read, the resulting files should be made readable and writable
	by group, without restricting the readability by others.

> Maybe it makes sense to provide the current semantics in some way too,
> but that cannot be done at the expense of ditching backwards
> compatibility; this bug has just wasted me two hours and broke
> repo.or.cz pushing for several hours.

I do not think this gripe after the semicolon belongs before the
three-dash lines.

> Cc: Heikki Orsila <heikki.orsila@iki.fi>
> Signed-off-by: Petr Baudis <pasky@rover.dkm.cz>
> ---
>
>   Oops, this testcase wouldn't really remove its test subrepository
> after itself properly, though the testsuite would still pass; of course
> this bug slipped through all the previous visual inspections of mine.
>
>   Sorry for the continuous noise. :-)

Hmm, I am very puzzled.

I am afraid I have to ask you for another round.  I applied only your
update to t/t1301 without the change to path.c, and the test passes, which
means either (1) the test is incorrect and does not exposing the breakage,
or (2) the breakage is not real and existing code is correct.