git.owner, hope everything is fine

git.owner, hope everything is fine

[Bruno Ribas]

Made final adjustments and hope it is OK now.

[PATCH] gitweb: Use the config file to set repository owner's name.

[Bruno Ribas]

Now gitweb checks if gitweb.owner exists before trying to get filesystem's
owner.

Allow to use configuration variable gitweb.owner set the repository owner,
it checks the gitweb.owner, if not set it uses filesystem directory's owner.

Useful when we 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).

Signed-off-by: Bruno Ribas <ribas@c3sl.ufpr.br>
---
 gitweb/gitweb.perl |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 8ef2735..c8fe22a 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1759,6 +1759,7 @@ sub git_get_project_owner {
 	my $owner;
 
 	return undef unless $project;
+	$git_dir = "$projectroot/$project";
 
 	if (!defined $gitweb_project_owner) {
 		git_get_project_list_from_file();
@@ -1767,8 +1768,11 @@ sub git_get_project_owner {
 	if (exists $gitweb_project_owner->{$project}) {
 		$owner = $gitweb_project_owner->{$project};
 	}
+	if (!defined $owner){
+		$owner = git_get_project_config('owner');
+	}
 	if (!defined $owner) {
-		$owner = get_file_owner("$projectroot/$project");
+		$owner = get_file_owner("$git_dir");
 	}
 
 	return $owner;
-- 
1.5.4.24.gce08d

[PATCH] gitweb: Update gitweb/README to include the new per-repository gitweb.owner

[Bruno Ribas]

Signed-off-by: Bruno Ribas <ribas@c3sl.ufpr.br>
---
 gitweb/README |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/gitweb/README b/gitweb/README
index 4c8bedf..2163071 100644
--- a/gitweb/README
+++ b/gitweb/README
@@ -233,6 +233,10 @@ You can use the following files in repository:
    Displayed in the project summary page. You can use multiple-valued
    gitweb.url repository configuration variable for that, but the file
    takes precendence.
+ * gitweb.owner
+   You can use the gitweb.owner repository configuration variable to set
+   repository's owner. It is displayed in the project list and summary
+   page. If it's not set, filesystem directory's owner is used.
  * various gitweb.* config variables (in config)
    Read description of %feature hash for detailed list, and some
    descriptions.
-- 
1.5.4.24.gce08d

Re: [PATCH] gitweb: Use the config file to set repository owner's name.

[Bruno Cesar Ribas]

Was it forgotten ? 8^)

On Fri, Feb 08, 2008 at 02:38:03PM -0200, Bruno Ribas wrote:
> Now gitweb checks if gitweb.owner exists before trying to get filesystem's
> owner.
> 
> Allow to use configuration variable gitweb.owner set the repository owner,
> it checks the gitweb.owner, if not set it uses filesystem directory's owner.
> 
> Useful when we 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).
> 
> Signed-off-by: Bruno Ribas <ribas@c3sl.ufpr.br>
> ---
>  gitweb/gitweb.perl |    6 +++++-
>  1 files changed, 5 insertions(+), 1 deletions(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 8ef2735..c8fe22a 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -1759,6 +1759,7 @@ sub git_get_project_owner {
>  	my $owner;
>  
>  	return undef unless $project;
> +	$git_dir = "$projectroot/$project";
>  
>  	if (!defined $gitweb_project_owner) {
>  		git_get_project_list_from_file();
> @@ -1767,8 +1768,11 @@ sub git_get_project_owner {
>  	if (exists $gitweb_project_owner->{$project}) {
>  		$owner = $gitweb_project_owner->{$project};
>  	}
> +	if (!defined $owner){
> +		$owner = git_get_project_config('owner');
> +	}
>  	if (!defined $owner) {
> -		$owner = get_file_owner("$projectroot/$project");
> +		$owner = get_file_owner("$git_dir");
>  	}
>  
>  	return $owner;
> -- 
> 1.5.4.24.gce08d

-- 
Bruno Ribas - ribas@c3sl.ufpr.br
http://web.inf.ufpr.br/ribas
C3SL: http://www.c3sl.ufpr.br 

Re: [PATCH] gitweb: Use the config file to set repository owner's name.

[Junio C Hamano]

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

> Was it forgotten ? 8^)

No, I have it on one topic, and I personally do not see an issue
with it, but I haven't merged it to 'pu' nor 'next' yet.  I
thought Jakub wanted to give Pasky the first refusal right, so
I've been waiting for a few days.

Adding gitweb.owner, last shot

[Bruno Ribas]

After some study about the insertion of a new repository configuration,
which sets repository owner, it was decided by the group that creating
another small file inside .git/ is not a good idea. So I started to bench
the viability to add just the gitweb.owner configuration, using gitweb.owner
together with gitweb.description there is no major performance downgrade
compared to $projects_list , as seen below:

8<-------
These times i got with a 1000projects running 2 dd to generate disk IO.
Here comes the resultm
NO projects_list  projects_list
16m30s69          15m10s74       default gitweb, using FS's owner
16m07s40          15m24s34       patched to get gitweb.owner
16m37s76          15m59s32       same above, but without gitweb.owner

Now results for a 1000projects on an idle machine.
NO projects_list  projects_list
1m19s08           1m09s55       default gitweb, using FS's owner
1m17s58           1m09s55       patched to get gitweb.owner
1m18s49           1m08s96       same above, but without gitweb.owner
8<-------

The idea of creating only the gitweb.owner can be a case of study to
centralize all gitweb repository configuration in one file. Maybe even
change the way $projects_list is formated, as gitweb.cgi needs to check
repository configuration to get repository description to generate
projecT_list page, we could just list repository's directories.

[PATCH] gitweb: Use the config file to set repository owner's name.

[Bruno Ribas]

From: Git Managment for C3SL <git@git.c3sl.ufpr.br>

Now gitweb checks if gitweb.owner exists before trying to get filesystem's
owner.

Allow to use configuration variable gitweb.owner set the repository owner,
it checks the gitweb.owner, if not set it uses filesystem directory's owner.

Useful when we 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).

Signed-off-by: Git Managment for C3SL <git@git.c3sl.ufpr.br>
---
 gitweb/gitweb.perl |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 8ef2735..e8a43b7 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1767,7 +1767,12 @@ sub git_get_project_owner {
 	if (exists $gitweb_project_owner->{$project}) {
 		$owner = $gitweb_project_owner->{$project};
 	}
-	if (!defined $owner) {
+
+	if (!defined $owner){
+		$owner = git_get_project_config('owner');
+	}
+
+	if (!$owner) {
 		$owner = get_file_owner("$projectroot/$project");
 	}
 
-- 
1.5.4.34.g053d9-dirty

Re: [PATCH] gitweb: Use the config file to set repository owner's name.

[Jakub Narebski]

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

> Allow to use configuration variable gitweb.owner set the repository owner,
> it checks the gitweb.owner, if not set it uses filesystem directory's owner.

> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 8ef2735..e8a43b7 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -1767,7 +1767,12 @@ sub git_get_project_owner {
>  	if (exists $gitweb_project_owner->{$project}) {
>  		$owner = $gitweb_project_owner->{$project};
>  	}
> -	if (!defined $owner) {
> +
> +	if (!defined $owner){
> +		$owner = git_get_project_config('owner');
> +	}
> +
> +	if (!$owner) {
>  		$owner = get_file_owner("$projectroot/$project");
>  	}
>  

First, I think the empty lines added are not needed.

Second, git_get_project_config() subroutine _REQUIRES_ for $git_dir to
be set. So you have to set $git_dir before checking repo config; then
you can reuse $git_dir in checking file owner.

-- 
Jakub Narebski
Poland
ShadeHawk on #git

Re: [PATCH] gitweb: Use the config file to set repository owner's name.

[Bruno Cesar Ribas]

On Fri, Feb 08, 2008 at 02:55:33AM -0800, Jakub Narebski wrote:
> Bruno Ribas <ribas@c3sl.ufpr.br> writes:
> 
><snip> 
> First, I think the empty lines added are not needed.

I made those empty lines because original code had same empty lines above, I
just let it to have same pattern, but I can remove. Should I remove?! I'll
resend without it, and with $git_dir set.
> 
> Second, git_get_project_config() subroutine _REQUIRES_ for $git_dir to
> be set. So you have to set $git_dir before checking repo config; then
> you can reuse $git_dir in checking file owner.
> 
> -- 
> Jakub Narebski
> Poland
> ShadeHawk on #git

-- 
Bruno Ribas - ribas@c3sl.ufpr.br
http://web.inf.ufpr.br/ribas
C3SL: http://www.c3sl.ufpr.br 

Re: [PATCH] gitweb: Use the config file to set repository owner's name.

[Bruno Cesar Ribas]

On Fri, Feb 08, 2008 at 11:53:27AM -0200, Bruno Cesar Ribas wrote:
> On Fri, Feb 08, 2008 at 02:55:33AM -0800, Jakub Narebski wrote:
> > Bruno Ribas <ribas@c3sl.ufpr.br> writes:
> > 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 8ef2735..e8a43b7 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -1767,7 +1767,12 @@ sub git_get_project_owner {
>       if (exists $gitweb_project_owner->{$project}) {
>               $owner = $gitweb_project_owner->{$project};
>       }
> -     if (!defined $owner) {
> +
> +     if (!defined $owner){
> +             $owner = git_get_project_config('owner');
> +     }
> +
> +     if (!$owner) {
>               $owner = get_file_owner("$projectroot/$project");
>       }
>


I that last 3lines should be inside the block that we call
git_get_project_config, don't you think?

> > First, I think the empty lines added are not needed.
> 
> I made those empty lines because original code had same empty lines above, I
> just let it to have same pattern, but I can remove. Should I remove?! I'll
> resend without it, and with $git_dir set.
> > 
> > Second, git_get_project_config() subroutine _REQUIRES_ for $git_dir to
> > be set. So you have to set $git_dir before checking repo config; then
> > you can reuse $git_dir in checking file owner.
> > 
> > -- 
> > Jakub Narebski
> > Poland
> > ShadeHawk on #git
> 
> -- 
> Bruno Ribas - ribas@c3sl.ufpr.br
> http://web.inf.ufpr.br/ribas
> C3SL: http://www.c3sl.ufpr.br 
> -
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Bruno Ribas - ribas@c3sl.ufpr.br
http://web.inf.ufpr.br/ribas
C3SL: http://www.c3sl.ufpr.br 

Re: [PATCH] gitweb: Use the config file to set repository owner's name.

[Jakub Narebski]

I have joined the two emails to reply only once.

On Fri, 8 Feb 2008, Bruno Cesar Ribas wrote:
> On Fri, Feb 08, 2008 at 02:55:33AM -0800, Jakub Narebski wrote:
>> Bruno Ribas <ribas@c3sl.ufpr.br> writes:
>>> 
>>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
>>> index 8ef2735..e8a43b7 100755
>>> --- a/gitweb/gitweb.perl
>>> +++ b/gitweb/gitweb.perl
>>> @@ -1767,7 +1767,12 @@ sub git_get_project_owner {
>>>       if (exists $gitweb_project_owner->{$project}) {
>>>               $owner = $gitweb_project_owner->{$project};
>>>       }
>>> -     if (!defined $owner) {
>>> +
>>> +     if (!defined $owner){
>>> +             $owner = git_get_project_config('owner');
>>> +     }
>>> +
>>> +     if (!$owner) {
>>>               $owner = get_file_owner("$projectroot/$project");
>>>       }

Another comment: why did you change from checking of "!defined $owner"
to checking "!$owner"? git_get_project_config('owner') returns undef
if gitweb.owner is not defined. With checking for defined we can avoid
false positives of owner being "0" (in practice I think this does not
matter) or "" (this could happen if somebody doesn't want for project
to have owner shown).

>> First, I think the empty lines added are not needed.
> 
> I made those empty lines because original code had same empty lines
> above, I just let it to have same pattern, but I can remove. Should I
> remove?!

The idea was for empty lines to separate blocks of code: variables 
declaration, initialization, finding an owner, and return value.
So I think that empty lines are not needed here. There were no empty 
lines between check for owner in the structure populated by 
git_get_project_list_from_file() and checking filesystem stat for 
project directory owner.

By the way, the git_get_project_list_from_file() interface is a bit 
strange...

> I that last 3lines should be inside the block that we call
> git_get_project_config, don't you think?

No. I think using "if (!defined $foo) { maybe define foo }..."
sequence is a good flow.

> I'll resend [...] with $git_dir set.

And with signoff corrected, I assume?

Please try to check if the code works with and without gitweb.owner set 
before sending new version of the patch...
-- 
Jakub Narebski
Poland

Re: [PATCH] gitweb: Use the config file to set repository owner's name.

[Bruno Cesar Ribas]

On Fri, Feb 08, 2008 at 04:33:54PM +0100, Jakub Narebski wrote:
> I have joined the two emails to reply only once.
> 
> On Fri, 8 Feb 2008, Bruno Cesar Ribas wrote:
> > On Fri, Feb 08, 2008 at 02:55:33AM -0800, Jakub Narebski wrote:
> >> Bruno Ribas <ribas@c3sl.ufpr.br> writes:
> >>> 
> >>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> >>> index 8ef2735..e8a43b7 100755
> >>> --- a/gitweb/gitweb.perl
> >>> +++ b/gitweb/gitweb.perl
> >>> @@ -1767,7 +1767,12 @@ sub git_get_project_owner {
> >>>       if (exists $gitweb_project_owner->{$project}) {
> >>>               $owner = $gitweb_project_owner->{$project};
> >>>       }
> >>> -     if (!defined $owner) {
> >>> +
> >>> +     if (!defined $owner){
> >>> +             $owner = git_get_project_config('owner');
> >>> +     }
> >>> +
> >>> +     if (!$owner) {
> >>>               $owner = get_file_owner("$projectroot/$project");
> >>>       }
> 
> Another comment: why did you change from checking of "!defined $owner"
> to checking "!$owner"? git_get_project_config('owner') returns undef
> if gitweb.owner is not defined. With checking for defined we can avoid
> false positives of owner being "0" (in practice I think this does not
> matter) or "" (this could happen if somebody doesn't want for project
> to have owner shown).

When I tested it returned empty, but i found out it was lack of NAME for the
user i was running tests.

> 
> 
> > I'll resend [...] with $git_dir set.
> 
> And with signoff corrected, I assume?

of course.
> 
> Please try to check if the code works with and without gitweb.owner set 
> before sending new version of the patch...

I always do that.
> -- 
> Jakub Narebski
> Poland

-- 
Bruno Ribas - ribas@c3sl.ufpr.br
http://web.inf.ufpr.br/ribas
C3SL: http://www.c3sl.ufpr.br