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

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

[Philippe Bruhat (BooK)]

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)

Signed-off-by: Philippe Bruhat (BooK) <book@cpan.org>
---
 t/t9400-git-cvsserver-server.sh |    2 +-
 t/t9401-git-cvsserver-crlf.sh   |    2 +-
 t/t9700-perl-git.sh             |    4 ++--
 t/test-lib.sh                   |    2 ++
 4 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh
index 64f947d..dc710f8 100755
--- a/t/t9400-git-cvsserver-server.sh
+++ b/t/t9400-git-cvsserver-server.sh
@@ -20,7 +20,7 @@ then
     say 'skipping git-cvsserver tests, cvs not found'
     test_done
 fi
-perl -e 'use DBI; use DBD::SQLite' >/dev/null 2>&1 || {
+$PERL_PATH -e 'use DBI; use DBD::SQLite' >/dev/null 2>&1 || {
     say 'skipping git-cvsserver tests, Perl SQLite interface unavailable'
     test_done
 }
diff --git a/t/t9401-git-cvsserver-crlf.sh b/t/t9401-git-cvsserver-crlf.sh
index aca40c1..c9e3dba 100755
--- a/t/t9401-git-cvsserver-crlf.sh
+++ b/t/t9401-git-cvsserver-crlf.sh
@@ -57,7 +57,7 @@ then
     say 'skipping git-cvsserver tests, perl not available'
     test_done
 fi
-perl -e 'use DBI; use DBD::SQLite' >/dev/null 2>&1 || {
+$PERL_PATH -e 'use DBI; use DBD::SQLite' >/dev/null 2>&1 || {
     say 'skipping git-cvsserver tests, Perl SQLite interface unavailable'
     test_done
 }
diff --git a/t/t9700-perl-git.sh b/t/t9700-perl-git.sh
index 4eb7d3f..3354e18 100755
--- a/t/t9700-perl-git.sh
+++ b/t/t9700-perl-git.sh
@@ -11,7 +11,7 @@ if ! test_have_prereq PERL; then
 	test_done
 fi
 
-perl -MTest::More -e 0 2>/dev/null || {
+$PERL_PATH -MTest::More -e 0 2>/dev/null || {
 	say "Perl Test::More unavailable, skipping test"
 	test_done
 }
@@ -48,6 +48,6 @@ test_expect_success \
 
 test_external_without_stderr \
     'Perl API' \
-    perl "$TEST_DIRECTORY"/t9700/test.pl
+    $PERL_PATH "$TEST_DIRECTORY"/t9700/test.pl
 
 test_done
diff --git a/t/test-lib.sh b/t/test-lib.sh
index f2ca536..54dd4d5 100644
--- 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
-- 
1.6.0.3.517.g759a

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

[Jeff King]

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.

> --- 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)))'\' >>$@

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

[Johannes Sixt]

Philippe Bruhat (BooK) schrieb:
> diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh
> index 64f947d..dc710f8 100755
> --- a/t/t9400-git-cvsserver-server.sh
> +++ b/t/t9400-git-cvsserver-server.sh
> @@ -20,7 +20,7 @@ then
>      say 'skipping git-cvsserver tests, cvs not found'
>      test_done
>  fi
> -perl -e 'use DBI; use DBD::SQLite' >/dev/null 2>&1 || {
> +$PERL_PATH -e 'use DBI; use DBD::SQLite' >/dev/null 2>&1 || {

Shouldn't this be "$PERL_PATH", i.e., double-quoted? (Ditto in the other
cases that you replaced.)

> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index f2ca536..54dd4d5 100644
> --- 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

Wouldn't

   ... && export PERL_PATH=perl

be a safer fall-back?

-- Hannes

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

[Philippe Bruhat (BooK)]

On Tue, Nov 10, 2009 at 07:23:15AM -0500, Jeff King wrote:
> 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)

PERL_PATH is a variable in the Makefile that, if not defined is set up
to /usr/bin/perl.

It is used to set the shebang line in the git-* perl scripts.

> 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.

Yes, this PERL_PATH.

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

Actually, my goal was to run Makefile.PL with the proper perl (see below).

