{"thread":{"id":"27643","subject":"[PATCH] t/Makefile: pass test opts to valgrind target properly","startedAt":"2011-06-17T08:29:57Z","lastAt":"2011-06-17T21:28:37Z","messageCount":3,"participants":["Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"170174","messageId":"20110617082956.GA20414@sigill.intra.peff.net","threadId":"27643","inReplyTo":null,"subject":"[PATCH] t/Makefile: pass test opts to valgrind target properly","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-06-17T08:29:57Z","receivedAt":"2011-06-17T08:29:57Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The valgrind target just reinvokes make with GIT_TEST_OPTS\nset to \"--valgrind\". However, it does this using an\nenvironment variable, which means GIT_TEST_OPTS in your\nconfig.mak would override it, and \"make valgrind\" would\nsimply run the test suite without valgrind on.\n\nInstead, we should pass GIT_TEST_OPTS on the command-line,\noverriding what's in config.mak, and take care to append to\nwhatever the user has there already.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nI'm attempting to run the whole test suite under valgrind for 1.7.6-rc2.\nIt's been a while since I've done a whole run, as there were some false\npositives in strspn on recent glibc which I've finally addressed.\n\nWe'll see what it comes up with later today. :)\n\n t/Makefile |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/t/Makefile b/t/Makefile\nindex 47cbeb6..9046ec9 100644\n--- a/t/Makefile\n+++ b/t/Makefile\n@@ -71,7 +71,7 @@ gitweb-test:\n \t$(MAKE) $(TGITWEB)\n \n valgrind:\n-\tGIT_TEST_OPTS=--valgrind $(MAKE)\n+\t$(MAKE) GIT_TEST_OPTS=\"$(GIT_TEST_OPTS) --valgrind\"\n \n # Smoke testing targets\n -include ../GIT-VERSION-FILE\n-- \n1.7.6.rc1.38.g97f64\n"},{"id":"170189","messageId":"20110617203632.GA27793@sigill.intra.peff.net","threadId":"27643","inReplyTo":"20110617082956.GA20414@sigill.intra.peff.net","subject":"[PATCH] tests: link shell libraries into valgrind directory","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-06-17T20:36:32Z","receivedAt":"2011-06-17T20:36:32Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jun 17, 2011 at 04:29:57AM -0400, Jeff King wrote:\n\n> I'm attempting to run the whole test suite under valgrind for 1.7.6-rc2.\n> It's been a while since I've done a whole run, as there were some false\n> positives in strspn on recent glibc which I've finally addressed.\n> \n> We'll see what it comes up with later today. :)\n\nIt took about 8 hours to run on my measly quad-core. The good news is\nthat there were no actual bugs. After fixing the strspn (and some\nstrcasestr) false positives at the valgrind level, the only patch I\nneeded to get every test script running successfully was:\n\n-- >8 --\nSubject: tests: link shell libraries into valgrind directory\n\nWhen we run tests under valgrind, we symlink anything\nexecutable that starts with git-* or test-* into a special\nvalgrind bin directory, and then make that our\nGIT_EXEC_PATH.\n\nHowever, shell libraries like git-sh-setup do not have the\nexecutable bit marked, and did not get symlinked.  This\nmeans that any test looking for shell libraries in our\nexec-path would fail to find them, even though that is a\nfine thing to do when testing against a regular git build\n(or in a git install, for that matter).\n\nt2300 demonstrated this problem. The fix is to symlink these\nshell libraries directly into the valgrind directory.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nAnother strategy would be to actually check the Makefile variables to\nsee what to symlink; but that's a pretty big change, and what we have\nnow works fine (with this exception).\n\n t/test-lib.sh |    9 +++++++--\n 1 files changed, 7 insertions(+), 2 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 64390d7..8c57a00 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -884,8 +884,13 @@ then\n \t}\n \n \tmake_valgrind_symlink () {\n-\t\t# handle only executables\n-\t\ttest -x \"$1\" || return\n+\t\t# handle only executables, unless they are shell libraries that\n+\t\t# need to be in the exec-path.  We will just use \"#!\" as a\n+\t\t# guess for a shell-script, since we have no idea what the user\n+\t\t# may have configured as the shell path.\n+\t\ttest -x \"$1\" ||\n+\t\ttest \"#!\" = \"$(head -c 2 <\"$1\")\" ||\n+\t\treturn;\n \n \t\tbase=$(basename \"$1\")\n \t\tsymlink_target=$GIT_BUILD_DIR/$base\n-- \n1.7.6.rc1.38.g97f64\n"},{"id":"170191","messageId":"7voc1wf8qi.fsf@alter.siamese.dyndns.org","threadId":"27643","inReplyTo":"20110617203632.GA27793@sigill.intra.peff.net","subject":"Re: [PATCH] tests: link shell libraries into valgrind directory","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-06-17T21:28:37Z","receivedAt":"2011-06-17T21:28:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks, will queue both.\n"}]}