git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] global: resolve Perl executable via PATH

From
Patrick Steinhardt <ps@pks.im>
Date
Apr 6, 2023, 08:19 UTC
Message-ID
<ZC6AdylF4TI41vnX@ncase>
In-Reply-To
<20230406032647.GA2092142@coredump.intra.peff.net>
On Wed, Apr 05, 2023 at 11:26:47PM -0400, Jeff King wrote:
Show 35 quoted lines
> On Wed, Apr 05, 2023 at 12:10:10PM +0200, Patrick Steinhardt wrote:
> 
> > The majority of Perl scripts we carry in Git have a `#!/usr/bin/perl`
> > shebang. This is not a portable location for the Perl interpreter and
> > may thus break on some systems that have the interpreter installed in a
> > different location. One such example is NixOS, where the only executable
> > installed in `/usr/bin` is env(1).
> > 
> > Convert the shebangs to resolve the location of the Perl interpreter via
> > env(1) to make these scripts more portable. While the location of env(1)
> > is not guaranteed by any standard either, in practice all distributions
> > including NixOS have it available at `/usr/bin/env`. We're also already
> > using this idiom in a small set of other scripts, and until now nobody
> > complained about them.
> > 
> > This makes the test suite pass on NixOS.
> 
> Can you tell us more about which tests failed?
> 
> Skimming over the list of files here, the first few examples:
> 
> >  Documentation/build-docdep.perl                    | 2 +-
> >  Documentation/cat-texi.perl                        | 2 +-
> >  Documentation/cmd-list.perl                        | 3 ++-
> >  Documentation/fix-texi.perl                        | 4 +++-
> >  Documentation/lint-fsck-msgids.perl                | 2 +-
> 
> will not be affected by your patch, because we never use their shebang
> lines at all (we say "$PERL_PATH cmd-list.perl" in the Makefile).
> 
> I did try removing /usr/bin/perl completely (and thus likewise had no
> perl in my path), setting PERL_PATH, and got a few broken tests, which
> could be fixed as below.
> 
> Does this fix the cases you saw, or are there others?

You know, let's just go with your patch. With PERL_PATH set it fixes all the issues I have observed. At some point in time I saw more issues than the one you fix here, but that's because my `config.mak` got lost without me noticing. Oops, embarassing.

Thanks!
Patrick
Show 59 quoted lines
> -- >8 --
> Subject: [PATCH] t/lib-httpd: pass PERL_PATH to CGI scripts
> 
> As discussed in t/README, tests should aim to use PERL_PATH rather than
> straight "perl". We usually do this automatically with a "perl" function
> in test-lib.sh, but a few cases need to be handled specially.
> 
> One such case is the apply-one-time-perl.sh CGI, which invokes plain
> "perl". It should be using $PERL_PATH, but to make that work, we must
> also instruct Apache to pass through the variable.
> 
> Prior to this patch, doing:
> 
>   mv /usr/bin/perl /usr/bin/my-perl
>   make PERL_PATH=/usr/bin/my-perl test
> 
> would fail t5702, t5703, and t5616. After this it passes. This is a
> pretty extreme case, as even if you install perl elsewhere, you'd likely
> still have it in your $PATH. A more realistic case is that you don't
> want to use the perl in your $PATH (because it's older, broken, etc) and
> expect PERL_PATH to consistently override that (since that's what it's
> documented to do). Removing it completely is just a convenient way of
> completely breaking it for testing purposes.
> 
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  t/lib-httpd/apache.conf            | 2 ++
>  t/lib-httpd/apply-one-time-perl.sh | 2 +-
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf
> index f43a25c1f10..9e6892970de 100644
> --- a/t/lib-httpd/apache.conf
> +++ b/t/lib-httpd/apache.conf
> @@ -101,6 +101,8 @@ PassEnv LC_ALL
>  Alias /dumb/ www/
>  Alias /auth/dumb/ www/auth/dumb/
>  
> +SetEnv PERL_PATH ${PERL_PATH}
> +
>  <LocationMatch /smart/>
>  	SetEnv GIT_EXEC_PATH ${GIT_EXEC_PATH}
>  	SetEnv GIT_HTTP_EXPORT_ALL
> diff --git a/t/lib-httpd/apply-one-time-perl.sh b/t/lib-httpd/apply-one-time-perl.sh
> index 09a0abdff7c..d7f9fed6aee 100644
> --- a/t/lib-httpd/apply-one-time-perl.sh
> +++ b/t/lib-httpd/apply-one-time-perl.sh
> @@ -13,7 +13,7 @@ then
>  	export LC_ALL
>  
>  	"$GIT_EXEC_PATH/git-http-backend" >out
> -	perl -pe "$(cat one-time-perl)" out >out_modified
> +	"$PERL_PATH" -pe "$(cat one-time-perl)" out >out_modified
>  
>  	if cmp -s out out_modified
>  	then
> -- 
> 2.40.0.824.g7b678b1f643
> 
Previous: Jeff KingNext: Jeff King
Message 24 of 27 in “global: resolve Perl executable via PATH”
  1. global: resolve Perl executable via PATHPatrick Steinhardt, Apr 5, 2023
  2. Felipe ContrerasApr 5, 2023
  3. Patrick SteinhardtApr 5, 2023
  4. Todd ZullingerApr 5, 2023
  5. Patrick SteinhardtApr 5, 2023
  6. Todd ZullingerApr 5, 2023
  7. Felipe ContrerasApr 5, 2023
  8. Patrick SteinhardtApr 5, 2023
  9. Junio C HamanoApr 5, 2023
  10. Felipe ContrerasApr 6, 2023
  11. Jeff KingApr 5, 2023
  12. Patrick SteinhardtApr 5, 2023
  13. Jeff KingApr 5, 2023
  14. Felipe ContrerasApr 6, 2023
  15. Jeff KingApr 6, 2023
  16. Ævar Arnfjörð BjarmasonApr 6, 2023
  17. Felipe ContrerasApr 18, 2023
  18. Patrick SteinhardtApr 6, 2023
  19. Kristoffer HaugsbakkApr 5, 2023
  20. Eric WongApr 5, 2023
  21. Felipe ContrerasApr 6, 2023
  22. Ævar Arnfjörð BjarmasonApr 6, 2023
  23. Jeff KingApr 6, 2023
  24. Patrick SteinhardtApr 6, 2023
  25. t/lib-httpd: pass PERL_PATH to CGI scriptsJeff King, Apr 6, 2023
  26. Junio C HamanoApr 6, 2023
  27. Felipe ContrerasApr 18, 2023

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.