{"thread":{"id":"30878","subject":"[RFC] test-lib.sh: preprocess to use PERL_PATH","startedAt":"2012-06-23T05:04:37Z","lastAt":"2012-06-23T13:11:31Z","messageCount":5,"participants":["Torsten Bögershausen","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"194122","messageId":"201206230704.38648.tboegi@web.de","threadId":"30878","inReplyTo":null,"subject":"[RFC] test-lib.sh: preprocess to use PERL_PATH","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2012-06-23T05:04:37Z","receivedAt":"2012-06-23T05:04:37Z","isPatch":false,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"All test cases found in t/*.sh must include test-lib instead of test-lib.sh\n\nReplace the inclusion of GIT-BUILD-OPTIONS with definitions that are filled in\nduring preprocessing.\n\nThe new test-lib doesn't need to to open GIT-BUILD-OPTIONS for each\ntest script, which speeds up the execution a bit.\n\nSigned-off-by: Torsten Bögershausen <tboegi@web.de>\n---\n\nNote: The diff of all files in t/* is not here in this patch - the mail seems not to got through when I add\nall the diffs here.\n\nSo this patch is incomplete and should show the idea.\n\nI ran the following script to do the changes:\nsedmulti.sh t/*\n(where sedmulti.sh is below:)\n\n====================\n#!/bin/sh\n\nif [ $# -lt 3 ]; then\n  echo \"usage sedmulti <fromtext> <totext> filename ...\" >&2\n        exit 1;\nfi\n\nfromtext=$1;\nshift;\ntotext=$1;\nshift;\n\nfor f in \"$@\"; do\n  echo sed -ire \"s%$fromtext%$totext%g\" $f\n  sed -ire \"s%$fromtext%$totext%g\" $f\ndone\n===================\n\n\n .gitignore    |    1 +\n Makefile      |    4 ++++\n t/README      |    8 ++++----\n t/test-lib.sh |   11 ++++++++---\n 4 files changed, 17 insertions(+), 7 deletions(-)\n\ndiff --git a/.gitignore b/.gitignore\nindex bf66648..62c60ce 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -192,6 +192,7 @@\n /test-sigchain\n /test-subprocess\n /test-svn-fe\n+t/test-lib\n /common-cmds.h\n *.tar.gz\n *.dsc\ndiff --git a/Makefile b/Makefile\nindex 0914133..d540ab6 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -442,6 +442,7 @@ SCRIPT_LIB += git-rebase--interactive\n SCRIPT_LIB += git-rebase--merge\n SCRIPT_LIB += git-sh-setup\n SCRIPT_LIB += git-sh-i18n\n+SCRIPT_LIB += t/test-lib\n \n SCRIPT_PERL += git-add--interactive.perl\n SCRIPT_PERL += git-difftool.perl\n@@ -2018,6 +2019,9 @@ sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \\\n     -e 's|@@LOCALEDIR@@|$(localedir_SQ)|g' \\\n     -e 's/@@NO_CURL@@/$(NO_CURL)/g' \\\n     -e 's/@@USE_GETTEXT_SCHEME@@/$(USE_GETTEXT_SCHEME)/g' \\\n+    -e 's|@@PERL_PATH@@|$(PERL_PATH)|g' \\\n+    -e 's|@@PYTHON_PATH@@|$(PYTHON_PATH)|g' \\\n+    -e 's|@@TAR@@|$(TAR)|g' \\\n     -e $(BROKEN_PATH_FIX) \\\n     $@.sh >$@+\n endef\ndiff --git a/t/README b/t/README\nindex 4c3ea25..fa23726 100644\n--- a/t/README\n+++ b/t/README\n@@ -203,7 +203,7 @@ the top-level test script, never name the file to match the above\n pattern.  The Makefile here considers all such files as the\n top-level test script and tries to run all of them.  Care is\n especially needed if you are creating a common test library\n-file, similar to test-lib.sh, because such a library file may\n+file, similar to test-lib, because such a library file may\n not be suitable for standalone execution.\n \n \n@@ -225,13 +225,13 @@ assignment to variable 'test_description', like this:\n \tand tries to run git-ls-files with option --frotz.'\n \n \n-Source 'test-lib.sh'\n+Source 'test-lib'\n --------------------\n \n After assigning test_description, the test script should source\n-test-lib.sh like this:\n+test-lib like this:\n \n-\t. ./test-lib.sh\n+\t. ./test-lib\n \n This test harness library does the following things:\n \ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 9e2b711..306fcd9 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -46,7 +46,7 @@ EDITOR=:\n # /usr/xpg4/bin/sh and /bin/ksh to bail out.  So keep the unsets\n # deriving from the command substitution clustered with the other\n # ones.\n-unset VISUAL EMAIL LANGUAGE COLUMNS $(perl -e '\n+unset VISUAL EMAIL LANGUAGE COLUMNS $(@@PERL_PATH@@ -e '\n \tmy @env = keys %ENV;\n \tmy $ok = join(\"|\", qw(\n \t\tTRACE\n@@ -107,7 +107,7 @@ export _x05 _x40 _z40 LF\n # test_description='Description of this test...\n # This test checks if command xyzzy does the right thing...\n # '\n-# . ./test-lib.sh\n+# . ./test-lib\n [ \"x$ORIGINAL_TERM\" != \"xdumb\" ] && (\n \t\tTERM=$ORIGINAL_TERM &&\n \t\texport TERM &&\n@@ -492,7 +492,12 @@ GIT_CONFIG_NOSYSTEM=1\n GIT_ATTR_NOSYSTEM=1\n export PATH GIT_EXEC_PATH GIT_TEMPLATE_DIR GIT_CONFIG_NOSYSTEM GIT_ATTR_NOSYSTEM\n \n-. \"$GIT_BUILD_DIR\"/GIT-BUILD-OPTIONS\n+SHELL_PATH='@SHELL_PATH@'\n+PERL_PATH='@@PERL_PATH@@'\n+DIFF='@@DIFF@@'\n+PYTHON_PATH='@@PYTHON_PATH@@'\n+TAR='@@TAR@@'\n+export PERL_PATH \n \n if test -z \"$GIT_TEST_CMP\"\n then\n-- \n1.7.9.1.3.gd6f4c.dirty\n"},{"id":"194123","messageId":"7vsjdmli06.fsf@alter.siamese.dyndns.org","threadId":"30878","inReplyTo":"201206230704.38648.tboegi@web.de","subject":"Re: [RFC] test-lib.sh: preprocess to use PERL_PATH","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-23T05:22:33Z","receivedAt":"2012-06-23T05:22:33Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Torsten Bögershausen <tboegi@web.de> writes:\n\n> All test cases found in t/*.sh must include test-lib instead of test-lib.sh\n\nPlease don't.  That is too much churning for too little gain, I am afraid.\n"},{"id":"194124","messageId":"4FE55372.3080008@web.de","threadId":"30878","inReplyTo":"7vsjdmli06.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC] test-lib.sh: preprocess to use PERL_PATH","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2012-06-23T05:26:10Z","receivedAt":"2012-06-23T05:26:10Z","isPatch":false,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 23.06.12 07:22, Junio C Hamano wrote:\n> Torsten Bögershausen <tboegi@web.de> writes:\n>\n>> All test cases found in t/*.sh must include test-lib instead of test-lib.sh\n> Please don't.  That is too much churning for too little gain, I am afraid.\nOk, would it be better to rename\n\nt/test-lib.sh -> t/test-lib.sh.sh\n\nand let the Makefile generate t/test-lib.sh?\n"},{"id":"194125","messageId":"7vipeilfe9.fsf@alter.siamese.dyndns.org","threadId":"30878","inReplyTo":"4FE55372.3080008@web.de","subject":"Re: [RFC] test-lib.sh: preprocess to use PERL_PATH","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-23T06:18:54Z","receivedAt":"2012-06-23T06:18:54Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Torsten Bögershausen <tboegi@web.de> writes:\n\n> On 23.06.12 07:22, Junio C Hamano wrote:\n>> Torsten Bögershausen <tboegi@web.de> writes:\n>>\n>>> All test cases found in t/*.sh must include test-lib instead of test-lib.sh\n>> Please don't.  That is too much churning for too little gain, I am afraid.\n> Ok, would it be better to rename\n>\n> t/test-lib.sh -> t/test-lib.sh.sh\n>\n> and let the Makefile generate t/test-lib.sh?\n\nIt isn't as bad as the patch posted, but not very much.\n\nThere are number of a lot lower impact options before you\ncontemplate such a large change, given that there is only one\ninvocation of bare \"perl\" before GIT-BUILD-OPTIONS is dot-sourced.\n\n (1) Perhaps that use does not have any portability issues, and we\n     can leave it as-is, with a comment to forbid people from\n     turning into \"$PERL_PATH\" and be done with it?\n\n (2) Perhaps that use can be rewritten in such a way that it does\n     not have to be done with perl in the first place?\n\n (3) Perhaps what that use of perl does can be delayed until we\n     dot-source GIT-BUILD-OPTIONS and have $PERL_PATH defined, in\n     which case we can move that use to a later position (and we can\n     turn that sole use of perl into \"$PERL_PATH\")?\n\n (3) Perhaps what test-lib.sh does before it dot-sources\n     GIT-BUILD-OPTIONS does not be affected if we dot-sourced\n     GIT-BUILD-OPTIONS a lot earlier (and we can turn that sole use\n     of perl into \"$PERL_PATH\")?\n\n\nFor example (this is not tested at all, nor I did not think it\nthrough), a patch that moves the definition of TEST_DIRECTORY which\nGIT_BUILD_DIR depends on higher, so that we can dot-source the file\na lot earlier, may look like this.\n\n\n t/test-lib.sh | 43 ++++++++++++++++++++++---------------------\n 1 file changed, 22 insertions(+), 21 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 9e2b711..f3e7cf9 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -34,6 +34,26 @@ esac\n # Keep the original TERM for say_color\n ORIGINAL_TERM=$TERM\n \n+# Test the binaries we have just built.  The tests are kept in\n+# t/ subdirectory and are run in 'trash directory' subdirectory.\n+if test -z \"$TEST_DIRECTORY\"\n+then\n+\t# We allow tests to override this, in case they want to run tests\n+\t# outside of t/, e.g. for running tests on the test library\n+\t# itself.\n+\tTEST_DIRECTORY=$(pwd)\n+fi\n+if test -z \"$TEST_OUTPUT_DIRECTORY\"\n+then\n+\t# Similarly, override this to store the test-results subdir\n+\t# elsewhere\n+\tTEST_OUTPUT_DIRECTORY=$TEST_DIRECTORY\n+fi\n+GIT_BUILD_DIR=\"$TEST_DIRECTORY\"/..\n+\n+. \"$GIT_BUILD_DIR\"/GIT-BUILD-OPTIONS\n+export PERL_PATH SHELL_PATH\n+\n # For repeatability, reset the environment to known value.\n LANG=C\n LC_ALL=C\n@@ -46,7 +66,7 @@ EDITOR=:\n # /usr/xpg4/bin/sh and /bin/ksh to bail out.  So keep the unsets\n # deriving from the command substitution clustered with the other\n # ones.\n-unset VISUAL EMAIL LANGUAGE COLUMNS $(perl -e '\n+unset VISUAL EMAIL LANGUAGE COLUMNS $(\"$PERL_PATH\" -e '\n \tmy @env = keys %ENV;\n \tmy $ok = join(\"|\", qw(\n \t\tTRACE\n@@ -229,7 +249,7 @@ trap 'die' EXIT\n \n # The user-facing functions are loaded from a separate file so that\n # test_perf subshells can have them too\n-. \"${TEST_DIRECTORY:-.}\"/test-lib-functions.sh\n+. \"$TEST_DIRECTORY/test-lib-functions.sh\"\n \n # You are not expected to call test_ok_ and test_failure_ directly, use\n # the text_expect_* functions instead.\n@@ -380,23 +400,6 @@ test_done () {\n \tesac\n }\n \n-# Test the binaries we have just built.  The tests are kept in\n-# t/ subdirectory and are run in 'trash directory' subdirectory.\n-if test -z \"$TEST_DIRECTORY\"\n-then\n-\t# We allow tests to override this, in case they want to run tests\n-\t# outside of t/, e.g. for running tests on the test library\n-\t# itself.\n-\tTEST_DIRECTORY=$(pwd)\n-fi\n-if test -z \"$TEST_OUTPUT_DIRECTORY\"\n-then\n-\t# Similarly, override this to store the test-results subdir\n-\t# elsewhere\n-\tTEST_OUTPUT_DIRECTORY=$TEST_DIRECTORY\n-fi\n-GIT_BUILD_DIR=\"$TEST_DIRECTORY\"/..\n-\n if test -n \"$valgrind\"\n then\n \tmake_symlink () {\n@@ -492,8 +495,6 @@ GIT_CONFIG_NOSYSTEM=1\n GIT_ATTR_NOSYSTEM=1\n export PATH GIT_EXEC_PATH GIT_TEMPLATE_DIR GIT_CONFIG_NOSYSTEM GIT_ATTR_NOSYSTEM\n \n-. \"$GIT_BUILD_DIR\"/GIT-BUILD-OPTIONS\n-\n if test -z \"$GIT_TEST_CMP\"\n then\n \tif test -n \"$GIT_TEST_CMP_USE_COPIED_CONTEXT\"\n"},{"id":"194132","messageId":"4FE5C083.8010903@web.de","threadId":"30878","inReplyTo":"7vipeilfe9.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC] test-lib.sh: preprocess to use PERL_PATH","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2012-06-23T13:11:31Z","receivedAt":"2012-06-23T13:11:31Z","isPatch":false,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 23.06.12 08:18, Junio C Hamano wrote:\n> Torsten Bögershausen <tboegi@web.de> writes:\n>\n>> On 23.06.12 07:22, Junio C Hamano wrote:\n>>> Torsten Bögershausen <tboegi@web.de> writes:\n>>>\n>>>> All test cases found in t/*.sh must include test-lib instead of test-lib.sh\n>>> Please don't.  That is too much churning for too little gain, I am afraid.\n>> Ok, would it be better to rename\n>>\n>> t/test-lib.sh -> t/test-lib.sh.sh\n>>\n>> and let the Makefile generate t/test-lib.sh?\n> It isn't as bad as the patch posted, but not very much.\n>\n> There are number of a lot lower impact options before you\n> contemplate such a large change, given that there is only one\n> invocation of bare \"perl\" before GIT-BUILD-OPTIONS is dot-sourced.\n>\n>  (1) Perhaps that use does not have any portability issues, and we\n>      can leave it as-is, with a comment to forbid people from\n>      turning into \"$PERL_PATH\" and be done with it?\n>\n>  (2) Perhaps that use can be rewritten in such a way that it does\n>      not have to be done with perl in the first place?\n>\n>  (3) Perhaps what that use of perl does can be delayed until we\n>      dot-source GIT-BUILD-OPTIONS and have $PERL_PATH defined, in\n>      which case we can move that use to a later position (and we can\n>      turn that sole use of perl into \"$PERL_PATH\")?\n>\n>  (3) Perhaps what test-lib.sh does before it dot-sources\n>      GIT-BUILD-OPTIONS does not be affected if we dot-sourced\n>      GIT-BUILD-OPTIONS a lot earlier (and we can turn that sole use\n>      of perl into \"$PERL_PATH\")?\n>\n>\n> For example (this is not tested at all, nor I did not think it\n> through), a patch that moves the definition of TEST_DIRECTORY which\n> GIT_BUILD_DIR depends on higher, so that we can dot-source the file\n> a lot earlier, may look like this.\n>\n>\n>  t/test-lib.sh | 43 ++++++++++++++++++++++---------------------\n>  1 file changed, 22 insertions(+), 21 deletions(-)\n>\n> diff --git a/t/test-lib.sh b/t/test-lib.sh\n> index 9e2b711..f3e7cf9 100644\n> --- a/t/test-lib.sh\n> +++ b/t/test-lib.sh\n> @@ -34,6 +34,26 @@ esac\n>  # Keep the original TERM for say_color\n>  ORIGINAL_TERM=$TERM\n>  \n> +# Test the binaries we have just built.  The tests are kept in\n> +# t/ subdirectory and are run in 'trash directory' subdirectory.\n> +if test -z \"$TEST_DIRECTORY\"\n> +then\n> +\t# We allow tests to override this, in case they want to run tests\n> +\t# outside of t/, e.g. for running tests on the test library\n> +\t# itself.\n> +\tTEST_DIRECTORY=$(pwd)\n> +fi\n> +if test -z \"$TEST_OUTPUT_DIRECTORY\"\n> +then\n> +\t# Similarly, override this to store the test-results subdir\n> +\t# elsewhere\n> +\tTEST_OUTPUT_DIRECTORY=$TEST_DIRECTORY\n> +fi\n> +GIT_BUILD_DIR=\"$TEST_DIRECTORY\"/..\n> +\n> +. \"$GIT_BUILD_DIR\"/GIT-BUILD-OPTIONS\n> +export PERL_PATH SHELL_PATH\n> +\n>  # For repeatability, reset the environment to known value.\n>  LANG=C\n>  LC_ALL=C\n> @@ -46,7 +66,7 @@ EDITOR=:\n>  # /usr/xpg4/bin/sh and /bin/ksh to bail out.  So keep the unsets\n>  # deriving from the command substitution clustered with the other\n>  # ones.\n> -unset VISUAL EMAIL LANGUAGE COLUMNS $(perl -e '\n> +unset VISUAL EMAIL LANGUAGE COLUMNS $(\"$PERL_PATH\" -e '\n>  \tmy @env = keys %ENV;\n>  \tmy $ok = join(\"|\", qw(\n>  \t\tTRACE\n> @@ -229,7 +249,7 @@ trap 'die' EXIT\n>  \n>  # The user-facing functions are loaded from a separate file so that\n>  # test_perf subshells can have them too\n> -. \"${TEST_DIRECTORY:-.}\"/test-lib-functions.sh\n> +. \"$TEST_DIRECTORY/test-lib-functions.sh\"\n>  \n>  # You are not expected to call test_ok_ and test_failure_ directly, use\n>  # the text_expect_* functions instead.\n> @@ -380,23 +400,6 @@ test_done () {\n>  \tesac\n>  }\n>  \n> -# Test the binaries we have just built.  The tests are kept in\n> -# t/ subdirectory and are run in 'trash directory' subdirectory.\n> -if test -z \"$TEST_DIRECTORY\"\n> -then\n> -\t# We allow tests to override this, in case they want to run tests\n> -\t# outside of t/, e.g. for running tests on the test library\n> -\t# itself.\n> -\tTEST_DIRECTORY=$(pwd)\n> -fi\n> -if test -z \"$TEST_OUTPUT_DIRECTORY\"\n> -then\n> -\t# Similarly, override this to store the test-results subdir\n> -\t# elsewhere\n> -\tTEST_OUTPUT_DIRECTORY=$TEST_DIRECTORY\n> -fi\n> -GIT_BUILD_DIR=\"$TEST_DIRECTORY\"/..\n> -\n>  if test -n \"$valgrind\"\n>  then\n>  \tmake_symlink () {\n> @@ -492,8 +495,6 @@ GIT_CONFIG_NOSYSTEM=1\n>  GIT_ATTR_NOSYSTEM=1\n>  export PATH GIT_EXEC_PATH GIT_TEMPLATE_DIR GIT_CONFIG_NOSYSTEM GIT_ATTR_NOSYSTEM\n>  \n> -. \"$GIT_BUILD_DIR\"/GIT-BUILD-OPTIONS\n> -\n>  if test -z \"$GIT_TEST_CMP\"\n>  then\n>  \tif test -n \"$GIT_TEST_CMP_USE_COPIED_CONTEXT\"\nExcellent!\nThanks: enjoyed & tested OK both on\n\n457f08c4777b552ad35 (where t4030 was broken when testing here)\n\nand\n\n>commit 9746b046e5651aa7277a0b853819e2d076d403c6\n>Date:   Fri Jun 22 22:20:27 2012 -0700\n"}]}