Re: [PATCH] Define $PERL_PATH in test-lib.sh

From: Jeff King <peff@peff.net>
Date: Nov 10, 2009, 12:23 UTC
Message-ID: <20091110122315.GA15906@coredump.intra.peff.net>
In-Reply-To: <1257850011-7544-1-git-send-email-book@cpan.org>

On Tue, Nov 10, 2009 at 11:46:51AM +0100, Philippe Bruhat (BooK) wrote:

> The main Makefile defines PERL_PATH as the perl to use in the shebang
> line of git*.perl commands. This ensures this will be the perl used
> to run the tests (in case another perl appears in $PATH before the one
> defined in $PERL_PATH)

I think this "the perl used to run the tests" needs to be clarified in the commit message. There are really three ways we use perl in the tests:

  1. To run to the git-* scripts themselves.

  2. To run a test snippet of perl as if we were a git-* script.

  3. To run random perl helper functions.

We already use PERL_PATH for (1). I don't think there is much point in worrying about (3). If the perl in your PATH is so broken that it can't be used for simple helpers, then you should fix your PATH.

Your patch seems to just fix (2), which I think is sane. But I wanted to note it, because when I read your commit message, I wasn't sure which you were doing.

Show 11 quoted lines

> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -730,6 +730,8 @@ esac
>  
>  test -z "$NO_PERL" && test_set_prereq PERL
>  
> +test -z "$NO_PERL" && test -z "$PERL_PATH" && export PERL_PATH=/usr/bin/perl
> +
>  # test whether the filesystem supports symbolic links
>  ln -s x y 2>/dev/null && test -h y 2>/dev/null && test_set_prereq SYMLINKS
>  rm -f y

Will this work if I just have PERL_PATH in my config.mak in the root directory? Should we be adding PERL_PATH to the generated GIT-BUILD-OPTIONS file in the root, which gets sourced by test-lib?

Something like the following (completely untested) patch?

diff --git a/Makefile b/Makefile
index a10a60c..b9a8145 100644
--- a/Makefile
+++ b/Makefile
@@ -1643,6 +1643,7 @@ GIT-CFLAGS: .FORCE-GIT-CFLAGS
 # and the first level quoting from the shell that runs "echo".
 GIT-BUILD-OPTIONS: .FORCE-GIT-BUILD-OPTIONS
 	@echo SHELL_PATH=\''$(subst ','\'',$(SHELL_PATH_SQ))'\' >$@
+	@echo PERL_PATH=\''$(subst ','\'',$(PERL_PATH_SQ))'\' >$@
 	@echo TAR=\''$(subst ','\'',$(subst ','\'',$(TAR)))'\' >>$@
 	@echo NO_CURL=\''$(subst ','\'',$(subst ','\'',$(NO_CURL)))'\' >>$@
 	@echo NO_PERL=\''$(subst ','\'',$(subst ','\'',$(NO_PERL)))'\' >>$@