{"thread":{"id":"30380","subject":"[PATCH] gitweb-lib.sh: Set up PATH to use perl from /usr/bin","startedAt":"2012-05-01T11:23:44Z","lastAt":"2012-05-01T20:54:54Z","messageCount":12,"participants":["Torsten Bögershausen","Zbigniew Jędrzejewski-Szmek","Jeff King","Junio C Hamano","Randal L. Schwartz"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"190417","messageId":"201205011323.45190.tboegi@web.de","threadId":"30380","inReplyTo":null,"subject":"[PATCH] gitweb-lib.sh: Set up PATH to use perl from /usr/bin","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2012-05-01T11:23:44Z","receivedAt":"2012-05-01T11:23:44Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"When there are different version of perl installed on the machine,\nthe $PATH may point out a different version of perl than /usr/bin.\nOne example is to have /opt/local/bin/perl before /usr/bin/perl.\n\nSanitize the PATH by adding /usr/bin at the beginning\n\nSigned-off-by: Torsten Bögershausen <tboegi@web.de>\n---\nOn my Mac OS machine t9501-gitweb-standalone-http-status.sh failed because\nperl was found under /opt/local/bin instead of of /usr/bin.\n\n/opt/local/bin is coming from Macports.\nThe problem with different perl installations on the same machine\nmay hit more people than just me.\n\nThere are different solutions, please help to find the best one:\n\na) Delete perl from /opt/local/bin\nb) Put /opt/local/bin at the end of the PATH \nc) Change gitweb-lib.sh to set up the PATH to /usr/bin, because that is what the\nfile gitweb_config.perl generated by gitweb-lib.sh expects.\n\n\n t/gitweb-lib.sh |    3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/t/gitweb-lib.sh b/t/gitweb-lib.sh\nindex 21d11d6..a016142 100644\n--- a/t/gitweb-lib.sh\n+++ b/t/gitweb-lib.sh\n@@ -113,4 +113,7 @@ perl -MCGI -MCGI::Util -MCGI::Carp -e 0 >/dev/null 2>&1 || {\n \ttest_done\n }\n \n+PATH=/usr/bin/:$PATH\n+export PATH\n+\n gitweb_init\n-- \n1.7.10.rc0.17.g74595.dirty\n"},{"id":"190426","messageId":"4FA00E09.2090708@in.waw.pl","threadId":"30380","inReplyTo":"201205011323.45190.tboegi@web.de","subject":"Re: [PATCH] gitweb-lib.sh: Set up PATH to use perl from /usr/bin","fromName":"Zbigniew Jędrzejewski-Szmek","fromEmail":"zbyszek@in.waw.pl","sentAt":"2012-05-01T16:23:37Z","receivedAt":"2012-05-01T16:23:37Z","isPatch":true,"sender":{"key":"zbyszek@in.waw.pl","avatar":"https://avatars.githubusercontent.com/u/349618?v=4"},"body":"On 05/01/2012 01:23 PM, Torsten Bögershausen wrote:\n> When there are different version of perl installed on the machine,\n> the $PATH may point out a different version of perl than /usr/bin.\n> One example is to have /opt/local/bin/perl before /usr/bin/perl.\n> \n> Sanitize the PATH by adding /usr/bin at the beginning\nHm, I see that most scripts have #!/usr/bin/perl, and only two have\n#!env perl [1]. So in general we usally rely on using perl in /usr/bin.\n\nBut your patch affects other stuff than perl, and unconditionally\nchanging PATH set by the user is not nice, as it affect programs called\nrecursively. Wouldn't simply replacing all calls to bare perl in\nt/gitweb-lib.sh with invocations of /usr/bin/perl be better?\n\n[1]\n% git grep 'env perl\\b'\ngit-relink.perl:#!/usr/bin/env perl\ngit-svn.perl:#!/usr/bin/env perl\n\n-\nZbyszek\n"},{"id":"190428","messageId":"20120501163420.GB15614@sigill.intra.peff.net","threadId":"30380","inReplyTo":"4FA00E09.2090708@in.waw.pl","subject":"Re: [PATCH] gitweb-lib.sh: Set up PATH to use perl from /usr/bin","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-01T16:34:20Z","receivedAt":"2012-05-01T16:34:20Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, May 01, 2012 at 06:23:37PM +0200, Zbigniew Jędrzejewski-Szmek wrote:\n\n> On 05/01/2012 01:23 PM, Torsten Bögershausen wrote:\n> > When there are different version of perl installed on the machine,\n> > the $PATH may point out a different version of perl than /usr/bin.\n> > One example is to have /opt/local/bin/perl before /usr/bin/perl.\n> > \n> > Sanitize the PATH by adding /usr/bin at the beginning\n> Hm, I see that most scripts have #!/usr/bin/perl, and only two have\n> #!env perl [1]. So in general we usally rely on using perl in /usr/bin.\n\nThe Makefile substitutes $PERL_PATH on the #!-line of each perl script\nduring its \"build\" step (which is really just copying the file to its\nfinal name and running \"chmod +x\").\n\nSo even though the source files say /usr/bin/perl, we are not relying on\nthat. If you look at the Makefile rule carefully, you will see that even\n\"#!/usr/bin/env perl\" gets replaced, too. Those scripts should probably\nbe updated, since the mention of env is simply confusing.\n\n> But your patch affects other stuff than perl, and unconditionally\n> changing PATH set by the user is not nice, as it affect programs called\n> recursively. Wouldn't simply replacing all calls to bare perl in\n> t/gitweb-lib.sh with invocations of /usr/bin/perl be better?\n\nYes, although they should use $PERL_PATH rather than hardcoding\n/usr/bin.\n\n-Peff\n"},{"id":"190429","messageId":"7vwr4vam0m.fsf@alter.siamese.dyndns.org","threadId":"30380","inReplyTo":"201205011323.45190.tboegi@web.de","subject":"Re: [PATCH] gitweb-lib.sh: Set up PATH to use perl from /usr/bin","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-01T16:44:57Z","receivedAt":"2012-05-01T16:44:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Torsten Bögershausen <tboegi@web.de> writes:\n\n> When there are different version of perl installed on the machine,\n> the $PATH may point out a different version of perl than /usr/bin.\n> One example is to have /opt/local/bin/perl before /usr/bin/perl.\n> ...\n> diff --git a/t/gitweb-lib.sh b/t/gitweb-lib.sh\n> index 21d11d6..a016142 100644\n> --- a/t/gitweb-lib.sh\n> +++ b/t/gitweb-lib.sh\n> @@ -113,4 +113,7 @@ perl -MCGI -MCGI::Util -MCGI::Carp -e 0 >/dev/null 2>&1 || {\n>  \ttest_done\n>  }\n>  \n> +PATH=/usr/bin/:$PATH\n> +export PATH\n> +\n>  gitweb_init\n\nThis is wrong.\n\nWhat makes you think /usr/bin always has saner version of tools than those\nin the directories that the user explicitly listed earlier on her $PATH?\n\nIf anything it should be honoring $PERL_PATH that is set in the Makefile.\n"},{"id":"190430","messageId":"7vsjfjalx6.fsf@alter.siamese.dyndns.org","threadId":"30380","inReplyTo":"4FA00E09.2090708@in.waw.pl","subject":"Re: [PATCH] gitweb-lib.sh: Set up PATH to use perl from /usr/bin","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-01T16:47:01Z","receivedAt":"2012-05-01T16:47:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Zbigniew Jędrzejewski-Szmek  <zbyszek@in.waw.pl> writes:\n\n> Hm, I see that most scripts have #!/usr/bin/perl, and only two have\n> #!env perl [1]. So in general we usally rely on using perl in /usr/bin.\n\nThe #!/usr/bin/env variants should be eradicated.  Our Makefile rewrites\n\"#!.*perl\" with \"#!$PERL_PATH\" in scripted Porcelains before installing,\nso /usr/bin/perl is the right thing to write there.\n"},{"id":"190431","messageId":"4FA0176B.50300@in.waw.pl","threadId":"30380","inReplyTo":"7vsjfjalx6.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] gitweb-lib.sh: Set up PATH to use perl from /usr/bin","fromName":"Zbigniew Jędrzejewski-Szmek","fromEmail":"zbyszek@in.waw.pl","sentAt":"2012-05-01T17:03:39Z","receivedAt":"2012-05-01T17:03:39Z","isPatch":true,"sender":{"key":"zbyszek@in.waw.pl","avatar":"https://avatars.githubusercontent.com/u/349618?v=4"},"body":"On 05/01/2012 06:47 PM, Junio C Hamano wrote:\n> Zbigniew Jędrzejewski-Szmek  <zbyszek@in.waw.pl> writes:\n> \n>> Hm, I see that most scripts have #!/usr/bin/perl, and only two have\n>> #!env perl [1]. So in general we usally rely on using perl in /usr/bin.\n> \n> The #!/usr/bin/env variants should be eradicated.  Our Makefile rewrites\n> \"#!.*perl\" with \"#!$PERL_PATH\" in scripted Porcelains before installing,\n> so /usr/bin/perl is the right thing to write there.\nThis would be trivial, as it is only two files.\n\nBut I don't see why we would use a different perl in\ngit-am.sh:                      perl -ne 'BEGIN { $subject = 0 }\ngit-am.sh:                      perl -M'POSIX qw(strftime)' -ne 'BEGIN { $subject = 0 }\ngit-request-pull.sh:ref=$(git ls-remote \"$url\" | perl -e \"$find_matching_ref\" \"$head\" \"$headrev\")\ngit-submodule.sh:       perl -e '\ntest-sha1.sh:                   perl -pe 'y/\\000/g/'\ntest-sha1.sh:                   perl -pe 'y/\\000/g/'\nand lot of files in t/. Shouldn't those be replaced too?\n\nJeff King wrote:\n> The Makefile substitutes $PERL_PATH on the #!-line of each perl script\n> during its \"build\" step (which is really just copying the file to its\n> final name and running \"chmod +x\").\nThank you for the explanation. I never noticed this, since I don't set\nPERL_PATH myself.\n\nZbyszek\n"},{"id":"190433","messageId":"20120501170810.GA22444@sigill.intra.peff.net","threadId":"30380","inReplyTo":"4FA0176B.50300@in.waw.pl","subject":"Re: [PATCH] gitweb-lib.sh: Set up PATH to use perl from /usr/bin","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-01T17:08:11Z","receivedAt":"2012-05-01T17:08:11Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, May 01, 2012 at 07:03:39PM +0200, Zbigniew Jędrzejewski-Szmek wrote:\n\n> But I don't see why we would use a different perl in\n> git-am.sh:                      perl -ne 'BEGIN { $subject = 0 }\n> git-am.sh:                      perl -M'POSIX qw(strftime)' -ne 'BEGIN { $subject = 0 }\n> git-request-pull.sh:ref=$(git ls-remote \"$url\" | perl -e \"$find_matching_ref\" \"$head\" \"$headrev\")\n> git-submodule.sh:       perl -e '\n> test-sha1.sh:                   perl -pe 'y/\\000/g/'\n> test-sha1.sh:                   perl -pe 'y/\\000/g/'\n> and lot of files in t/. Shouldn't those be replaced too?\n\nNo. There are two ways in which we use perl:\n\n  1. To run our complex scripts like gitweb, git-svn, etc. These require\n     a reasonably modern perl version, and the user must specify it with\n     PERL_PATH if it is not in /usr/bin.\n\n  2. To run little snippets that _could_ be written in sed or awk, but\n     which cause portability problems on crappy versions of those tools.\n     These should run under any version of perl5.\n\nIt's OK to use 'perl' from the path for (2), because we are not asking\nvery much of perl in that case.\n\nI think the patch we want is just:\n\ndiff --git a/t/gitweb-lib.sh b/t/gitweb-lib.sh\nindex 21d11d6..ae2dc46 100644\n--- a/t/gitweb-lib.sh\n+++ b/t/gitweb-lib.sh\n@@ -69,7 +69,7 @@ gitweb_run () {\n \t# written to web server logs, so we are not interested in that:\n \t# we are interested only in properly formatted errors/warnings\n \trm -f gitweb.log &&\n-\tperl -- \"$SCRIPT_NAME\" \\\n+\t\"$PERL_PATH\" -- \"$SCRIPT_NAME\" \\\n \t\t>gitweb.output 2>gitweb.log &&\n \tperl -w -e '\n \t\topen O, \">gitweb.headers\";\n\nno? Torsten, does that fix your problem?\n\n-Peff\n"},{"id":"190448","messageId":"4FA02274.6070601@web.de","threadId":"30380","inReplyTo":"20120501170810.GA22444@sigill.intra.peff.net","subject":"Re: [PATCH] gitweb-lib.sh: Set up PATH to use perl from /usr/bin","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2012-05-01T17:50:44Z","receivedAt":"2012-05-01T17:50:44Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"Thanks for all answers,\n> I think the patch we want is just:\n> \n> diff --git a/t/gitweb-lib.sh b/t/gitweb-lib.sh\n> index 21d11d6..ae2dc46 100644\n> --- a/t/gitweb-lib.sh\n> +++ b/t/gitweb-lib.sh\n> @@ -69,7 +69,7 @@ gitweb_run () {\n>  \t# written to web server logs, so we are not interested in that:\n>  \t# we are interested only in properly formatted errors/warnings\n>  \trm -f gitweb.log &&\n> -\tperl -- \"$SCRIPT_NAME\" \\\n> +\t\"$PERL_PATH\" -- \"$SCRIPT_NAME\" \\\n>  \t\t>gitweb.output 2>gitweb.log &&\n>  \tperl -w -e '\n>  \t\topen O, \">gitweb.headers\";\n> \n> no? Torsten, does that fix your problem?\nYes, it does.\n\nShould we go for that solution ?\n\n/Torsten\n"},{"id":"190450","messageId":"7v8vhbaitt.fsf@alter.siamese.dyndns.org","threadId":"30380","inReplyTo":"4FA02274.6070601@web.de","subject":"Re: [PATCH] gitweb-lib.sh: Set up PATH to use perl from /usr/bin","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-01T17:53:50Z","receivedAt":"2012-05-01T17:53:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Torsten Bögershausen <tboegi@web.de> writes:\n\n> Thanks for all answers,\n>> I think the patch we want is just:\n>> \n>> diff --git a/t/gitweb-lib.sh b/t/gitweb-lib.sh\n>> index 21d11d6..ae2dc46 100644\n>> --- a/t/gitweb-lib.sh\n>> +++ b/t/gitweb-lib.sh\n>> @@ -69,7 +69,7 @@ gitweb_run () {\n>>  \t# written to web server logs, so we are not interested in that:\n>>  \t# we are interested only in properly formatted errors/warnings\n>>  \trm -f gitweb.log &&\n>> -\tperl -- \"$SCRIPT_NAME\" \\\n>> +\t\"$PERL_PATH\" -- \"$SCRIPT_NAME\" \\\n>>  \t\t>gitweb.output 2>gitweb.log &&\n>>  \tperl -w -e '\n>>  \t\topen O, \">gitweb.headers\";\n>> \n>> no? Torsten, does that fix your problem?\n> Yes, it does.\n>\n> Should we go for that solution ?\n\nSounds good.\n"},{"id":"190451","messageId":"20120501175500.GA24258@sigill.intra.peff.net","threadId":"30380","inReplyTo":"4FA02274.6070601@web.de","subject":"[PATCH] t/gitweb-lib: use $PERL_PATH to run gitweb","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-01T17:55:00Z","receivedAt":"2012-05-01T17:55:00Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The current code runs \"perl gitweb.cgi\" to test gitweb. This\nwill use whatever version of perl happens to be first in the\nPATH. We are better off using the specific perl that the\nuser specified via PERL_PATH, which matches what gets put on\nthe #!-line of the built gitweb.cgi.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nOn Tue, May 01, 2012 at 07:50:44PM +0200, Torsten Bögershausen wrote:\n\n> > Torsten, does that fix your problem?\n> Yes, it does.\n\nOK, here it is with a commit message.\n\n t/gitweb-lib.sh |    2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/gitweb-lib.sh b/t/gitweb-lib.sh\nindex 21d11d6..ae2dc46 100644\n--- a/t/gitweb-lib.sh\n+++ b/t/gitweb-lib.sh\n@@ -69,7 +69,7 @@ gitweb_run () {\n \t# written to web server logs, so we are not interested in that:\n \t# we are interested only in properly formatted errors/warnings\n \trm -f gitweb.log &&\n-\tperl -- \"$SCRIPT_NAME\" \\\n+\t\"$PERL_PATH\" -- \"$SCRIPT_NAME\" \\\n \t\t>gitweb.output 2>gitweb.log &&\n \tperl -w -e '\n \t\topen O, \">gitweb.headers\";\n-- \n1.7.10.630.g31718\n"},{"id":"190475","messageId":"1335903498-583-1-git-send-email-zbyszek@in.waw.pl","threadId":"30380","inReplyTo":"20120501175500.GA24258@sigill.intra.peff.net","subject":"[PATCH] Consistently use perl from /usr/bin/ for scripts","fromName":"Zbigniew Jędrzejewski-Szmek","fromEmail":"zbyszek@in.waw.pl","sentAt":"2012-05-01T20:18:18Z","receivedAt":"2012-05-01T20:18:18Z","isPatch":true,"sender":{"key":"zbyszek@in.waw.pl","avatar":"https://avatars.githubusercontent.com/u/349618?v=4"},"body":"While the majority of scripts use '#!/usr/bin/perl', some use\n'#!/usr/bin/env perl'. In the end there is no difference, because the\nMakefile rewrites \"#!.*perl\" with \"#!$PERL_PATH\" in scripted\nPorcelains before installing. Nevertheless, the second form can be\nmisleading, because it suggests that perl found first in $PATH will be\nused.\n\nSuggested-by: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>\n---\n git-relink.perl |    2 +-\n git-svn.perl    |    2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/git-relink.perl b/git-relink.perl\nindex e136732..f29285c 100755\n--- a/git-relink.perl\n+++ b/git-relink.perl\n@@ -1,4 +1,4 @@\n-#!/usr/bin/env perl\n+#!/usr/bin/perl\n # Copyright 2005, Ryan Anderson <ryan@michonline.com>\n # Distribution permitted under the GPL v2, as distributed\n # by the Free Software Foundation.\ndiff --git a/git-svn.perl b/git-svn.perl\nindex 427da9e..9bec808 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -1,4 +1,4 @@\n-#!/usr/bin/env perl\n+#!/usr/bin/perl\n # Copyright (C) 2006, Eric Wong <normalperson@yhbt.net>\n # License: GPL v2 or later\n use 5.008;\n-- \n1.7.10.539.g8ecdfe\n"},{"id":"190481","messageId":"86vckfmxk1.fsf@red.stonehenge.com","threadId":"30380","inReplyTo":"1335903498-583-1-git-send-email-zbyszek@in.waw.pl","subject":"Re: [PATCH] Consistently use perl from /usr/bin/ for scripts","fromName":"Randal L. Schwartz","fromEmail":"merlyn@stonehenge.com","sentAt":"2012-05-01T20:54:54Z","receivedAt":"2012-05-01T20:54:54Z","isPatch":true,"sender":{"key":"merlyn@stonehenge.com","avatar":"https://gravatar.com/avatar/dc528d210743ff0333e6213f9ee7b33b23f1b7bc1f3c5a8c2d819074ecd7ab19?d=mp&s=160"},"body":">>>>> \"Zbigniew\" == Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl> writes:\n\nZbigniew> Suggested-by: Junio C Hamano <gitster@pobox.com>\nZbigniew> Signed-off-by: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>\n\nAnd I strongly support this change.  Too many things use that env trick,\nand it really isn't the proper solution to this.\n\n-- \nRandal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095\n<merlyn@stonehenge.com> <URL:http://www.stonehenge.com/merlyn/>\nSmalltalk/Perl/Unix consulting, Technical writing, Comedy, etc. etc.\nSee http://methodsandmessages.posterous.com/ for Smalltalk discussion\n"}]}