# [Patch] Use a default for a bad env config file variable 9 messages from 2010-08-10 to 2010-08-11. Participants: James, Matthieu Moy, Ævar Arnfjörð Bjarmason, Michael J Gruber, James Shubin, Jakub Narebski, Jonathan Nieder. Thread: https://gitlist.dev/t/24693 ## James, 2010-08-10 15:11 Subject: [Patch] Use a default for a bad env config file variable Message-ID: URL: https://gitlist.dev/e/AANLkTi%3D42V8%3D26oEwcNOHubuK%2BVAf-QaYnvCLAKQ0xtg%40mail.gmail.com ``` 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 From d29adf8c788b8a747bfd38dd7e10f684de9aa8e9 Mon Sep 17 00:00:00 2001 From: James Shubin 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 ``` ## Matthieu Moy, 2010-08-10 15:41 Subject: Re: [Patch] Use a default for a bad env config file variable Message-ID: URL: https://gitlist.dev/e/vpq4of2scen.fsf%40bauges.imag.fr In-Reply-To: ``` Hi, James 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/ ``` ## James, 2010-08-10 15:54 Subject: Re: [Patch] Use a default for a bad env config file variable Message-ID: URL: https://gitlist.dev/e/AANLkTi%3DUCJa9uT5i9TRB9hGsH1D8b9DgB49TPM5k1htQ%40mail.gmail.com In-Reply-To: ``` 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 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 --- 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 wrote: > Hi, > > > James 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/ > ``` ## Ævar Arnfjörð Bjarmason, 2010-08-10 16:02 Subject: Re: [Patch] Use a default for a bad env config file variable Message-ID: URL: https://gitlist.dev/e/AANLkTikDtK0vyRwJE3Un5uFkK7FzoWtiWpL8gPqLQjH5%40mail.gmail.com In-Reply-To: ``` On Tue, Aug 10, 2010 at 15:54, James 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 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 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 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. ``` ## Michael J Gruber, 2010-08-10 16:08 Subject: Re: [Patch] Use a default for a bad env config file variable Message-ID: <4C617975.8000501@drmicha.warpmail.net> URL: https://gitlist.dev/e/4C617975.8000501%40drmicha.warpmail.net In-Reply-To: ``` 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 > 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 > --- > 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 $@; > } > } > ``` ## James Shubin, 2010-08-10 16:47 Subject: [PATCH v3] Gitweb: Use a default for a bad env config file variable Message-ID: <1281458849.28858.18.camel@ping.CS.McGill.CA> URL: https://gitlist.dev/e/1281458849.28858.18.camel%40ping.CS.McGill.CA In-Reply-To: ``` From: James Shubin Signed-off-by: James Shubin --- 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 To: James Cc: Matthieu Moy , 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 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 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 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 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. ``` ## Jakub Narebski, 2010-08-10 22:42 Subject: Re: [PATCH v3] Gitweb: Use a default for a bad env config file variable Message-ID: URL: https://gitlist.dev/e/m3tyn22iqd.fsf%40localhost.localdomain In-Reply-To: <1281458849.28858.18.camel@ping.CS.McGill.CA> ``` James Shubin writes: > From: James Shubin 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 > --- > 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 ``` ## Jonathan Nieder, 2010-08-11 00:08 Subject: Re: [PATCH v3] Gitweb: Use a default for a bad env config file variable Message-ID: <20100811000846.GF2099@burratino> URL: https://gitlist.dev/e/20100811000846.GF2099%40burratino In-Reply-To: <1281458849.28858.18.camel@ping.CS.McGill.CA> ``` Hi James, James Shubin wrote: > Signed-off-by: James Shubin [...] > 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. ``` ## James Shubin, 2010-08-11 13:36 Subject: Re: [PATCH v3] Gitweb: Use a default for a bad env config file variable Message-ID: <1281533812.28858.50.camel@ping.CS.McGill.CA> URL: https://gitlist.dev/e/1281533812.28858.50.camel%40ping.CS.McGill.CA In-Reply-To: ``` 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 ```