threads / patch / 33526

patchcontrib/test-hg*.sh: Do not use PYTHON_PATH

Subject: [PATCH] contrib/test-hg*.sh: Do not use PYTHON_PATH

## tl;dr

5 messages between Apr 17, 2013 and Apr 17, 2013. Diffs are folded; open one to read it.

replies: 4people: 4as markdown or json

Torsten Bögershausen· Apr 17, 2013, 14:10 UTC · lore
The test cases in contrib/remote-helpers use mercurial and python.
Before the tests are run, we check if python can import
"mercurial" and "hggit".
To run this check, python pointed out by PYTHON_PATH is used.
This may not work when different python binaries exist,
and PYTHON_PATH is not set:
 Makefile sets it to the default /usr/bin/python
 The PATH may point out e.g. /sw/bin/python.
When /sw/bin/python has the mercurial module installed,
but /usr/bin/python has not, the test will not be run.

Git respects PYTHON_PATH, hg does not. Use python instead of $PYTHON_PATH to check for installed modules.

While at it, split exportX=Y into 2 lines
Signed-off-by: Torsten Bögershausen <tboegi@web.de>
---
 contrib/remote-helpers/test-hg-bidi.sh   | 14 +++++++++-----
 contrib/remote-helpers/test-hg-hg-git.sh | 12 +++++++-----
 contrib/remote-helpers/test-hg.sh        |  2 +-
 3 files changed, 17 insertions(+), 11 deletions(-)
Show changes to 3 files +17 −11

contrib/remote-helpers/test-hg-bidi.sh, contrib/remote-helpers/test-hg-hg-git.sh, contrib/remote-helpers/test-hg.sh

diff --git a/contrib/remote-helpers/test-hg-bidi.sh b/contrib/remote-helpers/test-hg-bidi.sh
index f368953..9f4a430 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
@@ -68,10 +68,13 @@ setup () {
 	) >> "$HOME"/.hgrc &&
 	git config --global remote-hg.hg-git-compat true
 
-	export HGEDITOR=/usr/bin/true
+	HGEDITOR=/usr/bin/true
+	export HGEDITOR
 
-	export GIT_AUTHOR_DATE="2007-01-01 00:00:00 +0230"
-	export GIT_COMMITTER_DATE="$GIT_AUTHOR_DATE"
+	GIT_AUTHOR_DATE="2007-01-01 00:00:00 +0230"
+	GIT_COMMITTER_DATE="$GIT_AUTHOR_DATE"
+
+	export GIT_AUTHOR_DATE GIT_COMMITTER_DATE
 }
 
 setup
@@ -88,7 +91,8 @@ test_expect_success 'encoding' '
 	git add alpha &&
 	git commit -m "add älphà" &&
 
-	export GIT_AUTHOR_NAME="tést èncödîng" &&
+	GIT_AUTHOR_NAME="tést èncödîng" &&
+	export GIT_AUTHOR_NAME &&
 	echo beta > beta &&
 	git add beta &&
 	git commit -m "add beta" &&
diff --git a/contrib/remote-helpers/test-hg-hg-git.sh b/contrib/remote-helpers/test-hg-hg-git.sh
index 253e65a..5414f81 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
@@ -103,10 +103,12 @@ setup () {
 	git config --global receive.denycurrentbranch warn
 	git config --global remote-hg.hg-git-compat true
 
-	export HGEDITOR=/usr/bin/true
+	HGEDITOR=/usr/bin/true
+	export HGEDITOR
 
-	export GIT_AUTHOR_DATE="2007-01-01 00:00:00 +0230"
-	export GIT_COMMITTER_DATE="$GIT_AUTHOR_DATE"
+  GIT_AUTHOR_DATE="2007-01-01 00:00:00 +0230"
+  GIT_COMMITTER_DATE="$GIT_AUTHOR_DATE"
+  export GIT_AUTHOR_DATE GIT_COMMITTER_DATE
 }
 
 setup
diff --git a/contrib/remote-helpers/test-hg.sh b/contrib/remote-helpers/test-hg.sh
index d5b8730..8614fa1 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
-- 
1.8.2.282.g4bc7171
Jonathan Nieder· Apr 17, 2013, 15:53 UTC · re: Torsten Bögershausen · lore

