On Fri, May 17, 2013 at 7:12 PM, Junio C Hamano <gits...@pobox.com> wrote: > Felipe Contreras <felipe.contre...@gmail.com> writes: > >> These remote helpers use 'env python', not PYTHON_PATH, so that's where >> we should check for the extensions. Otherwise, if 'python' is not >> PYTHON_PATH (e.g. /usr/bin/python: Makefile's default), there will be a >> mismatch between the python libraries actually accessible to the remote >> helpers. > > What I am reading here is that what the "helper" uses and what the > "test" checks to see if it can use the "helper" were different; and > this patch fixes that misalignment by testing what the "helper" > actually uses. > > So it is a right thing to do in that sense. > > I however am having this nagging feeling that I may be missing > something subtle here. Comments from others are very much welcome.
Yes, this is correct. Another way to skin this cat would be to do search/replace in a Makefile to burn in the PYTHON_PATH similar to how we do for the .sh scripts and other .py files in the main Makefile. The remote helpers are in contrib/ so they do not go through the main Makefile, which is the root cause. Longer-term, it would be good to treat these uniformly, but this is no worse for now. > Will queue but the result will be on tomorrow's pushout. > > Thanks. > >> Suggested by: Torsten Bögershausen <tbo...@web.de> >> Signed-off-by: Felipe Contreras <felipe.contre...@gmail.com> >> --- >> contrib/remote-helpers/test-bzr.sh | 2 +- >> contrib/remote-helpers/test-hg-bidi.sh | 2 +- >> contrib/remote-helpers/test-hg-hg-git.sh | 4 ++-- >> contrib/remote-helpers/test-hg.sh | 2 +- >> 4 files changed, 5 insertions(+), 5 deletions(-) >> >> diff --git a/contrib/remote-helpers/test-bzr.sh >> b/contrib/remote-helpers/test-bzr.sh >> index 5dfa070..2c89caa 100755 >> --- a/contrib/remote-helpers/test-bzr.sh >> +++ b/contrib/remote-helpers/test-bzr.sh >> @@ -12,7 +12,7 @@ if ! test_have_prereq PYTHON; then >> test_done >> fi >> >> -if ! "$PYTHON_PATH" -c 'import bzrlib'; then >> +if ! python -c 'import bzrlib'; then >> skip_all='skipping remote-bzr tests; bzr not available' >> test_done >> fi >> diff --git a/contrib/remote-helpers/test-hg-bidi.sh >> b/contrib/remote-helpers/test-hg-bidi.sh >> index f569697..2c693d0 100755 >> --- a/contrib/remote-helpers/test-hg-bidi.sh >> +++ b/contrib/remote-helpers/test-hg-bidi.sh >> @@ -15,7 +15,7 @@ if ! test_have_prereq PYTHON; then >> test_done >> fi >> >> -if ! "$PYTHON_PATH" -c 'import mercurial'; then >> +if ! python -c 'import mercurial'; then >> skip_all='skipping remote-hg tests; mercurial not available' >> test_done >> fi >> diff --git a/contrib/remote-helpers/test-hg-hg-git.sh >> b/contrib/remote-helpers/test-hg-hg-git.sh >> index 8440341..fbad2b9 100755 >> --- a/contrib/remote-helpers/test-hg-hg-git.sh >> +++ b/contrib/remote-helpers/test-hg-hg-git.sh >> @@ -15,12 +15,12 @@ if ! test_have_prereq PYTHON; then >> test_done >> fi >> >> -if ! "$PYTHON_PATH" -c 'import mercurial'; then >> +if ! python -c 'import mercurial'; then >> skip_all='skipping remote-hg tests; mercurial not available' >> test_done >> fi >> >> -if ! "$PYTHON_PATH" -c 'import hggit'; then >> +if ! python -c 'import hggit'; then >> skip_all='skipping remote-hg tests; hg-git not available' >> test_done >> fi >> diff --git a/contrib/remote-helpers/test-hg.sh >> b/contrib/remote-helpers/test-hg.sh >> index 8de2aa7..ce03fa3 100755 >> --- a/contrib/remote-helpers/test-hg.sh >> +++ b/contrib/remote-helpers/test-hg.sh >> @@ -15,7 +15,7 @@ if ! test_have_prereq PYTHON; then >> test_done >> fi >> >> -if ! "$PYTHON_PATH" -c 'import mercurial'; then >> +if ! python -c 'import mercurial'; then >> skip_all='skipping remote-hg tests; mercurial not available' >> test_done >> fi > -- > To unsubscribe from this list: send the line "unsubscribe git" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- David -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
