[Patch] Use a default for a bad env config file variable

[Patch] Use a default for a bad env config file variable

[James]

[-- Attachment #1: Type: text/plain, Size: 637 bytes --]

Hi git list, my name is James, and this is my first patch.

It's quite trivial really, all that changes is that if someone sets
the environment variable: $GITWEB_CONFIG_SYSTEM, and this points to
something like: /srv/gitosis/gitweb.conf, which doesn't actually
exist, then gitweb will default to trying out the built in default of
/etc/gitweb.conf (if it was built with that var).

This patch should make it easier for people who are configuring
gitweb+gitosis, so that a separate gitweb.conf config file can be used
to call the main config, but which doesn't null out the system
defaults if it is missing.

Thank you in advance,
_James

[-- Attachment #2: 0001-Use-a-default-for-a-bad-env-config-file-variable.patch --]
[-- Type: application/octet-stream, Size: 760 bytes --]

From d29adf8c788b8a747bfd38dd7e10f684de9aa8e9 Mon Sep 17 00:00:00 2001
From: James Shubin <purpleidea@gmail.com>
Date: Tue, 10 Aug 2010 10:30:22 -0400
Subject: [PATCH] Use a default for a bad env config file variable.

---
 gitweb/gitweb.perl |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 4efeebc..43294e1 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -605,6 +605,10 @@ sub evaluate_gitweb_config {
 	} elsif (-e $GITWEB_CONFIG_SYSTEM) {
 		do $GITWEB_CONFIG_SYSTEM;
 		die $@ if $@;
+	# if config file from env is missing, then try the default anyways
+	} elsif (-e "++GITWEB_CONFIG_SYSTEM++") {
+		do "++GITWEB_CONFIG_SYSTEM++";
+		die $@ if $@;
 	}
 }
 
-- 
1.7.0.4

Re: [Patch] Use a default for a bad env config file variable

[Matthieu Moy]

Hi,

James <purpleidea@gmail.com> writes:

> Hi git list, my name is James, and this is my first patch.

Then, I'll have to be the first person directing you to
Documentation/SubmittingPatches ;-).

The custom here is to send patches inline (git send-email can help),
not attached. Read about Signed-off-by too.

(Don't know perl and gitweb enough to judge on the content
unfortunately, sorry)

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [Patch] Use a default for a bad env config file variable

[James]

Sorry about that,
I guess I had only read the README.
Hope this is better:

>From d29adf8c788b8a747bfd38dd7e10f684de9aa8e9 Mon Sep 17 00:00:00 2001
From: James Shubin <purpleidea@gmail.com>
Date: Tue, 10 Aug 2010 10:30:22 -0400
Subject: [PATCH] Use a default for a bad env config file variable.


Signed-off-by: James Shubin <purpleidea@gmail.com>
---
 gitweb/gitweb.perl |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 4efeebc..43294e1 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -605,6 +605,10 @@ sub evaluate_gitweb_config {
 	} elsif (-e $GITWEB_CONFIG_SYSTEM) {
 		do $GITWEB_CONFIG_SYSTEM;
 		die $@ if $@;
+	# if config file from env is missing, then try the default anyways
+	} elsif (-e "++GITWEB_CONFIG_SYSTEM++") {
+		do "++GITWEB_CONFIG_SYSTEM++";
+		die $@ if $@;
 	}
 }

-- 
1.7.0.4



On 8/10/10, Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> wrote:
> Hi,
>
>
>  James <purpleidea@gmail.com> writes:
>
>  > Hi git list, my name is James, and this is my first patch.
>
>
> Then, I'll have to be the first person directing you to
>  Documentation/SubmittingPatches ;-).
>
>  The custom here is to send patches inline (git send-email can help),
>  not attached. Read about Signed-off-by too.
>
>  (Don't know perl and gitweb enough to judge on the content
>  unfortunately, sorry)
>
>
>  --
>  Matthieu Moy
>  http://www-verimag.imag.fr/~moy/
>

Re: [Patch] Use a default for a bad env config file variable

[Ævar Arnfjörð Bjarmason]