Re: [PATCH] contrib/test-hg*.sh: Do not use PYTHON_PATH

Hi,
Torsten Bögershausen wrote:
> Git respects PYTHON_PATH, hg does not.

Probably more relevant is that contrib/remote-helpers/git-remote-hg has a shebang line of "#!/usr/bin/env python" and hence does not respect PYTHON_PATH.

> Use python instead of $PYTHON_PATH to check for installed modules.
Makes sense to me.
> While at it, split exportX=Y into 2 lines

Would you mind splitting this change (which also looks good) into a separate patch? It makes life debugging, cherry-picking, reading, reverting when needed, and so on easier.

Hope that helps, Jonathan

Junio C Hamano· Apr 17, 2013, 16:45 UTC · re: Torsten Bögershausen · lore

Re: [PATCH] contrib/test-hg*.sh: Do not use PYTHON_PATH

Torsten Bögershausen <tboegi@web.de> writes:
Show 11 quoted lines
> The test cases in contrib/remote-helpers use mercurial and python.
> Before the tests are run, we check if python can import
> "mercurial" and "hggit".
> To run this check, python pointed out by PYTHON_PATH is used.
> This may not work when different python binaries exist,
> and PYTHON_PATH is not set:
>  Makefile sets it to the default /usr/bin/python
>  The PATH may point out e.g. /sw/bin/python.
> When /sw/bin/python has the mercurial module installed,
> but /usr/bin/python has not, the test will not be run.
> Git respects PYTHON_PATH, hg does not.
The above problem analysis looks sensible.
> Use python instead of $PYTHON_PATH to check for installed modules.

But I am not sure if I agree with the solution. Going back to the analysis, I find this:

> This may not work when different python binaries exist,
> and PYTHON_PATH is not set:

Isn't the real problem that PYTHON_PATH is not set to point at the instance of a python with mercurial module for it? Why not make sure it is set and can be seen by tests correctly?

I do not offhand know where in the contrib/remote-helpers/ part the user is expected to tweak PYTHON_PATH variable, and if the variable is correctly arranged to be exported to the environment when running the tests, but your problem analysis tells me that that is the part you would want to fix. If a default-fallback value for PYTHON_PATH is the easiest solution to the problem, you could even solve that in the "export it while running the tests" logic.

Perhaps adding
	PYTHON_PATH ?= python
	export PYTHON_PATH

to contrib/remote-helpers/Makefile and changing nothing else would be a starting point for a more reasonable fix to the issue?

> While at it, split exportX=Y into 2 lines

That is an important portability fix, but as you said "while at it", it is orthogonal.

