{"thread":{"id":"33526","subject":"[PATCH] contrib/test-hg*.sh: Do not use PYTHON_PATH","startedAt":"2013-04-17T14:10:48Z","lastAt":"2013-04-17T19:17:57Z","messageCount":5,"participants":["Torsten Bögershausen","Jonathan Nieder","Junio C Hamano","Felipe Contreras"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"214609","messageId":"201304171610.49384.tboegi@web.de","threadId":"33526","inReplyTo":null,"subject":"[PATCH] contrib/test-hg*.sh: Do not use PYTHON_PATH","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2013-04-17T14:10:48Z","receivedAt":"2013-04-17T14:10:48Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"The test cases in contrib/remote-helpers use mercurial and python.\nBefore the tests are run, we check if python can import\n\"mercurial\" and \"hggit\".\nTo run this check, python pointed out by PYTHON_PATH is used.\nThis may not work when different python binaries exist,\nand PYTHON_PATH is not set:\n Makefile sets it to the default /usr/bin/python\n The PATH may point out e.g. /sw/bin/python.\nWhen /sw/bin/python has the mercurial module installed,\nbut /usr/bin/python has not, the test will not be run.\n\nGit respects PYTHON_PATH, hg does not.\nUse python instead of $PYTHON_PATH to check for installed modules.\n\nWhile at it, split exportX=Y into 2 lines\n\nSigned-off-by: Torsten Bögershausen <tboegi@web.de>\n---\n contrib/remote-helpers/test-hg-bidi.sh   | 14 +++++++++-----\n contrib/remote-helpers/test-hg-hg-git.sh | 12 +++++++-----\n contrib/remote-helpers/test-hg.sh        |  2 +-\n 3 files changed, 17 insertions(+), 11 deletions(-)\n\ndiff --git a/contrib/remote-helpers/test-hg-bidi.sh b/contrib/remote-helpers/test-hg-bidi.sh\nindex f368953..9f4a430 100755\n--- a/contrib/remote-helpers/test-hg-bidi.sh\n+++ b/contrib/remote-helpers/test-hg-bidi.sh\n@@ -15,7 +15,7 @@ if ! test_have_prereq PYTHON; then\n \ttest_done\n fi\n \n-if ! \"$PYTHON_PATH\" -c 'import mercurial'; then\n+if ! python -c 'import mercurial'; then\n \tskip_all='skipping remote-hg tests; mercurial not available'\n \ttest_done\n fi\n@@ -68,10 +68,13 @@ setup () {\n \t) >> \"$HOME\"/.hgrc &&\n \tgit config --global remote-hg.hg-git-compat true\n \n-\texport HGEDITOR=/usr/bin/true\n+\tHGEDITOR=/usr/bin/true\n+\texport HGEDITOR\n \n-\texport GIT_AUTHOR_DATE=\"2007-01-01 00:00:00 +0230\"\n-\texport GIT_COMMITTER_DATE=\"$GIT_AUTHOR_DATE\"\n+\tGIT_AUTHOR_DATE=\"2007-01-01 00:00:00 +0230\"\n+\tGIT_COMMITTER_DATE=\"$GIT_AUTHOR_DATE\"\n+\n+\texport GIT_AUTHOR_DATE GIT_COMMITTER_DATE\n }\n \n setup\n@@ -88,7 +91,8 @@ test_expect_success 'encoding' '\n \tgit add alpha &&\n \tgit commit -m \"add älphà\" &&\n \n-\texport GIT_AUTHOR_NAME=\"tést èncödîng\" &&\n+\tGIT_AUTHOR_NAME=\"tést èncödîng\" &&\n+\texport GIT_AUTHOR_NAME &&\n \techo beta > beta &&\n \tgit add beta &&\n \tgit commit -m \"add beta\" &&\ndiff --git a/contrib/remote-helpers/test-hg-hg-git.sh b/contrib/remote-helpers/test-hg-hg-git.sh\nindex 253e65a..5414f81 100755\n--- a/contrib/remote-helpers/test-hg-hg-git.sh\n+++ b/contrib/remote-helpers/test-hg-hg-git.sh\n@@ -15,12 +15,12 @@ if ! test_have_prereq PYTHON; then\n \ttest_done\n fi\n \n-if ! \"$PYTHON_PATH\" -c 'import mercurial'; then\n+if ! python -c 'import mercurial'; then\n \tskip_all='skipping remote-hg tests; mercurial not available'\n \ttest_done\n fi\n \n-if ! \"$PYTHON_PATH\" -c 'import hggit'; then\n+if ! python -c 'import hggit'; then\n \tskip_all='skipping remote-hg tests; hg-git not available'\n \ttest_done\n fi\n@@ -103,10 +103,12 @@ setup () {\n \tgit config --global receive.denycurrentbranch warn\n \tgit config --global remote-hg.hg-git-compat true\n \n-\texport HGEDITOR=/usr/bin/true\n+\tHGEDITOR=/usr/bin/true\n+\texport HGEDITOR\n \n-\texport GIT_AUTHOR_DATE=\"2007-01-01 00:00:00 +0230\"\n-\texport GIT_COMMITTER_DATE=\"$GIT_AUTHOR_DATE\"\n+  GIT_AUTHOR_DATE=\"2007-01-01 00:00:00 +0230\"\n+  GIT_COMMITTER_DATE=\"$GIT_AUTHOR_DATE\"\n+  export GIT_AUTHOR_DATE GIT_COMMITTER_DATE\n }\n \n setup\ndiff --git a/contrib/remote-helpers/test-hg.sh b/contrib/remote-helpers/test-hg.sh\nindex d5b8730..8614fa1 100755\n--- a/contrib/remote-helpers/test-hg.sh\n+++ b/contrib/remote-helpers/test-hg.sh\n@@ -15,7 +15,7 @@ if ! test_have_prereq PYTHON; then\n \ttest_done\n fi\n \n-if ! \"$PYTHON_PATH\" -c 'import mercurial'; then\n+if ! python -c 'import mercurial'; then\n \tskip_all='skipping remote-hg tests; mercurial not available'\n \ttest_done\n fi\n-- \n1.8.2.282.g4bc7171\n"},{"id":"214617","messageId":"20130417155326.GB3499@elie.Belkin","threadId":"33526","inReplyTo":"201304171610.49384.tboegi@web.de","subject":"Re: [PATCH] contrib/test-hg*.sh: Do not use PYTHON_PATH","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-04-17T15:53:26Z","receivedAt":"2013-04-17T15:53:26Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nTorsten Bögershausen wrote:\n\n> Git respects PYTHON_PATH, hg does not.\n\nProbably more relevant is that contrib/remote-helpers/git-remote-hg\nhas a shebang line of \"#!/usr/bin/env python\" and hence does not\nrespect PYTHON_PATH.\n\n> Use python instead of $PYTHON_PATH to check for installed modules.\n\nMakes sense to me.\n\n> While at it, split exportX=Y into 2 lines\n\nWould you mind splitting this change (which also looks good) into a\nseparate patch?  It makes life debugging, cherry-picking, reading,\nreverting when needed, and so on easier.\n\nHope that helps,\nJonathan\n"},{"id":"214620","messageId":"7vobddf6n6.fsf@alter.siamese.dyndns.org","threadId":"33526","inReplyTo":"201304171610.49384.tboegi@web.de","subject":"Re: [PATCH] contrib/test-hg*.sh: Do not use PYTHON_PATH","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-17T16:45:17Z","receivedAt":"2013-04-17T16:45:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Torsten Bögershausen <tboegi@web.de> writes:\n\n> The test cases in contrib/remote-helpers use mercurial and python.\n> Before the tests are run, we check if python can import\n> \"mercurial\" and \"hggit\".\n> To run this check, python pointed out by PYTHON_PATH is used.\n> This may not work when different python binaries exist,\n> and PYTHON_PATH is not set:\n>  Makefile sets it to the default /usr/bin/python\n>  The PATH may point out e.g. /sw/bin/python.\n> When /sw/bin/python has the mercurial module installed,\n> but /usr/bin/python has not, the test will not be run.\n> Git respects PYTHON_PATH, hg does not.\n\nThe above problem analysis looks sensible.\n\n> Use python instead of $PYTHON_PATH to check for installed modules.\n\nBut I am not sure if I agree with the solution.  Going back to the\nanalysis, I find this:\n\n> This may not work when different python binaries exist,\n> and PYTHON_PATH is not set:\n\nIsn't the real problem that PYTHON_PATH is not set to point at the\ninstance of a python with mercurial module for it?  Why not make\nsure it is set and can be seen by tests correctly?\n\nI do not offhand know where in the contrib/remote-helpers/ part the\nuser is expected to tweak PYTHON_PATH variable, and if the variable\nis correctly arranged to be exported to the environment when running\nthe tests, but your problem analysis tells me that that is the part\nyou would want to fix.  If a default-fallback value for PYTHON_PATH\nis the easiest solution to the problem, you could even solve that in\nthe \"export it while running the tests\" logic.\n\nPerhaps adding\n\n\tPYTHON_PATH ?= python\n\texport PYTHON_PATH\n\nto contrib/remote-helpers/Makefile and changing nothing else would\nbe a starting point for a more reasonable fix to the issue?\n\n> While at it, split exportX=Y into 2 lines\n\nThat is an important portability fix, but as you said \"while at it\",\nit is orthogonal.\n\n>\n> Signed-off-by: Torsten Bögershausen <tboegi@web.de>\n> ---\n>  contrib/remote-helpers/test-hg-bidi.sh   | 14 +++++++++-----\n>  contrib/remote-helpers/test-hg-hg-git.sh | 12 +++++++-----\n>  contrib/remote-helpers/test-hg.sh        |  2 +-\n>  3 files changed, 17 insertions(+), 11 deletions(-)\n>\n> diff --git a/contrib/remote-helpers/test-hg-bidi.sh b/contrib/remote-helpers/test-hg-bidi.sh\n> index f368953..9f4a430 100755\n> --- a/contrib/remote-helpers/test-hg-bidi.sh\n> +++ b/contrib/remote-helpers/test-hg-bidi.sh\n> @@ -15,7 +15,7 @@ if ! test_have_prereq PYTHON; then\n>  \ttest_done\n>  fi\n>  \n> -if ! \"$PYTHON_PATH\" -c 'import mercurial'; then\n> +if ! python -c 'import mercurial'; then\n>  \tskip_all='skipping remote-hg tests; mercurial not available'\n>  \ttest_done\n>  fi\n> @@ -68,10 +68,13 @@ setup () {\n>  \t) >> \"$HOME\"/.hgrc &&\n>  \tgit config --global remote-hg.hg-git-compat true\n>  \n> -\texport HGEDITOR=/usr/bin/true\n> +\tHGEDITOR=/usr/bin/true\n> +\texport HGEDITOR\n>  \n> -\texport GIT_AUTHOR_DATE=\"2007-01-01 00:00:00 +0230\"\n> -\texport GIT_COMMITTER_DATE=\"$GIT_AUTHOR_DATE\"\n> +\tGIT_AUTHOR_DATE=\"2007-01-01 00:00:00 +0230\"\n> +\tGIT_COMMITTER_DATE=\"$GIT_AUTHOR_DATE\"\n> +\n> +\texport GIT_AUTHOR_DATE GIT_COMMITTER_DATE\n>  }\n>  \n>  setup\n> @@ -88,7 +91,8 @@ test_expect_success 'encoding' '\n>  \tgit add alpha &&\n>  \tgit commit -m \"add älphà\" &&\n>  \n> -\texport GIT_AUTHOR_NAME=\"tést èncödîng\" &&\n> +\tGIT_AUTHOR_NAME=\"tést èncödîng\" &&\n> +\texport GIT_AUTHOR_NAME &&\n>  \techo beta > beta &&\n>  \tgit add beta &&\n>  \tgit commit -m \"add beta\" &&\n> diff --git a/contrib/remote-helpers/test-hg-hg-git.sh b/contrib/remote-helpers/test-hg-hg-git.sh\n> index 253e65a..5414f81 100755\n> --- a/contrib/remote-helpers/test-hg-hg-git.sh\n> +++ b/contrib/remote-helpers/test-hg-hg-git.sh\n> @@ -15,12 +15,12 @@ if ! test_have_prereq PYTHON; then\n>  \ttest_done\n>  fi\n>  \n> -if ! \"$PYTHON_PATH\" -c 'import mercurial'; then\n> +if ! python -c 'import mercurial'; then\n>  \tskip_all='skipping remote-hg tests; mercurial not available'\n>  \ttest_done\n>  fi\n>  \n> -if ! \"$PYTHON_PATH\" -c 'import hggit'; then\n> +if ! python -c 'import hggit'; then\n>  \tskip_all='skipping remote-hg tests; hg-git not available'\n>  \ttest_done\n>  fi\n> @@ -103,10 +103,12 @@ setup () {\n>  \tgit config --global receive.denycurrentbranch warn\n>  \tgit config --global remote-hg.hg-git-compat true\n>  \n> -\texport HGEDITOR=/usr/bin/true\n> +\tHGEDITOR=/usr/bin/true\n> +\texport HGEDITOR\n>  \n> -\texport GIT_AUTHOR_DATE=\"2007-01-01 00:00:00 +0230\"\n> -\texport GIT_COMMITTER_DATE=\"$GIT_AUTHOR_DATE\"\n> +  GIT_AUTHOR_DATE=\"2007-01-01 00:00:00 +0230\"\n> +  GIT_COMMITTER_DATE=\"$GIT_AUTHOR_DATE\"\n> +  export GIT_AUTHOR_DATE GIT_COMMITTER_DATE\n>  }\n>  \n>  setup\n> diff --git a/contrib/remote-helpers/test-hg.sh b/contrib/remote-helpers/test-hg.sh\n> index d5b8730..8614fa1 100755\n> --- a/contrib/remote-helpers/test-hg.sh\n> +++ b/contrib/remote-helpers/test-hg.sh\n> @@ -15,7 +15,7 @@ if ! test_have_prereq PYTHON; then\n>  \ttest_done\n>  fi\n>  \n> -if ! \"$PYTHON_PATH\" -c 'import mercurial'; then\n> +if ! python -c 'import mercurial'; then\n>  \tskip_all='skipping remote-hg tests; mercurial not available'\n>  \ttest_done\n>  fi\n"},{"id":"214630","messageId":"CAMP44s2RTKLO3nhBJcm_KAwq9iHt8ZSXeFuYksYv_JebK-eqrg@mail.gmail.com","threadId":"33526","inReplyTo":"201304171610.49384.tboegi@web.de","subject":"Re: [PATCH] contrib/test-hg*.sh: Do not use PYTHON_PATH","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-04-17T18:36:28Z","receivedAt":"2013-04-17T18:36:28Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, Apr 17, 2013 at 9:10 AM, Torsten Bögershausen <tboegi@web.de> wrote:\n> The test cases in contrib/remote-helpers use mercurial and python.\n> Before the tests are run, we check if python can import\n> \"mercurial\" and \"hggit\".\n> To run this check, python pointed out by PYTHON_PATH is used.\n> This may not work when different python binaries exist,\n> and PYTHON_PATH is not set:\n>  Makefile sets it to the default /usr/bin/python\n>  The PATH may point out e.g. /sw/bin/python.\n> When /sw/bin/python has the mercurial module installed,\n> but /usr/bin/python has not, the test will not be run.\n>\n> Git respects PYTHON_PATH, hg does not.\n> Use python instead of $PYTHON_PATH to check for installed modules.\n\nAnd this would fail if the distribution doesn't have a 'python'\nbinary, and instead has python2, python3, etc.\n\n> While at it, split exportX=Y into 2 lines\n\nDo it in a separate patch.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"214633","messageId":"CAMP44s1i5Pw5-Cbn3v_fGPLScjcs3HbUNkSpCgCngsTNnb1DJg@mail.gmail.com","threadId":"33526","inReplyTo":"CAMP44s2RTKLO3nhBJcm_KAwq9iHt8ZSXeFuYksYv_JebK-eqrg@mail.gmail.com","subject":"Re: [PATCH] contrib/test-hg*.sh: Do not use PYTHON_PATH","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-04-17T19:17:57Z","receivedAt":"2013-04-17T19:17:57Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, Apr 17, 2013 at 1:36 PM, Felipe Contreras\n<felipe.contreras@gmail.com> wrote:\n> On Wed, Apr 17, 2013 at 9:10 AM, Torsten Bögershausen <tboegi@web.de> wrote:\n>> The test cases in contrib/remote-helpers use mercurial and python.\n>> Before the tests are run, we check if python can import\n>> \"mercurial\" and \"hggit\".\n>> To run this check, python pointed out by PYTHON_PATH is used.\n>> This may not work when different python binaries exist,\n>> and PYTHON_PATH is not set:\n>>  Makefile sets it to the default /usr/bin/python\n>>  The PATH may point out e.g. /sw/bin/python.\n>> When /sw/bin/python has the mercurial module installed,\n>> but /usr/bin/python has not, the test will not be run.\n>>\n>> Git respects PYTHON_PATH, hg does not.\n>> Use python instead of $PYTHON_PATH to check for installed modules.\n>\n> And this would fail if the distribution doesn't have a 'python'\n> binary, and instead has python2, python3, etc.\n\nAlso, it would fail if mercurial is installed for python2, and python\nis really python3.\n\n-- \nFelipe Contreras\n"}]}