[PATCH] gitweb-lib.sh: Set up PATH to use perl from /usr/bin

[PATCH] gitweb-lib.sh: Set up PATH to use perl from /usr/bin

[Torsten Bögershausen]

When there are different version of perl installed on the machine,
the $PATH may point out a different version of perl than /usr/bin.
One example is to have /opt/local/bin/perl before /usr/bin/perl.

Sanitize the PATH by adding /usr/bin at the beginning

Signed-off-by: Torsten Bögershausen <tboegi@web.de>
---
On my Mac OS machine t9501-gitweb-standalone-http-status.sh failed because
perl was found under /opt/local/bin instead of of /usr/bin.

/opt/local/bin is coming from Macports.
The problem with different perl installations on the same machine
may hit more people than just me.

There are different solutions, please help to find the best one:

a) Delete perl from /opt/local/bin
b) Put /opt/local/bin at the end of the PATH 
c) Change gitweb-lib.sh to set up the PATH to /usr/bin, because that is what the
file gitweb_config.perl generated by gitweb-lib.sh expects.


 t/gitweb-lib.sh |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/t/gitweb-lib.sh b/t/gitweb-lib.sh
index 21d11d6..a016142 100644
--- a/t/gitweb-lib.sh
+++ b/t/gitweb-lib.sh
@@ -113,4 +113,7 @@ perl -MCGI -MCGI::Util -MCGI::Carp -e 0 >/dev/null 2>&1 || {
 	test_done
 }
 
+PATH=/usr/bin/:$PATH
+export PATH
+
 gitweb_init
-- 
1.7.10.rc0.17.g74595.dirty

Re: [PATCH] gitweb-lib.sh: Set up PATH to use perl from /usr/bin

[Zbigniew Jędrzejewski-Szmek]

On 05/01/2012 01:23 PM, Torsten Bögershausen wrote:
> When there are different version of perl installed on the machine,
> the $PATH may point out a different version of perl than /usr/bin.
> One example is to have /opt/local/bin/perl before /usr/bin/perl.
> 
> Sanitize the PATH by adding /usr/bin at the beginning
Hm, I see that most scripts have #!/usr/bin/perl, and only two have
#!env perl [1]. So in general we usally rely on using perl in /usr/bin.

But your patch affects other stuff than perl, and unconditionally
changing PATH set by the user is not nice, as it affect programs called
recursively. Wouldn't simply replacing all calls to bare perl in
t/gitweb-lib.sh with invocations of /usr/bin/perl be better?

[1]
% git grep 'env perl\b'
git-relink.perl:#!/usr/bin/env perl
git-svn.perl:#!/usr/bin/env perl

-
Zbyszek

Re: [PATCH] gitweb-lib.sh: Set up PATH to use perl from /usr/bin

[Jeff King]

On Tue, May 01, 2012 at 06:23:37PM +0200, Zbigniew Jędrzejewski-Szmek wrote:

> On 05/01/2012 01:23 PM, Torsten Bögershausen wrote:
> > When there are different version of perl installed on the machine,
> > the $PATH may point out a different version of perl than /usr/bin.
> > One example is to have /opt/local/bin/perl before /usr/bin/perl.
> > 
> > Sanitize the PATH by adding /usr/bin at the beginning
> Hm, I see that most scripts have #!/usr/bin/perl, and only two have
> #!env perl [1]. So in general we usally rely on using perl in /usr/bin.

The Makefile substitutes $PERL_PATH on the #!-line of each perl script
during its "build" step (which is really just copying the file to its
final name and running "chmod +x").

So even though the source files say /usr/bin/perl, we are not relying on
that. If you look at the Makefile rule carefully, you will see that even
"#!/usr/bin/env perl" gets replaced, too. Those scripts should probably
be updated, since the mention of env is simply confusing.

> But your patch affects other stuff than perl, and unconditionally
> changing PATH set by the user is not nice, as it affect programs called
> recursively. Wouldn't simply replacing all calls to bare perl in
> t/gitweb-lib.sh with invocations of /usr/bin/perl be better?

Yes, although they should use $PERL_PATH rather than hardcoding
/usr/bin.

-Peff

Re: [PATCH] gitweb-lib.sh: Set up PATH to use perl from /usr/bin

[Junio C Hamano]

Zbigniew Jędrzejewski-Szmek  <zbyszek@in.waw.pl> writes:

> Hm, I see that most scripts have #!/usr/bin/perl, and only two have
> #!env perl [1]. So in general we usally rely on using perl in /usr/bin.

The #!/usr/bin/env variants should be eradicated.  Our Makefile rewrites
"#!.*perl" with "#!$PERL_PATH" in scripted Porcelains before installing,
so /usr/bin/perl is the right thing to write there.

Re: [PATCH] gitweb-lib.sh: Set up PATH to use perl from /usr/bin

[Zbigniew Jędrzejewski-Szmek]

