{"thread":{"id":"21778","subject":"[PATCH 0/4] Run test suite without dashed commands in PATH","startedAt":"2009-11-28T18:38:53Z","lastAt":"2009-11-30T07:07:52Z","messageCount":14,"participants":["Matthew Ogilvie","Junio C Hamano","Jeff King","Nanako Shiraishi"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"128633","messageId":"1259433537-3963-1-git-send-email-mmogilvi_git@miniinfo.net","threadId":"21778","inReplyTo":null,"subject":"[PATCH 0/4] Run test suite without dashed commands in PATH","fromName":"Matthew Ogilvie","fromEmail":"mmogilvi_git@miniinfo.net","sentAt":"2009-11-28T18:38:53Z","receivedAt":"2009-11-28T18:38:53Z","isPatch":true,"sender":{"key":"mmogilvi_git@miniinfo.net","avatar":null},"body":"This patch series runs the test suite without the dashed commands\nin the PATH.  This improves the usefulness of the test suite\nas examples of how to do things, and more importantly,\nminimizes any possibility of regressions in bindir-installed\nscripts that might prevent them from working in a standard\ninstall where most dashed commands are not in the PATH (git-cvsserver\nwas broken in this way until Sep 2009: d2feb01aa5).\n\nThe scripts in the \"test-bin\" directory that patch 3 creates\ncan also be useful for manually testing a build of git without\ninstalling it; you typically don't need to set any environment\nvariables (except maybe PATH) for the test-bin scripts to use\nthe build properly.\n\nTrivial note: This is a cleaned up resend of part of a hodgepodge\ncvsserver patch series that I sent last January.\n\n---------\n\nBy the way, has anyone considered the possibility of splitting\nup the large directory at the top of the git source tree?  I\nsuspect that no one is interested, or it would have been done\nalready, but I thought I would mention it anyway.  Perhaps split\noff separate directories for libgit.a, builtins, other C-based\nexcutables, test support executables, scripts, output execdir, and\noutput intermediate (object) files for each of the other\ndirectories.  Or some subset of these.\n\n--\nMatthew Ogilvie   [mmogilvi_git@miniinfo.net]\n\nMatthew Ogilvie (4):\n  t2300: use documented technique to invoke git-sh-setup\n  t3409 t4107 t7406: use dashless commands\n  build dashless \"test-bin\" directory similar to installed bindir\n  run test suite without dashed git-commands in PATH\n\n .gitignore                         |    1 +\n INSTALL                            |    8 +++++-\n Makefile                           |   46 +++++++++++++++++++++++++----------\n t/README                           |    8 ++++++\n t/t2300-cd-to-toplevel.sh          |    2 +-\n t/t3409-rebase-preserve-merges.sh  |    6 ++--\n t/t4107-apply-ignore-whitespace.sh |   20 ++++++++--------\n t/t7406-submodule-update.sh        |    4 +-\n t/test-lib.sh                      |   33 ++++++++++++++++---------\n test-bin-wrapper.sh                |   13 ++++++++++\n 10 files changed, 99 insertions(+), 42 deletions(-)\n create mode 100644 test-bin-wrapper.sh\n"},{"id":"128636","messageId":"1259433537-3963-2-git-send-email-mmogilvi_git@miniinfo.net","threadId":"21778","inReplyTo":"1259433537-3963-1-git-send-email-mmogilvi_git@miniinfo.net","subject":"[PATCH 1/4] t2300: use documented technique to invoke git-sh-setup","fromName":"Matthew Ogilvie","fromEmail":"mmogilvi_git@miniinfo.net","sentAt":"2009-11-28T18:38:54Z","receivedAt":"2009-11-28T18:38:54Z","isPatch":true,"sender":{"key":"mmogilvi_git@miniinfo.net","avatar":null},"body":"This is needed to allow the test suite to run against a standard\ninstall bin directory instead of GIT_EXEC_PATH.\n\nSigned-off-by: Matthew Ogilvie <mmogilvi_git@miniinfo.net>\n---\n t/t2300-cd-to-toplevel.sh |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/t/t2300-cd-to-toplevel.sh b/t/t2300-cd-to-toplevel.sh\nindex 3b01ad2..9965bc5 100755\n--- a/t/t2300-cd-to-toplevel.sh\n+++ b/t/t2300-cd-to-toplevel.sh\n@@ -8,7 +8,7 @@ test_cd_to_toplevel () {\n \ttest_expect_success $3 \"$2\" '\n \t\t(\n \t\t\tcd '\"'$1'\"' &&\n-\t\t\t. git-sh-setup &&\n+\t\t\t. \"$(git --exec-path)\"/git-sh-setup &&\n \t\t\tcd_to_toplevel &&\n \t\t\t[ \"$(pwd -P)\" = \"$TOPLEVEL\" ]\n \t\t)\n-- \n1.6.4.GIT\n"},{"id":"128634","messageId":"1259433537-3963-3-git-send-email-mmogilvi_git@miniinfo.net","threadId":"21778","inReplyTo":"1259433537-3963-2-git-send-email-mmogilvi_git@miniinfo.net","subject":"[PATCH 2/4] t3409 t4107 t7406: use dashless commands","fromName":"Matthew Ogilvie","fromEmail":"mmogilvi_git@miniinfo.net","sentAt":"2009-11-28T18:38:55Z","receivedAt":"2009-11-28T18:38:55Z","isPatch":true,"sender":{"key":"mmogilvi_git@miniinfo.net","avatar":null},"body":"This is needed to allow test suite to run against a standard\ninstall bin directory instead of GIT_EXEC_PATH.\n\nSigned-off-by: Matthew Ogilvie <mmogilvi_git@miniinfo.net>\n---\n t/t3409-rebase-preserve-merges.sh  |    6 +++---\n t/t4107-apply-ignore-whitespace.sh |   20 ++++++++++----------\n t/t7406-submodule-update.sh        |    4 ++--\n 3 files changed, 15 insertions(+), 15 deletions(-)\n\ndiff --git a/t/t3409-rebase-preserve-merges.sh b/t/t3409-rebase-preserve-merges.sh\nindex 297d165..8f785e7 100755\n--- a/t/t3409-rebase-preserve-merges.sh\n+++ b/t/t3409-rebase-preserve-merges.sh\n@@ -32,14 +32,14 @@ export GIT_AUTHOR_EMAIL\n test_expect_success 'setup for merge-preserving rebase' \\\n \t'echo First > A &&\n \tgit add A &&\n-\tgit-commit -m \"Add A1\" &&\n+\tgit commit -m \"Add A1\" &&\n \tgit checkout -b topic &&\n \techo Second > B &&\n \tgit add B &&\n-\tgit-commit -m \"Add B1\" &&\n+\tgit commit -m \"Add B1\" &&\n \tgit checkout -f master &&\n \techo Third >> A &&\n-\tgit-commit -a -m \"Modify A2\" &&\n+\tgit commit -a -m \"Modify A2\" &&\n \n \tgit clone ./. clone1 &&\n \tcd clone1 &&\ndiff --git a/t/t4107-apply-ignore-whitespace.sh b/t/t4107-apply-ignore-whitespace.sh\nindex 484654d..b04fc8f 100755\n--- a/t/t4107-apply-ignore-whitespace.sh\n+++ b/t/t4107-apply-ignore-whitespace.sh\n@@ -136,37 +136,37 @@ void print_int(int num) {\n EOF\n \n test_expect_success 'file creation' '\n-\tgit-apply patch1.patch\n+\tgit apply patch1.patch\n '\n \n test_expect_success 'patch2 fails (retab)' '\n-\ttest_must_fail git-apply patch2.patch\n+\ttest_must_fail git apply patch2.patch\n '\n \n test_expect_success 'patch2 applies with --ignore-whitespace' '\n-\tgit-apply --ignore-whitespace patch2.patch\n+\tgit apply --ignore-whitespace patch2.patch\n '\n \n test_expect_success 'patch2 reverse applies with --ignore-space-change' '\n-\tgit-apply -R --ignore-space-change patch2.patch\n+\tgit apply -R --ignore-space-change patch2.patch\n '\n \n git config apply.ignorewhitespace change\n \n test_expect_success 'patch2 applies (apply.ignorewhitespace = change)' '\n-\tgit-apply patch2.patch\n+\tgit apply patch2.patch\n '\n \n test_expect_success 'patch3 fails (missing string at EOL)' '\n-\ttest_must_fail git-apply patch3.patch\n+\ttest_must_fail git apply patch3.patch\n '\n \n test_expect_success 'patch4 fails (missing EOL at EOF)' '\n-\ttest_must_fail git-apply patch4.patch\n+\ttest_must_fail git apply patch4.patch\n '\n \n test_expect_success 'patch5 applies (leading whitespace)' '\n-\tgit-apply patch5.patch\n+\tgit apply patch5.patch\n '\n \n test_expect_success 'patches do not mangle whitespace' '\n@@ -175,11 +175,11 @@ test_expect_success 'patches do not mangle whitespace' '\n \n test_expect_success 're-create file (with --ignore-whitespace)' '\n \trm -f main.c &&\n-\tgit-apply patch1.patch\n+\tgit apply patch1.patch\n '\n \n test_expect_success 'patch5 fails (--no-ignore-whitespace)' '\n-\ttest_must_fail git-apply --no-ignore-whitespace patch5.patch\n+\ttest_must_fail git apply --no-ignore-whitespace patch5.patch\n '\n \n test_done\ndiff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh\nindex 2d33d9e..8e2449d 100755\n--- a/t/t7406-submodule-update.sh\n+++ b/t/t7406-submodule-update.sh\n@@ -14,8 +14,8 @@ submodule and \"git submodule update --rebase/--merge\" does not detach the HEAD.\n \n compare_head()\n {\n-    sha_master=`git-rev-list --max-count=1 master`\n-    sha_head=`git-rev-list --max-count=1 HEAD`\n+    sha_master=`git rev-list --max-count=1 master`\n+    sha_head=`git rev-list --max-count=1 HEAD`\n \n     test \"$sha_master\" = \"$sha_head\"\n }\n-- \n1.6.4.GIT\n"},{"id":"128637","messageId":"1259433537-3963-4-git-send-email-mmogilvi_git@miniinfo.net","threadId":"21778","inReplyTo":"1259433537-3963-3-git-send-email-mmogilvi_git@miniinfo.net","subject":"[PATCH 3/4] build dashless \"test-bin\" directory similar to installed bindir","fromName":"Matthew Ogilvie","fromEmail":"mmogilvi_git@miniinfo.net","sentAt":"2009-11-28T18:38:56Z","receivedAt":"2009-11-28T18:38:56Z","isPatch":true,"sender":{"key":"mmogilvi_git@miniinfo.net","avatar":null},"body":"The new test-bin directory contains wrapper scripts\nfor executables that will be installed into the standard\nbindir.  It explicitly does not contain most dashed-commands.\nThe scripts automatically set environment variables to run out\nof the source tree, not the installed directory.\n\nThis will allow running the test suite without dashed commands in\nthe PATH, and provides a simplified way to run custom built git\nexecutables without installing them first.\n\ntest-bin also contains wrappers for some test suite support executables,\nwhere the test suite will soon make use of them.\n\nSigned-off-by: Matthew Ogilvie <mmogilvi_git@miniinfo.net>\n---\n .gitignore          |    1 +\n INSTALL             |    8 +++++++-\n Makefile            |   46 +++++++++++++++++++++++++++++++++-------------\n test-bin-wrapper.sh |   13 +++++++++++++\n 4 files changed, 54 insertions(+), 14 deletions(-)\n create mode 100644 test-bin-wrapper.sh\n\ndiff --git a/.gitignore b/.gitignore\nindex ac02a58..c981e4c 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -10,6 +10,7 @@\n /git-apply\n /git-archimport\n /git-archive\n+/test-bin/\n /git-bisect\n /git-bisect--helper\n /git-blame\ndiff --git a/INSTALL b/INSTALL\nindex be504c9..d092d33 100644\n--- a/INSTALL\n+++ b/INSTALL\n@@ -39,7 +39,13 @@ Issues of note:\n    with --disable-transition option to avoid this.\n \n  - You can use git after building but without installing if you\n-   wanted to.  Various git commands need to find other git\n+   want to.\n+\n+   The simplest option is to use the wrapper scripts that are built\n+   and saved into `pwd`/test-bin, by either putting test-bin in your\n+   PATH, or invoking the scripts in test-bin using their full paths.\n+\n+   Alternatively, various git commands need to find other git\n    commands and scripts to do their work, so you would need to\n    arrange a few environment variables to tell them that their\n    friends will be found in your built source area instead of at\ndiff --git a/Makefile b/Makefile\nindex 5a0b3d4..e153ff0 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -416,6 +416,15 @@ ALL_PROGRAMS = $(PROGRAMS) $(SCRIPTS)\n # what 'all' will build but not install in gitexecdir\n OTHER_PROGRAMS = git$X\n \n+# what test wrappers are needed and 'install' will install, in bindir\n+BINDIR_PROGRAMS_NEED_X += git\n+BINDIR_PROGRAMS_NEED_X += git-upload-pack\n+BINDIR_PROGRAMS_NEED_X += git-receive-pack\n+BINDIR_PROGRAMS_NEED_X += git-upload-archive\n+BINDIR_PROGRAMS_NEED_X += git-shell\n+\n+BINDIR_PROGRAMS_NO_X += git-cvsserver\n+\n # Set paths to tools early so that they can be used for version tests.\n ifndef SHELL_PATH\n \tSHELL_PATH = /bin/sh\n@@ -1690,19 +1699,27 @@ endif\n \n ### Testing rules\n \n-TEST_PROGRAMS += test-chmtime$X\n-TEST_PROGRAMS += test-ctype$X\n-TEST_PROGRAMS += test-date$X\n-TEST_PROGRAMS += test-delta$X\n-TEST_PROGRAMS += test-dump-cache-tree$X\n-TEST_PROGRAMS += test-genrandom$X\n-TEST_PROGRAMS += test-match-trees$X\n-TEST_PROGRAMS += test-parse-options$X\n-TEST_PROGRAMS += test-path-utils$X\n-TEST_PROGRAMS += test-sha1$X\n-TEST_PROGRAMS += test-sigchain$X\n+TEST_PROGRAMS_NEED_X += test-chmtime\n+TEST_PROGRAMS_NEED_X += test-ctype\n+TEST_PROGRAMS_NEED_X += test-date\n+TEST_PROGRAMS_NEED_X += test-delta\n+TEST_PROGRAMS_NEED_X += test-dump-cache-tree\n+TEST_PROGRAMS_NEED_X += test-genrandom\n+TEST_PROGRAMS_NEED_X += test-match-trees\n+TEST_PROGRAMS_NEED_X += test-parse-options\n+TEST_PROGRAMS_NEED_X += test-path-utils\n+TEST_PROGRAMS_NEED_X += test-sha1\n+TEST_PROGRAMS_NEED_X += test-sigchain\n+\n+TEST_PROGRAMS = $(patsubst %,%$X,$(TEST_PROGRAMS_NEED_X))\n \n-all:: $(TEST_PROGRAMS)\n+test_bindir_programs := $(patsubst %,test-bin/%,$(BINDIR_PROGRAMS_NEED_X) $(BINDIR_PROGRAMS_NO_X) $(TEST_PROGRAMS_NEED_X))\n+\n+all:: $(TEST_PROGRAMS) $(test_bindir_programs)\n+\n+test-bin/%: test-bin-wrapper.sh GIT-CFLAGS\n+\t@mkdir -p test-bin\n+\t$(QUIET_GEN)sed -e 's|__GIT_EXEC_PATH__|$(shell pwd)|' -e 's|__PROG__|$(@F)|' < $< > $@ && chmod +x $@\n \n # GNU make supports exporting all variables by \"export\" without parameters.\n # However, the environment gets quite big, and some programs have problems\n@@ -1763,11 +1780,13 @@ endif\n gitexec_instdir_SQ = $(subst ','\\'',$(gitexec_instdir))\n export gitexec_instdir\n \n+install_bindir_programs := $(patsubst %,%$X,$(BINDIR_PROGRAMS_NEED_X)) $(BINDIR_PROGRAMS_NO_X)\n+\n install: all\n \t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(bindir_SQ)'\n \t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'\n \t$(INSTALL) $(ALL_PROGRAMS) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'\n-\t$(INSTALL) git$X git-upload-pack$X git-receive-pack$X git-upload-archive$X git-shell$X git-cvsserver '$(DESTDIR_SQ)$(bindir_SQ)'\n+\t$(INSTALL) $(install_bindir_programs) '$(DESTDIR_SQ)$(bindir_SQ)'\n \t$(MAKE) -C templates DESTDIR='$(DESTDIR_SQ)' install\n ifndef NO_PERL\n \t$(MAKE) -C perl prefix='$(prefix_SQ)' DESTDIR='$(DESTDIR_SQ)' install\n@@ -1878,6 +1897,7 @@ clean:\n \t\t$(LIB_FILE) $(XDIFF_LIB)\n \t$(RM) $(ALL_PROGRAMS) $(BUILT_INS) git$X\n \t$(RM) $(TEST_PROGRAMS)\n+\t$(RM) -r test-bin\n \t$(RM) *.spec *.pyc *.pyo */*.pyc */*.pyo common-cmds.h TAGS tags cscope*\n \t$(RM) -r autom4te.cache\n \t$(RM) config.log config.mak.autogen config.mak.append config.status config.cache\ndiff --git a/test-bin-wrapper.sh b/test-bin-wrapper.sh\nnew file mode 100644\nindex 0000000..a01eee1\n--- /dev/null\n+++ b/test-bin-wrapper.sh\n@@ -0,0 +1,13 @@\n+#!/bin/sh\n+\n+# test-bin-wrapper.sh: Template for git executable wrapper scripts\n+# to run test suite against sandbox, but with only bindir-installed\n+# executables in PATH.  The Makefile copies this into various\n+# files in test-bin, substituting __GIT_EXEC_PATH__ and __PROG__.\n+\n+GIT_EXEC_PATH=\"__GIT_EXEC_PATH__\"\n+GIT_TEMPLATE_DIR=\"__GIT_EXEC_PATH__/templates/blt\"\n+GITPERLLIB=\"__GIT_EXEC_PATH__/perl/blib/lib\"\n+export GIT_EXEC_PATH GIT_TEMPLATE_DIR GITPERLLIB\n+\n+exec \"${GIT_EXEC_PATH}/__PROG__\" \"$@\"\n-- \n1.6.4.GIT\n"},{"id":"128635","messageId":"1259433537-3963-5-git-send-email-mmogilvi_git@miniinfo.net","threadId":"21778","inReplyTo":"1259433537-3963-4-git-send-email-mmogilvi_git@miniinfo.net","subject":"[PATCH 4/4] run test suite without dashed git-commands in PATH","fromName":"Matthew Ogilvie","fromEmail":"mmogilvi_git@miniinfo.net","sentAt":"2009-11-28T18:38:57Z","receivedAt":"2009-11-28T18:38:57Z","isPatch":true,"sender":{"key":"mmogilvi_git@miniinfo.net","avatar":null},"body":"Only put test-bin in the PATH (do not put GIT_EXEC_PATH in the PATH),\nto emulate the default installed user environment, and\nensure all the programs run correctly in such an\nenvironment.  This is now the default, although\nit can be overridden with a --with-dashes test option when running\ntests.\n\nSigned-off-by: Matthew Ogilvie <mmogilvi_git@miniinfo.net>\n---\n t/README      |    8 ++++++++\n t/test-lib.sh |   33 +++++++++++++++++++++------------\n 2 files changed, 29 insertions(+), 12 deletions(-)\n\ndiff --git a/t/README b/t/README\nindex d8f6c7d..b61c34b 100644\n--- a/t/README\n+++ b/t/README\n@@ -75,6 +75,14 @@ appropriately before running \"make\".\n \tAs the names depend on the tests' file names, it is safe to\n \trun the tests with this option in parallel.\n \n+--with-dashes::\n+\tBy default tests are run without dashed forms of\n+\tcommands (like git-commit) in the PATH (it only uses\n+\twrappers from TOP/git-bin).  Use this option to include TOP\n+\tin the PATH, which conains all the dashed forms of commands.\n+\tThis option is currently implied by other options like --valgrind\n+\tand GIT_TEST_INSTALLED.\n+\n Skipping Tests\n --------------\n \ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex ec3336a..54570ac 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -105,6 +105,8 @@ do\n \t\tverbose=t; shift ;;\n \t-q|--q|--qu|--qui|--quie|--quiet)\n \t\tquiet=t; shift ;;\n+\t--with-dashes)\n+\t\twith_dashes=t; shift ;;\n \t--no-color)\n \t\tcolor=; shift ;;\n \t--no-python)\n@@ -551,19 +553,8 @@ test_done () {\n # Test the binaries we have just built.  The tests are kept in\n # t/ subdirectory and are run in 'trash directory' subdirectory.\n TEST_DIRECTORY=$(pwd)\n-if test -z \"$valgrind\"\n+if test -n \"$valgrind\"\n then\n-\tif test -z \"$GIT_TEST_INSTALLED\"\n-\tthen\n-\t\tPATH=$TEST_DIRECTORY/..:$PATH\n-\t\tGIT_EXEC_PATH=$TEST_DIRECTORY/..\n-\telse\n-\t\tGIT_EXEC_PATH=$($GIT_TEST_INSTALLED/git --exec-path)  ||\n-\t\terror \"Cannot run git from $GIT_TEST_INSTALLED.\"\n-\t\tPATH=$GIT_TEST_INSTALLED:$TEST_DIRECTORY/..:$PATH\n-\t\tGIT_EXEC_PATH=${GIT_TEST_EXEC_PATH:-$GIT_EXEC_PATH}\n-\tfi\n-else\n \tmake_symlink () {\n \t\ttest -h \"$2\" &&\n \t\ttest \"$1\" = \"$(readlink \"$2\")\" || {\n@@ -625,6 +616,24 @@ else\n \tPATH=$GIT_VALGRIND/bin:$PATH\n \tGIT_EXEC_PATH=$GIT_VALGRIND/bin\n \texport GIT_VALGRIND\n+elif test -n \"$GIT_TEST_INSTALLED\" ; then\n+\tGIT_EXEC_PATH=$($GIT_TEST_INSTALLED/git --exec-path)  ||\n+\terror \"Cannot run git from $GIT_TEST_INSTALLED.\"\n+\tPATH=$GIT_TEST_INSTALLED:$TEST_DIRECTORY/..:$PATH\n+\tGIT_EXEC_PATH=${GIT_TEST_EXEC_PATH:-$GIT_EXEC_PATH}\n+else # normal case, use ../test-bin only unless $with_dashes:\n+\tgit_bin_dir=\"$TEST_DIRECTORY/../test-bin\"\n+\tif ! test -x \"$git_bin_dir/git\" ; then\n+\t\tif test -z \"$with_dashes\" ; then\n+\t\t\tsay \"$git_bin_dir/git is not executable; using GIT_EXEC_PATH\"\n+\t\tfi\n+\t\twith_dashes=t\n+\tfi\n+\tPATH=\"$git_bin_dir:$PATH\"\n+\tGIT_EXEC_PATH=$TEST_DIRECTORY/..\n+\tif test -n \"$with_dashes\" ; then\n+\t\tPATH=\"$TEST_DIRECTORY/..:$PATH\"\n+\tfi\n fi\n GIT_TEMPLATE_DIR=$(pwd)/../templates/blt\n unset GIT_CONFIG\n-- \n1.6.4.GIT\n"},{"id":"128638","messageId":"7vtywefn88.fsf@alter.siamese.dyndns.org","threadId":"21778","inReplyTo":"1259433537-3963-4-git-send-email-mmogilvi_git@miniinfo.net","subject":"Re: [PATCH 3/4] build dashless \"test-bin\" directory similar to installed bindir","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-11-28T19:44:39Z","receivedAt":"2009-11-28T19:44:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthew Ogilvie <mmogilvi_git@miniinfo.net> writes:\n\n> The new test-bin directory contains wrapper scripts for executables that\n> will be installed into the standard bindir.  It explicitly does not\n> contain most dashed-commands.  The scripts automatically set environment\n> variables to run out of the source tree, not the installed directory.\n\nEven though I haven't tried the series, I like most of the things I see in\nthis series.\n\n - Patch #1 and #2 are good and are independent from the later patches, as\n   without them running tests with GIT_TEST_INSTALLED would not work.\n\n   By the way, 6720721 (test-lib.sh: Allow running the test suite against\n   installed git, 2009-03-16) failed to document the feature in t/README.\n   Could you please fix this while you are at it?\n\n - Running tests before installation in an environment slightly closer to\n   the final installation (i.e. lacks dashed commands in the $PATH during\n   the time tests are run) is a good direction to go.\n\n - I like the Makefile changes that uses these new BINDIR_PROGRAMS_NEED_X,\n   TEST_PROGRAMS_NEED_X.  Parameterizing commands listed in the actions\n   part is good.\n\n - It certainly is _possible_ to add $(pwd)/test-bin to $PATH instead of\n   the established practice of using GIT_EXEC_PATH for every day/permanent\n   use without installation, but I doubt we should _encourage_ it for a\n   few reasons:\n\n   . The set-up will force one extra exec due to the wrapper; this is good\n     for the purpose of running tests, but unnecessary for a set-up for\n     every day/permanent use by people, compared with the already working\n     approach.  The user needs to change an environment variable _anyway_\n     (either GIT_EXEC_PATH with the traditional approach, or PATH with\n     your patch).\n\n   . The new component to be added to $PATH shouldn't be named \"test-bin/\"\n     if it is meant for every day/permanent use.\n\n   . Advertising this forces the Makefile build test-bin/ contents from\n     \"all\" target.  I think test-bin/ should only depend on \"test\" (iow,\n     after \"make all && make install\" there shouldn't have to be \"test-bin\"\n     directory.\n\n   I would rather treat it an unintended side-effect that you can add that\n   directory to the $PATH.  It is designed to work in such an environment\n   (otherwise the tests won't exercise the version of git they are meant\n   to test), but I do not think it is _meant_ to be _used_  by end users\n   that way.  And an unintended side-effect does not have to be mentioned\n   in INSTALL (especially with the directory name with \"test\" in it).\n"},{"id":"128640","messageId":"20091128194910.GA17605@coredump.intra.peff.net","threadId":"21778","inReplyTo":"7vtywefn88.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/4] build dashless \"test-bin\" directory similar to installed bindir","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-11-28T19:49:11Z","receivedAt":"2009-11-28T19:49:11Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Nov 28, 2009 at 11:44:39AM -0800, Junio C Hamano wrote:\n\n>    . Advertising this forces the Makefile build test-bin/ contents from\n>      \"all\" target.  I think test-bin/ should only depend on \"test\" (iow,\n>      after \"make all && make install\" there shouldn't have to be \"test-bin\"\n>      directory.\n\nWould implementing it that way mean that:\n\n  make && cd t && make\n\ndoes not work (or worse, might silently use stale information in\ntest-bin)? Dealing with this is part of the reason the valgrind code\n(which similarly sets up a pseudo-installed directory) does everything\nin test-lib.sh, and not as part of the make process.\n\n-Peff\n"},{"id":"128652","messageId":"7vaay6f4ce.fsf@alter.siamese.dyndns.org","threadId":"21778","inReplyTo":"20091128194910.GA17605@coredump.intra.peff.net","subject":"Re: [PATCH 3/4] build dashless \"test-bin\" directory similar to installed bindir","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-11-29T02:32:33Z","receivedAt":"2009-11-29T02:32:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Sat, Nov 28, 2009 at 11:44:39AM -0800, Junio C Hamano wrote:\n>\n>>    . Advertising this forces the Makefile build test-bin/ contents from\n>>      \"all\" target.  I think test-bin/ should only depend on \"test\" (iow,\n>>      after \"make all && make install\" there shouldn't have to be \"test-bin\"\n>>      directory.\n>\n> Would implementing it that way mean that:\n>\n>   make && cd t && make\n>\n> does not work (or worse, might silently use stale information in\n> test-bin)?\n\nWhy can't t/Makefile have a dependency on its 'default' target that goes\nup and prepares test-bin/, i.e. \"cd .. && make test-bin-stuff\"?\n"},{"id":"128654","messageId":"20091129025251.GA1771@comcast.net","threadId":"21778","inReplyTo":"7vtywefn88.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/4] build dashless \"test-bin\" directory similar to installed bindir","fromName":"Matthew Ogilvie","fromEmail":"mmogilvi_git@miniinfo.net","sentAt":"2009-11-29T02:52:51Z","receivedAt":"2009-11-29T02:52:51Z","isPatch":true,"sender":{"key":"mmogilvi_git@miniinfo.net","avatar":null},"body":"On Sat, Nov 28, 2009 at 11:44:39AM -0800, Junio C Hamano wrote:\n>  - Patch #1 and #2 are good and are independent from the later patches, as\n>    without them running tests with GIT_TEST_INSTALLED would not work.\n> \n>    By the way, 6720721 (test-lib.sh: Allow running the test suite against\n>    installed git, 2009-03-16) failed to document the feature in t/README.\n>    Could you please fix this while you are at it?\n\nSure, I'll include another patch for this when I re-roll the\nseries.  It will probably mention something about still needing\na build so that it can access the test-* support executables.\n\n>  - It certainly is _possible_ to add $(pwd)/test-bin to $PATH instead of\n>    the established practice of using GIT_EXEC_PATH for every day/permanent\n>    use without installation, but I doubt we should _encourage_ it for a\n>    few reasons:\n> \n>    . The set-up will force one extra exec due to the wrapper; this is good\n>      for the purpose of running tests, but unnecessary for a set-up for\n>      every day/permanent use by people, compared with the already working\n>      approach.  The user needs to change an environment variable _anyway_\n>      (either GIT_EXEC_PATH with the traditional approach, or PATH with\n>      your patch).\n> \n>    . The new component to be added to $PATH shouldn't be named \"test-bin/\"\n>      if it is meant for every day/permanent use.\n> \n>    . Advertising this forces the Makefile build test-bin/ contents from\n>      \"all\" target.  I think test-bin/ should only depend on \"test\" (iow,\n>      after \"make all && make install\" there shouldn't have to be \"test-bin\"\n>      directory.\n> \n>    I would rather treat it an unintended side-effect that you can add that\n>    directory to the $PATH.  It is designed to work in such an environment\n>    (otherwise the tests won't exercise the version of git they are meant\n>    to test), but I do not think it is _meant_ to be _used_  by end users\n>    that way.  And an unintended side-effect does not have to be mentioned\n>    in INSTALL (especially with the directory name with \"test\" in it).\n\nI personally like the idea of being able to use an uninstalled\nbuild without touching environment variables at all.  Just specify\nthe full path to the the version you want to run on the\ncommand line, as in: ~/SANDBOX/test-bin/git WHATEVER\nEspecially handy for trying \"ssh MACHINE /PATH/SANDBOX/...\".\n\nFYI: There are already a number of test suite support executables\nbuilt by \"make all\" (test-*).  It might be hard to eliminate them\nfrom \"all\" without risking stale executables.  As Jeff\nKing <peff@peff.net> pointed out in a separate email, some people\n(including me) often don't use the top-level \"make test\" target\nto run tests.\n\nI'm still thinking about this.  I've noted some possible changes\nto the patch series below, some of which are mutually exclusive.\nAny opinions?\n\nOptions geared towards isolating/hiding test-bin:\n\n  1. Scrap the part of the patch that modifies INSTALL.\n\n  2. Perhaps use hardlinks, symlinks, and/or copies within test-bin,\n     instead of wrapper scripts, to eliminate the extra exec.  Since\n     test-lib.sh already sets up necessary environment variables,\n     they don't strictly need to be set in the wrappers as\n     well.  On the other hand, hardlinks and copies are potentially\n     vulnerable to stale executable issues, and symlinks typically\n     don't work on Windows.  \n\n  3. Scrap pre-built test-bin completely, and switch to a solution\n     that more closely resembles the valgrind option (have test-lib.sh\n     build the directory).  This can't use the same makefile variables\n     to define the contents of the directory, though.\n\nOptions geared towards making something like test-bin an official way\nto run an uninstalled build:\n\n  4. Rename test-bin.  Perhaps \"bin-wrappers\", \"bin-dashless\",\n     \"bin-install\", \"bin\", or \"bindir\".  Any preferences?\n\n  5. The current patch doesn't quite handle the simple\n     \"~/SANDBOX/test-bin/gitSOMETHING WHATEVER\" idiom perfectly\n     if the executable (gitSOMETHING) tries to run additional\n     git commands without adjusting the PATH first.  I could enhance\n     the wrapper to prefix test-bin onto the PATH just in case it\n     isn't there already.\n\nOther cleanup options:\n\n  6. There is a stale script issue if someone does something like:\n       make\n       cp -a . /some/other/path\n       cd /some/other/path\n       [optional modifications, without a \"make clean\"]\n       make\n       [run tests; uses wrong executables...]\n     Including GIT-CFLAGS as a makefile dependency for the\n     wrappers was intended to address this, but looking\n     closer, I don't think it works.  Perhaps I should\n     include $(shell pwd) in GIT-CFLAGS, or make a new GIT-PWD target\n     that works similarly to GIT-CFLAGS.  Without this, a workaround\n     (and probably best option overall) is to do a \"make clean\" after\n     copying a sandbox.\n\n  7. Enable similar dashless environment when\n     GIT_TEST_INSTALLED and/or valgrind are enabled?\n\n  8. Include wrappers for other dashed-commands in test-bin, which\n     would always fail, in case someone runs tests with an installed\n     GIT_EXEC_PATH already in their PATH.  This might catch a new test\n     using dashes in such an environment.  I don't really think this\n     is worth it, though.  Most people don't have GIT_EXEC_PATH in their\n     PATH, and some such person would notice any problems soon.\n\n  9. This may be outside the scope of this patch series, but perhaps\n     git executables could try to find argv[0] in the PATH\n     (if argv[0] is not absolute), and see if they can find various other\n     executables (GIT_EXEC_PATH) and data files (perl, templates,\n     etc) using paths relative to itself.  This may include\n     manually dereferencing argv[0] if it is a symlink.  GIT_EXEC_PATH\n     and friends still takes precedence, but only fallback on\n     compile-time defaults if \"find relative to argv[0]\" fails.\n     It looks like Makefile RUNTIME_PREFIX enables something like\n     this, but it is currently disabled by default on most platforms.\n\n--\nMatthew Ogilvie   [mmogilvi_git@miniinfo.net]\n"},{"id":"128657","messageId":"20091129034313.GA28379@coredump.intra.peff.net","threadId":"21778","inReplyTo":"7vaay6f4ce.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/4] build dashless \"test-bin\" directory similar to installed bindir","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-11-29T03:43:13Z","receivedAt":"2009-11-29T03:43:13Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Nov 28, 2009 at 06:32:33PM -0800, Junio C Hamano wrote:\n\n> > Would implementing it that way mean that:\n> >\n> >   make && cd t && make\n> >\n> > does not work (or worse, might silently use stale information in\n> > test-bin)?\n> \n> Why can't t/Makefile have a dependency on its 'default' target that goes\n> up and prepares test-bin/, i.e. \"cd .. && make test-bin-stuff\"?\n\nYeah, that would work (I really should have phrased my other response\nless as a critique and more as \"please don't break this workflow\"). But\nI don't think the default target would be enough. I would also expect\n\n  make && cd t && make tXXXX-YYYY.sh\n\nto work correctly (and to be pedantic, I am actually more interested in\nthe equivalent situation that one has one window looking at code and\ncompiling, and another window running a test script, but they are\nfunctionally equivalent here).\n\nI also like to be able to simply run ./tXXXX-YYYY.sh. I can accept\nlosing that if there is something to be gained, but if we can keep it, I\nsuspect I am not the only one who uses it.\n\n-Peff\n"},{"id":"128658","messageId":"20091129042355.GA2252@comcast.net","threadId":"21778","inReplyTo":"7vaay6f4ce.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/4] build dashless \"test-bin\" directory similar to installed bindir","fromName":"Matthew Ogilvie","fromEmail":"mmogilvi_git@miniinfo.net","sentAt":"2009-11-29T04:23:55Z","receivedAt":"2009-11-29T04:23:55Z","isPatch":true,"sender":{"key":"mmogilvi_git@miniinfo.net","avatar":null},"body":"On Sat, Nov 28, 2009 at 06:32:33PM -0800, Junio C Hamano wrote:\n> Why can't t/Makefile have a dependency on its 'default' target that goes\n> up and prepares test-bin/, i.e. \"cd .. && make test-bin-stuff\"?\n\nThat wouldn't help with someone manually running specific tests,\nwhich is a common way to run tests when working on git:\n\n   hackhackhack && make && cd t && ./tXXXX-*.sh\n\nPerhaps we could invoke make from test-lib.sh, but at that point\nit starts to look more attractive to just do it the same way as the\nvalgrind option, even though that means duplicating the appropriate\nlist of executables in test-lib.sh.\n\nAlso, \"make all\" already builds some test suite support\nbinaries (test-*).  Is it really worth any effort to leave\na few short wrapper scripts out of \"make all\" when it is already\nbuilding several test binaries?\n\n(See also my much longer email response that crossed this one in the\nmail.)\n\n--\nMatthew Ogilvie   [mmogilvi_git@miniinfo.net]\n"},{"id":"128659","messageId":"7vzl65ex6u.fsf@alter.siamese.dyndns.org","threadId":"21778","inReplyTo":"20091129034313.GA28379@coredump.intra.peff.net","subject":"Re: [PATCH 3/4] build dashless \"test-bin\" directory similar to installed bindir","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-11-29T05:07:05Z","receivedAt":"2009-11-29T05:07:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n>   make && cd t && make tXXXX-YYYY.sh\n>\n> to work correctly ...\n>\n> I also like to be able to simply run ./tXXXX-YYYY.sh.\n\nI think both can be done by running \"cd .. && make test-bin-stuff\" from\ntest-lib.sh if you wanted to.  Isn't it essentially what you do for\nvalgrind?\n"},{"id":"128660","messageId":"20091129051047.GA6135@coredump.intra.peff.net","threadId":"21778","inReplyTo":"7vzl65ex6u.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/4] build dashless \"test-bin\" directory similar to installed bindir","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-11-29T05:10:47Z","receivedAt":"2009-11-29T05:10:47Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Nov 28, 2009 at 09:07:05PM -0800, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> >   make && cd t && make tXXXX-YYYY.sh\n> >\n> > to work correctly ...\n> >\n> > I also like to be able to simply run ./tXXXX-YYYY.sh.\n> \n> I think both can be done by running \"cd .. && make test-bin-stuff\" from\n> test-lib.sh if you wanted to.  Isn't it essentially what you do for\n> valgrind?\n\nYes, except that the valgrind setup doesn't actually interact with the\nMakefile; it just assumes that git-* is interesting. I would be happy\nwith a solution that is triggered from test-lib. It may even be possible\nto refactor the valgrind stuff, but I'm not sure. What Dscho eventually\nsubmitted doesn't bear much resemblence to my original attempt, and I've\nforgotten all of the details.\n\n-Peff\n"},{"id":"128729","messageId":"20091130160752.6117@nanako3.lavabit.com","threadId":"21778","inReplyTo":"20091129025251.GA1771@comcast.net","subject":"Re: [PATCH 3/4] build dashless \"test-bin\" directory similar to installed bindir","fromName":"Nanako Shiraishi","fromEmail":"nanako3@lavabit.com","sentAt":"2009-11-30T07:07:52Z","receivedAt":"2009-11-30T07:07:52Z","isPatch":true,"sender":{"key":"nanako3@lavabit.com","avatar":"https://gravatar.com/avatar/3777b9e201c5883a62b1a6fdf7c53f2d712d1d80989146063ea861e33aad72a8?d=mp&s=160"},"body":"Quoting Matthew Ogilvie <mmogilvi_git@miniinfo.net>\n\n>   4. Rename test-bin.  Perhaps \"bin-wrappers\", \"bin-dashless\",\n>      \"bin-install\", \"bin\", or \"bindir\".  Any preferences?\n\n\"gitrun\" or \"rungit\"?\n\nI prefer the latter because too many files begin with \"git\" after compiling.\n\n-- \nNanako Shiraishi\nhttp://ivory.ap.teacup.com/nanako3/\n"}]}