>   3. To run random perl helper functions.

I didn't replace those ones, because any perl is good enough to do some
perl -i.bak -pe 's/foo/bar/' processing.

> 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.

The Perl in my PATH doesn't have Error.pm installed, but /usr/bin/perl
has it. When the Makefile.PL is run by /usr/bin/perl, the private-Error.pm
module is not copied in blib. The git-* perl scripts are using the perl
in PERL_PATH, and therefore can't load Error.pm (not in blib during make test),
causing test breakage.

This is the original reason for this patch. I fixed the general (2) case,
deliberately avoiding to replace every call to perl (3).

> 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.

Yes.

-- 
 Philippe Bruhat (BooK)

 Trust only in incompetence. You will never be disappointed.
                                    (Moral from Groo The Wanderer #16 (Epic))

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

[Philippe Bruhat (BooK)]

On Tue, Nov 10, 2009 at 01:26:53PM +0100, Johannes Sixt wrote:
> >  
> > +test -z "$NO_PERL" && test -z "$PERL_PATH" && export PERL_PATH=/usr/bin/perl
> 
> Wouldn't
> 
>    ... && export PERL_PATH=perl
> 
> be a safer fall-back?

/usr/bin/perl is the value used in the top-level Makefile.
I used this for consistency.

-- 
 Philippe Bruhat (BooK)

 The greatest monster of them all is ignorance.
                             (Moral to Pal'n Drumm Story in Groo #89 (Epic))

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

[Junio C Hamano]

"Philippe Bruhat (BooK)" <book@cpan.org> writes:

> On Tue, Nov 10, 2009 at 01:26:53PM +0100, Johannes Sixt wrote:
>> >  
>> > +test -z "$NO_PERL" && test -z "$PERL_PATH" && export PERL_PATH=/usr/bin/perl
>> 
>> Wouldn't
>> 
>>    ... && export PERL_PATH=perl
>> 
>> be a safer fall-back?
>
> /usr/bin/perl is the value used in the top-level Makefile.
> I used this for consistency.

Hmm, but that means two separate definitions in ./Makefile and
t/test-lib.sh must be kept in sync forever, and there is not even a
comment next to the line that requires such care in your patch to help
people who might want to change these lines in the future.

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

[Philippe Bruhat (BooK)]

On Tue, Nov 10, 2009 at 12:17:26PM -0800, Junio C Hamano wrote:
> "Philippe Bruhat (BooK)" <book@cpan.org> writes:
> 
> > On Tue, Nov 10, 2009 at 01:26:53PM +0100, Johannes Sixt wrote:
> >> >  
> >> > +test -z "$NO_PERL" && test -z "$PERL_PATH" && export PERL_PATH=/usr/bin/perl
> >> 
> >> Wouldn't
> >> 
> >>    ... && export PERL_PATH=perl
> >> 
> >> be a safer fall-back?
> >
> > /usr/bin/perl is the value used in the top-level Makefile.
> > I used this for consistency.
> 
> Hmm, but that means two separate definitions in ./Makefile and
> t/test-lib.sh must be kept in sync forever, and there is not even a
> comment next to the line that requires such care in your patch to help
> people who might want to change these lines in the future.

Is there a way to obtain whatever value was computed in the Makefile,
or should I just add a comment in all-caps saying "keep this in sync
with the default value in the top level Makefile"? (and a more detailed
commit message)

-- 
 Philippe Bruhat (BooK)

 Freedom is not an individual effort. Yours comes only when you grant others
 theirs.                             (Moral from Groo The Wanderer #5 (Epic))

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

[Jeff King]

On Wed, Nov 11, 2009 at 09:40:14AM +0100, Philippe Bruhat (BooK) wrote:

> > Hmm, but that means two separate definitions in ./Makefile and
> > t/test-lib.sh must be kept in sync forever, and there is not even a
> > comment next to the line that requires such care in your patch to help
> > people who might want to change these lines in the future.
> 
> Is there a way to obtain whatever value was computed in the Makefile,
> or should I just add a comment in all-caps saying "keep this in sync
> with the default value in the top level Makefile"? (and a more detailed
> commit message)

Yes. Did you miss the second half of my other message?

  http://article.gmane.org/gmane.comp.version-control.git/132561

-Peff

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

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Tue, Nov 10, 2009 at 11:46:51AM +0100, Philippe Bruhat (BooK) wrote:
> (snip)
> 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?

Philippe, could you please help getting this topic unstuck with a "it
works" or "it doesn't and here is a better solution"?

> 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)))'\' >>$@

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

