Re: [PATCH] Added sub get_owner_file which checks if there's a file with project owner name

> Bruno Ribas <ribas@c3sl.ufpr.br> writes:
> 
> > This file ($projectroot/$project/owner) is good to have when we don't want to
> > maintain a project  list AND when we share same SSH account for all projects,
> > using ssh_acl for example.
> >
> > Signed-off-by: Bruno Ribas <ribas@c3sl.ufpr.br>
> 
> This explanation is a bit too complicated; it explains farther
> reasons, instead of immediate ones: you don't want to maintain project
> list file, and all repository directories have to have the same owner
> (for example when the same SSH account is shared for all projects,
> using ssh_acl to control access instead).

> 
> Besides with new faster config reader we probably would want to allow
> to use config file to set owner, instead of adding yet another file to
> the repo area; see commit 0e121a2cd42d28bc4034feedf8a13c5a91f85bd3
>   "gitweb: Use config file for repository description and URLs"
> This would have the advantage that you could use system config
> (/etc/gitconfig) to set fallback owner instead of relying on
> filesystem.  I'm not sure what should be the preference, though:
> gitweb.owner, then $GIT_DIR/owner, or vice versa?  I guess that
> reading $GIT_DIR/owner should take preference, as it is needed also
> for projects list page, where ordinary we didn't read individual
> repositories configuration.

> 
> I guess that it is meant to be post 1.5.4, isn't it?
> 
> > ---
> >  gitweb/gitweb.perl |   14 ++++++++++++++
> >  1 files changed, 14 insertions(+), 0 deletions(-)
> > 
> > diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> > index 6256641..fac5f78 100755
> > --- a/gitweb/gitweb.perl
> > +++ b/gitweb/gitweb.perl
> > @@ -1754,6 +1754,15 @@ sub git_get_project_list_from_file {
> >  	}
> >  }
> >  
> > +sub get_owner_file {
> > +    my $owner_file = shift;
> 
> Here you use spaces instead of tabs in indent.
> 
> > +
> > +	open my $fd, "$owner_file" or return undef;
> 
> 	open my $fd, $owner_file or return undef;
> 
> would be simpler.
> 
> > +	my $owner = <$fd>;
> > +	close $fd;
> > +	return to_utf8($owner);
> > +}
> 
> I wonder if we should just bite the bullet and replace all such by
> generic subroutine called e.g. read_singleline_file, or something like
> that.  Or perhaps not, if we want to read alternatively from config,
> with different config variable key names and different preferences of
> file/config priority...
> 
> > +
> >  sub git_get_project_owner {
> >  	my $project = shift;
> >  	my $owner;
> > @@ -1767,6 +1776,11 @@ sub git_get_project_owner {
> >  	if (exists $gitweb_project_owner->{$project}) {
> >  		$owner = $gitweb_project_owner->{$project};
> >  	}
> > +
> > +    if ( -f "$projectroot/$project/owner" ) {
> > +        $owner = get_owner_file("$projectroot/$project/owner");
> > +    }
> > +
> 
> Here you use spaces. I think that you can lose spaces around condition
> in the above 'if'.
> 
> >  	if (!defined $owner) {
> >  		$owner = get_file_owner("$projectroot/$project");
> >  	}
> > -- 
> > 1.5.3.8
> 
> I hope that doesn't mean that this patch is based on v1.5.3.8
> gitweb...
> 
> -- 
> Jakub Narebski
> Poland
> ShadeHawk on #git