* [PATCH] Define $PERL_PATH in test-lib.sh @ 2009-11-10 10:46 Philippe Bruhat (BooK) 2009-11-10 12:23 ` Jeff King 2009-11-10 12:26 ` [PATCH] Define $PERL_PATH in test-lib.sh Johannes Sixt 0 siblings, 2 replies; 18+ messages in thread From: Philippe Bruhat (BooK) @ 2009-11-10 10:46 UTC (permalink / raw) To: git; +Cc: 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 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH] Define $PERL_PATH in test-lib.sh 2009-11-10 10:46 [PATCH] Define $PERL_PATH in test-lib.sh Philippe Bruhat (BooK) @ 2009-11-10 12:23 ` Jeff King 2009-11-10 13:33 ` Philippe Bruhat (BooK) 2009-11-15 9:12 ` Junio C Hamano 2009-11-10 12:26 ` [PATCH] Define $PERL_PATH in test-lib.sh Johannes Sixt 1 sibling, 2 replies; 18+ messages in thread From: Jeff King @ 2009-11-10 12:23 UTC (permalink / raw) To: Philippe Bruhat (BooK); +Cc: git 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)))'\' >>$@ ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH] Define $PERL_PATH in test-lib.sh 2009-11-10 12:23 ` Jeff King @ 2009-11-10 13:33 ` Philippe Bruhat (BooK) 2009-11-15 9:12 ` Junio C Hamano 1 sibling, 0 replies; 18+ messages in thread From: Philippe Bruhat (BooK) @ 2009-11-10 13:33 UTC (permalink / raw) To: Jeff King; +Cc: Philippe Bruhat (BooK), git 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)) ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] Define $PERL_PATH in test-lib.sh 2009-11-10 12:23 ` Jeff King 2009-11-10 13:33 ` Philippe Bruhat (BooK) @ 2009-11-15 9:12 ` Junio C Hamano 2009-11-16 23:48 ` Philippe Bruhat (BooK) 1 sibling, 1 reply; 18+ messages in thread From: Junio C Hamano @ 2009-11-15 9:12 UTC (permalink / raw) To: Philippe Bruhat (BooK); +Cc: Jeff King, git 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)))'\' >>$@ ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] Define $PERL_PATH in test-lib.sh 2009-11-15 9:12 ` Junio C Hamano @ 2009-11-16 23:48 ` Philippe Bruhat (BooK) 2009-11-16 23:53 ` [PATCH] Make sure $PERL_PATH is defined when the test suite is run Philippe Bruhat (BooK) 0 siblings, 1 reply; 18+ messages in thread From: Philippe Bruhat (BooK) @ 2009-11-16 23:48 UTC (permalink / raw) To: Junio C Hamano; +Cc: Jeff King, git 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)) ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] Make sure $PERL_PATH is defined when the test suite is run. 2009-11-16 23:48 ` Philippe Bruhat (BooK) @ 2009-11-16 23:53 ` Philippe Bruhat (BooK) 2009-11-17 0:10 ` Junio C Hamano 0 siblings, 1 reply; 18+ messages in thread From: Philippe Bruhat (BooK) @ 2009-11-16 23:53 UTC (permalink / raw) To: git; +Cc: 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 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH] Make sure $PERL_PATH is defined when the test suite is run. 2009-11-16 23:53 ` [PATCH] Make sure $PERL_PATH is defined when the test suite is run Philippe Bruhat (BooK) @ 2009-11-17 0:10 ` Junio C Hamano 2009-11-17 0:17 ` Philippe Bruhat (BooK) 2009-11-17 0:20 ` Philippe Bruhat (BooK) 0 siblings, 2 replies; 18+ messages in thread From: Junio C Hamano @ 2009-11-17 0:10 UTC (permalink / raw) To: Philippe Bruhat (BooK); +Cc: git "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? ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] Make sure $PERL_PATH is defined when the test suite is run. 2009-11-17 0:10 ` Junio C Hamano @ 2009-11-17 0:17 ` Philippe Bruhat (BooK) 2009-11-17 0:20 ` Philippe Bruhat (BooK) 1 sibling, 0 replies; 18+ messages in thread From: Philippe Bruhat (BooK) @ 2009-11-17 0:17 UTC (permalink / raw) To: Junio C Hamano; +Cc: Philippe Bruhat (BooK), git 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)) ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] Make sure $PERL_PATH is defined when the test suite is run. 2009-11-17 0:10 ` Junio C Hamano 2009-11-17 0:17 ` Philippe Bruhat (BooK) @ 2009-11-17 0:20 ` Philippe Bruhat (BooK) 2009-11-17 8:30 ` Johannes Sixt 1 sibling, 1 reply; 18+ messages in thread From: Philippe Bruhat (BooK) @ 2009-11-17 0:20 UTC (permalink / raw) To: git; +Cc: 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 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH] Make sure $PERL_PATH is defined when the test suite is run. 2009-11-17 0:20 ` Philippe Bruhat (BooK) @ 2009-11-17 8:30 ` Johannes Sixt 2009-11-17 8:35 ` Philippe Bruhat (BooK) 2009-11-17 8:42 ` Philippe Bruhat (BooK) 0 siblings, 2 replies; 18+ messages in thread From: Johannes Sixt @ 2009-11-17 8:30 UTC (permalink / raw) To: Philippe Bruhat (BooK); +Cc: git 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] Make sure $PERL_PATH is defined when the test suite is run. 2009-11-17 8:30 ` Johannes Sixt @ 2009-11-17 8:35 ` Philippe Bruhat (BooK) 2009-11-17 18:25 ` Junio C Hamano 2009-11-17 8:42 ` Philippe Bruhat (BooK) 1 sibling, 1 reply; 18+ messages in thread From: Philippe Bruhat (BooK) @ 2009-11-17 8:35 UTC (permalink / raw) To: Johannes Sixt; +Cc: git 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!) ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] Make sure $PERL_PATH is defined when the test suite is run. 2009-11-17 8:35 ` Philippe Bruhat (BooK) @ 2009-11-17 18:25 ` Junio C Hamano 0 siblings, 0 replies; 18+ messages in thread From: Junio C Hamano @ 2009-11-17 18:25 UTC (permalink / raw) To: Philippe Bruhat (BooK); +Cc: Johannes Sixt, git "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. ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] Make sure $PERL_PATH is defined when the test suite is run. 2009-11-17 8:30 ` Johannes Sixt 2009-11-17 8:35 ` Philippe Bruhat (BooK) @ 2009-11-17 8:42 ` Philippe Bruhat (BooK) 1 sibling, 0 replies; 18+ messages in thread From: Philippe Bruhat (BooK) @ 2009-11-17 8:42 UTC (permalink / raw) To: git; +Cc: 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 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH] Define $PERL_PATH in test-lib.sh 2009-11-10 10:46 [PATCH] Define $PERL_PATH in test-lib.sh Philippe Bruhat (BooK) 2009-11-10 12:23 ` Jeff King @ 2009-11-10 12:26 ` Johannes Sixt 2009-11-10 13:34 ` Philippe Bruhat (BooK) 1 sibling, 1 reply; 18+ messages in thread From: Johannes Sixt @ 2009-11-10 12:26 UTC (permalink / raw) To: Philippe Bruhat (BooK); +Cc: git 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] Define $PERL_PATH in test-lib.sh 2009-11-10 12:26 ` [PATCH] Define $PERL_PATH in test-lib.sh Johannes Sixt @ 2009-11-10 13:34 ` Philippe Bruhat (BooK) 2009-11-10 20:17 ` Junio C Hamano 0 siblings, 1 reply; 18+ messages in thread From: Philippe Bruhat (BooK) @ 2009-11-10 13:34 UTC (permalink / raw) To: Johannes Sixt; +Cc: git 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)) ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] Define $PERL_PATH in test-lib.sh 2009-11-10 13:34 ` Philippe Bruhat (BooK) @ 2009-11-10 20:17 ` Junio C Hamano 2009-11-11 8:40 ` Philippe Bruhat (BooK) 0 siblings, 1 reply; 18+ messages in thread From: Junio C Hamano @ 2009-11-10 20:17 UTC (permalink / raw) To: Philippe Bruhat (BooK); +Cc: Johannes Sixt, git "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. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] Define $PERL_PATH in test-lib.sh 2009-11-10 20:17 ` Junio C Hamano @ 2009-11-11 8:40 ` Philippe Bruhat (BooK) 2009-11-11 8:43 ` Jeff King 0 siblings, 1 reply; 18+ messages in thread From: Philippe Bruhat (BooK) @ 2009-11-11 8:40 UTC (permalink / raw) To: Junio C Hamano; +Cc: Johannes Sixt, git 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)) ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] Define $PERL_PATH in test-lib.sh 2009-11-11 8:40 ` Philippe Bruhat (BooK) @ 2009-11-11 8:43 ` Jeff King 0 siblings, 0 replies; 18+ messages in thread From: Jeff King @ 2009-11-11 8:43 UTC (permalink / raw) To: Philippe Bruhat (BooK); +Cc: Junio C Hamano, Johannes Sixt, git 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2009-11-17 18:25 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2009-11-10 10:46 [PATCH] Define $PERL_PATH in test-lib.sh Philippe Bruhat (BooK) 2009-11-10 12:23 ` Jeff King 2009-11-10 13:33 ` Philippe Bruhat (BooK) 2009-11-15 9:12 ` Junio C Hamano 2009-11-16 23:48 ` Philippe Bruhat (BooK) 2009-11-16 23:53 ` [PATCH] Make sure $PERL_PATH is defined when the test suite is run Philippe Bruhat (BooK) 2009-11-17 0:10 ` Junio C Hamano 2009-11-17 0:17 ` Philippe Bruhat (BooK) 2009-11-17 0:20 ` Philippe Bruhat (BooK) 2009-11-17 8:30 ` Johannes Sixt 2009-11-17 8:35 ` Philippe Bruhat (BooK) 2009-11-17 18:25 ` Junio C Hamano 2009-11-17 8:42 ` Philippe Bruhat (BooK) 2009-11-10 12:26 ` [PATCH] Define $PERL_PATH in test-lib.sh Johannes Sixt 2009-11-10 13:34 ` Philippe Bruhat (BooK) 2009-11-10 20:17 ` Junio C Hamano 2009-11-11 8:40 ` Philippe Bruhat (BooK) 2009-11-11 8:43 ` Jeff King
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).