[Philippe Bruhat (BooK)]

On Sun, Nov 15, 2009 at 01:12:37AM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
> 
> > On Tue, Nov 10, 2009 at 11:46:51AM +0100, Philippe Bruhat (BooK) wrote:
> > (snip)
> > 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?
> 
> Philippe, could you please help getting this topic unstuck with a "it
> works" or "it doesn't and here is a better solution"?
> 

I took Jeff's patch the main Makefile, removed my patch to test-lib.sh,
and it worked. That is to say, the test suite failed on the perl tests
when the first perl in the PATH was my local perl without Error.pm
installed. With the changes, the test suite passed, even with my local
perl first in the PATH.

Patch with a reworked commit message follows.

-- 
 Philippe Bruhat (BooK)

 The truly stupid always find a way to create disaster.
                                                (Moral from Groo #10 (Image))

[PATCH] Make sure $PERL_PATH is defined when the test suite is run.

[Philippe Bruhat (BooK)]

Some test scripts run Perl scripts as if they were git-* scripts, and
thus need to use the same perl that will be put in the shebang line of
git*.perl commands. $PERL_PATH therefore needs to be used instead of
a bare "perl".

The tests can fail if another perl is found in $PATH before the one
defined in $PERL_PATH.

Example test failure caused by this: the perl defined in $PERL_PATH has
Error.pm installed, and therefore the Git.pm's Makefile.PL doesn't install
the private copy. The perl from $PATH doesn't have Error.pm installed, and
all git*.perl scripts invoked during the test will fail loading Error.pm.

Makefile patch by Jeff King <peff@peff.net>.

Signed-off-by: Philippe Bruhat (BooK) <book@cpan.org>
---
 Makefile                        |    1 +
 t/t9400-git-cvsserver-server.sh |    2 +-
 t/t9401-git-cvsserver-crlf.sh   |    2 +-
 t/t9700-perl-git.sh             |    4 ++--
 4 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index 35f5294..8e8f981 100644
--- a/Makefile
+++ b/Makefile
@@ -1633,6 +1633,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)))'\' >>$@
diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh
index 64f947d..dc710f8 100755
--- a/t/t9400-git-cvsserver-server.sh
+++ b/t/t9400-git-cvsserver-server.sh
@@ -20,7 +20,7 @@ then
     say 'skipping git-cvsserver tests, cvs not found'
     test_done
 fi
