Date: Wed, 2 Jan 2013 03:59:35 -0500 From: Jeff King To: Junio C Hamano Cc: Dan McGee , GIT Mailing-list , Florian Achleitner , David Michael Barr , "Eric S. Raymond" Subject: Re: Test failures with python versions when building git 1.8.1 Message-ID: <20130102085935.GB9328@sigill.intra.peff.net> References: <7va9ss5fhq.fsf@alter.siamese.dyndns.org> <20130102065345.GA8685@sigill.intra.peff.net> <7v1ue459yh.fsf@alter.siamese.dyndns.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <7v1ue459yh.fsf@alter.siamese.dyndns.org> On Tue, Jan 01, 2013 at 11:18:46PM -0800, Junio C Hamano wrote: > Jeff King writes: > > > [1] This symlink is doubly wrong, because any use of symbolic links > > in the test scripts needs to depend on the SYMLINKS prereq, and this > > does not. > > Yeah, I think we have discussed this once already in > > http://thread.gmane.org/gmane.comp.version-control.git/210688/focus=210714 Thanks for the pointer; it looks like nothing productive came of the earlier discussion. To give a hat trick of failure to this line of code, I notice that the existing code also does not properly put quotes around $GIT_BUILD_DIR. > > [2] In both the current code and what I showed above, the test scripts > > depend on things in contrib/. This is probably a bad idea in > > general, as the quality of what goes into contrib is not as closely > > watched (especially with respect to things like portability). > > Certainly I would not have known to look more carefully at a patch > > to contrib/svn-fe for breakage to the test suite. > > As long as such tests are made skippable with appropriate > prerequisites, I do not think it is bad to have their tests in t/; I > would say it is rather better than having them in contrib/ and leave > it not run by anybody, which happened to some of the stuff in > contrib/ already. Good point. While my sense of decorum wants to keep contrib totally split out, from a practical point of view, it is better to have more people run the tests and report failures than not. Whether we end up doing something with contrib and tests or not, the patch below gives a minimal fix in the meantime. Dan, does it fix your problem? -- >8 -- Subject: [PATCH] t9020: don't run python from $PATH In t9020, we symlink in a python script from contrib to help with the testing. However, we don't munge its #!-line, which means we may run the wrong python (we want the one in PYTHON_PATH). On top of this, we use a symlink without checking the SYMLINKS prereq, and we fail to properly quote GIT_BUILD_DIR, which may have spaces. Instead of symlinking, let's just write a small script which will feed the contrib script to PYTHON_PATH. To avoid quoting issues, we just export the variables the script needs to run. Signed-off-by: Jeff King --- t/t9020-remote-svn.sh | 5 ++++- t/test-lib.sh | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/t/t9020-remote-svn.sh b/t/t9020-remote-svn.sh index 4f2dfe0..416623b 100755 --- a/t/t9020-remote-svn.sh +++ b/t/t9020-remote-svn.sh @@ -14,7 +14,10 @@ export PATH="$HOME:$PATH" # We override svnrdump by placing a symlink to the svnrdump-emulator in . export PATH="$HOME:$PATH" -ln -sf $GIT_BUILD_DIR/contrib/svn-fe/svnrdump_sim.py "$HOME/svnrdump" +export GIT_BUILD_DIR +write_script svnrdump <<\EOF +exec "$PYTHON_PATH" "$GIT_BUILD_DIR"/contrib/svn-fe/svnrdump_sim.py "$@" +EOF init_git () { rm -fr .git && diff --git a/t/test-lib.sh b/t/test-lib.sh index f50f834..c17db19 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -45,7 +45,7 @@ fi fi . "$GIT_BUILD_DIR"/GIT-BUILD-OPTIONS -export PERL_PATH SHELL_PATH +export PERL_PATH SHELL_PATH PYTHON_PATH # if --tee was passed, write the output not only to the terminal, but # additionally to the file test-results/$BASENAME.out, too. -- 1.8.1.rc3.4.gf3a2f57