On 05/01/2012 06:47 PM, Junio C Hamano wrote:
> Zbigniew Jędrzejewski-Szmek  <zbyszek@in.waw.pl> writes:
> 
>> Hm, I see that most scripts have #!/usr/bin/perl, and only two have
>> #!env perl [1]. So in general we usally rely on using perl in /usr/bin.
> 
> The #!/usr/bin/env variants should be eradicated.  Our Makefile rewrites
> "#!.*perl" with "#!$PERL_PATH" in scripted Porcelains before installing,
> so /usr/bin/perl is the right thing to write there.
This would be trivial, as it is only two files.

But I don't see why we would use a different perl in
git-am.sh:                      perl -ne 'BEGIN { $subject = 0 }
git-am.sh:                      perl -M'POSIX qw(strftime)' -ne 'BEGIN { $subject = 0 }
git-request-pull.sh:ref=$(git ls-remote "$url" | perl -e "$find_matching_ref" "$head" "$headrev")
git-submodule.sh:       perl -e '
test-sha1.sh:                   perl -pe 'y/\000/g/'
test-sha1.sh:                   perl -pe 'y/\000/g/'
and lot of files in t/. Shouldn't those be replaced too?

Jeff King wrote:
> The Makefile substitutes $PERL_PATH on the #!-line of each perl script
> during its "build" step (which is really just copying the file to its
> final name and running "chmod +x").
Thank you for the explanation. I never noticed this, since I don't set
PERL_PATH myself.

Zbyszek

Re: [PATCH] gitweb-lib.sh: Set up PATH to use perl from /usr/bin

[Jeff King]

On Tue, May 01, 2012 at 07:03:39PM +0200, Zbigniew Jędrzejewski-Szmek wrote:

> But I don't see why we would use a different perl in
> git-am.sh:                      perl -ne 'BEGIN { $subject = 0 }
> git-am.sh:                      perl -M'POSIX qw(strftime)' -ne 'BEGIN { $subject = 0 }
> git-request-pull.sh:ref=$(git ls-remote "$url" | perl -e "$find_matching_ref" "$head" "$headrev")
> git-submodule.sh:       perl -e '
> test-sha1.sh:                   perl -pe 'y/\000/g/'
> test-sha1.sh:                   perl -pe 'y/\000/g/'
> and lot of files in t/. Shouldn't those be replaced too?

No. There are two ways in which we use perl:

  1. To run our complex scripts like gitweb, git-svn, etc. These require
     a reasonably modern perl version, and the user must specify it with
     PERL_PATH if it is not in /usr/bin.

  2. To run little snippets that _could_ be written in sed or awk, but
     which cause portability problems on crappy versions of those tools.
     These should run under any version of perl5.

It's OK to use 'perl' from the path for (2), because we are not asking
very much of perl in that case.

I think the patch we want is just:

