Bruno Ribas <ribas@xxxxxxxxxxxx> 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@xxxxxxxxxxxx> 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 - To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html