# [PATCH] gitweb-lib.sh: Set up PATH to use perl from /usr/bin 12 messages from 2012-05-01 to 2012-05-01. Participants: Torsten Bögershausen, Zbigniew Jędrzejewski-Szmek, Jeff King, Junio C Hamano, Randal L. Schwartz. Thread: https://gitlist.dev/t/30380 ## Torsten Bögershausen, 2012-05-01 11:23 Subject: [PATCH] gitweb-lib.sh: Set up PATH to use perl from /usr/bin Message-ID: <201205011323.45190.tboegi@web.de> URL: https://gitlist.dev/e/201205011323.45190.tboegi%40web.de ``` 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 --- 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 ``` ## Zbigniew Jędrzejewski-Szmek, 2012-05-01 16:23 Subject: Re: [PATCH] gitweb-lib.sh: Set up PATH to use perl from /usr/bin Message-ID: <4FA00E09.2090708@in.waw.pl> URL: https://gitlist.dev/e/4FA00E09.2090708%40in.waw.pl In-Reply-To: <201205011323.45190.tboegi@web.de> ``` 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 ``` ## Jeff King, 2012-05-01 16:34 Subject: Re: [PATCH] gitweb-lib.sh: Set up PATH to use perl from /usr/bin Message-ID: <20120501163420.GB15614@sigill.intra.peff.net> URL: https://gitlist.dev/e/20120501163420.GB15614%40sigill.intra.peff.net In-Reply-To: <4FA00E09.2090708@in.waw.pl> ``` 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 ``` ## Junio C Hamano, 2012-05-01 16:44 Subject: Re: [PATCH] gitweb-lib.sh: Set up PATH to use perl from /usr/bin Message-ID: <7vwr4vam0m.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vwr4vam0m.fsf%40alter.siamese.dyndns.org In-Reply-To: <201205011323.45190.tboegi@web.de> ``` Torsten Bögershausen 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. ``` ## Junio C Hamano, 2012-05-01 16:47 Subject: Re: [PATCH] gitweb-lib.sh: Set up PATH to use perl from /usr/bin Message-ID: <7vsjfjalx6.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vsjfjalx6.fsf%40alter.siamese.dyndns.org In-Reply-To: <4FA00E09.2090708@in.waw.pl> ``` Zbigniew Jędrzejewski-Szmek 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. ``` ## Zbigniew Jędrzejewski-Szmek, 2012-05-01 17:03 Subject: Re: [PATCH] gitweb-lib.sh: Set up PATH to use perl from /usr/bin Message-ID: <4FA0176B.50300@in.waw.pl> URL: https://gitlist.dev/e/4FA0176B.50300%40in.waw.pl In-Reply-To: <7vsjfjalx6.fsf@alter.siamese.dyndns.org> ``` On 05/01/2012 06:47 PM, Junio C Hamano wrote: > Zbigniew Jędrzejewski-Szmek 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 ``` ## Jeff King, 2012-05-01 17:08 Subject: Re: [PATCH] gitweb-lib.sh: Set up PATH to use perl from /usr/bin Message-ID: <20120501170810.GA22444@sigill.intra.peff.net> URL: https://gitlist.dev/e/20120501170810.GA22444%40sigill.intra.peff.net In-Reply-To: <4FA0176B.50300@in.waw.pl> ``` 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 ``` ## Torsten Bögershausen, 2012-05-01 17:50 Subject: Re: [PATCH] gitweb-lib.sh: Set up PATH to use perl from /usr/bin Message-ID: <4FA02274.6070601@web.de> URL: https://gitlist.dev/e/4FA02274.6070601%40web.de In-Reply-To: <20120501170810.GA22444@sigill.intra.peff.net> ``` 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 ``` ## Junio C Hamano, 2012-05-01 17:53 Subject: Re: [PATCH] gitweb-lib.sh: Set up PATH to use perl from /usr/bin Message-ID: <7v8vhbaitt.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7v8vhbaitt.fsf%40alter.siamese.dyndns.org In-Reply-To: <4FA02274.6070601@web.de> ``` Torsten Bögershausen 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. ``` ## Jeff King, 2012-05-01 17:55 Subject: [PATCH] t/gitweb-lib: use $PERL_PATH to run gitweb Message-ID: <20120501175500.GA24258@sigill.intra.peff.net> URL: https://gitlist.dev/e/20120501175500.GA24258%40sigill.intra.peff.net In-Reply-To: <4FA02274.6070601@web.de> ``` 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 --- 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 ``` ## Zbigniew Jędrzejewski-Szmek, 2012-05-01 20:18 Subject: [PATCH] Consistently use perl from /usr/bin/ for scripts Message-ID: <1335903498-583-1-git-send-email-zbyszek@in.waw.pl> URL: https://gitlist.dev/e/1335903498-583-1-git-send-email-zbyszek%40in.waw.pl In-Reply-To: <20120501175500.GA24258@sigill.intra.peff.net> ``` 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 Signed-off-by: Zbigniew Jędrzejewski-Szmek --- 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 # 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 # License: GPL v2 or later use 5.008; -- 1.7.10.539.g8ecdfe ``` ## Randal L. Schwartz, 2012-05-01 20:54 Subject: Re: [PATCH] Consistently use perl from /usr/bin/ for scripts Message-ID: <86vckfmxk1.fsf@red.stonehenge.com> URL: https://gitlist.dev/e/86vckfmxk1.fsf%40red.stonehenge.com In-Reply-To: <1335903498-583-1-git-send-email-zbyszek@in.waw.pl> ``` >>>>> "Zbigniew" == Zbigniew Jędrzejewski-Szmek writes: Zbigniew> Suggested-by: Junio C Hamano Zbigniew> Signed-off-by: Zbigniew Jędrzejewski-Szmek 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 Smalltalk/Perl/Unix consulting, Technical writing, Comedy, etc. etc. See http://methodsandmessages.posterous.com/ for Smalltalk discussion ```