On Tue, Aug 10, 2010 at 15:54, James <purpleidea@gmail.com> wrote:
> Sorry about that,
> I guess I had only read the README.
> Hope this is better:

Not really, no. You should send the patches you produce with
git-format-patch with git-send-email, and try sending to yourself
first and apply it with git-am (this is all mentioned in
SubmittingPatches).

This is what your new patch looks like after being applied with
git-am:

    commit 7be6207e8923cd7c4c48243f5257a0fdba6bfa0a
    Author: James <purpleidea@gmail.com>
    Date:   Tue Aug 10 11:54:43 2010 -0400

        Use a default for a bad env config file variable

        Sorry about that,
        I guess I had only read the README.
        Hope this is better:

        From d29adf8c788b8a747bfd38dd7e10f684de9aa8e9 Mon Sep 17 00:00:00 2001
        From: James Shubin <purpleidea@gmail.com>
        Date: Tue, 10 Aug 2010 10:30:22 -0400
        Subject: [PATCH] Use a default for a bad env config file variable.

        Signed-off-by: James Shubin <purpleidea@gmail.com>

    diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
    index 4efeebc..43294e1 100755
    --- a/gitweb/gitweb.perl
    +++ b/gitweb/gitweb.perl
    @@ -605,6 +605,10 @@ sub evaluate_gitweb_config {
            } elsif (-e $GITWEB_CONFIG_SYSTEM) {
                    do $GITWEB_CONFIG_SYSTEM;
                    die $@ if $@;
    +       # if config file from env is missing, then try the default anyways
    +       } elsif (-e "++GITWEB_CONFIG_SYSTEM++") {
    +               do "++GITWEB_CONFIG_SYSTEM++";
    +               die $@ if $@;
            }
     }

I.e. your message has become part of the patch. To include commentary
on resend add it after -- and before the diffstat (also in
SubmittingPatches).

Thanks.

[PATCH v3] Gitweb: Use a default for a bad env config file variable

[James Shubin]

From: James Shubin <purpleidea@gmail.com>
Signed-off-by: James Shubin <purpleidea@gmail.com>
---
It's quite trivial really, all that changes is that if someone sets
the environment variable: $GITWEB_CONFIG_SYSTEM, and this points to
something like: /srv/gitosis/gitweb.conf, which doesn't actually
exist, then gitweb will default to trying out the built in default of
/etc/gitweb.conf (if it was built with that value).

This patch should make it easier for people who are configuring
gitweb+gitosis, so that a separate gitweb.conf config file can be used
to call the main config, but which doesn't null out the system
defaults if it is missing.

PS: thanks to everyone for their patience with my first patch.
This applied cleanly with git am, let me know if I should do anything
else differently.

 gitweb/gitweb.perl |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 4efeebc..43294e1 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -605,6 +605,10 @@ sub evaluate_gitweb_config {
 	} elsif (-e $GITWEB_CONFIG_SYSTEM) {
 		do $GITWEB_CONFIG_SYSTEM;
 		die $@ if $@;
+	# if config file from env is missing, then try the default anyways
+	} elsif (-e "++GITWEB_CONFIG_SYSTEM++") {
+		do "++GITWEB_CONFIG_SYSTEM++";
+		die $@ if $@;
 	}
 }
 
-- 
1.7.0.4


-----Original Message-----
From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
To: James <purpleidea@gmail.com>
Cc: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>, git@vger.kernel.org
Subject: Re: [Patch] Use a default for a bad env config file variable
Date: Tue, 10 Aug 2010 16:02:48 +0000

On Tue, Aug 10, 2010 at 15:54, James <purpleidea@gmail.com> wrote:
> Sorry about that,
> I guess I had only read the README.
> Hope this is better:

Not really, no. You should send the patches you produce with
git-format-patch with git-send-email, and try sending to yourself
first and apply it with git-am (this is all mentioned in
SubmittingPatches).

