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

[Petr Baudis <pasky@suse.cz>]

On Thu, Jul 10, 2008 at 10:34:57PM -0700, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> 
> > 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.
> >
> > Donn't our existing tests catch this, and if the answer is no because we
> > don't have any, could you add some?

(Will do. Wanted to do it but forgot. :-)

> >> diff --git a/path.c b/path.c
> >> index 5983255..75c5915 100644
> >> --- a/path.c
> >> +++ b/path.c
> >> @@ -269,7 +269,7 @@ int adjust_shared_perm(const char *path)
> >>  	mode = st.st_mode;
> >>  
> >>  	if (shared_repository) {
> >> -		int tweak = shared_repository;
> >> +		int tweak = (mode & 0777) | shared_repository;
> >>  		if (!(mode & S_IWUSR))
> >>  			tweak &= ~0222;
> >>  		mode = (mode & ~0777) | tweak;
> >
> > I think this change is good.  shared_repository has always been about
> > widening the access and not about limiting.
> 
> Having said that, you really should protect this behaviour from regression
> with a test case.  I do not see practical difference for sane umask
> values.
> 
> What umask are you using, and which file in the repository gets affected?
> In the old code I see we do have checks for S_IXUSR and tweaks on S_IXGRP
> and S_IXOTH, but this should make a difference only if your umask blocks
> executable bit and the file in question is executable.  Was it an
> executable hook copied from template?

My umask is 002, and the difference is visible at all the files in the
repository (in existing repositories, update will cause new object files
have the missing permission, making for the most confusing behaviour).

With sane Git versions, the permissions when shared=1 are rwxrwxr-x,
with recent Git however, it is rwxrwx--- (modulo the exec bits).

-- 
				Petr "Pasky" Baudis
The last good thing written in C++ was the Pachelbel Canon. -- J. Olson