{"thread":{"id":"49809","subject":"[PATCH 0/5] tests: various improvements to the GIT_TEST_INSTALLED feature","startedAt":"2018-11-12T13:48:34Z","lastAt":"2018-11-15T12:41:23Z","messageCount":27,"participants":["Johannes Schindelin via GitGitGadget","Junio C Hamano","Jeff King","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"363016","messageId":"pull.73.git.gitgitgadget@gmail.com","threadId":"49809","inReplyTo":null,"subject":"[PATCH 0/5] tests: various improvements to the GIT_TEST_INSTALLED feature","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-11-12T13:48:32Z","receivedAt":"2018-11-12T13:48:34Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"By setting the GIT_TEST_INSTALLED variable to the path of an installed Git\nexecutable, it is possible to run the test suite also on a specific\ninstalled version (as opposed to a version built from scratch).\n\nThe only thing this needs that is unlikely to be installed is the test\nhelper(s).\n\nHowever, there have been a few rough edges around that, identified in my\n(still ongoing) work to support building Git in Visual Studio (where we do\nnot want to run GNU Make, and where we have no canonical way to create, say,\nhard-linked copies of the built-in commands, and other work to let Git for\nWindows play better with BusyBox.\n\nTriggered by a comment of AEvar\n[https://public-inbox.org/git/20181102223743.4331-1-avarab@gmail.com/], I\nhereby contribute these assorted fixes for the GIT_TEST_INSTALLED feature.\n\nJohannes Schindelin (5):\n tests: fix GIT_TEST_INSTALLED's PATH to include t/helper/\n tests: respect GIT_TEST_INSTALLED when initializing repositories\n t/lib-gettext: test installed git-sh-i18n if GIT_TEST_INSTALLED is set\n tests: do not require Git to be built when testing an installed Git\n tests: explicitly use `git.exe` on Windows\n\n Makefile | 1 +\n t/lib-gettext.sh | 7 ++++++-\n t/test-lib-functions.sh | 3 ++-\n t/test-lib.sh | 15 ++++++++++-----\n 4 files changed, 19 insertions(+), 7 deletions(-)\n\n\nbase-commit: 8858448bb49332d353febc078ce4a3abcc962efe\nPublished-As: https://github.com/gitgitgadget/git/releases/tags/pr-73%2Fdscho%2Ftest-git-installed-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-73/dscho/test-git-installed-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/73\n-- \ngitgitgadget\n"},{"id":"363017","messageId":"2b04f9f086e0361345fb38effd61a4b1ef4ac22d.1542030510.git.gitgitgadget@gmail.com","threadId":"49809","inReplyTo":"pull.73.git.gitgitgadget@gmail.com","subject":"[PATCH 1/5] tests: fix GIT_TEST_INSTALLED's PATH to include t/helper/","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-11-12T13:48:33Z","receivedAt":"2018-11-12T13:48:38Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nWe really need to be able to find the test helpers... Really. This\nchange was forgotten when we moved the test helpers into t/helper/\n\nSigned-off-by: Johannes Schindelin \n---\n t/test-lib.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 47a99aa0ed..832ede5099 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -957,7 +957,7 @@ elif test -n \"$GIT_TEST_INSTALLED\"\n 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:$GIT_BUILD_DIR:$PATH\n+\tPATH=$GIT_TEST_INSTALLED:$GIT_BUILD_DIR/t/helper:$PATH\n \tGIT_EXEC_PATH=${GIT_TEST_EXEC_PATH:-$GIT_EXEC_PATH}\n else # normal case, use ../bin-wrappers only unless $with_dashes:\n \tgit_bin_dir=\"$GIT_BUILD_DIR/bin-wrappers\"\n-- \ngitgitgadget\n\n"},{"id":"363018","messageId":"eddea552e414964e19413c02fb07167432e8aaa0.1542030510.git.gitgitgadget@gmail.com","threadId":"49809","inReplyTo":"pull.73.git.gitgitgadget@gmail.com","subject":"[PATCH 3/5] t/lib-gettext: test installed git-sh-i18n if GIT_TEST_INSTALLED is set","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-11-12T13:48:36Z","receivedAt":"2018-11-12T13:48:38Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nIt makes very, very little sense to test the built git-sh-i18n when the\nuser asked specifically to test another one.\n\nSigned-off-by: Johannes Schindelin \n---\n t/lib-gettext.sh | 7 ++++++-\n 1 file changed, 6 insertions(+), 1 deletion(-)\n\ndiff --git a/t/lib-gettext.sh b/t/lib-gettext.sh\nindex eec757f104..9eb160c997 100644\n--- a/t/lib-gettext.sh\n+++ b/t/lib-gettext.sh\n@@ -10,7 +10,12 @@ GIT_TEXTDOMAINDIR=\"$GIT_BUILD_DIR/po/build/locale\"\n GIT_PO_PATH=\"$GIT_BUILD_DIR/po\"\n export GIT_TEXTDOMAINDIR GIT_PO_PATH\n \n-. \"$GIT_BUILD_DIR\"/git-sh-i18n\n+if test -n \"$GIT_TEST_INSTALLED\"\n+then\n+\t. \"$(git --exec-path)\"/git-sh-i18n\n+else\n+\t. \"$GIT_BUILD_DIR\"/git-sh-i18n\n+fi\n \n if test_have_prereq GETTEXT && ! test_have_prereq GETTEXT_POISON\n then\n-- \ngitgitgadget\n\n"},{"id":"363019","messageId":"948b3dc146fe353fbab6057c1376fa0e787a444f.1542030510.git.gitgitgadget@gmail.com","threadId":"49809","inReplyTo":"pull.73.git.gitgitgadget@gmail.com","subject":"[PATCH 2/5] tests: respect GIT_TEST_INSTALLED when initializing repositories","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-11-12T13:48:34Z","receivedAt":"2018-11-12T13:48:38Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nIt really makes very, very little sense to use a different git\nexecutable than the one the caller indicated via setting the environment\nvariable GIT_TEST_INSTALLED.\n\nSigned-off-by: Johannes Schindelin \n---\n t/test-lib-functions.sh | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex 78d8c3783b..801cc9b2ef 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -900,7 +900,8 @@ test_create_repo () {\n \tmkdir -p \"$repo\"\n \t(\n \t\tcd \"$repo\" || error \"Cannot setup test environment\"\n-\t\t\"$GIT_EXEC_PATH/git-init\" \"--template=$GIT_BUILD_DIR/templates/blt/\" >&3 2>&4 ||\n+\t\t\"${GIT_TEST_INSTALLED:-$GIT_EXEC_PATH}/git\" init \\\n+\t\t\t\"--template=$GIT_BUILD_DIR/templates/blt/\" >&3 2>&4 ||\n \t\terror \"cannot run git init -- have you built things yet?\"\n \t\tmv .git/hooks .git/hooks-disabled\n \t) || exit\n-- \ngitgitgadget\n\n"},{"id":"363020","messageId":"316e215e54b921330b91c763255eb25f475a64ae.1542030510.git.gitgitgadget@gmail.com","threadId":"49809","inReplyTo":"pull.73.git.gitgitgadget@gmail.com","subject":"[PATCH 4/5] tests: do not require Git to be built when testing an installed Git","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-11-12T13:48:37Z","receivedAt":"2018-11-12T13:48:40Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nWe really only need the test helpers in that case, but that is not what\nwe test for. So let's skip the test for now when we know that we want to\ntest an installed Git.\n\nSigned-off-by: Johannes Schindelin \n---\n t/test-lib.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 832ede5099..1ea20dc2dc 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -51,7 +51,7 @@ export LSAN_OPTIONS\n \n ################################################################\n # It appears that people try to run tests without building...\n-\"$GIT_BUILD_DIR/git\" >/dev/null\n+test -n \"$GIT_TEST_INSTALLED\" || \"$GIT_BUILD_DIR/git\" >/dev/null ||\n if test $? != 1\n then\n \techo >&2 'error: you do not seem to have built git yet.'\n-- \ngitgitgadget\n\n"},{"id":"363021","messageId":"cd314e1384312cd5b0c0031efd40c6442074e11c.1542030510.git.gitgitgadget@gmail.com","threadId":"49809","inReplyTo":"pull.73.git.gitgitgadget@gmail.com","subject":"[PATCH 5/5] tests: explicitly use `git.exe` on Windows","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-11-12T13:48:38Z","receivedAt":"2018-11-12T13:48:41Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nIn the bin-wrappers/* scripts, we already take pains to use `git.exe`\nrather than `git`, as this could pick up the wrong thing on Windows\n(i.e. if there exists a `git` file or directory in the build directory).\n\nNow we do the same in the tests' start-up code.\n\nThis also helps when testing an installed Git, as there might be even\nmore likely some stray file or directory in the way.\n\nNote: the only way we can record whether the `.exe` suffix is by writing\nit to the `GIT-BUILD-OPTIONS` file and sourcing it at the beginning of\n`t/test-lib.sh`. This is not a requirement introduced by this patch, but\nwe move the call to be able to use the `$X` variable that holds the file\nextension, if any.\n\nNote also: the many, many calls to `git this` and `git that` are\nunaffected, as the regular PATH search will find the `.exe` files on\nWindows (and not be confused by a directory of the name `git` that is\nin one of the directories listed in the `PATH` variable), while\n`/path/to/git` would not, per se, know that it is looking for an\nexecutable and happily prefer such a directory.\n\nSigned-off-by: Johannes Schindelin \n---\n Makefile | 1 +\n t/test-lib-functions.sh | 2 +-\n t/test-lib.sh | 13 +++++++++----\n 3 files changed, 11 insertions(+), 5 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex bbfbb4292d..5df0118ce9 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2590,6 +2590,7 @@ GIT-BUILD-OPTIONS: FORCE\n \t@echo NO_UNIX_SOCKETS=\\''$(subst ','\\'',$(subst ','\\'',$(NO_UNIX_SOCKETS)))'\\' >>$@+\n \t@echo PAGER_ENV=\\''$(subst ','\\'',$(subst ','\\'',$(PAGER_ENV)))'\\' >>$@+\n \t@echo DC_SHA1=\\''$(subst ','\\'',$(subst ','\\'',$(DC_SHA1)))'\\' >>$@+\n+\t@echo X=\\'$(X)\\' >>$@+\n ifdef TEST_OUTPUT_DIRECTORY\n \t@echo TEST_OUTPUT_DIRECTORY=\\''$(subst ','\\'',$(subst ','\\'',$(TEST_OUTPUT_DIRECTORY)))'\\' >>$@+\n endif\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex 801cc9b2ef..c167b2e1af 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -900,7 +900,7 @@ test_create_repo () {\n \tmkdir -p \"$repo\"\n \t(\n \t\tcd \"$repo\" || error \"Cannot setup test environment\"\n-\t\t\"${GIT_TEST_INSTALLED:-$GIT_EXEC_PATH}/git\" init \\\n+\t\t\"${GIT_TEST_INSTALLED:-$GIT_EXEC_PATH}/git$X\" init \\\n \t\t\t\"--template=$GIT_BUILD_DIR/templates/blt/\" >&3 2>&4 ||\n \t\terror \"cannot run git init -- have you built things yet?\"\n \t\tmv .git/hooks .git/hooks-disabled\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 1ea20dc2dc..3e2a9ce76d 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -49,18 +49,23 @@ export ASAN_OPTIONS\n : ${LSAN_OPTIONS=abort_on_error=1}\n export LSAN_OPTIONS\n \n+if test ! -f \"$GIT_BUILD_DIR\"/GIT-BUILD-OPTIONS\n+then\n+\techo >&2 'error: GIT-BUILD-OPTIONS missing (has Git been built?).'\n+\texit 1\n+fi\n+. \"$GIT_BUILD_DIR\"/GIT-BUILD-OPTIONS\n+export PERL_PATH SHELL_PATH\n+\n ################################################################\n # It appears that people try to run tests without building...\n-test -n \"$GIT_TEST_INSTALLED\" || \"$GIT_BUILD_DIR/git\" >/dev/null ||\n+test -n \"$GIT_TEST_INSTALLED\" || \"$GIT_BUILD_DIR/git$X\" >/dev/null ||\n if test $? != 1\n then\n \techo >&2 'error: you do not seem to have built git yet.'\n \texit 1\n fi\n \n-. \"$GIT_BUILD_DIR\"/GIT-BUILD-OPTIONS\n-export PERL_PATH SHELL_PATH\n-\n # if --tee was passed, write the output not only to the terminal, but\n # additionally to the file test-results/$BASENAME.out, too.\n case \"$GIT_TEST_TEE_STARTED, $* \" in\n-- \ngitgitgadget\n"},{"id":"363310","messageId":"xmqqmuqcp8e2.fsf@gitster-ct.c.googlers.com","threadId":"49809","inReplyTo":"2b04f9f086e0361345fb38effd61a4b1ef4ac22d.1542030510.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/5] tests: fix GIT_TEST_INSTALLED's PATH to include t/helper/","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-11-14T04:53:25Z","receivedAt":"2018-11-14T04:53:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> From: Johannes Schindelin \n>\n> We really need to be able to find the test helpers... Really. This\n> change was forgotten when we moved the test helpers into t/helper/\n\nYeah, it is unfortunate that more and more things have internal\nwhitebox test that can only be checked with test-helper binaries,\nwhich makes it impossible to fully testing installed git, but that\nis not a new issue this fix introduces. This at least allows us to\nkeep using the test-helpers from freshly built Git while using the\ninstalled version of Git for most of the tests.\n\nThanks.\n\n> Signed-off-by: Johannes Schindelin \n> ---\n> t/test-lib.sh | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/t/test-lib.sh b/t/test-lib.sh\n> index 47a99aa0ed..832ede5099 100644\n> --- a/t/test-lib.sh\n> +++ b/t/test-lib.sh\n> @@ -957,7 +957,7 @@ elif test -n \"$GIT_TEST_INSTALLED\"\n> 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:$GIT_BUILD_DIR:$PATH\n> +\tPATH=$GIT_TEST_INSTALLED:$GIT_BUILD_DIR/t/helper:$PATH\n> \tGIT_EXEC_PATH=${GIT_TEST_EXEC_PATH:-$GIT_EXEC_PATH}\n> else # normal case, use ../bin-wrappers only unless $with_dashes:\n> \tgit_bin_dir=\"$GIT_BUILD_DIR/bin-wrappers\"\n"},{"id":"363311","messageId":"xmqqin10p8aq.fsf@gitster-ct.c.googlers.com","threadId":"49809","inReplyTo":"948b3dc146fe353fbab6057c1376fa0e787a444f.1542030510.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/5] tests: respect GIT_TEST_INSTALLED when initializing repositories","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-11-14T04:55:25Z","receivedAt":"2018-11-14T04:55:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> From: Johannes Schindelin \n>\n> It really makes very, very little sense to use a different git\n> executable than the one the caller indicated via setting the environment\n> variable GIT_TEST_INSTALLED.\n\nMakes perfect sense. Shouldn't we be asking where the template\ndirectory of the installed version is and using it instead of the\nfreshly built one, no?\n\n>\n> Signed-off-by: Johannes Schindelin \n> ---\n> t/test-lib-functions.sh | 3 ++-\n> 1 file changed, 2 insertions(+), 1 deletion(-)\n>\n> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\n> index 78d8c3783b..801cc9b2ef 100644\n> --- a/t/test-lib-functions.sh\n> +++ b/t/test-lib-functions.sh\n> @@ -900,7 +900,8 @@ test_create_repo () {\n> \tmkdir -p \"$repo\"\n> \t(\n> \t\tcd \"$repo\" || error \"Cannot setup test environment\"\n> -\t\t\"$GIT_EXEC_PATH/git-init\" \"--template=$GIT_BUILD_DIR/templates/blt/\" >&3 2>&4 ||\n> +\t\t\"${GIT_TEST_INSTALLED:-$GIT_EXEC_PATH}/git\" init \\\n> +\t\t\t\"--template=$GIT_BUILD_DIR/templates/blt/\" >&3 2>&4 ||\n> \t\terror \"cannot run git init -- have you built things yet?\"\n> \t\tmv .git/hooks .git/hooks-disabled\n> \t) || exit\n"},{"id":"363312","messageId":"xmqqefbop89n.fsf@gitster-ct.c.googlers.com","threadId":"49809","inReplyTo":"eddea552e414964e19413c02fb07167432e8aaa0.1542030510.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 3/5] t/lib-gettext: test installed git-sh-i18n if GIT_TEST_INSTALLED is set","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-11-14T04:56:04Z","receivedAt":"2018-11-14T04:56:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> From: Johannes Schindelin \n>\n> It makes very, very little sense to test the built git-sh-i18n when the\n> user asked specifically to test another one.\n>\n> Signed-off-by: Johannes Schindelin \n> ---\n\nYup. Makes sense.\n\n> t/lib-gettext.sh | 7 ++++++-\n> 1 file changed, 6 insertions(+), 1 deletion(-)\n>\n> diff --git a/t/lib-gettext.sh b/t/lib-gettext.sh\n> index eec757f104..9eb160c997 100644\n> --- a/t/lib-gettext.sh\n> +++ b/t/lib-gettext.sh\n> @@ -10,7 +10,12 @@ GIT_TEXTDOMAINDIR=\"$GIT_BUILD_DIR/po/build/locale\"\n> GIT_PO_PATH=\"$GIT_BUILD_DIR/po\"\n> export GIT_TEXTDOMAINDIR GIT_PO_PATH\n> \n> -. \"$GIT_BUILD_DIR\"/git-sh-i18n\n> +if test -n \"$GIT_TEST_INSTALLED\"\n> +then\n> +\t. \"$(git --exec-path)\"/git-sh-i18n\n> +else\n> +\t. \"$GIT_BUILD_DIR\"/git-sh-i18n\n> +fi\n> \n> if test_have_prereq GETTEXT && ! test_have_prereq GETTEXT_POISON\n> then\n"},{"id":"363313","messageId":"xmqqa7mcp80u.fsf@gitster-ct.c.googlers.com","threadId":"49809","inReplyTo":"316e215e54b921330b91c763255eb25f475a64ae.1542030510.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 4/5] tests: do not require Git to be built when testing an installed Git","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-11-14T05:01:21Z","receivedAt":"2018-11-14T05:01:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> From: Johannes Schindelin \n>\n> We really only need the test helpers in that case, but that is not what\n> we test for. So let's skip the test for now when we know that we want to\n> test an installed Git.\n\nTrue, but... hopefully we are making sure t/helpers/ has been built\nin some other ways, though, right? I do not offhand see how tests\nwould work in a pristine checkout with \"cd t && sh t0000-*.sh\" as\nt/test-lib.sh is not running $(MAKE) itself (and the design of the\ntest-lib.sh, as can be seen in the check this part of it makes, is\nto just abort if we cannot test) with this patch (and PATCH 1/5)\nunder the test-installed mode, though.\n\n> Signed-off-by: Johannes Schindelin \n> ---\n> t/test-lib.sh | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/t/test-lib.sh b/t/test-lib.sh\n> index 832ede5099..1ea20dc2dc 100644\n> --- a/t/test-lib.sh\n> +++ b/t/test-lib.sh\n> @@ -51,7 +51,7 @@ export LSAN_OPTIONS\n> \n> ################################################################\n> # It appears that people try to run tests without building...\n> -\"$GIT_BUILD_DIR/git\" >/dev/null\n> +test -n \"$GIT_TEST_INSTALLED\" || \"$GIT_BUILD_DIR/git\" >/dev/null ||\n> if test $? != 1\n> then\n> \techo >&2 'error: you do not seem to have built git yet.'\n"},{"id":"363314","messageId":"xmqq5zx0p7ej.fsf@gitster-ct.c.googlers.com","threadId":"49809","inReplyTo":"cd314e1384312cd5b0c0031efd40c6442074e11c.1542030510.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 5/5] tests: explicitly use `git.exe` on Windows","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-11-14T05:14:44Z","receivedAt":"2018-11-14T05:14:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> diff --git a/Makefile b/Makefile\n> index bbfbb4292d..5df0118ce9 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -2590,6 +2590,7 @@ GIT-BUILD-OPTIONS: FORCE\n> \t@echo NO_UNIX_SOCKETS=\\''$(subst ','\\'',$(subst ','\\'',$(NO_UNIX_SOCKETS)))'\\' >>$@+\n> \t@echo PAGER_ENV=\\''$(subst ','\\'',$(subst ','\\'',$(PAGER_ENV)))'\\' >>$@+\n> \t@echo DC_SHA1=\\''$(subst ','\\'',$(subst ','\\'',$(DC_SHA1)))'\\' >>$@+\n> +\t@echo X=\\'$(X)\\' >>$@+\n\nMade me wonder if a single letter $(X) is a bit too cute to expose\nto the outside world; as a narrowly confined local convention in\nthis single Makefile, it was perfectly fine. But it should do for\nnow. Its terseness is attractive, and our eyes (I do not speak for\nthose new to our codebase and build structure) are already used to\nseeing $X suffix. Somebody may later come and complain but I am OK\nto rename it to something like $EXE at that time, not now.\n\n> ifdef TEST_OUTPUT_DIRECTORY\n> \t@echo TEST_OUTPUT_DIRECTORY=\\''$(subst ','\\'',$(subst ','\\'',$(TEST_OUTPUT_DIRECTORY)))'\\' >>$@+\n> endif\n> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\n> index 801cc9b2ef..c167b2e1af 100644\n> --- a/t/test-lib-functions.sh\n> +++ b/t/test-lib-functions.sh\n> @@ -900,7 +900,7 @@ test_create_repo () {\n> \tmkdir -p \"$repo\"\n> \t(\n> \t\tcd \"$repo\" || error \"Cannot setup test environment\"\n> -\t\t\"${GIT_TEST_INSTALLED:-$GIT_EXEC_PATH}/git\" init \\\n> +\t\t\"${GIT_TEST_INSTALLED:-$GIT_EXEC_PATH}/git$X\" init \\\n\nGood.\n\n> \t\t\t\"--template=$GIT_BUILD_DIR/templates/blt/\" >&3 2>&4 ||\n> \t\terror \"cannot run git init -- have you built things yet?\"\n> \t\tmv .git/hooks .git/hooks-disabled\n> diff --git a/t/test-lib.sh b/t/test-lib.sh\n> index 1ea20dc2dc..3e2a9ce76d 100644\n> --- a/t/test-lib.sh\n> +++ b/t/test-lib.sh\n> @@ -49,18 +49,23 @@ export ASAN_OPTIONS\n> : ${LSAN_OPTIONS=abort_on_error=1}\n> export LSAN_OPTIONS\n> \n> +if test ! -f \"$GIT_BUILD_DIR\"/GIT-BUILD-OPTIONS\n> +then\n> +\techo >&2 'error: GIT-BUILD-OPTIONS missing (has Git been built?).'\n> +\texit 1\n> +fi\n\nOK, this tells us that we at least attempted to build git once, even\nunder test-installed mode, and hopefully people won't run $(MAKE) and\nimmediately ^C it only to fool us by leaving this file while keeping\ngit binary and t/helpers/ binary unbuilt.\n\n> +. \"$GIT_BUILD_DIR\"/GIT-BUILD-OPTIONS\n> +export PERL_PATH SHELL_PATH\n> +\n> ################################################################\n> # It appears that people try to run tests without building...\n> -test -n \"$GIT_TEST_INSTALLED\" || \"$GIT_BUILD_DIR/git\" >/dev/null ||\n> +test -n \"$GIT_TEST_INSTALLED\" || \"$GIT_BUILD_DIR/git$X\" >/dev/null ||\n\nThe latter half of this change is a good one. Given what the\nproposed log message of this patch says\n\n Note also: the many, many calls to `git this` and `git that` are\n unaffected, as the regular PATH search will find the `.exe` files on\n Windows (and not be confused by a directory of the name `git` that is\n in one of the directories listed in the `PATH` variable), while\n `/path/to/git` would not, per se, know that it is looking for an\n executable and happily prefer such a directory.\n\nwhich I read as \"path-prefixed invocation, i.e. some/path/to/git, is\nbad, it must be spelled some/path/to/git.exe\", I am surprised that\ntests ever worked on Windows without these five patches, as this\nline used to read like this:\n\n\t\"$GIT_BUILD_DIR/git\" >/dev/null\n\tif test $? != 1\n\tthen\n\t\t...\n\nWouldn't \"$GIT_BUILD_DIR/git\" have failed (and \"executable not\nfound\" hopefully won't produce exit code 1) and stopped the test\nsuite from running even after you built git and not under the\ntest-installed-git mode?\n\n> if test $? != 1\n> then\n> \techo >&2 'error: you do not seem to have built git yet.'\n> \texit 1\n> fi\n> \n> -. \"$GIT_BUILD_DIR\"/GIT-BUILD-OPTIONS\n> -export PERL_PATH SHELL_PATH\n> -\n> # if --tee was passed, write the output not only to the terminal, but\n> # additionally to the file test-results/$BASENAME.out, too.\n> case \"$GIT_TEST_TEE_STARTED, $* \" in\n"},{"id":"363353","messageId":"20181114125206.GA32308@sigill.intra.peff.net","threadId":"49809","inReplyTo":"316e215e54b921330b91c763255eb25f475a64ae.1542030510.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 4/5] tests: do not require Git to be built when testing an installed Git","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-11-14T12:52:06Z","receivedAt":"2018-11-14T12:52:09Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Nov 12, 2018 at 05:48:37AM -0800, Johannes Schindelin via GitGitGadget wrote:\n\n> diff --git a/t/test-lib.sh b/t/test-lib.sh\n> index 832ede5099..1ea20dc2dc 100644\n> --- a/t/test-lib.sh\n> +++ b/t/test-lib.sh\n> @@ -51,7 +51,7 @@ export LSAN_OPTIONS\n> \n> ################################################################\n> # It appears that people try to run tests without building...\n> -\"$GIT_BUILD_DIR/git\" >/dev/null\n> +test -n \"$GIT_TEST_INSTALLED\" || \"$GIT_BUILD_DIR/git\" >/dev/null ||\n> if test $? != 1\n> then\n> \techo >&2 'error: you do not seem to have built git yet.'\n\nThe original runs the command independently and then checks $?. Your\nreplacement chains the ||. I think it works, because the only case that\nis different is if running git returns 0 (in which case we currently\ncomplain, but the new code would quietly accept this).\n\nThat should never happen, but if it does we'd probably want to complain.\nAnd it's pretty subtle all around. Maybe this would be a bit clearer:\n\n if test -n \"$GIT_TEST_INSTALLED\"\n then\n\t: assume installed git is OK\n else\n\t\"$GIT_BUILD_DIR/git\" >/dev/null\n\tif test $? != 1\n\tthen\n\t\t... die ...\n\tfi\n fi\n\nThough arguably we should be checking that there is a git in our path in\nthe first instance. So maybe:\n\n if test -n \"$GIT_TEST_INSTALLED\"\n\t\"$GIT_TEST_INSTALLED/git\" >/dev/null\n else\n\t\"$GIT_BUILD_DIR/git\" >/dev/null\n fi\n if test $? != 1 ...\n\n-Peff\n"},{"id":"363355","messageId":"nycvar.QRO.7.76.6.1811141413170.39@tvgsbejvaqbjf.bet","threadId":"49809","inReplyTo":"xmqqin10p8aq.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 2/5] tests: respect GIT_TEST_INSTALLED when initializing repositories","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-11-14T13:16:37Z","receivedAt":"2018-11-14T13:16:44Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Wed, 14 Nov 2018, Junio C Hamano wrote:\n\n> \"Johannes Schindelin via GitGitGadget\" \n> writes:\n> \n> > From: Johannes Schindelin \n> >\n> > It really makes very, very little sense to use a different git\n> > executable than the one the caller indicated via setting the environment\n> > variable GIT_TEST_INSTALLED.\n> \n> Makes perfect sense. Shouldn't we be asking where the template\n> directory of the installed version is and using it instead of the\n> freshly built one, no?\n\nIt would make sense, but we don't know how to get that information, do we?\n\n$ git grep DEFAULT_GIT_TEMPLATE_DIR\nMakefile: -DDEFAULT_GIT_TEMPLATE_DIR='\"$(template_dir_SQ)\"'\nbuiltin/init-db.c:#ifndef DEFAULT_GIT_TEMPLATE_DIR\nbuiltin/init-db.c:#define DEFAULT_GIT_TEMPLATE_DIR \"/usr/share/git-core/templates\"\nbuiltin/init-db.c: template_dir = to_free = system_path(DEFAULT_GIT_TEMPLATE_DIR);\ncontrib/vscode/init.sh: '-DDEFAULT_GIT_TEMPLATE_DIR=\"$(template_dir_SQ)\"' \\\n\nIn other words, the Makefile defines the DEFAULT_GIT_TEMPLATE_DIR, and the\nonly user in the code is init-db.c which uses it in copy_templates().\n\nAnd changing the code *now* to let us query Git where it thinks its\ntemplates should be won't work, as this patch is about using the installed\nGit (at whatever pre-compiled version that might be).\n\nCiao,\nDscho\n\n> >\n> > Signed-off-by: Johannes Schindelin \n> > ---\n> > t/test-lib-functions.sh | 3 ++-\n> > 1 file changed, 2 insertions(+), 1 deletion(-)\n> >\n> > diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\n> > index 78d8c3783b..801cc9b2ef 100644\n> > --- a/t/test-lib-functions.sh\n> > +++ b/t/test-lib-functions.sh\n> > @@ -900,7 +900,8 @@ test_create_repo () {\n> > \tmkdir -p \"$repo\"\n> > \t(\n> > \t\tcd \"$repo\" || error \"Cannot setup test environment\"\n> > -\t\t\"$GIT_EXEC_PATH/git-init\" \"--template=$GIT_BUILD_DIR/templates/blt/\" >&3 2>&4 ||\n> > +\t\t\"${GIT_TEST_INSTALLED:-$GIT_EXEC_PATH}/git\" init \\\n> > +\t\t\t\"--template=$GIT_BUILD_DIR/templates/blt/\" >&3 2>&4 ||\n> > \t\terror \"cannot run git init -- have you built things yet?\"\n> > \t\tmv .git/hooks .git/hooks-disabled\n> > \t) || exit\n> \n"},{"id":"363356","messageId":"nycvar.QRO.7.76.6.1811141417160.39@tvgsbejvaqbjf.bet","threadId":"49809","inReplyTo":"xmqqa7mcp80u.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 4/5] tests: do not require Git to be built when testing an installed Git","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-11-14T13:20:18Z","receivedAt":"2018-11-14T13:20:25Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Wed, 14 Nov 2018, Junio C Hamano wrote:\n\n> \"Johannes Schindelin via GitGitGadget\" \n> writes:\n> \n> > From: Johannes Schindelin \n> >\n> > We really only need the test helpers in that case, but that is not what\n> > we test for. So let's skip the test for now when we know that we want to\n> > test an installed Git.\n> \n> True, but... hopefully we are making sure t/helpers/ has been built\n> in some other ways, though, right?\n\nWe do it implicitly, in the test cases that use the helpers.\n\nHowever, t/test-lib.sh does not particularly verify that the test helpers\nhave been built.\n\nAnd I think that is a good thing: we do have several test scripts, I would\nthink, that do not require any test helper to begin with. These scripts\ncan work pretty well without having to build anything (read: on a machine\nwhere there is not even a toolchain available to build things).\n\nBesides, it is pretty much only t/helper/test-tool these days, and it is\nunlikely that anybody has a `test-tool` in their `PATH`. If they do,\nthey'll find out soon enough iff they test with `GIT_TEST_INSTALLED` and\niff they do not have the test helper(s) in t/helper/ ;-)\n\nCiao,\nDscho\n\n> I do not offhand see how tests would work in a pristine checkout with\n> \"cd t && sh t0000-*.sh\" as t/test-lib.sh is not running $(MAKE) itself\n> (and the design of the test-lib.sh, as can be seen in the check this\n> part of it makes, is to just abort if we cannot test) with this patch\n> (and PATCH 1/5) under the test-installed mode, though.\n> \n> > Signed-off-by: Johannes Schindelin \n> > ---\n> > t/test-lib.sh | 2 +-\n> > 1 file changed, 1 insertion(+), 1 deletion(-)\n> >\n> > diff --git a/t/test-lib.sh b/t/test-lib.sh\n> > index 832ede5099..1ea20dc2dc 100644\n> > --- a/t/test-lib.sh\n> > +++ b/t/test-lib.sh\n> > @@ -51,7 +51,7 @@ export LSAN_OPTIONS\n> > \n> > ################################################################\n> > # It appears that people try to run tests without building...\n> > -\"$GIT_BUILD_DIR/git\" >/dev/null\n> > +test -n \"$GIT_TEST_INSTALLED\" || \"$GIT_BUILD_DIR/git\" >/dev/null ||\n> > if test $? != 1\n> > then\n> > \techo >&2 'error: you do not seem to have built git yet.'\n> \n"},{"id":"363357","messageId":"nycvar.QRO.7.76.6.1811141422270.39@tvgsbejvaqbjf.bet","threadId":"49809","inReplyTo":"xmqq5zx0p7ej.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 5/5] tests: explicitly use `git.exe` on Windows","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-11-14T13:24:45Z","receivedAt":"2018-11-14T13:24:52Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Wed, 14 Nov 2018, Junio C Hamano wrote:\n\n> \"Johannes Schindelin via GitGitGadget\" \n> writes:\n> \n> > diff --git a/Makefile b/Makefile\n> > index bbfbb4292d..5df0118ce9 100644\n> > --- a/Makefile\n> > +++ b/Makefile\n> > @@ -2590,6 +2590,7 @@ GIT-BUILD-OPTIONS: FORCE\n> > \t@echo NO_UNIX_SOCKETS=\\''$(subst ','\\'',$(subst ','\\'',$(NO_UNIX_SOCKETS)))'\\' >>$@+\n> > \t@echo PAGER_ENV=\\''$(subst ','\\'',$(subst ','\\'',$(PAGER_ENV)))'\\' >>$@+\n> > \t@echo DC_SHA1=\\''$(subst ','\\'',$(subst ','\\'',$(DC_SHA1)))'\\' >>$@+\n> > +\t@echo X=\\'$(X)\\' >>$@+\n> \n> Made me wonder if a single letter $(X) is a bit too cute to expose\n> to the outside world; as a narrowly confined local convention in\n> this single Makefile, it was perfectly fine. But it should do for\n> now. Its terseness is attractive, and our eyes (I do not speak for\n> those new to our codebase and build structure) are already used to\n> seeing $X suffix. Somebody may later come and complain but I am OK\n> to rename it to something like $EXE at that time, not now.\n> \n> > ifdef TEST_OUTPUT_DIRECTORY\n> > \t@echo TEST_OUTPUT_DIRECTORY=\\''$(subst ','\\'',$(subst ','\\'',$(TEST_OUTPUT_DIRECTORY)))'\\' >>$@+\n> > endif\n> > diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\n> > index 801cc9b2ef..c167b2e1af 100644\n> > --- a/t/test-lib-functions.sh\n> > +++ b/t/test-lib-functions.sh\n> > @@ -900,7 +900,7 @@ test_create_repo () {\n> > \tmkdir -p \"$repo\"\n> > \t(\n> > \t\tcd \"$repo\" || error \"Cannot setup test environment\"\n> > -\t\t\"${GIT_TEST_INSTALLED:-$GIT_EXEC_PATH}/git\" init \\\n> > +\t\t\"${GIT_TEST_INSTALLED:-$GIT_EXEC_PATH}/git$X\" init \\\n> \n> Good.\n> \n> > \t\t\t\"--template=$GIT_BUILD_DIR/templates/blt/\" >&3 2>&4 ||\n> > \t\terror \"cannot run git init -- have you built things yet?\"\n> > \t\tmv .git/hooks .git/hooks-disabled\n> > diff --git a/t/test-lib.sh b/t/test-lib.sh\n> > index 1ea20dc2dc..3e2a9ce76d 100644\n> > --- a/t/test-lib.sh\n> > +++ b/t/test-lib.sh\n> > @@ -49,18 +49,23 @@ export ASAN_OPTIONS\n> > : ${LSAN_OPTIONS=abort_on_error=1}\n> > export LSAN_OPTIONS\n> > \n> > +if test ! -f \"$GIT_BUILD_DIR\"/GIT-BUILD-OPTIONS\n> > +then\n> > +\techo >&2 'error: GIT-BUILD-OPTIONS missing (has Git been built?).'\n> > +\texit 1\n> > +fi\n> \n> OK, this tells us that we at least attempted to build git once, even\n> under test-installed mode, and hopefully people won't run $(MAKE) and\n> immediately ^C it only to fool us by leaving this file while keeping\n> git binary and t/helpers/ binary unbuilt.\n> \n> > +. \"$GIT_BUILD_DIR\"/GIT-BUILD-OPTIONS\n> > +export PERL_PATH SHELL_PATH\n> > +\n> > ################################################################\n> > # It appears that people try to run tests without building...\n> > -test -n \"$GIT_TEST_INSTALLED\" || \"$GIT_BUILD_DIR/git\" >/dev/null ||\n> > +test -n \"$GIT_TEST_INSTALLED\" || \"$GIT_BUILD_DIR/git$X\" >/dev/null ||\n> \n> The latter half of this change is a good one. Given what the\n> proposed log message of this patch says\n> \n> Note also: the many, many calls to `git this` and `git that` are\n> unaffected, as the regular PATH search will find the `.exe` files on\n> Windows (and not be confused by a directory of the name `git` that is\n> in one of the directories listed in the `PATH` variable), while\n> `/path/to/git` would not, per se, know that it is looking for an\n> executable and happily prefer such a directory.\n> \n> which I read as \"path-prefixed invocation, i.e. some/path/to/git, is\n> bad, it must be spelled some/path/to/git.exe\", I am surprised that\n> tests ever worked on Windows without these five patches, as this\n> line used to read like this:\n> \n> \t\"$GIT_BUILD_DIR/git\" >/dev/null\n> \tif test $? != 1\n> \tthen\n> \t\t...\n> \n> Wouldn't \"$GIT_BUILD_DIR/git\" have failed (and \"executable not\n> found\" hopefully won't produce exit code 1) and stopped the test\n> suite from running even after you built git and not under the\n> test-installed-git mode?\n\n\"$GIT_BUILD_DIR/git\" did not fail until I regularly built with Visual\nStudio (and my Visual Studio project generator generates a directory named\n\"git\" to live alongside \"git.exe\").\n\nAnd when it failed, I patched Git for Windows. Fast-forward, years later I\nmanaged to contribute the patch we are discussing right now ;-)\n\nSo yes, it is primarily a concern when testing Git in specific setups\nwhere a \"git\" directory can live next to the \"git.exe\" that we want to\ntest. Not necessarily a big deal for most developers on Windows.\n\nCiao,\nDscho\n\n> \n> > if test $? != 1\n> > then\n> > \techo >&2 'error: you do not seem to have built git yet.'\n> > \texit 1\n> > fi\n> > \n> > -. \"$GIT_BUILD_DIR\"/GIT-BUILD-OPTIONS\n> > -export PERL_PATH SHELL_PATH\n> > -\n> > # if --tee was passed, write the output not only to the terminal, but\n> > # additionally to the file test-results/$BASENAME.out, too.\n> > case \"$GIT_TEST_TEE_STARTED, $* \" in\n> \n"},{"id":"363358","messageId":"nycvar.QRO.7.76.6.1811141441030.39@tvgsbejvaqbjf.bet","threadId":"49809","inReplyTo":"20181114125206.GA32308@sigill.intra.peff.net","subject":"Re: [PATCH 4/5] tests: do not require Git to be built when testing an installed Git","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-11-14T13:41:48Z","receivedAt":"2018-11-14T13:41:59Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Peff,\n\nOn Wed, 14 Nov 2018, Jeff King wrote:\n\n> On Mon, Nov 12, 2018 at 05:48:37AM -0800, Johannes Schindelin via GitGitGadget wrote:\n> \n> > diff --git a/t/test-lib.sh b/t/test-lib.sh\n> > index 832ede5099..1ea20dc2dc 100644\n> > --- a/t/test-lib.sh\n> > +++ b/t/test-lib.sh\n> > @@ -51,7 +51,7 @@ export LSAN_OPTIONS\n> > \n> > ################################################################\n> > # It appears that people try to run tests without building...\n> > -\"$GIT_BUILD_DIR/git\" >/dev/null\n> > +test -n \"$GIT_TEST_INSTALLED\" || \"$GIT_BUILD_DIR/git\" >/dev/null ||\n> > if test $? != 1\n> > then\n> > \techo >&2 'error: you do not seem to have built git yet.'\n> \n> The original runs the command independently and then checks $?. Your\n> replacement chains the ||. I think it works, because the only case that\n> is different is if running git returns 0 (in which case we currently\n> complain, but the new code would quietly accept this).\n> \n> That should never happen, but if it does we'd probably want to complain.\n> And it's pretty subtle all around. Maybe this would be a bit clearer:\n> \n> if test -n \"$GIT_TEST_INSTALLED\"\n> then\n> \t: assume installed git is OK\n> else\n> \t\"$GIT_BUILD_DIR/git\" >/dev/null\n> \tif test $? != 1\n> \tthen\n> \t\t... die ...\n> \tfi\n> fi\n> \n> Though arguably we should be checking that there is a git in our path in\n> the first instance. So maybe:\n> \n> if test -n \"$GIT_TEST_INSTALLED\"\n> \t\"$GIT_TEST_INSTALLED/git\" >/dev/null\n> else\n> \t\"$GIT_BUILD_DIR/git\" >/dev/null\n> fi\n> if test $? != 1 ...\n\nYou're right. I did it using `\"${GIT_TEST_INSTALLED:-$GIT_BUILD_DIR}/git\"`\nand I also adjust the error message now.\n\nWill be fixed in the next iteration,\nDscho\n\n> \n> -Peff\n> \n"},{"id":"363368","messageId":"xmqqefbnn2be.fsf@gitster-ct.c.googlers.com","threadId":"49809","inReplyTo":"nycvar.QRO.7.76.6.1811141422270.39@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 5/5] tests: explicitly use `git.exe` on Windows","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-11-14T14:47:33Z","receivedAt":"2018-11-14T14:47:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n>> The latter half of this change is a good one. Given what the\n>> proposed log message of this patch says\n>> \n>> Note also: the many, many calls to `git this` and `git that` are\n>> unaffected, as the regular PATH search will find the `.exe` files on\n>> Windows (and not be confused by a directory of the name `git` that is\n>> in one of the directories listed in the `PATH` variable), while\n>> `/path/to/git` would not, per se, know that it is looking for an\n>> executable and happily prefer such a directory.\n>> \n>> which I read as \"path-prefixed invocation, i.e. some/path/to/git, is\n>> bad, it must be spelled some/path/to/git.exe\", I am surprised that\n>> tests ever worked on Windows without these five patches, as this\n>> line used to read like this:\n>> \n>> \t\"$GIT_BUILD_DIR/git\" >/dev/null\n>> \tif test $? != 1\n>> \tthen\n>> \t\t...\n>> \n>> Wouldn't \"$GIT_BUILD_DIR/git\" have failed (and \"executable not\n>> found\" hopefully won't produce exit code 1) and stopped the test\n>> suite from running even after you built git and not under the\n>> test-installed-git mode?\n>\n> \"$GIT_BUILD_DIR/git\" did not fail until I regularly built with Visual\n> Studio (and my Visual Studio project generator generates a directory named\n> \"git\" to live alongside \"git.exe\").\n>\n> And when it failed, I patched Git for Windows. Fast-forward, years later I\n> managed to contribute the patch we are discussing right now ;-)\n\nOK, I still cannot read it out of what you wrote in the proposed log\nmessage without aid, but in essense the crux of the problem is that\ninvoking \"some/path/to/git\" finds \"some/path/to/git.exe\" only when\nthere is no \"some/path/to/git\" directory at the same time, and if\nthat directory exists, tries and fails to execute that directory,\nand the change in this patch protects us from that problem.\n\nDid I get it right? I'd really prefer to see it more clearly\nwritten in the log message so the next person who reads \"git log\"\ndo not have to ask the same question to you.\n\nAssuming that I read it correctly, I think this patch as a whole\nmakes tons of sense as a change to make the invocation more robust.\n\nThanks.\n"},{"id":"363369","messageId":"xmqqa7mbn1r4.fsf@gitster-ct.c.googlers.com","threadId":"49809","inReplyTo":"nycvar.QRO.7.76.6.1811141413170.39@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 2/5] tests: respect GIT_TEST_INSTALLED when initializing repositories","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-11-14T14:59:43Z","receivedAt":"2018-11-14T14:59:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> It would make sense, but we don't know how to get that information, do we?\n> ...\n> And changing the code *now* to let us query Git where it thinks its\n> templates should be won't work, as this patch is about using the installed\n> Git (at whatever pre-compiled version that might be).\n\nIt won't work, but we can add something like \"git var templatedir\"\nto help those who want to further improve the test-installed mode\nnext year, preparing for better future by sowing seeds now.\n\nIn the meantime, using the same temlate dir as before is probably\nthe best we can do. Two and a half tangential thoughts are:\n\n - But then, we need to make sure $GIT_BUILD_DIR/templates/blt/\n is populated, if we do rely on them (i.e. we probably want to\n make sure we have built).\n\n - Yet, once installed, the contents of the templatedir can be\n arbitrarily munged by the end user, so anything that depends on\n what is in the template won't work as a reliable test piece.\n\n - Among what's in templates/blt/, we explicitly disable hooks at\n the beginning of the test repository creation to ensure no hooks\n interfere what we test by default, but we will get affected by\n what is in info/excludes. The contents of freshly-built one is\n empty, so it is unlikely that this will cause problems, but if we\n use installed templates, we cannot control what's in there,\n letting the tests get affected to random things the end-user\n happens to have.\n\nSo after all, if we were to change anything, it might make better\nsense not to install anything from any templatedir.\n\nBut of course, that is orthogonal to the test-install mode. If we\nwant to make the test more robust by emptying the templates, we\nshould do that also for the test-freshly-baked mode, too.\n"},{"id":"363379","messageId":"pull.73.v2.git.gitgitgadget@gmail.com","threadId":"49809","inReplyTo":"pull.73.git.gitgitgadget@gmail.com","subject":"[PATCH v2 0/5] tests: various improvements to the GIT_TEST_INSTALLED feature","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-11-14T16:32:02Z","receivedAt":"2018-11-14T16:32:06Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"By setting the GIT_TEST_INSTALLED variable to the path of an installed Git\nexecutable, it is possible to run the test suite also on a specific\ninstalled version (as opposed to a version built from scratch).\n\nThe only thing this needs that is unlikely to be installed is the test\nhelper(s).\n\nHowever, there have been a few rough edges around that, identified in my\n(still ongoing) work to support building Git in Visual Studio (where we do\nnot want to run GNU Make, and where we have no canonical way to create, say,\nhard-linked copies of the built-in commands, and other work to let Git for\nWindows play better with BusyBox.\n\nTriggered by a comment of AEvar\n[https://public-inbox.org/git/20181102223743.4331-1-avarab@gmail.com/], I\nhereby contribute these assorted fixes for the GIT_TEST_INSTALLED feature.\n\nChanges since v1:\n\n * Now we verify in test-lib.sh also in the GIT_TEST_INSTALLED case whether\n the Git executable is working (thanks, Peff!).\n * The commit message of 5/5 was touched up.\n\nJohannes Schindelin (5):\n tests: fix GIT_TEST_INSTALLED's PATH to include t/helper/\n tests: respect GIT_TEST_INSTALLED when initializing repositories\n t/lib-gettext: test installed git-sh-i18n if GIT_TEST_INSTALLED is set\n tests: do not require Git to be built when testing an installed Git\n tests: explicitly use `git.exe` on Windows\n\n Makefile | 1 +\n t/lib-gettext.sh | 7 ++++++-\n t/test-lib-functions.sh | 3 ++-\n t/test-lib.sh | 22 ++++++++++++++++------\n 4 files changed, 25 insertions(+), 8 deletions(-)\n\n\nbase-commit: d166e6afe5f257217836ef24a73764eba390c58d\nPublished-As: https://github.com/gitgitgadget/git/releases/tags/pr-73%2Fdscho%2Ftest-git-installed-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-73/dscho/test-git-installed-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/73\n\nRange-diff vs v1:\n\n 1: 2b04f9f086 = 1: 3b68e0fe8a tests: fix GIT_TEST_INSTALLED's PATH to include t/helper/\n 2: 948b3dc146 = 2: 80d50d5932 tests: respect GIT_TEST_INSTALLED when initializing repositories\n 3: eddea552e4 = 3: 49e408677a t/lib-gettext: test installed git-sh-i18n if GIT_TEST_INSTALLED is set\n 4: 316e215e54 < -: ---------- tests: do not require Git to be built when testing an installed Git\n -: ---------- > 4: b801dc8027 tests: do not require Git to be built when testing an installed Git\n 5: cd314e1384 ! 5: fbdb659de6 tests: explicitly use `git.exe` on Windows\n @@ -2,6 +2,17 @@\n \n tests: explicitly use `git.exe` on Windows\n \n + On Windows, when we refer to `/an/absolute/path/to/git`, it magically\n + resolves `git.exe` at that location. Except if something of the name\n + `git` exists next to that `git.exe`. So if we call `$BUILD_DIR/git`, it\n + will find `$BUILD_DIR/git.exe` *only* if there is not, say, a directory\n + called `$BUILD_DIR/git`.\n +\n + Such a directory, however, exists in Git for Windows when building with\n + Visual Studio (our Visual Studio project generator defaults to putting\n + the build files into a directory whose name is the base name of the\n + corresponding `.exe`).\n +\n In the bin-wrappers/* scripts, we already take pains to use `git.exe`\n rather than `git`, as this could pick up the wrong thing on Windows\n (i.e. if there exists a `git` file or directory in the build directory).\n @@ -68,11 +79,12 @@\n +\n ################################################################\n # It appears that people try to run tests without building...\n --test -n \"$GIT_TEST_INSTALLED\" || \"$GIT_BUILD_DIR/git\" >/dev/null ||\n -+test -n \"$GIT_TEST_INSTALLED\" || \"$GIT_BUILD_DIR/git$X\" >/dev/null ||\n +-\"${GIT_TEST_INSTALLED:-$GIT_BUILD_DIR}/git\" >/dev/null\n ++\"${GIT_TEST_INSTALLED:-$GIT_BUILD_DIR}/git$X\" >/dev/null\n if test $? != 1\n then\n - \techo >&2 'error: you do not seem to have built git yet.'\n + \tif test -n \"$GIT_TEST_INSTALLED\"\n +@@\n \texit 1\n fi\n \n\n-- \ngitgitgadget\n"},{"id":"363380","messageId":"3b68e0fe8aeb53e39df9862a5db3b414ed578824.1542213121.git.gitgitgadget@gmail.com","threadId":"49809","inReplyTo":"pull.73.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v2 1/5] tests: fix GIT_TEST_INSTALLED's PATH to include t/helper/","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-11-14T16:32:04Z","receivedAt":"2018-11-14T16:32:08Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nWe really need to be able to find the test helpers... Really. This\nchange was forgotten when we moved the test helpers into t/helper/\n\nSigned-off-by: Johannes Schindelin \n---\n t/test-lib.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex aba66cafa2..93883580a8 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -957,7 +957,7 @@ elif test -n \"$GIT_TEST_INSTALLED\"\n 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:$GIT_BUILD_DIR:$PATH\n+\tPATH=$GIT_TEST_INSTALLED:$GIT_BUILD_DIR/t/helper:$PATH\n \tGIT_EXEC_PATH=${GIT_TEST_EXEC_PATH:-$GIT_EXEC_PATH}\n else # normal case, use ../bin-wrappers only unless $with_dashes:\n \tgit_bin_dir=\"$GIT_BUILD_DIR/bin-wrappers\"\n-- \ngitgitgadget\n\n"},{"id":"363381","messageId":"80d50d5932779592a1ea92d0b13e8d0429453128.1542213121.git.gitgitgadget@gmail.com","threadId":"49809","inReplyTo":"pull.73.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v2 2/5] tests: respect GIT_TEST_INSTALLED when initializing repositories","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-11-14T16:32:06Z","receivedAt":"2018-11-14T16:32:09Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nIt really makes very, very little sense to use a different git\nexecutable than the one the caller indicated via setting the environment\nvariable GIT_TEST_INSTALLED.\n\nSigned-off-by: Johannes Schindelin \n---\n t/test-lib-functions.sh | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex d158c8d0bf..3472716651 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -923,7 +923,8 @@ test_create_repo () {\n \tmkdir -p \"$repo\"\n \t(\n \t\tcd \"$repo\" || error \"Cannot setup test environment\"\n-\t\t\"$GIT_EXEC_PATH/git-init\" \"--template=$GIT_BUILD_DIR/templates/blt/\" >&3 2>&4 ||\n+\t\t\"${GIT_TEST_INSTALLED:-$GIT_EXEC_PATH}/git\" init \\\n+\t\t\t\"--template=$GIT_BUILD_DIR/templates/blt/\" >&3 2>&4 ||\n \t\terror \"cannot run git init -- have you built things yet?\"\n \t\tmv .git/hooks .git/hooks-disabled\n \t) || exit\n-- \ngitgitgadget\n\n"},{"id":"363382","messageId":"49e408677ad8e1b34e4b13a5eb759b1ead69cd50.1542213121.git.gitgitgadget@gmail.com","threadId":"49809","inReplyTo":"pull.73.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v2 3/5] t/lib-gettext: test installed git-sh-i18n if GIT_TEST_INSTALLED is set","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-11-14T16:32:08Z","receivedAt":"2018-11-14T16:32:11Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nIt makes very, very little sense to test the built git-sh-i18n when the\nuser asked specifically to test another one.\n\nSigned-off-by: Johannes Schindelin \n---\n t/lib-gettext.sh | 7 ++++++-\n 1 file changed, 6 insertions(+), 1 deletion(-)\n\ndiff --git a/t/lib-gettext.sh b/t/lib-gettext.sh\nindex eec757f104..9eb160c997 100644\n--- a/t/lib-gettext.sh\n+++ b/t/lib-gettext.sh\n@@ -10,7 +10,12 @@ GIT_TEXTDOMAINDIR=\"$GIT_BUILD_DIR/po/build/locale\"\n GIT_PO_PATH=\"$GIT_BUILD_DIR/po\"\n export GIT_TEXTDOMAINDIR GIT_PO_PATH\n \n-. \"$GIT_BUILD_DIR\"/git-sh-i18n\n+if test -n \"$GIT_TEST_INSTALLED\"\n+then\n+\t. \"$(git --exec-path)\"/git-sh-i18n\n+else\n+\t. \"$GIT_BUILD_DIR\"/git-sh-i18n\n+fi\n \n if test_have_prereq GETTEXT && ! test_have_prereq GETTEXT_POISON\n then\n-- \ngitgitgadget\n\n"},{"id":"363383","messageId":"b801dc80276cf6fd9727f0ead7054c5ea58465b3.1542213121.git.gitgitgadget@gmail.com","threadId":"49809","inReplyTo":"pull.73.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v2 4/5] tests: do not require Git to be built when testing an installed Git","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-11-14T16:32:10Z","receivedAt":"2018-11-14T16:32:13Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nWe really only need the test helpers to be built in the worktree in that\ncase, but that is not what we test for.\n\nOn the other hand it is a perfect opportunity to verify that\n`GIT_TEST_INSTALLED` points to a working Git.\n\nSo let's test the appropriate Git executable. While at it, also adjust\nthe error message in the `GIT_TEST_INSTALLED` case.\n\nThis patch is best viewed with `-w --patience`.\n\nHelped-by: Jeff King \nSigned-off-by: Johannes Schindelin \n---\n t/test-lib.sh | 9 +++++++--\n 1 file changed, 7 insertions(+), 2 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 93883580a8..3d3a65ed0e 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -51,10 +51,15 @@ export LSAN_OPTIONS\n \n ################################################################\n # It appears that people try to run tests without building...\n-\"$GIT_BUILD_DIR/git\" >/dev/null\n+\"${GIT_TEST_INSTALLED:-$GIT_BUILD_DIR}/git\" >/dev/null\n if test $? != 1\n then\n-\techo >&2 'error: you do not seem to have built git yet.'\n+\tif test -n \"$GIT_TEST_INSTALLED\"\n+\tthen\n+\t\techo >&2 \"error: there is no working Git at '$GIT_TEST_INSTALLED'\"\n+\telse\n+\t\techo >&2 'error: you do not seem to have built git yet.'\n+\tfi\n \texit 1\n fi\n \n-- \ngitgitgadget\n\n"},{"id":"363384","messageId":"fbdb659de688fc32c85641b30bf565adafeb5dd6.1542213121.git.gitgitgadget@gmail.com","threadId":"49809","inReplyTo":"pull.73.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v2 5/5] tests: explicitly use `git.exe` on Windows","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-11-14T16:32:11Z","receivedAt":"2018-11-14T16:32:15Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nOn Windows, when we refer to `/an/absolute/path/to/git`, it magically\nresolves `git.exe` at that location. Except if something of the name\n`git` exists next to that `git.exe`. So if we call `$BUILD_DIR/git`, it\nwill find `$BUILD_DIR/git.exe` *only* if there is not, say, a directory\ncalled `$BUILD_DIR/git`.\n\nSuch a directory, however, exists in Git for Windows when building with\nVisual Studio (our Visual Studio project generator defaults to putting\nthe build files into a directory whose name is the base name of the\ncorresponding `.exe`).\n\nIn the bin-wrappers/* scripts, we already take pains to use `git.exe`\nrather than `git`, as this could pick up the wrong thing on Windows\n(i.e. if there exists a `git` file or directory in the build directory).\n\nNow we do the same in the tests' start-up code.\n\nThis also helps when testing an installed Git, as there might be even\nmore likely some stray file or directory in the way.\n\nNote: the only way we can record whether the `.exe` suffix is by writing\nit to the `GIT-BUILD-OPTIONS` file and sourcing it at the beginning of\n`t/test-lib.sh`. This is not a requirement introduced by this patch, but\nwe move the call to be able to use the `$X` variable that holds the file\nextension, if any.\n\nNote also: the many, many calls to `git this` and `git that` are\nunaffected, as the regular PATH search will find the `.exe` files on\nWindows (and not be confused by a directory of the name `git` that is\nin one of the directories listed in the `PATH` variable), while\n`/path/to/git` would not, per se, know that it is looking for an\nexecutable and happily prefer such a directory.\n\nSigned-off-by: Johannes Schindelin \n---\n Makefile | 1 +\n t/test-lib-functions.sh | 2 +-\n t/test-lib.sh | 13 +++++++++----\n 3 files changed, 11 insertions(+), 5 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 016fdcdb81..21b3978744 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2591,6 +2591,7 @@ GIT-BUILD-OPTIONS: FORCE\n \t@echo NO_UNIX_SOCKETS=\\''$(subst ','\\'',$(subst ','\\'',$(NO_UNIX_SOCKETS)))'\\' >>$@+\n \t@echo PAGER_ENV=\\''$(subst ','\\'',$(subst ','\\'',$(PAGER_ENV)))'\\' >>$@+\n \t@echo DC_SHA1=\\''$(subst ','\\'',$(subst ','\\'',$(DC_SHA1)))'\\' >>$@+\n+\t@echo X=\\'$(X)\\' >>$@+\n ifdef TEST_OUTPUT_DIRECTORY\n \t@echo TEST_OUTPUT_DIRECTORY=\\''$(subst ','\\'',$(subst ','\\'',$(TEST_OUTPUT_DIRECTORY)))'\\' >>$@+\n endif\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex 3472716651..274cbc2d6e 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -923,7 +923,7 @@ test_create_repo () {\n \tmkdir -p \"$repo\"\n \t(\n \t\tcd \"$repo\" || error \"Cannot setup test environment\"\n-\t\t\"${GIT_TEST_INSTALLED:-$GIT_EXEC_PATH}/git\" init \\\n+\t\t\"${GIT_TEST_INSTALLED:-$GIT_EXEC_PATH}/git$X\" init \\\n \t\t\t\"--template=$GIT_BUILD_DIR/templates/blt/\" >&3 2>&4 ||\n \t\terror \"cannot run git init -- have you built things yet?\"\n \t\tmv .git/hooks .git/hooks-disabled\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 3d3a65ed0e..e12addc324 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -49,9 +49,17 @@ export ASAN_OPTIONS\n : ${LSAN_OPTIONS=abort_on_error=1}\n export LSAN_OPTIONS\n \n+if test ! -f \"$GIT_BUILD_DIR\"/GIT-BUILD-OPTIONS\n+then\n+\techo >&2 'error: GIT-BUILD-OPTIONS missing (has Git been built?).'\n+\texit 1\n+fi\n+. \"$GIT_BUILD_DIR\"/GIT-BUILD-OPTIONS\n+export PERL_PATH SHELL_PATH\n+\n ################################################################\n # It appears that people try to run tests without building...\n-\"${GIT_TEST_INSTALLED:-$GIT_BUILD_DIR}/git\" >/dev/null\n+\"${GIT_TEST_INSTALLED:-$GIT_BUILD_DIR}/git$X\" >/dev/null\n if test $? != 1\n then\n \tif test -n \"$GIT_TEST_INSTALLED\"\n@@ -63,9 +71,6 @@ then\n \texit 1\n fi\n \n-. \"$GIT_BUILD_DIR\"/GIT-BUILD-OPTIONS\n-export PERL_PATH SHELL_PATH\n-\n # if --tee was passed, write the output not only to the terminal, but\n # additionally to the file test-results/$BASENAME.out, too.\n case \"$GIT_TEST_TEE_STARTED, $* \" in\n-- \ngitgitgadget\n"},{"id":"363396","messageId":"20181114213851.GA2960@sigill.intra.peff.net","threadId":"49809","inReplyTo":"nycvar.QRO.7.76.6.1811141413170.39@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 2/5] tests: respect GIT_TEST_INSTALLED when initializing repositories","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-11-14T21:38:51Z","receivedAt":"2018-11-14T21:38:54Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 14, 2018 at 02:16:37PM +0100, Johannes Schindelin wrote:\n\n> > Makes perfect sense. Shouldn't we be asking where the template\n> > directory of the installed version is and using it instead of the\n> > freshly built one, no?\n> \n> It would make sense, but we don't know how to get that information, do we?\n> \n> $ git grep DEFAULT_GIT_TEMPLATE_DIR\n> Makefile: -DDEFAULT_GIT_TEMPLATE_DIR='\"$(template_dir_SQ)\"'\n> builtin/init-db.c:#ifndef DEFAULT_GIT_TEMPLATE_DIR\n> builtin/init-db.c:#define DEFAULT_GIT_TEMPLATE_DIR \"/usr/share/git-core/templates\"\n> builtin/init-db.c: template_dir = to_free = system_path(DEFAULT_GIT_TEMPLATE_DIR);\n> contrib/vscode/init.sh: '-DDEFAULT_GIT_TEMPLATE_DIR=\"$(template_dir_SQ)\"' \\\n> \n> In other words, the Makefile defines the DEFAULT_GIT_TEMPLATE_DIR, and the\n> only user in the code is init-db.c which uses it in copy_templates().\n> \n> And changing the code *now* to let us query Git where it thinks its\n> templates should be won't work, as this patch is about using the installed\n> Git (at whatever pre-compiled version that might be).\n\nDo we actually care where the templates are? I thought the point was to\noverride for the built Git to use the built templates instead of the\ninstalled one. For an installed Git, shouldn't we not be overriding the\ntemplates at all? I.e.:\n\n if test -n \"$GIT_TEST_INSTALLED\"\n then\n\t\"$GIT_TEST_INSTALLED/git\" init\n else\n\t\"$GIT_ExEC_PATH/git\" init --template=\"$GIT_BUILD_DIR/templates/blt\"\n fi >&3 2>&4\n\n(That's all leaving aside the question of whether we ought to be using a\nclean template dir instead of this).\n\n-Peff\n"},{"id":"363428","messageId":"nycvar.QRO.7.76.6.1811151329150.41@tvgsbejvaqbjf.bet","threadId":"49809","inReplyTo":"20181114213851.GA2960@sigill.intra.peff.net","subject":"Re: [PATCH 2/5] tests: respect GIT_TEST_INSTALLED when initializing repositories","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-11-15T12:29:58Z","receivedAt":"2018-11-15T12:30:25Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Wed, 14 Nov 2018, Jeff King wrote:\n\n> On Wed, Nov 14, 2018 at 02:16:37PM +0100, Johannes Schindelin wrote:\n> \n> > > Makes perfect sense. Shouldn't we be asking where the template\n> > > directory of the installed version is and using it instead of the\n> > > freshly built one, no?\n> > \n> > It would make sense, but we don't know how to get that information, do we?\n> > \n> > $ git grep DEFAULT_GIT_TEMPLATE_DIR\n> > Makefile: -DDEFAULT_GIT_TEMPLATE_DIR='\"$(template_dir_SQ)\"'\n> > builtin/init-db.c:#ifndef DEFAULT_GIT_TEMPLATE_DIR\n> > builtin/init-db.c:#define DEFAULT_GIT_TEMPLATE_DIR \"/usr/share/git-core/templates\"\n> > builtin/init-db.c: template_dir = to_free = system_path(DEFAULT_GIT_TEMPLATE_DIR);\n> > contrib/vscode/init.sh: '-DDEFAULT_GIT_TEMPLATE_DIR=\"$(template_dir_SQ)\"' \\\n> > \n> > In other words, the Makefile defines the DEFAULT_GIT_TEMPLATE_DIR, and the\n> > only user in the code is init-db.c which uses it in copy_templates().\n> > \n> > And changing the code *now* to let us query Git where it thinks its\n> > templates should be won't work, as this patch is about using the installed\n> > Git (at whatever pre-compiled version that might be).\n> \n> Do we actually care where the templates are? I thought the point was to\n> override for the built Git to use the built templates instead of the\n> installed one. For an installed Git, shouldn't we not be overriding the\n> templates at all? I.e.:\n> \n> if test -n \"$GIT_TEST_INSTALLED\"\n> then\n> \t\"$GIT_TEST_INSTALLED/git\" init\n> else\n> \t\"$GIT_ExEC_PATH/git\" init --template=\"$GIT_BUILD_DIR/templates/blt\"\n> fi >&3 2>&4\n> \n> (That's all leaving aside the question of whether we ought to be using a\n> clean template dir instead of this).\n\nI fear that that might buy us a ton of trouble. Just like we override the\nsystem config, we should override the templates.\n\nCiao,\nDscho\n"},{"id":"363430","messageId":"20181115124119.GA25562@sigill.intra.peff.net","threadId":"49809","inReplyTo":"nycvar.QRO.7.76.6.1811151329150.41@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 2/5] tests: respect GIT_TEST_INSTALLED when initializing repositories","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-11-15T12:41:20Z","receivedAt":"2018-11-15T12:41:23Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Nov 15, 2018 at 01:29:58PM +0100, Johannes Schindelin wrote:\n\n> > Do we actually care where the templates are? I thought the point was to\n> > override for the built Git to use the built templates instead of the\n> > installed one. For an installed Git, shouldn't we not be overriding the\n> > templates at all? I.e.:\n> > \n> > if test -n \"$GIT_TEST_INSTALLED\"\n> > then\n> > \t\"$GIT_TEST_INSTALLED/git\" init\n> > else\n> > \t\"$GIT_ExEC_PATH/git\" init --template=\"$GIT_BUILD_DIR/templates/blt\"\n> > fi >&3 2>&4\n> > \n> > (That's all leaving aside the question of whether we ought to be using a\n> > clean template dir instead of this).\n> \n> I fear that that might buy us a ton of trouble. Just like we override the\n> system config, we should override the templates.\n\nYes, it might. I guess it just seems plausible to me that somebody would\nexpect GIT_TEST_INSTALLED to be as close to the installed experience as\npossible. I dunno. I do not use it myself.\n\nAt any rate, my point was that for GIT_TEST_INSTALLED, either:\n\n 1. We can use a known-clean set of templates (either our local\n templates/blt, or an even-cleaner empty set).\n\nor\n\n 2. We do not need to specify any template, and it will just use\n whatever it came installed with.\n\nAnd in either case, we do not have to worry about asking it \"where are\nyour templates?\".\n\n-Peff\n"}]}