This is what your new patch looks like after being applied with
git-am:

    commit 7be6207e8923cd7c4c48243f5257a0fdba6bfa0a
    Author: James <purpleidea@gmail.com>
    Date:   Tue Aug 10 11:54:43 2010 -0400

        Use a default for a bad env config file variable

        Sorry about that,
        I guess I had only read the README.
        Hope this is better:

        From d29adf8c788b8a747bfd38dd7e10f684de9aa8e9 Mon Sep 17 00:00:00 2001
        From: James Shubin <purpleidea@gmail.com>
        Date: Tue, 10 Aug 2010 10:30:22 -0400
        Subject: [PATCH] Use a default for a bad env config file variable.

        Signed-off-by: James Shubin <purpleidea@gmail.com>

    diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
    index 4efeebc..43294e1 100755
    --- a/gitweb/gitweb.perl
    +++ b/gitweb/gitweb.perl
    @@ -605,6 +605,10 @@ sub evaluate_gitweb_config {
            } elsif (-e $GITWEB_CONFIG_SYSTEM) {
                    do $GITWEB_CONFIG_SYSTEM;
                    die $@ if $@;
    +       # if config file from env is missing, then try the default anyways
    +       } elsif (-e "++GITWEB_CONFIG_SYSTEM++") {
    +               do "++GITWEB_CONFIG_SYSTEM++";
    +               die $@ if $@;
            }
     }

I.e. your message has become part of the patch. To include commentary
on resend add it after -- and before the diffstat (also in
SubmittingPatches).

Thanks.

Re: [PATCH v3] Gitweb: Use a default for a bad env config file variable

[Jakub Narebski]

James Shubin <purpleidea@gmail.com> writes:

> From: James Shubin <purpleidea@gmail.com>

Small nitpick: you need line like the above only if the From: header
in your email is diferent from the authorship you want to have in
commit, i.e. when you are sending email from other email account, or
when you are (re)sending someone's else patches.

In this patch situation it is not, I think, necessary.

> Signed-off-by: James Shubin <purpleidea@gmail.com>
> ---
> It's quite trivial really, all that changes is that if someone sets
> the environment variable: $GITWEB_CONFIG_SYSTEM, and this points to
> something like: /srv/gitosis/gitweb.conf, which doesn't actually
> exist, then gitweb will default to trying out the built in default of
> /etc/gitweb.conf (if it was built with that value).
> 
> This patch should make it easier for people who are configuring
> gitweb+gitosis, so that a separate gitweb.conf config file can be used
> to call the main config, but which doesn't null out the system
> defaults if it is missing.

First, why it is needed?  Why can't you just have GITWEB_CONFIG_SYSTEM
(or GITWEB_CONFIG) environment variable visible to gitweb.cgi that
points to existing file?

Second, is there any history behind providing this fallback only for
$GITWEB_CONFIG_SYSTEM variable, and not for $GITWEB_CONFIG?  Currently
gitweb use environment variable if it exists, falling back to build-time
value (might be default), and using first of $GITWEB_CONFIG and 
$GITWEB_CONFIG_SYSTEM that exists.

> PS: thanks to everyone for their patience with my first patch.
> This applied cleanly with git am, let me know if I should do anything
> else differently.
> 
>  gitweb/gitweb.perl |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 4efeebc..43294e1 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -605,6 +605,10 @@ sub evaluate_gitweb_config {
>  	} elsif (-e $GITWEB_CONFIG_SYSTEM) {
>  		do $GITWEB_CONFIG_SYSTEM;
>  		die $@ if $@;
> +	# if config file from env is missing, then try the default anyways

Minor nitpick: Actually that is not default, but build-time value, which
has a default.

> +	} elsif (-e "++GITWEB_CONFIG_SYSTEM++") {
> +		do "++GITWEB_CONFIG_SYSTEM++";
> +		die $@ if $@;
>  	}
>  }
>  
> -- 
> 1.7.0.4

-- 
Jakub Narebski
Poland
ShadeHawk on #git

Re: [PATCH v3] Gitweb: Use a default for a bad env config file variable

[James Shubin]

Thanks for all the "sending patches" input; it's useful, even if my
patch isn't ;)
I suppose I was trying to provide more defaults to work around some
weirdly setup servers that I have inherited. Upon reconsideration, I'll
withdraw the patch.
Thanks for your time, and thanks for your nitpicks.

_James