-perl -e 'use DBI; use DBD::SQLite' >/dev/null 2>&1 || {
+$PERL_PATH -e 'use DBI; use DBD::SQLite' >/dev/null 2>&1 || {
     say 'skipping git-cvsserver tests, Perl SQLite interface unavailable'
     test_done
 }
diff --git a/t/t9401-git-cvsserver-crlf.sh b/t/t9401-git-cvsserver-crlf.sh
index aca40c1..c9e3dba 100755
--- a/t/t9401-git-cvsserver-crlf.sh
+++ b/t/t9401-git-cvsserver-crlf.sh
@@ -57,7 +57,7 @@ then
     say 'skipping git-cvsserver tests, perl not available'
     test_done
 fi
-perl -e 'use DBI; use DBD::SQLite' >/dev/null 2>&1 || {
+$PERL_PATH -e 'use DBI; use DBD::SQLite' >/dev/null 2>&1 || {
     say 'skipping git-cvsserver tests, Perl SQLite interface unavailable'
     test_done
 }
diff --git a/t/t9700-perl-git.sh b/t/t9700-perl-git.sh
index 4eb7d3f..3354e18 100755
--- a/t/t9700-perl-git.sh
+++ b/t/t9700-perl-git.sh
@@ -11,7 +11,7 @@ if ! test_have_prereq PERL; then
 	test_done
 fi
 
-perl -MTest::More -e 0 2>/dev/null || {
+$PERL_PATH -MTest::More -e 0 2>/dev/null || {
 	say "Perl Test::More unavailable, skipping test"
 	test_done
 }
@@ -48,6 +48,6 @@ test_expect_success \
 
 test_external_without_stderr \
     'Perl API' \
-    perl "$TEST_DIRECTORY"/t9700/test.pl
+    $PERL_PATH "$TEST_DIRECTORY"/t9700/test.pl
 
 test_done
-- 
1.6.0.3.517.g759a

Re: [PATCH] Make sure $PERL_PATH is defined when the test suite is run.

[Junio C Hamano]

"Philippe Bruhat (BooK)" <book@cpan.org> writes:

> diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh
> index 64f947d..dc710f8 100755
> --- a/t/t9400-git-cvsserver-server.sh
> +++ b/t/t9400-git-cvsserver-server.sh
> @@ -20,7 +20,7 @@ then
>      say 'skipping git-cvsserver tests, cvs not found'
>      test_done
>  fi
> -perl -e 'use DBI; use DBD::SQLite' >/dev/null 2>&1 || {
> +$PERL_PATH -e 'use DBI; use DBD::SQLite' >/dev/null 2>&1 || {
>      say 'skipping git-cvsserver tests, Perl SQLite interface unavailable'
>      test_done
>  }

Shouldn't these $PERL_PATH all be quoted inside double-quotes?

Re: [PATCH] Make sure $PERL_PATH is defined when the test suite is run.

[Philippe Bruhat (BooK)]

On Mon, Nov 16, 2009 at 04:10:13PM -0800, Junio C Hamano wrote:
> "Philippe Bruhat (BooK)" <book@cpan.org> writes:
> 
> > diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh
> > index 64f947d..dc710f8 100755
> > --- a/t/t9400-git-cvsserver-server.sh
> > +++ b/t/t9400-git-cvsserver-server.sh
> > @@ -20,7 +20,7 @@ then
> >      say 'skipping git-cvsserver tests, cvs not found'
> >      test_done
> >  fi
> > -perl -e 'use DBI; use DBD::SQLite' >/dev/null 2>&1 || {
> > +$PERL_PATH -e 'use DBI; use DBD::SQLite' >/dev/null 2>&1 || {
> >      say 'skipping git-cvsserver tests, Perl SQLite interface unavailable'
> >      test_done
> >  }
> 
> Shouldn't these $PERL_PATH all be quoted inside double-quotes?

I have no idea. I assume it's to protect against paths with a space in
them, so yes, probably.

Amending my patch and sending again.

-- 
 Philippe Bruhat (BooK)

 When you double-cross a friend, you triple-cross yourself.
                                     (Moral from Groo The Wanderer #8 (Epic))

[PATCH] Make sure $PERL_PATH is defined when the test suite is run.

[Philippe Bruhat (BooK)]

Some test scripts run Perl scripts as if they were git-* scripts, and
thus need to use the same perl that will be put in the shebang line of
git*.perl commands. $PERL_PATH therefore needs to be used instead of
a bare "perl".

The tests can fail if another perl is found in $PATH before the one
defined in $PERL_PATH.

Example test failure caused by this: the perl defined in $PERL_PATH has
Error.pm installed, and therefore the Git.pm's Makefile.PL doesn't install
the private copy. The perl from $PATH doesn't have Error.pm installed, and
all git*.perl scripts invoked during the test will fail loading Error.pm.

Makefile patch by Jeff King <peff@peff.net>.

Signed-off-by: Philippe Bruhat (BooK) <book@cpan.org>
---
 Makefile                        |    1 +
 t/t9400-git-cvsserver-server.sh |    2 +-
 t/t9401-git-cvsserver-crlf.sh   |    2 +-
 t/t9700-perl-git.sh             |    4 ++--
 4 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index 35f5294..8e8f981 100644
--- a/Makefile
+++ b/Makefile
@@ -1633,6 +1633,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)))'\' >>$@
diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh
index 64f947d..c2ec3cb 100755
--- a/t/t9400-git-cvsserver-server.sh
+++ b/t/t9400-git-cvsserver-server.sh
@@ -20,7 +20,7 @@ then
     say 'skipping git-cvsserver tests, cvs not found'
     test_done
 fi
-perl -e 'use DBI; use DBD::SQLite' >/dev/null 2>&1 || {
+"$PERL_PATH" -e 'use DBI; use DBD::SQLite' >/dev/null 2>&1 || {
     say 'skipping git-cvsserver tests, Perl SQLite interface unavailable'
     test_done
 }
diff --git a/t/t9401-git-cvsserver-crlf.sh b/t/t9401-git-cvsserver-crlf.sh
index aca40c1..40637d6 100755
--- a/t/t9401-git-cvsserver-crlf.sh
+++ b/t/t9401-git-cvsserver-crlf.sh
@@ -57,7 +57,7 @@ then
     say 'skipping git-cvsserver tests, perl not available'
     test_done
 fi
-perl -e 'use DBI; use DBD::SQLite' >/dev/null 2>&1 || {
+"$PERL_PATH" -e 'use DBI; use DBD::SQLite' >/dev/null 2>&1 || {
     say 'skipping git-cvsserver tests, Perl SQLite interface unavailable'
     test_done
 }
diff --git a/t/t9700-perl-git.sh b/t/t9700-perl-git.sh
index 4eb7d3f..96a2e55 100755
--- a/t/t9700-perl-git.sh
+++ b/t/t9700-perl-git.sh
@@ -11,7 +11,7 @@ if ! test_have_prereq PERL; then
 	test_done
 fi
 
-perl -MTest::More -e 0 2>/dev/null || {
+"$PERL_PATH" -MTest::More -e 0 2>/dev/null || {
 	say "Perl Test::More unavailable, skipping test"
 	test_done
 }
@@ -48,6 +48,6 @@ test_expect_success \
 
 test_external_without_stderr \
     'Perl API' \
-    perl "$TEST_DIRECTORY"/t9700/test.pl
+    $PERL_PATH "$TEST_DIRECTORY"/t9700/test.pl
 
 test_done
-- 
1.6.0.3.517.g759a

Re: [PATCH] Make sure $PERL_PATH is defined when the test suite is run.

[Johannes Sixt]

Philippe Bruhat (BooK) schrieb:
> --- a/Makefile
> +++ b/Makefile
> @@ -1633,6 +1633,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))'\' >$@

Make it: ... >>$@

>  	@echo TAR=\''$(subst ','\'',$(subst ','\'',$(TAR)))'\' >>$@
>  	@echo NO_CURL=\''$(subst ','\'',$(subst ','\'',$(NO_CURL)))'\' >>$@
>  	@echo NO_PERL=\''$(subst ','\'',$(subst ','\'',$(NO_PERL)))'\' >>$@

>  test_external_without_stderr \
>      'Perl API' \
> -    perl "$TEST_DIRECTORY"/t9700/test.pl
> +    $PERL_PATH "$TEST_DIRECTORY"/t9700/test.pl

This one needs the double-quotes as well.

-- Hannes

Re: [PATCH] Make sure $PERL_PATH is defined when the test suite is run.

[Philippe Bruhat (BooK)]

On Tue, Nov 17, 2009 at 09:30:17AM +0100, Johannes Sixt wrote:
> Philippe Bruhat (BooK) schrieb:
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -1633,6 +1633,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))'\' >$@
> 
> Make it: ... >>$@

This proves late commits needs many extra pair of eyes. :-)

> >  	@echo TAR=\''$(subst ','\'',$(subst ','\'',$(TAR)))'\' >>$@
> >  	@echo NO_CURL=\''$(subst ','\'',$(subst ','\'',$(NO_CURL)))'\' >>$@
> >  	@echo NO_PERL=\''$(subst ','\'',$(subst ','\'',$(NO_PERL)))'\' >>$@
> 
> >  test_external_without_stderr \
> >      'Perl API' \
> > -    perl "$TEST_DIRECTORY"/t9700/test.pl
> > +    $PERL_PATH "$TEST_DIRECTORY"/t9700/test.pl
> 
> This one needs the double-quotes as well.

Thanks. Sending again. (sorry for the noise)

-- 
 Philippe Bruhat (BooK)

 "Did I err?"      (Groo, in too many issues to count - ...and *YES* he did!)

[PATCH] Make sure $PERL_PATH is defined when the test suite is run.

[Philippe Bruhat (BooK)]

Some test scripts run Perl scripts as if they were git-* scripts, and
thus need to use the same perl that will be put in the shebang line of
git*.perl commands. $PERL_PATH therefore needs to be used instead of
a bare "perl".

The tests can fail if another perl is found in $PATH before the one
defined in $PERL_PATH.

Example test failure caused by this: the perl defined in $PERL_PATH has
Error.pm installed, and therefore the Git.pm's Makefile.PL doesn't install
the private copy. The perl from $PATH doesn't have Error.pm installed, and
all git*.perl scripts invoked during the test will fail loading Error.pm.

Makefile patch by Jeff King <peff@peff.net>.

Signed-off-by: Philippe Bruhat (BooK) <book@cpan.org>
---
 Makefile                        |    1 +
 t/t9400-git-cvsserver-server.sh |    2 +-
 t/t9401-git-cvsserver-crlf.sh   |    2 +-
 t/t9700-perl-git.sh             |    4 ++--
 4 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index 35f5294..287c7fc 100644
--- a/Makefile
+++ b/Makefile
@@ -1633,6 +1633,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)))'\' >>$@
diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh
index 64f947d..c2ec3cb 100755
--- a/t/t9400-git-cvsserver-server.sh
+++ b/t/t9400-git-cvsserver-server.sh
@@ -20,7 +20,7 @@ then
     say 'skipping git-cvsserver tests, cvs not found'
     test_done
 fi