diff --git a/t/gitweb-lib.sh b/t/gitweb-lib.sh
index 21d11d6..ae2dc46 100644
--- a/t/gitweb-lib.sh
+++ b/t/gitweb-lib.sh
@@ -69,7 +69,7 @@ gitweb_run () {
 	# written to web server logs, so we are not interested in that:
 	# we are interested only in properly formatted errors/warnings
 	rm -f gitweb.log &&
-	perl -- "$SCRIPT_NAME" \
+	"$PERL_PATH" -- "$SCRIPT_NAME" \
 		>gitweb.output 2>gitweb.log &&
 	perl -w -e '
 		open O, ">gitweb.headers";

no? Torsten, does that fix your problem?

-Peff

Re: [PATCH] gitweb-lib.sh: Set up PATH to use perl from /usr/bin

[Torsten Bögershausen]

Thanks for all answers,
> I think the patch we want is just:
> 
> diff --git a/t/gitweb-lib.sh b/t/gitweb-lib.sh
> index 21d11d6..ae2dc46 100644
> --- a/t/gitweb-lib.sh
> +++ b/t/gitweb-lib.sh
> @@ -69,7 +69,7 @@ gitweb_run () {
>  	# written to web server logs, so we are not interested in that:
>  	# we are interested only in properly formatted errors/warnings
>  	rm -f gitweb.log &&
> -	perl -- "$SCRIPT_NAME" \
> +	"$PERL_PATH" -- "$SCRIPT_NAME" \
>  		>gitweb.output 2>gitweb.log &&
>  	perl -w -e '
>  		open O, ">gitweb.headers";
> 
> no? Torsten, does that fix your problem?
Yes, it does.

Should we go for that solution ?

/Torsten

Re: [PATCH] gitweb-lib.sh: Set up PATH to use perl from /usr/bin

[Junio C Hamano]

Torsten Bögershausen <tboegi@web.de> writes:

> Thanks for all answers,
>> I think the patch we want is just:
>> 
>> diff --git a/t/gitweb-lib.sh b/t/gitweb-lib.sh
>> index 21d11d6..ae2dc46 100644
>> --- a/t/gitweb-lib.sh
>> +++ b/t/gitweb-lib.sh
>> @@ -69,7 +69,7 @@ gitweb_run () {
>>  	# written to web server logs, so we are not interested in that:
>>  	# we are interested only in properly formatted errors/warnings
>>  	rm -f gitweb.log &&
>> -	perl -- "$SCRIPT_NAME" \
>> +	"$PERL_PATH" -- "$SCRIPT_NAME" \
>>  		>gitweb.output 2>gitweb.log &&
>>  	perl -w -e '
>>  		open O, ">gitweb.headers";
>> 
>> no? Torsten, does that fix your problem?
> Yes, it does.
>
> Should we go for that solution ?

Sounds good.

[PATCH] t/gitweb-lib: use $PERL_PATH to run gitweb

[Jeff King]

The current code runs "perl gitweb.cgi" to test gitweb. This
will use whatever version of perl happens to be first in the
PATH. We are better off using the specific perl that the
user specified via PERL_PATH, which matches what gets put on
the #!-line of the built gitweb.cgi.

Signed-off-by: Jeff King <peff@peff.net>
---
On Tue, May 01, 2012 at 07:50:44PM +0200, Torsten Bögershausen wrote:

> > Torsten, does that fix your problem?
> Yes, it does.

OK, here it is with a commit message.

 t/gitweb-lib.sh |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/gitweb-lib.sh b/t/gitweb-lib.sh
index 21d11d6..ae2dc46 100644
--- a/t/gitweb-lib.sh
+++ b/t/gitweb-lib.sh
@@ -69,7 +69,7 @@ gitweb_run () {
 	# written to web server logs, so we are not interested in that:
 	# we are interested only in properly formatted errors/warnings
 	rm -f gitweb.log &&
-	perl -- "$SCRIPT_NAME" \
+	"$PERL_PATH" -- "$SCRIPT_NAME" \
 		>gitweb.output 2>gitweb.log &&
 	perl -w -e '
 		open O, ">gitweb.headers";
-- 
1.7.10.630.g31718

[PATCH] Consistently use perl from /usr/bin/ for scripts

[Zbigniew Jędrzejewski-Szmek]

While the majority of scripts use '#!/usr/bin/perl', some use
'#!/usr/bin/env perl'. In the end there is no difference, because the
Makefile rewrites "#!.*perl" with "#!$PERL_PATH" in scripted
Porcelains before installing. Nevertheless, the second form can be
misleading, because it suggests that perl found first in $PATH will be
used.

Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
---
 git-relink.perl |    2 +-
 git-svn.perl    |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/git-relink.perl b/git-relink.perl
index e136732..f29285c 100755
--- a/git-relink.perl
+++ b/git-relink.perl
@@ -1,4 +1,4 @@
-#!/usr/bin/env perl
+#!/usr/bin/perl
 # Copyright 2005, Ryan Anderson <ryan@michonline.com>
 # Distribution permitted under the GPL v2, as distributed
 # by the Free Software Foundation.
diff --git a/git-svn.perl b/git-svn.perl
index 427da9e..9bec808 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -1,4 +1,4 @@
-#!/usr/bin/env perl
+#!/usr/bin/perl
 # Copyright (C) 2006, Eric Wong <normalperson@yhbt.net>
 # License: GPL v2 or later
 use 5.008;
-- 
1.7.10.539.g8ecdfe

Re: [PATCH] Consistently use perl from /usr/bin/ for scripts

[Randal L. Schwartz]

>>>>> "Zbigniew" == Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl> writes:

Zbigniew> Suggested-by: Junio C Hamano <gitster@pobox.com>
Zbigniew> Signed-off-by: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>

And I strongly support this change.  Too many things use that env trick,
and it really isn't the proper solution to this.

-- 
Randal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095
<merlyn@stonehenge.com> <URL:http://www.stonehenge.com/merlyn/>
Smalltalk/Perl/Unix consulting, Technical writing, Comedy, etc. etc.
See http://methodsandmessages.posterous.com/ for Smalltalk discussion

Re: [PATCH] gitweb-lib.sh: Set up PATH to use perl from /usr/bin

[Junio C Hamano]

Torsten Bögershausen <tboegi@web.de> writes:

> When there are different version of perl installed on the machine,
> the $PATH may point out a different version of perl than /usr/bin.
> One example is to have /opt/local/bin/perl before /usr/bin/perl.
> ...
> diff --git a/t/gitweb-lib.sh b/t/gitweb-lib.sh
> index 21d11d6..a016142 100644
> --- a/t/gitweb-lib.sh
> +++ b/t/gitweb-lib.sh
> @@ -113,4 +113,7 @@ perl -MCGI -MCGI::Util -MCGI::Carp -e 0 >/dev/null 2>&1 || {
>  	test_done
>  }
>  
> +PATH=/usr/bin/:$PATH
> +export PATH
> +
>  gitweb_init

This is wrong.

What makes you think /usr/bin always has saner version of tools than those
in the directories that the user explicitly listed earlier on her $PATH?

If anything it should be honoring $PERL_PATH that is set in the Makefile.