git-cvsserver doesn't respect core.sharedrepository

git-cvsserver doesn't respect core.sharedrepository

[Andy Parkins]

Hello,

I'm using git-cvsserver with a repository that has core.sharedrepository set 
to "group".  After a commit has been pushed using git-cvsserver I have to 
correct the permissions again.

I suspect then that git-cvsserver is somehow bypassing git's normal fixing of 
the permissions bits.  I'm not entirely sure though, as I can't entirely see 
how the calling of path.c:adjust_shared_perm() can be avoided.


Andy
-- 
Dr Andy Parkins, M Eng (hons), MIEE
andyparkins@gmail.com

Re: git-cvsserver doesn't respect core.sharedrepository

[Johannes Schindelin]

Hi,

On Tue, 13 Feb 2007, Andy Parkins wrote:

> I'm using git-cvsserver with a repository that has core.sharedrepository set 
> to "group".  After a commit has been pushed using git-cvsserver I have to 
> correct the permissions again.

Are you sure?

Since ref updates are not write-into-existing-file, but rather 
write-into-new-file-and-replace-old, it should work, no?

Ciao,
Dscho

Re: git-cvsserver doesn't respect core.sharedrepository

[Andy Parkins]

On Tuesday 2007 February 13 15:11, Johannes Schindelin wrote:

> Since ref updates are not write-into-existing-file, but rather
> write-into-new-file-and-replace-old, it should work, no?

That perhaps might be the explanation for the bad behaviour.  The ref is being 
updated in git-cvsserver by writing the new head hash into the lockfile then 
doing

    unlink($reffile);
    rename($lockfile, $reffile);

The lockfile is made with whatever umask is in place, so the 
sharedrepository=group is being ignored.  The solution then is to use the 
permissions of the existing reffile when creating the lockfile.

Thanks Johannes.  One little sentence and it all becomes clear :-)


Andy
-- 
Dr Andy Parkins, M Eng (hons), MIEE
andyparkins@gmail.com

Re: git-cvsserver doesn't respect core.sharedrepository

[Nicolas Pitre]

On Tue, 13 Feb 2007, Andy Parkins wrote:

> That perhaps might be the explanation for the bad behaviour.  The ref is being 
> updated in git-cvsserver by writing the new head hash into the lockfile then 
> doing
> 
>     unlink($reffile);
>     rename($lockfile, $reffile);

Wouldn't it be much better if it used git-update-ref instead?  This 
manual ref updating looks like it has a potential for races to me.

Also git-update-ref should probably be used with -m to create reflog 
entries.


Nicolas

Re: git-cvsserver doesn't respect core.sharedrepository

[Johannes Schindelin]

Hi,

On Tue, 13 Feb 2007, Andy Parkins wrote:

> On Tuesday 2007 February 13 15:11, Johannes Schindelin wrote:
> 
> > Since ref updates are not write-into-existing-file, but rather
> > write-into-new-file-and-replace-old, it should work, no?
> 
> That perhaps might be the explanation for the bad behaviour.

Ummm. What I tried to say is that this is intended behaviour, not bad 
behaviour. The file does not have to have write permissions for the group. 
The _directory_ has to have them.

Ciao,
Dscho

Re: git-cvsserver doesn't respect core.sharedrepository

[Andy Parkins]

On Tuesday 2007, February 13, Johannes Schindelin wrote:

> Ummm. What I tried to say is that this is intended behaviour, not bad
> behaviour. The file does not have to have write permissions for the
> group. The _directory_ has to have them.

It's not just write permissions.  The default umask could also prevent 
read access by the group - the new ref needs to be readable by the 
group.


Andy
-- 
Dr Andrew Parkins, M Eng (Hons), AMIEE
andyparkins@gmail.com

Re: git-cvsserver doesn't respect core.sharedrepository

[Martin Langhoff]

On 2/14/07, Andy Parkins <andyparkins@gmail.com> wrote:
> On Tuesday 2007, February 13, Johannes Schindelin wrote:
>
> > Ummm. What I tried to say is that this is intended behaviour, not bad
> > behaviour. The file does not have to have write permissions for the
> > group. The _directory_ has to have them.
>
> It's not just write permissions.  The default umask could also prevent
> read access by the group - the new ref needs to be readable by the
> group.

I agree. cvsserver depends on the user's umask just like an oldstyle
cvs server. This is probably wrong, and you are right - we should
respect core.sharedrepository.

The recent change to use git-update-ref should have fixed part of the
problem. The other part of the problem is the handling of the sqlite
files. All the other operations that leave permanent files in the repo
are using git plumbing so we are safe.

To fix the sqlite files handling, the fix is to change the umask early
on if core.sharedrepository is true.

cheers,


martin