Show 96 quoted lines
>
> Signed-off-by: Torsten Bögershausen <tboegi@web.de>
> ---
>  contrib/remote-helpers/test-hg-bidi.sh   | 14 +++++++++-----
>  contrib/remote-helpers/test-hg-hg-git.sh | 12 +++++++-----
>  contrib/remote-helpers/test-hg.sh        |  2 +-
>  3 files changed, 17 insertions(+), 11 deletions(-)
>
> diff --git a/contrib/remote-helpers/test-hg-bidi.sh b/contrib/remote-helpers/test-hg-bidi.sh
> index f368953..9f4a430 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
> @@ -68,10 +68,13 @@ setup () {
>  	) >> "$HOME"/.hgrc &&
>  	git config --global remote-hg.hg-git-compat true
>  
> -	export HGEDITOR=/usr/bin/true
> +	HGEDITOR=/usr/bin/true
> +	export HGEDITOR
>  
> -	export GIT_AUTHOR_DATE="2007-01-01 00:00:00 +0230"
> -	export GIT_COMMITTER_DATE="$GIT_AUTHOR_DATE"
> +	GIT_AUTHOR_DATE="2007-01-01 00:00:00 +0230"
> +	GIT_COMMITTER_DATE="$GIT_AUTHOR_DATE"
> +
> +	export GIT_AUTHOR_DATE GIT_COMMITTER_DATE
>  }
>  
>  setup
> @@ -88,7 +91,8 @@ test_expect_success 'encoding' '
>  	git add alpha &&
>  	git commit -m "add älphà" &&
>  
> -	export GIT_AUTHOR_NAME="tést èncödîng" &&
> +	GIT_AUTHOR_NAME="tést èncödîng" &&
> +	export GIT_AUTHOR_NAME &&
>  	echo beta > beta &&
>  	git add beta &&
>  	git commit -m "add beta" &&
> diff --git a/contrib/remote-helpers/test-hg-hg-git.sh b/contrib/remote-helpers/test-hg-hg-git.sh
> index 253e65a..5414f81 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
> @@ -103,10 +103,12 @@ setup () {
>  	git config --global receive.denycurrentbranch warn
>  	git config --global remote-hg.hg-git-compat true
>  
> -	export HGEDITOR=/usr/bin/true
> +	HGEDITOR=/usr/bin/true
> +	export HGEDITOR
>  
> -	export GIT_AUTHOR_DATE="2007-01-01 00:00:00 +0230"
> -	export GIT_COMMITTER_DATE="$GIT_AUTHOR_DATE"
> +  GIT_AUTHOR_DATE="2007-01-01 00:00:00 +0230"
> +  GIT_COMMITTER_DATE="$GIT_AUTHOR_DATE"
> +  export GIT_AUTHOR_DATE GIT_COMMITTER_DATE
>  }
>  
>  setup
> diff --git a/contrib/remote-helpers/test-hg.sh b/contrib/remote-helpers/test-hg.sh
> index d5b8730..8614fa1 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
Felipe Contreras· Apr 17, 2013, 18:36 UTC · re: Torsten Bögershausen · lore

Re: [PATCH] contrib/test-hg*.sh: Do not use PYTHON_PATH

On Wed, Apr 17, 2013 at 9:10 AM, Torsten Bögershausen <tboegi@web.de> wrote:
Show 13 quoted lines
> The test cases in contrib/remote-helpers use mercurial and python.
> Before the tests are run, we check if python can import
> "mercurial" and "hggit".
> To run this check, python pointed out by PYTHON_PATH is used.
> This may not work when different python binaries exist,
> and PYTHON_PATH is not set:
>  Makefile sets it to the default /usr/bin/python
>  The PATH may point out e.g. /sw/bin/python.
> When /sw/bin/python has the mercurial module installed,
> but /usr/bin/python has not, the test will not be run.
>
> Git respects PYTHON_PATH, hg does not.
> Use python instead of $PYTHON_PATH to check for installed modules.

And this would fail if the distribution doesn't have a 'python' binary, and instead has python2, python3, etc.

> While at it, split exportX=Y into 2 lines
Do it in a separate patch.
Cheers.
-- 
Felipe Contreras
Felipe Contreras· Apr 17, 2013, 19:17 UTC · re: Felipe Contreras · lore

Re: [PATCH] contrib/test-hg*.sh: Do not use PYTHON_PATH

On Wed, Apr 17, 2013 at 1:36 PM, Felipe Contreras <felipe.contreras@gmail.com> wrote:

Show 17 quoted lines
> On Wed, Apr 17, 2013 at 9:10 AM, Torsten Bögershausen <tboegi@web.de> wrote:
>> The test cases in contrib/remote-helpers use mercurial and python.
>> Before the tests are run, we check if python can import
>> "mercurial" and "hggit".
>> To run this check, python pointed out by PYTHON_PATH is used.
>> This may not work when different python binaries exist,
>> and PYTHON_PATH is not set:
>>  Makefile sets it to the default /usr/bin/python
>>  The PATH may point out e.g. /sw/bin/python.
>> When /sw/bin/python has the mercurial module installed,
>> but /usr/bin/python has not, the test will not be run.
>>
>> Git respects PYTHON_PATH, hg does not.
>> Use python instead of $PYTHON_PATH to check for installed modules.
>
> And this would fail if the distribution doesn't have a 'python'
> binary, and instead has python2, python3, etc.

Also, it would fail if mercurial is installed for python2, and python is really python3.

-- 
Felipe Contreras

← back to recent threads