-----Original Message-----
> First, why it is needed?  Why can't you just have GITWEB_CONFIG_SYSTEM
> (or GITWEB_CONFIG) environment variable visible to gitweb.cgi that
> points to existing file?


Second, is there any history behind providing this fallback only for
$GITWEB_CONFIG_SYSTEM variable, and not for $GITWEB_CONFIG?  Currently
gitweb use environment variable if it exists, falling back to build-time
value (might be default), and using first of $GITWEB_CONFIG and 
$GITWEB_CONFIG_SYSTEM that exists.

> PS: thanks to everyone for their patience with my first patch.
> This applied cleanly with git am, let me know if I should do anything
> else differently.
> 
>  gitweb/gitweb.perl |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 4efeebc..43294e1 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -605,6 +605,10 @@ sub evaluate_gitweb_config {
>  	} elsif (-e $GITWEB_CONFIG_SYSTEM) {
>  		do $GITWEB_CONFIG_SYSTEM;
>  		die $@ if $@;
> +	# if config file from env is missing, then try the default anyways

Minor nitpick: Actually that is not default, but build-time value, which
has a default.

> +	} elsif (-e "++GITWEB_CONFIG_SYSTEM++") {
> +		do "++GITWEB_CONFIG_SYSTEM++";
> +		die $@ if $@;
>  	}
>  }
>  
> -- 
> 1.7.0.4

Re: [PATCH v3] Gitweb: Use a default for a bad env config file variable

[Jonathan Nieder]

Hi James,

James Shubin wrote:

> Signed-off-by: James Shubin <purpleidea@gmail.com>
[...]
> It's quite trivial really, all that changes is that if someone sets
> the environment variable: $GITWEB_CONFIG_SYSTEM, and this points to
> something like: /srv/gitosis/gitweb.conf, which doesn't actually
> exist, then gitweb will default to trying out the built in default of
> /etc/gitweb.conf (if it was built with that value).
[...]
|	our $GITWEB_CONFIG = $ENV{'GITWEB_CONFIG'} || "++GITWEB_CONFIG++";
|	our $GITWEB_CONFIG_SYSTEM = $ENV{'GITWEB_CONFIG_SYSTEM'} || "++GITWEB_CONFIG_SYSTEM++"; # die if there are errors parsing config file
|	if (-e $GITWEB_CONFIG) {
|		do $GITWEB_CONFIG;
|		die $@ if $@;
|	} elsif (-e $GITWEB_CONFIG_SYSTEM) {
|		do $GITWEB_CONFIG_SYSTEM;
|		die $@ if $@;
|	} elsif (-e "++GITWEB_CONFIG_SYSTEM++") {
|		...

Interesting.  I am a bit nervous that this might be confusing.
cc-ing Jakub for input.

Re: [Patch] Use a default for a bad env config file variable

[Michael J Gruber]

James venit, vidit, dixit 10.08.2010 17:54:
> Sorry about that,
> I guess I had only read the README.
> Hope this is better:
> 
> From d29adf8c788b8a747bfd38dd7e10f684de9aa8e9 Mon Sep 17 00:00:00 2001
> From: James Shubin <purpleidea@gmail.com>
> Date: Tue, 10 Aug 2010 10:30:22 -0400
> Subject: [PATCH] Use a default for a bad env config file variable.

Now, make that commit subject

gitweb: Use a default for a bad env config file variable

(i.e. insert "gitweb: ", delete the stop) and you're good to go ;)

Michael

> 
> 
> Signed-off-by: James Shubin <purpleidea@gmail.com>
> ---
>  gitweb/gitweb.perl |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 4efeebc..43294e1 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -605,6 +605,10 @@ sub evaluate_gitweb_config {
>  	} elsif (-e $GITWEB_CONFIG_SYSTEM) {
>  		do $GITWEB_CONFIG_SYSTEM;
>  		die $@ if $@;
> +	# if config file from env is missing, then try the default anyways
> +	} elsif (-e "++GITWEB_CONFIG_SYSTEM++") {
> +		do "++GITWEB_CONFIG_SYSTEM++";
> +		die $@ if $@;
>  	}
>  }
>