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

From: Jakub Narebski <jnareb@gmail.com>
Date: Jan 29, 2008, 11:26 UTC
Message-ID: <m3lk6898kg.fsf@localhost.localdomain>
In-Reply-To: <1201577766-11601-1-git-send-email-ribas@c3sl.ufpr.br>

Bruno Ribas <ribas@c3sl.ufpr.br> writes:

Show 5 quoted lines

> 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?

Show 14 quoted lines

> ---
>  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...

Show 13 quoted lines

> +
>  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'.

Show 5 quoted lines

>  	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