-perl -e 'use DBI; use DBD::SQLite' >/dev/null 2>&1 || {
+"$PERL_PATH" -e 'use DBI; use DBD::SQLite' >/dev/null 2>&1 || {
     say 'skipping git-cvsserver tests, Perl SQLite interface unavailable'
     test_done
 }
diff --git a/t/t9401-git-cvsserver-crlf.sh b/t/t9401-git-cvsserver-crlf.sh
index aca40c1..40637d6 100755
--- a/t/t9401-git-cvsserver-crlf.sh
+++ b/t/t9401-git-cvsserver-crlf.sh
@@ -57,7 +57,7 @@ then
     say 'skipping git-cvsserver tests, perl not available'
     test_done
 fi
-perl -e 'use DBI; use DBD::SQLite' >/dev/null 2>&1 || {
+"$PERL_PATH" -e 'use DBI; use DBD::SQLite' >/dev/null 2>&1 || {
     say 'skipping git-cvsserver tests, Perl SQLite interface unavailable'
     test_done
 }
diff --git a/t/t9700-perl-git.sh b/t/t9700-perl-git.sh
index 4eb7d3f..8686086 100755
--- a/t/t9700-perl-git.sh
+++ b/t/t9700-perl-git.sh
@@ -11,7 +11,7 @@ if ! test_have_prereq PERL; then
 	test_done
 fi
 
-perl -MTest::More -e 0 2>/dev/null || {
+"$PERL_PATH" -MTest::More -e 0 2>/dev/null || {
 	say "Perl Test::More unavailable, skipping test"
 	test_done
 }
@@ -48,6 +48,6 @@ test_expect_success \
 
 test_external_without_stderr \
     'Perl API' \
-    perl "$TEST_DIRECTORY"/t9700/test.pl
+    "$PERL_PATH" "$TEST_DIRECTORY"/t9700/test.pl
 
 test_done
-- 
1.6.0.3.517.g759a

Re: [PATCH] Make sure $PERL_PATH is defined when the test suite is run.

[Junio C Hamano]

"Philippe Bruhat (BooK)" <book@cpan.org> writes:

>> Make it: ... >>$@
>
> This proves late commits needs many extra pair of eyes. :-)
> ...
>> This one needs the double-quotes as well.
>
> Thanks. Sending again. (sorry for the noise)

Thanks, I also missed them. Will re-queue.