{"thread":{"id":"17248","subject":"What's cooking in git.git (Jan 2009, #04; Mon, 19)","startedAt":"2009-01-19T09:13:30Z","lastAt":"2009-01-29T14:54:11Z","messageCount":73,"participants":["Junio C Hamano","Kjetil Barvik","Johannes Schindelin","Jeff King","Boyd Stephen Smith Jr.","Johannes Sixt","Thomas Rast","Linus Torvalds","Mark Brown","Mark Adler"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"101112","messageId":"7vbpu3r745.fsf@gitster.siamese.dyndns.org","threadId":"17248","inReplyTo":null,"subject":"What's cooking in git.git (Jan 2009, #04; Mon, 19)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-01-19T09:13:30Z","receivedAt":"2009-01-19T09:13:30Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Here are the topics that have been cooking. Commits prefixed with '-' are\nonly in 'pu' while commits prefixed with '+' are in 'next'. The ones\nmarked with '.' do not appear in any of the branches, but I am still\nholding onto them.\n\nThe topics list the commits in reverse chronological order. The topics\nmeant to be merged to the maintenance series have \"maint-\" in their names.\n\n----------------------------------------------------------------\n[New Topics]\n\n* jk/color-parse (Sat Jan 17 10:38:46 2009 -0500) 2 commits\n + expand --pretty=format color options\n + color: make it easier for non-config to parse color specs\n\n* sb/hook-cleanup (Sat Jan 17 04:02:55 2009 +0100) 5 commits\n + run_hook(): allow more than 9 hook arguments\n + run_hook(): check the executability of the hook before filling\n argv\n + api-run-command.txt: talk about run_hook()\n + Move run_hook() from builtin-commit.c into run-command.c (libgit)\n + checkout: don't crash on file checkout before running post-\n checkout hook\n\n* js/maint-all-implies-HEAD (Sat Jan 17 22:27:08 2009 -0800) 2 commits\n - bundle: allow the same ref to be given more than once\n - revision walker: include a detached HEAD in --all\n\n* tr/previous-branch (Sat Jan 17 19:08:12 2009 +0100) 6 commits\n - Fix parsing of @{-1}@{1}\n - interpret_nth_last_branch(): avoid traversing the reflog twice\n - checkout: implement \"-\" abbreviation, add docs and tests\n - sha1_name: support @{-N} syntax in get_sha1()\n - sha1_name: tweak @{-N} lookup\n - checkout: implement \"@{-N}\" shortcut name for N-th last branch\n\n* rs/ctype (Sat Jan 17 16:50:37 2009 +0100) 4 commits\n + Add is_regex_special()\n + Change NUL char handling of isspecial()\n + Reformat ctype.c\n + Add ctype test\n\n* mh/unify-color (Sun Jan 18 21:39:12 2009 +0100) 2 commits\n - move the color variables to color.c\n - handle color.ui at a central place\n\n* jf/am-failure-report (Sun Jan 18 19:34:31 2009 -0800) 2 commits\n + git-am: re-fix the diag message printing\n + git-am: Make it easier to see which patch failed\n\n* cb/add-pathspec (Wed Jan 14 15:54:35 2009 +0100) 2 commits\n - remove pathspec_match, use match_pathspec instead\n - clean up pathspec matching\n\n* sg/maint-gitdir-in-subdir (Fri Jan 16 16:37:33 2009 +0100) 1 commit\n + Fix gitdir detection when in subdir of gitdir\n\nThis has my \"don't do the fullpath if you are directly inside .git\"\nsquashed in, so it should be much safer.\n\n* am/maint-push-doc (Sun Jan 18 15:36:58 2009 +0100) 4 commits\n + Documentation: avoid using undefined parameters\n + Documentation: mention branches rather than heads\n + Documentation: remove a redundant elaboration\n + Documentation: git push repository can also be a remote\n\n* sp/runtime-prefix (Sun Jan 18 13:00:15 2009 +0100) 5 commits\n - Windows: Revert to default paths and convert them by\n RUNTIME_PREFIX\n - Modify setup_path() to only add git_exec_path() to PATH\n - Add calls to git_extract_argv0_path() in programs that call\n git_config_*\n - git_extract_argv0_path(): Move check for valid argv0 from caller\n to callee\n - Move computation of absolute paths from Makefile to runtime (in\n preparation for RUNTIME_PREFIX)\n\n----------------------------------------------------------------\n[Stalled and may need help and prodding to go forward]\n\n* jc/blame (Wed Jun 4 22:58:40 2008 -0700) 2 commits\n + blame: show \"previous\" information in --porcelain/--incremental\n format\n + git-blame: refactor code to emit \"porcelain format\" output\n\nThis gives Porcelains (like gitweb) the information on the commit _before_\nthe one that the final blame is laid on, which should save them one\nrev-parse to dig further. The line number in the \"previous\" information\nmay need refining, and sanity checking code for reference counting may\nneed to be resurrected before this can move forward.\n\n* db/foreign-scm (Sun Jan 11 15:12:10 2009 -0500) 3 commits\n - Support fetching from foreign VCSes\n - Add specification of git-vcs helpers\n - Add \"vcs\" config option in remotes\n\nThe \"spec\" did not seem quite well cooked yet, but in the longer term I\nthink something like this to allow interoperating with other SCMs as if\nthe other end is a native git repository is a very worthy goal.\n\n----------------------------------------------------------------\n[Actively cooking]\n\n* kb/lstat-cache (Sun Jan 18 16:14:54 2009 +0100) 5 commits\n + lstat_cache(): introduce clear_lstat_cache() function\n + lstat_cache(): introduce invalidate_lstat_cache() function\n + lstat_cache(): introduce has_dirs_only_path() function\n + lstat_cache(): introduce has_symlink_or_noent_leading_path()\n function\n + lstat_cache(): more cache effective symlink/directory detection\n\nThis is the tenth round, now in 'next'.\n\n* lh/submodule-tree-traversal (Mon Jan 12 00:45:55 2009 +0100) 3 commits\n - builtin-ls-tree: enable traversal of submodules\n - archive.c: enable traversal of submodules\n - tree.c: add support for traversal of submodules\n\nStill getting active reviews.\n\n* lt/maint-wrap-zlib (Wed Jan 7 19:54:47 2009 -0800) 1 commit\n + Wrap inflate and other zlib routines for better error reporting\n\nNeeds the \"free our memory upon seeing Z_MEM_ERROR and try again\" bits\nextracted from Shawn's patch on top of this one.\n\n* jk/signal-cleanup (Sun Jan 11 06:36:49 2009 -0500) 3 commits\n - pager: do wait_for_pager on signal death\n - refactor signal handling for cleanup functions\n - chain kill signals for cleanup functions\n\nSorry, I lost track. What is the status of this one?\n\n* js/diff-color-words (Sat Jan 17 17:29:48 2009 +0100) 7 commits\n - color-words: make regex configurable via attributes\n - color-words: expand docs with precise semantics\n - color-words: enable REG_NEWLINE to help user\n - color-words: take an optional regular expression describing words\n - color-words: change algorithm to allow for 0-character word\n boundaries\n - color-words: refactor word splitting and use ALLOC_GROW()\n - Add color_fwrite_lines(), a function coloring each line\n individually\n\nDscho's series that was done in response to Thomas's original; two agreed\nto work together on this codebase.\n\n* ks/maint-mailinfo-folded (Tue Jan 13 01:21:04 2009 +0300) 5 commits\n - mailinfo: tests for RFC2047 examples\n - mailinfo: add explicit test for mails like '\n (A U Thor)'\n - mailinfo: more smarter removal of rfc822 comments from 'From'\n + mailinfo: 'From:' header should be unfold as well\n + mailinfo: correctly handle multiline 'Subject:' header\n\nI think \"more smarter\" one is too aggressive for our purpose. Perhaps not\nremoving comments at all would be what we want.\n\n* js/patience-diff (Thu Jan 1 17:39:37 2009 +0100) 3 commits\n + bash completions: Add the --patience option\n + Introduce the diff option '--patience'\n + Implement the patience diff algorithm\n\n* js/notes (Tue Jan 13 20:57:16 2009 +0100) 6 commits\n + git-notes: fix printing of multi-line notes\n + notes: fix core.notesRef documentation\n + Add an expensive test for git-notes\n + Speed up git notes lookup\n + Add a script to edit/inspect notes\n + Introduce commit notes\n\n* sc/gitweb-category (Fri Dec 12 00:45:12 2008 +0100) 3 commits\n - gitweb: Optional grouping of projects by category\n - gitweb: Split git_project_list_body in two functions\n - gitweb: Modularized git_get_project_description to be more generic\n\n----------------------------------------------------------------\n[Graduated to \"master\"]\n\n* ds/uintmax-config (Mon Nov 3 09:14:28 2008 -0900) 1 commit\n + autoconf: Enable threaded delta search when pthreads are supported\n\nSee if anybody screams.\n\n* gb/gitweb-opml (Fri Jan 2 13:49:30 2009 +0100) 2 commits\n + gitweb: suggest name for OPML view\n + gitweb: don't use pathinfo for global actions\n\n* mv/apply-parse-opt (Fri Jan 9 22:21:36 2009 -0800) 2 commits\n + Resurrect \"git apply --flags -\" to read from the standard input\n + parse-opt: migrate builtin-apply.\n\n* tr/rebase-root (Fri Jan 2 23:28:29 2009 +0100) 4 commits\n + rebase: update documentation for --root\n + rebase -i: learn to rebase root commit\n + rebase: learn to rebase root commit\n + rebase -i: execute hook only after argument checking\n\nLooked reasonable.\n\n* mh/maint-commit-color-status (Thu Jan 8 19:53:05 2009 +0100) 2 commits\n + git-status -v: color diff output when color.ui is set\n + git-commit: color status output when color.ui is set\n\n* rs/maint-shortlog-foldline (Tue Jan 6 21:41:06 2009 +0100) 1 commit\n + shortlog: handle multi-line subjects like log --pretty=oneline et.\n al. do\n\n* rs/fgrep (Sat Jan 10 00:18:34 2009 +0100) 2 commits\n + grep: don't call regexec() for fixed strings\n + grep -w: forward to next possible position after rejected match\n\n* as/autocorrect-alias (Sun Jan 4 18:16:01 2009 +0100) 1 commit\n + git.c: make autocorrected aliases work\n\n* tr/maint-no-index-fixes (Wed Jan 7 12:15:30 2009 +0100) 3 commits\n + diff --no-index -q: fix endless loop\n + diff --no-index: test for pager after option parsing\n + diff: accept -- when using --no-index\n\n* jc/maint-format-patch (Sat Jan 10 12:41:33 2009 -0800) 1 commit\n + format-patch: show patch text for the root commit\n\n* ap/clone-into-empty (Sun Jan 11 15:19:12 2009 +0300) 2 commits\n + Allow cloning to an existing empty directory\n + add is_dot_or_dotdot inline function\n\n* gb/gitweb-patch (Thu Dec 18 08:13:19 2008 +0100) 4 commits\n + gitweb: link to patch(es) view in commit(diff) and (short)log view\n + gitweb: add patches view\n + gitweb: change call pattern for git_commitdiff\n + gitweb: add patch view\n\n----------------------------------------------------------------\n[Will merge to \"master\" soon]\n\n* kb/am-directory (Wed Jan 14 16:29:59 2009 -0800) 2 commits\n + git-am: fix shell quoting\n + git-am: add --directory= option\n\nThis is \"third-time-lucky, perhaps?\" resurrection. I do not think I'd be\nusing this very often, but it originated from a real user request.\n\n* jc/maint-format-patch-o-relative (Mon Jan 12 15:18:02 2009 -0800) 1 commit\n + Teach format-patch to handle output directory relative to cwd\n\n----------------------------------------------------------------\n[On Hold]\n\n* jk/renamelimit (Sat May 3 13:58:42 2008 -0700) 1 commit\n . diff: enable \"too large a rename\" warning when -M/-C is explicitly\n asked for\n\n* jc/stripspace (Sun Mar 9 00:30:35 2008 -0800) 6 commits\n . git-am --forge: add Signed-off-by: line for the author\n . git-am: clean-up Signed-off-by: lines\n . stripspace: add --log-clean option to clean up signed-off-by:\n lines\n . stripspace: use parse_options()\n . Add \"git am -s\" test\n . git-am: refactor code to add signed-off-by line for the committer\n\n* jc/post-simplify (Fri Aug 15 01:34:51 2008 -0700) 2 commits\n . revision --simplify-merges: incremental simplification\n . revision --simplify-merges: prepare for incremental simplification\n\n* jk/valgrind (Thu Oct 23 04:30:45 2008 +0000) 2 commits\n . valgrind: ignore ldso errors\n . add valgrind support in test scripts\n\n* wp/add-patch-find (Thu Nov 27 04:08:03 2008 +0000) 3 commits\n . In add --patch, Handle K,k,J,j slightly more gracefully.\n . Add / command in add --patch\n . git-add -i/-p: Change prompt separater from slash to comma\n\n* jc/grafts (Wed Jul 2 17:14:12 2008 -0700) 1 commit\n . [BROKEN wrt shallow clones] Ignore graft during object transfer\n\n* jc/replace (Fri Oct 31 09:21:39 2008 -0700) 1 commit\n . WIP\n"},{"id":"101124","messageId":"864ozvv7d3.fsf@broadpark.no","threadId":"17248","inReplyTo":"7vbpu3r745.fsf@gitster.siamese.dyndns.org","subject":"Re: What's cooking in git.git (Jan 2009, #04; Mon, 19)","fromName":"Kjetil Barvik","fromEmail":"barvik@broadpark.no","sentAt":"2009-01-19T11:54:32Z","receivedAt":"2009-01-19T11:54:32Z","isPatch":false,"sender":{"key":"barvik@broadpark.no","avatar":null},"body":"Junio C Hamano writes:\n\n> ----------------------------------------------------------------\n> [Actively cooking]\n>\n> * kb/lstat-cache (Sun Jan 18 16:14:54 2009 +0100) 5 commits\n> + lstat_cache(): introduce clear_lstat_cache() function\n> + lstat_cache(): introduce invalidate_lstat_cache() function\n> + lstat_cache(): introduce has_dirs_only_path() function\n> + lstat_cache(): introduce has_symlink_or_noent_leading_path()\n> function\n> + lstat_cache(): more cache effective symlink/directory detection\n>\n> This is the tenth round, now in 'next'.\n\n Thanks!! Nice to see that the patch is going forward. And I have to\n admit that it was very fun to make that patch.\n\n How long is the 'merge window' in Linux Kernel terms for this round\n (to the next release of GIT)?\n\n I have a second idea to an improvement, which also looks quite good\n for the moment, and I am sort of wondering how fast I must work. :-)\n\n -- kjetil\n"},{"id":"101131","messageId":"alpine.DEB.1.00.0901191407470.3586@pacific.mpi-cbg.de","threadId":"17248","inReplyTo":"7vbpu3r745.fsf@gitster.siamese.dyndns.org","subject":"Re: What's cooking in git.git (Jan 2009, #04; Mon, 19)","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-19T13:08:48Z","receivedAt":"2009-01-19T13:08:48Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 19 Jan 2009, Junio C Hamano wrote:\n\n> * js/diff-color-words (Sat Jan 17 17:29:48 2009 +0100) 7 commits\n> - color-words: make regex configurable via attributes\n> - color-words: expand docs with precise semantics\n> - color-words: enable REG_NEWLINE to help user\n> - color-words: take an optional regular expression describing words\n> - color-words: change algorithm to allow for 0-character word\n> boundaries\n> - color-words: refactor word splitting and use ALLOC_GROW()\n> - Add color_fwrite_lines(), a function coloring each line\n> individually\n> \n> Dscho's series that was done in response to Thomas's original; two agreed\n> to work together on this codebase.\n\nI am actually pretty comfortable with this series now.\n\n> * jk/valgrind (Thu Oct 23 04:30:45 2008 +0000) 2 commits\n> . valgrind: ignore ldso errors\n> . add valgrind support in test scripts\n\nCould you put this in pu, at least, please?\n\nThanks,\nDscho\n"},{"id":"101132","messageId":"alpine.DEB.1.00.0901191408540.3586@pacific.mpi-cbg.de","threadId":"17248","inReplyTo":"864ozvv7d3.fsf@broadpark.no","subject":"Re: What's cooking in git.git (Jan 2009, #04; Mon, 19)","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-19T13:09:48Z","receivedAt":"2009-01-19T13:09:48Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 19 Jan 2009, Kjetil Barvik wrote:\n\n\n> How long is the 'merge window' in Linux Kernel terms for this round \n> (to the next release of GIT)?\n\nThere is no \"merge window\", but rather an \"-rc\" period for 1.x versions. \nWhich has been largely ignored :-)\n\nCiao,\nDscho\n"},{"id":"101213","messageId":"20090120043030.GD30714@sigill.intra.peff.net","threadId":"17248","inReplyTo":"7vbpu3r745.fsf@gitster.siamese.dyndns.org","subject":"Re: What's cooking in git.git (Jan 2009, #04; Mon, 19)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-01-20T04:30:30Z","receivedAt":"2009-01-20T04:30:30Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jan 19, 2009 at 01:13:30AM -0800, Junio C Hamano wrote:\n\n> * jk/color-parse (Sat Jan 17 10:38:46 2009 -0500) 2 commits\n> + expand --pretty=format color options\n> + color: make it easier for non-config to parse color specs\n\nI posted a revised version of 1/2 based on René's work, but it looks\nlike you have the original. So here it is on top of what's in next.\n\n-- >8 --\nFrom: René Scharfe \n\noptimize color_parse_mem\n\nCommit 5ef8d77a implemented color_parse_mem, a function for\nparsing colors from a non-NUL-terminated string, by simply\nallocating a new NUL-terminated string and calling\ncolor_parse. This had a small but measurable speed impact on\na user format that used the advanced color parsing. E.g.,\n\n # uses quick parsing\n $ time ./git log --pretty=tformat:'%Credfoo%Creset' >/dev/null\n real 0m0.673s\n user 0m0.652s\n sys 0m0.016s\n\n # uses color_parse_mem\n $ time ./git log --pretty=tformat:'%C(red)foo%C(reset)' >/dev/null\n real 0m0.692s\n user 0m0.660s\n sys 0m0.032s\n\nThis patch implements color_parse_mem as the primary\nfunction, with color_parse as a wrapper for strings. This\ngives comparable timings to the first case above.\n\nOriginal patch by René. Commit message and debugging by Jeff\nKing.\n\nSigned-off-by: Jeff King \n---\n color.c | 38 +++++++++++++++++++++-----------------\n 1 files changed, 21 insertions(+), 17 deletions(-)\n\ndiff --git a/color.c b/color.c\nindex 54a3da1..915d7a9 100644\n--- a/color.c\n+++ b/color.c\n@@ -41,29 +41,40 @@ static int parse_attr(const char *name, int len)\n \n void color_parse(const char *value, const char *var, char *dst)\n {\n+\tcolor_parse_mem(value, strlen(value), var, dst);\n+}\n+\n+void color_parse_mem(const char *value, int value_len, const char *var,\n+\t\tchar *dst)\n+{\n \tconst char *ptr = value;\n+\tint len = value_len;\n \tint attr = -1;\n \tint fg = -2;\n \tint bg = -2;\n \n-\tif (!strcasecmp(value, \"reset\")) {\n+\tif (!strncasecmp(value, \"reset\", len)) {\n \t\tstrcpy(dst, \"\\033[m\");\n \t\treturn;\n \t}\n \n \t/* [fg [bg]] [attr] */\n-\twhile (*ptr) {\n+\twhile (len > 0) {\n \t\tconst char *word = ptr;\n-\t\tint val, len = 0;\n+\t\tint val, wordlen = 0;\n \n-\t\twhile (word[len] && !isspace(word[len]))\n-\t\t\tlen++;\n+\t\twhile (len > 0 && !isspace(word[wordlen])) {\n+\t\t\twordlen++;\n+\t\t\tlen--;\n+\t\t}\n \n-\t\tptr = word + len;\n-\t\twhile (*ptr && isspace(*ptr))\n+\t\tptr = word + wordlen;\n+\t\twhile (len > 0 && isspace(*ptr)) {\n \t\t\tptr++;\n+\t\t\tlen--;\n+\t\t}\n \n-\t\tval = parse_color(word, len);\n+\t\tval = parse_color(word, wordlen);\n \t\tif (val >= -1) {\n \t\t\tif (fg == -2) {\n \t\t\t\tfg = val;\n@@ -75,7 +86,7 @@ void color_parse(const char *value, const char *var, char *dst)\n \t\t\t}\n \t\t\tgoto bad;\n \t\t}\n-\t\tval = parse_attr(word, len);\n+\t\tval = parse_attr(word, wordlen);\n \t\tif (val < 0 || attr != -1)\n \t\t\tgoto bad;\n \t\tattr = val;\n@@ -115,7 +126,7 @@ void color_parse(const char *value, const char *var, char *dst)\n \t*dst = 0;\n \treturn;\n bad:\n-\tdie(\"bad color value '%s' for variable '%s'\", value, var);\n+\tdie(\"bad color value '%.*s' for variable '%s'\", value_len, value, var);\n }\n \n int git_config_colorbool(const char *var, const char *value, int stdout_is_tty)\n@@ -191,10 +202,3 @@ int color_fprintf_ln(FILE *fp, const char *color, const char *fmt, ...)\n \tva_end(args);\n \treturn r;\n }\n-\n-void color_parse_mem(const char *value, int len, const char *var, char *dst)\n-{\n-\tchar *tmp = xmemdupz(value, len);\n-\tcolor_parse(tmp, var, dst);\n-\tfree(tmp);\n-}\n-- \n1.6.1.335.g0366b.dirty\n"},{"id":"101214","messageId":"20090120044021.GE30714@sigill.intra.peff.net","threadId":"17248","inReplyTo":"7vbpu3r745.fsf@gitster.siamese.dyndns.org","subject":"Re: What's cooking in git.git (Jan 2009, #04; Mon, 19)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-01-20T04:40:21Z","receivedAt":"2009-01-20T04:40:21Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jan 19, 2009 at 01:13:30AM -0800, Junio C Hamano wrote:\n\n> * jk/signal-cleanup (Sun Jan 11 06:36:49 2009 -0500) 3 commits\n> - pager: do wait_for_pager on signal death\n> - refactor signal handling for cleanup functions\n> - chain kill signals for cleanup functions\n> \n> Sorry, I lost track. What is the status of this one?\n\nI need to clean up and re-send. The three improvements needed are:\n\n - there is a related Windows cleanup from JSixt, which I will send\n when I re-post\n\n - the test needs a few tweaks to be portable to Windows\n\n - Some of the signal handlers should be guarded from inserting\n themselves multiple times. I don't think any are dangerous to run\n twice (they generally traverse a list, cleaning up files, and then\n remove the list elements), but I'm not sure that you can't get some\n stupid behavior, like inserting one handler per diff'd file, which\n will unnecessarily allocate memory.\n\nThis series fixes pager handling for interrupted git programs. There is\nalso a related fix that needs to be done for forked git programs. I\nposted a \"how about this\" patch to use run_command for external git\nprograms, but it has some serious problems (\"git bogus\" no longer\nreports an error!).\n\nI have unfortunately not had very much git time lately, but I'll try to\ncome up with something for both cases this week.\n\n-Peff\n"},{"id":"101215","messageId":"20090120044447.GF30714@sigill.intra.peff.net","threadId":"17248","inReplyTo":"alpine.DEB.1.00.0901191407470.3586@pacific.mpi-cbg.de","subject":"Re: What's cooking in git.git (Jan 2009, #04; Mon, 19)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-01-20T04:44:47Z","receivedAt":"2009-01-20T04:44:47Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jan 19, 2009 at 02:08:48PM +0100, Johannes Schindelin wrote:\n\n> > * jk/valgrind (Thu Oct 23 04:30:45 2008 +0000) 2 commits\n> > . valgrind: ignore ldso errors\n> > . add valgrind support in test scripts\n> \n> Could you put this in pu, at least, please?\n\nI don't think I've really touched this since it was posted. One of the\nthings I didn't like about it was that the valgrind wrapper directory\nwas created in the Makefile. I think creating it inside the trash\ndirectory for each test run that wants to use valgrind makes more sense\n(probably as .git/valgrind, which is unlikely to hurt anything but will\nstay out of the way of most of the tests).\n\nI doubt I will have the chance to look at it anytime soon, so please\nfeel free to pick up the topic if you are interested.\n\n-Peff\n"},{"id":"101216","messageId":"200901192317.23079.bss@iguanasuicide.net","threadId":"17248","inReplyTo":"7vbpu3r745.fsf@gitster.siamese.dyndns.org","subject":"Re: What's cooking in git.git (Jan 2009, #04; Mon, 19)","fromName":"Boyd Stephen Smith Jr.","fromEmail":"bss@iguanasuicide.net","sentAt":"2009-01-20T05:17:13Z","receivedAt":"2009-01-20T05:17:13Z","isPatch":false,"sender":{"key":"bss@iguanasuicide.net","avatar":"https://gravatar.com/avatar/84b95eeff194b816c1568b1339e63e4b229825298664a9037b9f1ec713ead1e3?d=mp&s=160"},"body":"On Monday 19 January 2009, Junio C Hamano wrote \nabout 'What's cooking in git.git (Jan 2009, #04; Mon, 19)':\n>Here are the topics that have been cooking. Commits prefixed with '-'\n> are only in 'pu' while commits prefixed with '+' are in 'next'. The\n> ones marked with '.' do not appear in any of the branches, but I am\n> still holding onto them.\n\nIs there anywhere you are publishing these refs? Of course, I see the \ncommits in 'pu', but sometimes I would like to merge something you have \nin 'next'/'pu' into a branch based on 'master' or one of my local \nbranches, and I have to go hunting for the commit SHA.\n\nIt's not a big deal: qgit, gitk, and 'git log'+grep all solve the issue \nquickly enough, and I don't want to add to your workload. I was just \nhoping they were already published and I could simply add a remote to my \nconfig to get them.\n\nCurrently, I'm just using:\n* remote origin\n URL: git://git.kernel.org/pub/scm/git/git.git\n Remote branch merged with 'git pull' while on branch master\n master\n Tracked remote branches\n html maint man master next pu todo\nand I get this:\n$ git pull origin jk/color-parse\nfatal: Couldn't find remote ref jk/color-parse\n-- \nBoyd Stephen Smith Jr. ,= ,-_-. =. \nbss@iguanasuicide.net ((_/)o o(\\_))\nICQ: 514984 YM/AIM: DaTwinkDaddy `-'(. .)`-' \nhttp://iguanasuicide.net/ \\_/ \n"},{"id":"101226","messageId":"7vfxjelap4.fsf@gitster.siamese.dyndns.org","threadId":"17248","inReplyTo":"20090120044021.GE30714@sigill.intra.peff.net","subject":"Re: What's cooking in git.git (Jan 2009, #04; Mon, 19)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-01-20T07:04:55Z","receivedAt":"2009-01-20T07:04:55Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Mon, Jan 19, 2009 at 01:13:30AM -0800, Junio C Hamano wrote:\n>\n>> * jk/signal-cleanup (Sun Jan 11 06:36:49 2009 -0500) 3 commits\n>> - pager: do wait_for_pager on signal death\n>> - refactor signal handling for cleanup functions\n>> - chain kill signals for cleanup functions\n>> \n>> Sorry, I lost track. What is the status of this one?\n>\n> I need to clean up and re-send. The three improvements needed are:\n> ...\n\nThanks.\n"},{"id":"101230","messageId":"49758367.7040706@viscovery.net","threadId":"17248","inReplyTo":"20090120044021.GE30714@sigill.intra.peff.net","subject":"Re: What's cooking in git.git (Jan 2009, #04; Mon, 19)","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2009-01-20T07:55:19Z","receivedAt":"2009-01-20T07:55:19Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Jeff King schrieb:\n> - the test needs a few tweaks to be portable to Windows\n\nWhile this is true, the workaround I have in my tree is so ugly that its\ndiscussion would hold back this series unnecessarily. So, please don't\nwait for the fixup of the test.\n\n[My intention is to send test suite fixups for Windows as a separate\nseries, which would include the fixup for this case, too.]\n\n-- Hannes\n"},{"id":"101235","messageId":"200901200957.54603.trast@student.ethz.ch","threadId":"17248","inReplyTo":"200901192317.23079.bss@iguanasuicide.net","subject":"Re: What's cooking in git.git (Jan 2009, #04; Mon, 19)","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2009-01-20T08:57:50Z","receivedAt":"2009-01-20T08:57:50Z","isPatch":false,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Boyd Stephen Smith Jr. wrote:\n> Is there anywhere you are publishing these refs? Of course, I see the \n> commits in 'pu', but sometimes I would like to merge something you have \n> in 'next'/'pu' into a branch based on 'master' or one of my local \n> branches, and I have to go hunting for the commit SHA.\n[...]\n> $ git pull origin jk/color-parse\n> fatal: Couldn't find remote ref jk/color-parse\n\nYou could try the script I posted here:\n\n http://article.gmane.org/gmane.comp.version-control.git/106129\n\nJust 'git resurrect -m jk/color-parse' and you should be good to go.\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"101250","messageId":"alpine.DEB.1.00.0901201447290.5159@intel-tinevez-2-302","threadId":"17248","inReplyTo":"20090120044447.GF30714@sigill.intra.peff.net","subject":"valgrind patches, was Re: What's cooking in git.git (Jan 2009, #04; Mon, 19)","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-20T13:51:49Z","receivedAt":"2009-01-20T13:51:49Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 19 Jan 2009, Jeff King wrote:\n\n> One of the things I didn't like about it was that the valgrind wrapper \n> directory was created in the Makefile.\n\nI agree.\n\n> I think creating it inside the trash directory for each test run that \n> wants to use valgrind makes more sense (probably as .git/valgrind, which \n> is unlikely to hurt anything but will stay out of the way of most of the \n> tests).\n\nHere I disagree. But I think that test-lib.sh should create it on-demand, \nand it should traverse all executables in all paths listed in $PATH, \nreplacing the ones that start with \"git-\" (\"git\" itself should be the \nfirst one) that are no scripts by symlinks to the valgrind script (which \nshould therefore live in t/), and those that _are_ scripts by symlinks to \n$GIT_ROOT/$NAME.\n\nI'll work on it.\n\nCiao,\nDscho\n"},{"id":"101251","messageId":"20090120141810.GA10688@sigill.intra.peff.net","threadId":"17248","inReplyTo":"49758367.7040706@viscovery.net","subject":"Re: What's cooking in git.git (Jan 2009, #04; Mon, 19)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-01-20T14:18:10Z","receivedAt":"2009-01-20T14:18:10Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jan 20, 2009 at 08:55:19AM +0100, Johannes Sixt wrote:\n\n> > - the test needs a few tweaks to be portable to Windows\n> \n> While this is true, the workaround I have in my tree is so ugly that its\n> discussion would hold back this series unnecessarily. So, please don't\n> wait for the fixup of the test.\n\nMy goal was to just accept multiple exit codes in the test. I'll cc you\nwhen I send out the new one, and you can comment.\n\n-Peff\n"},{"id":"101252","messageId":"20090120141932.GB10688@sigill.intra.peff.net","threadId":"17248","inReplyTo":"alpine.DEB.1.00.0901201447290.5159@intel-tinevez-2-302","subject":"Re: valgrind patches, was Re: What's cooking in git.git (Jan 2009, #04; Mon, 19)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-01-20T14:19:32Z","receivedAt":"2009-01-20T14:19:32Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jan 20, 2009 at 02:51:49PM +0100, Johannes Schindelin wrote:\n\n> > I think creating it inside the trash directory for each test run that \n> > wants to use valgrind makes more sense (probably as .git/valgrind, which \n> > is unlikely to hurt anything but will stay out of the way of most of the \n> > tests).\n> \n> Here I disagree. But I think that test-lib.sh should create it on-demand, \n> and it should traverse all executables in all paths listed in $PATH, \n> replacing the ones that start with \"git-\" (\"git\" itself should be the \n> first one) that are no scripts by symlinks to the valgrind script (which \n> should therefore live in t/), and those that _are_ scripts by symlinks to \n> $GIT_ROOT/$NAME.\n\nHow will you deal with race conditions between two simultaneously\nrunning scripts? I.e., where are you going to put it?\n\n> I'll work on it.\n\nGreat.\n\n-Peff\n"},{"id":"101255","messageId":"alpine.DEB.1.00.0901201545570.5159@intel-tinevez-2-302","threadId":"17248","inReplyTo":"20090120141932.GB10688@sigill.intra.peff.net","subject":"Re: valgrind patches, was Re: What's cooking in git.git (Jan 2009, #04; Mon, 19)","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-20T14:50:28Z","receivedAt":"2009-01-20T14:50:28Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 20 Jan 2009, Jeff King wrote:\n\n> On Tue, Jan 20, 2009 at 02:51:49PM +0100, Johannes Schindelin wrote:\n> \n> > > I think creating it inside the trash directory for each test run \n> > > that wants to use valgrind makes more sense (probably as \n> > > .git/valgrind, which is unlikely to hurt anything but will stay out \n> > > of the way of most of the tests).\n> > \n> > Here I disagree. But I think that test-lib.sh should create it \n> > on-demand, and it should traverse all executables in all paths listed \n> > in $PATH, replacing the ones that start with \"git-\" (\"git\" itself \n> > should be the first one) that are no scripts by symlinks to the \n> > valgrind script (which should therefore live in t/), and those that \n> > _are_ scripts by symlinks to $GIT_ROOT/$NAME.\n> \n> How will you deal with race conditions between two simultaneously \n> running scripts? I.e., where are you going to put it?\n\nThere are no race conditions, as for every git executable, a symbolic link \nis created, pointing to the valgrind.sh script [*1*].\n\nBesides, what with valgrind being a memory hog, you'd be nuts to call \nvalgrinded scripts simultaneously.\n\nCiao,\nDscho\n\n[*1*] Before anybody complains about symbolic links not being available on \nWindows, or $GIT_SHELL not being heeded by the valgrind.sh script: get \nvalgrind to compile on those platforms, and _then_ we'll talk again.\n"},{"id":"101257","messageId":"alpine.DEB.1.00.0901201602410.5159@intel-tinevez-2-302","threadId":"17248","inReplyTo":"alpine.DEB.1.00.0901201545570.5159@intel-tinevez-2-302","subject":"[PATCH 1/2] Add valgrind support in test scripts","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-20T15:04:28Z","receivedAt":"2009-01-20T15:04:28Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nThis patch adds the ability to use valgrind's memcheck tool to\ndiagnose memory problems in git while running the test scripts. It\nworks by placing a \"fake\" git in the front of the test script's PATH;\nthis fake git runs the real git under valgrind. It also points the\nexec-path such that any stand-alone dashed git programs are run using\nthe same script. In this way we avoid having to modify the actual git\ncode in any way.\n\nTo be certain that every call to any git executable is intercepted,\nthe PATH is searched for executables beginning with \"git-\"; Scripts\nare excluded however.\n\nValgrind can be used by specifying \"GIT_TEST_OPTS=--valgrind\" in the\nmake invocation. Any invocation of git that finds any errors under\nvalgrind will exit with failure code 126. Any valgrind output will go\nto the usual stderr channel for tests (i.e., /dev/null, unless -v has\nbeen specified).\n\nIf you need to pass options to valgrind -- you might want to run\nanother tool than memcheck, for example -- you can set the environment\nvariable GIT_VALGRIND_OPTIONS.\n\nA few default suppressions are included, since libz seems to\ntrigger quite a few false positives. We'll assume that libz\nworks and that we can ignore any errors which are reported\nthere.\n\nInitial patch and all the hard work by Jeff King.\n\nSigned-off-by: Johannes Schindelin \n---\n\n\tAFAIR libz reserves only aligned memory, and does all operations \n\tin an aligned manner, but it is safe (even if it accesses \n\tuninitialized memory, it does not use the results anyway).\n\n t/test-lib.sh | 39 +++++++++++++++++++++++++++++++++++++--\n t/valgrind/.gitignore | 2 ++\n t/valgrind/default.supp | 21 +++++++++++++++++++++\n t/valgrind/valgrind.sh | 12 ++++++++++++\n 4 files changed, 72 insertions(+), 2 deletions(-)\n create mode 100644 t/valgrind/.gitignore\n create mode 100644 t/valgrind/default.supp\n create mode 100755 t/valgrind/valgrind.sh\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 41d5a59..1daae9b 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -94,6 +94,8 @@ do\n \t--no-python)\n \t\t# noop now...\n \t\tshift ;;\n+\t--va|--val|--valg|--valgr|--valgri|--valgrin|--valgrind)\n+\t\tvalgrind=t; shift ;;\n \t*)\n \t\tbreak ;;\n \tesac\n@@ -467,8 +469,41 @@ 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-PATH=$TEST_DIRECTORY/..:$PATH\n-GIT_EXEC_PATH=$(pwd)/..\n+if test -z \"$valgrind\"\n+then\n+\tPATH=$TEST_DIRECTORY/..:$PATH\n+\tGIT_EXEC_PATH=$TEST_DIRECTORY/..\n+else\n+\t# override all git executables in PATH and TEST_DIRECTORY/..\n+\tGIT_VALGRIND=$TEST_DIRECTORY/valgrind\n+\tmkdir -p \"$GIT_VALGRIND\"\n+\tOLDIFS=$IFS\n+\tIFS=:\n+\tfor path in $PATH:$TEST_DIRECTORY/..\n+\tdo\n+\t\tls \"$TEST_DIRECTORY\"/../git \"$path\"/git-* 2> /dev/null |\n+\t\twhile read file\n+\t\tdo\n+\t\t\t# handle only executables\n+\t\t\ttest -x \"$file\" || continue\n+\n+\t\t\tbase=$(basename \"$file\")\n+\t\t\ttest ! -h \"$GIT_VALGRIND\"/\"$base\" || continue\n+\n+\t\t\tif test \"#!\" = \"$(head -c 2 < \"$file\")\"\n+\t\t\tthen\n+\t\t\t\t# do not override scripts\n+\t\t\t\tln -s ../../\"$base\" \"$GIT_VALGRIND\"/\"$base\"\n+\t\t\telse\n+\t\t\t\tln -s valgrind.sh \"$GIT_VALGRIND\"/\"$base\"\n+\t\t\tfi\n+\t\tdone\n+\tdone\n+\tIFS=$OLDIFS\n+\tPATH=$GIT_VALGRIND:$PATH\n+\tGIT_EXEC_PATH=$GIT_VALGRIND\n+\texport GIT_VALGRIND\n+fi\n GIT_TEMPLATE_DIR=$(pwd)/../templates/blt\n unset GIT_CONFIG\n GIT_CONFIG_NOSYSTEM=1\ndiff --git a/t/valgrind/.gitignore b/t/valgrind/.gitignore\nnew file mode 100644\nindex 0000000..d781a63\n--- /dev/null\n+++ b/t/valgrind/.gitignore\n@@ -0,0 +1,2 @@\n+/git\n+/git-*\ndiff --git a/t/valgrind/default.supp b/t/valgrind/default.supp\nnew file mode 100644\nindex 0000000..2482b3b\n--- /dev/null\n+++ b/t/valgrind/default.supp\n@@ -0,0 +1,21 @@\n+{\n+\tignore-zlib-errors-cond\n+\tMemcheck:Cond\n+\tobj:*libz.so*\n+}\n+\n+{\n+\tignore-zlib-errors-value4\n+\tMemcheck:Value4\n+\tobj:*libz.so*\n+}\n+\n+{\n+\twriting-data-from-zlib-triggers-errors\n+\tMemcheck:Param\n+\twrite(buf)\n+\tobj:/lib/ld-*.so\n+\tfun:write_in_full\n+\tfun:write_buffer\n+\tfun:write_loose_object\n+}\ndiff --git a/t/valgrind/valgrind.sh b/t/valgrind/valgrind.sh\nnew file mode 100755\nindex 0000000..24f3a4e\n--- /dev/null\n+++ b/t/valgrind/valgrind.sh\n@@ -0,0 +1,12 @@\n+#!/bin/sh\n+\n+base=$(basename \"$0\")\n+\n+exec valgrind -q --error-exitcode=126 \\\n+\t--leak-check=no \\\n+\t--suppressions=\"$GIT_VALGRIND/default.supp\" \\\n+\t--gen-suppressions=all \\\n+\t--log-fd=4 \\\n+\t--input-fd=4 \\\n+\t$GIT_VALGRIND_OPTIONS \\\n+\t\"$GIT_VALGRIND\"/../../\"$base\" \"$@\"\n-- \n1.6.1.243.g6c8bb35\n"},{"id":"101258","messageId":"alpine.DEB.1.00.0901201604530.5159@intel-tinevez-2-302","threadId":"17248","inReplyTo":"alpine.DEB.1.00.0901201602410.5159@intel-tinevez-2-302","subject":"[PATCH 2/2] valgrind: ignore ldso errors","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-20T15:05:03Z","receivedAt":"2009-01-20T15:05:03Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nFrom: Jeff King \n\nOn some Linux systems, we get a host of Cond and Addr errors\nfrom calls to dlopen that are caused by nss modules. We\nshould be able to safely ignore anything happening in\nld-*.so as \"not our problem.\"\n\nSigned-off-by: Jeff King \nSigned-off-by: Junio C Hamano \n---\n\n\tThis is Peff's patch, unchanged.\n\n t/valgrind/default.supp | 12 ++++++++++++\n 1 files changed, 12 insertions(+), 0 deletions(-)\n\ndiff --git a/t/valgrind/default.supp b/t/valgrind/default.supp\nindex 2482b3b..1013847 100644\n--- a/t/valgrind/default.supp\n+++ b/t/valgrind/default.supp\n@@ -11,6 +11,18 @@\n }\n \n {\n+\tignore-ldso-cond\n+\tMemcheck:Cond\n+\tobj:*ld-*.so\n+}\n+\n+{\n+\tignore-ldso-addr8\n+\tMemcheck:Addr8\n+\tobj:*ld-*.so\n+}\n+\n+{\n \twriting-data-from-zlib-triggers-errors\n \tMemcheck:Param\n \twrite(buf)\n-- \n1.6.1.243.g6c8bb35\n"},{"id":"101315","messageId":"20090120232439.GA17746@coredump.intra.peff.net","threadId":"17248","inReplyTo":"alpine.DEB.1.00.0901201545570.5159@intel-tinevez-2-302","subject":"Re: valgrind patches, was Re: What's cooking in git.git (Jan 2009, #04; Mon, 19)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-01-20T23:24:39Z","receivedAt":"2009-01-20T23:24:39Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jan 20, 2009 at 03:50:28PM +0100, Johannes Schindelin wrote:\n\n> > How will you deal with race conditions between two simultaneously \n> > running scripts? I.e., where are you going to put it?\n> \n> There are no race conditions, as for every git executable, a symbolic link \n> is created, pointing to the valgrind.sh script [*1*].\n\nHmm. I suppose that would work, since every test run is trying to create\nthe same state.\n\n> Besides, what with valgrind being a memory hog, you'd be nuts to call \n> valgrinded scripts simultaneously.\n\nI have to disagree there. I think there are two obvious usage patterns:\n\n - run script $X specifically under valgrind to track down a bug\n\n - run the whole test suite under valgrind occasionally to find\n latent bugs that wouldn't otherwise show up\n\nIn the latter, you want a pretty beefy box. When I did the original\npatches, I ran through the whole test suite under valgrind. It took\nseveral hours on a 6GB quad-core box, using \"-j4\". I would hate for it\nto have taken an entire day. :)\n\n-Peff\n"},{"id":"101321","messageId":"alpine.DEB.1.00.0901210105470.19014@racer","threadId":"17248","inReplyTo":"20090120232439.GA17746@coredump.intra.peff.net","subject":"Re: valgrind patches, was Re: What's cooking in git.git (Jan 2009, #04; Mon, 19)","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-21T00:10:22Z","receivedAt":"2009-01-21T00:10:22Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 20 Jan 2009, Jeff King wrote:\n\n> On Tue, Jan 20, 2009 at 03:50:28PM +0100, Johannes Schindelin wrote:\n> \n> > > How will you deal with race conditions between two simultaneously \n> > > running scripts? I.e., where are you going to put it?\n> > \n> > There are no race conditions, as for every git executable, a symbolic \n> > link is created, pointing to the valgrind.sh script [*1*].\n> \n> Hmm. I suppose that would work, since every test run is trying to create \n> the same state.\n\nYep, that's what I meant with \"no race\".\n\n> > Besides, what with valgrind being a memory hog, you'd be nuts to call \n> > valgrinded scripts simultaneously.\n> \n> I have to disagree there. I think there are two obvious usage patterns:\n> \n> - run script $X specifically under valgrind to track down a bug\n> \n> - run the whole test suite under valgrind occasionally to find\n> latent bugs that wouldn't otherwise show up\n> \n> In the latter, you want a pretty beefy box. When I did the original\n> patches, I ran through the whole test suite under valgrind. It took\n> several hours on a 6GB quad-core box, using \"-j4\". I would hate for it\n> to have taken an entire day. :)\n\nHeh. Okay. I was convinced that your valgrind patch predated my -j \npatch...\n\nIn any case, I already found a bug in the nth_last series, thanks to your \nwork, which I'll send in a minute.\n\nCiao,\nDscho\n"},{"id":"101322","messageId":"20090121001219.GA18169@coredump.intra.peff.net","threadId":"17248","inReplyTo":"alpine.DEB.1.00.0901201602410.5159@intel-tinevez-2-302","subject":"Re: [PATCH 1/2] Add valgrind support in test scripts","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-01-21T00:12:19Z","receivedAt":"2009-01-21T00:12:19Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jan 20, 2009 at 04:04:28PM +0100, Johannes Schindelin wrote:\n\n> +else\n> +\t# override all git executables in PATH and TEST_DIRECTORY/..\n> +\tGIT_VALGRIND=$TEST_DIRECTORY/valgrind\n> +\tmkdir -p \"$GIT_VALGRIND\"\n\nIsn't this mkdir unnecessary, since it is actually part of the\nrepository (i.e., there is a gitignore there already).\n\nHowever, I think it makes more sense to put the symlink cruft into\n\"$GIT_VALGRIND/bin\". That way you can clean up the cruft very easily. In\nwhich case you do need to \"mkdir\" that directory.\n\n> +\tOLDIFS=$IFS\n> +\tIFS=:\n> +\tfor path in $PATH:$TEST_DIRECTORY/..\n> +\tdo\n> +\t\tls \"$TEST_DIRECTORY\"/../git \"$path\"/git-* 2> /dev/null |\n\nWhy aren't these both \"$path\"/ ?\n\nBut more importantly, do we really need to bother overriding the whole\n$PATH? In theory, we aren't calling anything git-* that isn't in\n\"$TEST_DIRECTORY/..\". And while it might be nice to catch it if we do,\nit seems like detecting that is totally orthogonal to running valgrind,\nand we get different behavior from valgrind versus not. And I think the\ntwo should be as similar as possible (with the obvious except of\nactually, you know, running valgrind).\n\n> +\t\t\tbase=$(basename \"$file\")\n> +\t\t\ttest ! -h \"$GIT_VALGRIND\"/\"$base\" || continue\n> +\n> +\t\t\tif test \"#!\" = \"$(head -c 2 < \"$file\")\"\n> +\t\t\tthen\n> +\t\t\t\t# do not override scripts\n> +\t\t\t\tln -s ../../\"$base\" \"$GIT_VALGRIND\"/\"$base\"\n> +\t\t\telse\n> +\t\t\t\tln -s valgrind.sh \"$GIT_VALGRIND\"/\"$base\"\n> +\t\t\tfi\n\nIt would be nice to actually detect errors. But you have to\ndifferentiate between EEXIST and other errors, which is a pain. And you\ncan't use \"ln -sf\" because it isn't atomic.\n\nCopying would solve that (provided you copied to a tempfile and did\nan atomic rename). Or writing this snippet as a C helper.\n\n> --- /dev/null\n> +++ b/t/valgrind/valgrind.sh\n> @@ -0,0 +1,12 @@\n> +#!/bin/sh\n> +\n> +base=$(basename \"$0\")\n> +\n> +exec valgrind -q --error-exitcode=126 \\\n> +\t--leak-check=no \\\n> +\t--suppressions=\"$GIT_VALGRIND/default.supp\" \\\n> +\t--gen-suppressions=all \\\n> +\t--log-fd=4 \\\n> +\t--input-fd=4 \\\n> +\t$GIT_VALGRIND_OPTIONS \\\n> +\t\"$GIT_VALGRIND\"/../../\"$base\" \"$@\"\n\nHm. My version had to do some magic with the GIT_EXEC_PATH, but I think\nthat is because I didn't set GIT_EXEC_PATH in the first place. If yours\nworks (and I haven't really tested it -- I remember it being a real pain\nin the butt to make sure valgrind was getting called from every code\npath), then I like your approach much better.\n\n-Peff\n"},{"id":"101323","messageId":"20090121001551.GB18169@coredump.intra.peff.net","threadId":"17248","inReplyTo":"alpine.DEB.1.00.0901210105470.19014@racer","subject":"Re: valgrind patches, was Re: What's cooking in git.git (Jan 2009, #04; Mon, 19)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-01-21T00:15:51Z","receivedAt":"2009-01-21T00:15:51Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jan 21, 2009 at 01:10:22AM +0100, Johannes Schindelin wrote:\n\n> > Hmm. I suppose that would work, since every test run is trying to create \n> > the same state.\n> \n> Yep, that's what I meant with \"no race\".\n\nRight, but it is still possible to screw it up, if your creation process\ndoes a delete-create. But it looks like you did it correctly in your\npatch (try to create, and if you fail because it's there, assume it's\nright).\n\n> Heh. Okay. I was convinced that your valgrind patch predated my -j \n> patch...\n\nI think I did an early version that did predate it, but then another\nround afterwards.\n\n> In any case, I already found a bug in the nth_last series, thanks to your \n> work, which I'll send in a minute.\n\nYay! It's nice when infrastructure work like this actually pays off.\n\nThanks for picking up this topic...I can drop the size of my\never-growing git todo list by one. :)\n\n-Peff\n"},{"id":"101326","messageId":"alpine.DEB.1.00.0901210119510.19014@racer","threadId":"17248","inReplyTo":"20090121001551.GB18169@coredump.intra.peff.net","subject":"Re: valgrind patches, was Re: What's cooking in git.git (Jan 2009, #04; Mon, 19)","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-21T00:28:01Z","receivedAt":"2009-01-21T00:28:01Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 20 Jan 2009, Jeff King wrote:\n\n> On Wed, Jan 21, 2009 at 01:10:22AM +0100, Johannes Schindelin wrote:\n> \n> > > Hmm. I suppose that would work, since every test run is trying to create \n> > > the same state.\n> > \n> > Yep, that's what I meant with \"no race\".\n> \n> Right, but it is still possible to screw it up, if your creation process \n> does a delete-create. But it looks like you did it correctly in your \n> patch (try to create, and if you fail because it's there, assume it's \n> right).\n\nActually, I test first if it is there, and only if it is not, try to \ncreate the symlink.\n\nNow, there is still a very minor chance for a race, namely if two \nprocesses happen to test the existence of the missing symlink at exactly \nthe same time, and both do not find it, so both processes will try to \ncreate it.\n\nHowever, the symlink creation is not checked for success, so the processes \nwill still both run just fine.\n\nThere is a very subtle problem, though. If you screw with your \nconfiguration, replacing a link in t/valgrind/ by a script, my code will \nnot try to undo it. However, I think that's really asking for trouble, \nand you can get out of the mess by \"rm -r t/valgrind/git*\".\n\nAnother problem which is potentially much more troublesome is this: \nwhen there was a script by a certain name, my code would symlink it \nto $GIT_DIR/$BASENAME (actually a relative path, but you get the \nidea). If that script is turned into a builtin -- this list has certainly \nknown a certain person to push for that kind of conversion :-) -- that \nfact is not picked up.\n\nBut I think I have an easy solution for that.\n\n> > In any case, I already found a bug in the nth_last series, thanks to \n> > your work, which I'll send in a minute.\n> \n> Yay! It's nice when infrastructure work like this actually pays off.\n\nYep! Thanks!\n\n> Thanks for picking up this topic...I can drop the size of my \n> ever-growing git todo list by one. :)\n\nActually, don't remind me... of my TODO list.\n\nCiao,\nDscho\n"},{"id":"101327","messageId":"20090121003739.GA18373@coredump.intra.peff.net","threadId":"17248","inReplyTo":"alpine.DEB.1.00.0901210119510.19014@racer","subject":"Re: valgrind patches, was Re: What's cooking in git.git (Jan 2009, #04; Mon, 19)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-01-21T00:37:39Z","receivedAt":"2009-01-21T00:37:39Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jan 21, 2009 at 01:28:01AM +0100, Johannes Schindelin wrote:\n\n> Actually, I test first if it is there, and only if it is not, try to \n> create the symlink.\n> \n> Now, there is still a very minor chance for a race, namely if two \n> processes happen to test the existence of the missing symlink at exactly \n> the same time, and both do not find it, so both processes will try to \n> create it.\n\nYep. Though I find \"minor chance\" when it comes to races to really mean\n\"annoying to debug\". But...\n\n> However, the symlink creation is not checked for success, so the processes \n> will still both run just fine.\n\nYes, so there is no race in what is there currently. It's just sad that\nwe can't detect any actual errors.\n\n> There is a very subtle problem, though. If you screw with your \n> configuration, replacing a link in t/valgrind/ by a script, my code will \n> not try to undo it. However, I think that's really asking for trouble, \n> and you can get out of the mess by \"rm -r t/valgrind/git*\".\n\nI think we can safely ignore such mucking about in the valgrind\ndirectory as craziness.\n\n> Another problem which is potentially much more troublesome is this: \n> when there was a script by a certain name, my code would symlink it \n> to $GIT_DIR/$BASENAME (actually a relative path, but you get the \n> idea). If that script is turned into a builtin -- this list has certainly \n> known a certain person to push for that kind of conversion :-) -- that \n> fact is not picked up.\n\nYes. One way around this is to generate a \"want\" and a \"have\" list, and\nthen just operate on the differences. Something like (totally untested):\n\n (cd $GIT_VALGRIND && ls) | sort >have\n (cd $TEST_DIRECTORY/.. && ls git git-*) | sort >want\n comm -23 have want | xargs -r rm -v\n comm -13 have want | while read f; do ln -s ../../$f $GIT_VALGRIND/$f; done\n\nand then you are also cleaning every time you create.\n\n-Peff\n"},{"id":"101328","messageId":"alpine.DEB.1.00.0901210130030.19014@racer","threadId":"17248","inReplyTo":"20090121001219.GA18169@coredump.intra.peff.net","subject":"Re: [PATCH 1/2] Add valgrind support in test scripts","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-21T00:41:13Z","receivedAt":"2009-01-21T00:41:13Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 20 Jan 2009, Jeff King wrote:\n\n> On Tue, Jan 20, 2009 at 04:04:28PM +0100, Johannes Schindelin wrote:\n> \n> > +else\n> > +\t# override all git executables in PATH and TEST_DIRECTORY/..\n> > +\tGIT_VALGRIND=$TEST_DIRECTORY/valgrind\n> > +\tmkdir -p \"$GIT_VALGRIND\"\n> \n> Isn't this mkdir unnecessary, since it is actually part of the\n> repository (i.e., there is a gitignore there already).\n> \n> However, I think it makes more sense to put the symlink cruft into\n> \"$GIT_VALGRIND/bin\". That way you can clean up the cruft very easily. In\n> which case you do need to \"mkdir\" that directory.\n\nHmm. I actually liked the hierarchy to be shallow, but I could be \nconvinced...\n\n> > +\tOLDIFS=$IFS\n> > +\tIFS=:\n> > +\tfor path in $PATH:$TEST_DIRECTORY/..\n> > +\tdo\n> > +\t\tls \"$TEST_DIRECTORY\"/../git \"$path\"/git-* 2> /dev/null |\n> \n> Why aren't these both \"$path\"/ ?\n\nYeah. Makes it more readable, doesn't it?\n\n> But more importantly, do we really need to bother overriding the whole \n> $PATH? In theory, we aren't calling anything git-* that isn't in \n> \"$TEST_DIRECTORY/..\". And while it might be nice to catch it if we do, \n> it seems like detecting that is totally orthogonal to running valgrind, \n> and we get different behavior from valgrind versus not. And I think the \n> two should be as similar as possible (with the obvious except of \n> actually, you know, running valgrind).\n\nActually, the two _are_ orthogonal from the technical viewpoint.\n\nBut with the infrastructure we have in place, it was already very easy to \nmake sure that calls to a Git program we no longer ship are caught.\n\nI vividly remember such a bug costing me 3 hours of my life, and a few \nhairs.\n\nSo I think \"as it's already _that_ easy, we should catch them bugs, too\".\n\nNeeds some documentation though, I agree.\n\n> > +\t\t\tbase=$(basename \"$file\")\n> > +\t\t\ttest ! -h \"$GIT_VALGRIND\"/\"$base\" || continue\n> > +\n> > +\t\t\tif test \"#!\" = \"$(head -c 2 < \"$file\")\"\n> > +\t\t\tthen\n> > +\t\t\t\t# do not override scripts\n> > +\t\t\t\tln -s ../../\"$base\" \"$GIT_VALGRIND\"/\"$base\"\n> > +\t\t\telse\n> > +\t\t\t\tln -s valgrind.sh \"$GIT_VALGRIND\"/\"$base\"\n> > +\t\t\tfi\n> \n> It would be nice to actually detect errors. But you have to\n> differentiate between EEXIST and other errors, which is a pain. And you\n> can't use \"ln -sf\" because it isn't atomic.\n\nI really would not care all that much about that. \n'GIT_TEST_OPTS==--valgrind make test' should be run by experts. And even \nif it is a dummy driving the test, the next \"make\" call should take care \nof that.\n\n> Copying would solve that (provided you copied to a tempfile and did\n> an atomic rename). Or writing this snippet as a C helper.\n\nNah, that is really too much work for such a rare thing. Think about it. \nThe symlinks are set up once. And even if you do that with -j50, there is \nhardly a chance that two processes conflict with each other, and even if \nthey do, they do the same thing.\n\nNo, what I really want to fix is a script being replaced by a binary.\n\n> > --- /dev/null\n> > +++ b/t/valgrind/valgrind.sh\n> > @@ -0,0 +1,12 @@\n> > +#!/bin/sh\n> > +\n> > +base=$(basename \"$0\")\n> > +\n> > +exec valgrind -q --error-exitcode=126 \\\n> > +\t--leak-check=no \\\n> > +\t--suppressions=\"$GIT_VALGRIND/default.supp\" \\\n> > +\t--gen-suppressions=all \\\n> > +\t--log-fd=4 \\\n> > +\t--input-fd=4 \\\n> > +\t$GIT_VALGRIND_OPTIONS \\\n> > +\t\"$GIT_VALGRIND\"/../../\"$base\" \"$@\"\n> \n> Hm. My version had to do some magic with the GIT_EXEC_PATH, but I think\n> that is because I didn't set GIT_EXEC_PATH in the first place. If yours\n> works (and I haven't really tested it -- I remember it being a real pain\n> in the butt to make sure valgrind was getting called from every code\n> path), then I like your approach much better.\n\nI set GIT_EXEC_PATH... to $GIT_VALGRIND.\n\nCiao,\nDscho\n"},{"id":"101329","messageId":"alpine.DEB.1.00.0901210209580.19014@racer","threadId":"17248","inReplyTo":"20090121001219.GA18169@coredump.intra.peff.net","subject":"[PATCH 1/2 v2] Add valgrind support in test scripts","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-21T01:10:17Z","receivedAt":"2009-01-21T01:10:17Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nThis patch adds the ability to use valgrind's memcheck tool to\ndiagnose memory problems in git while running the test scripts. It\nworks by placing a \"fake\" git in the front of the test script's PATH;\nthis fake git runs the real git under valgrind. It also points the\nexec-path such that any stand-alone dashed git programs are run using\nthe same script. In this way we avoid having to modify the actual git\ncode in any way.\n\nTo be certain that every call to any git executable is intercepted,\nthe PATH is searched for executables beginning with \"git-\"; Scripts\nare excluded however.\n\nValgrind can be used by specifying \"GIT_TEST_OPTS=--valgrind\" in the\nmake invocation. Any invocation of git that finds any errors under\nvalgrind will exit with failure code 126. Any valgrind output will go\nto the usual stderr channel for tests (i.e., /dev/null, unless -v has\nbeen specified).\n\nIf you need to pass options to valgrind -- you might want to run\nanother tool than memcheck, for example -- you can set the environment\nvariable GIT_VALGRIND_OPTIONS.\n\nA few default suppressions are included, since libz seems to\ntrigger quite a few false positives. We'll assume that libz\nworks and that we can ignore any errors which are reported\nthere.\n\nInitial patch and all the hard work by Jeff King.\n\nSigned-off-by: Johannes Schindelin \n---\n\n\tChanges vs v1:\n\n\t- the symlinks will be created in t/valgrind/bin/ again, as it is\n\t easier to remove the whole directory than weed out the unwanted\n\t files from t/valgrind/.\n\n\t- symbolic links are inspected for correct targets now, and if they\n\t point somewhere else than expected, they are removed (this can\n\t error out if the file could not be removed) and recreated.\n\n\t- if the executable ends in \".sh\" or \".perl\", the target will be\n\t set to a non-existing file (to catch invocations, erroring out).\n\n\t- the Git binaries from the root are actually found now (IFS is\n\t only interpreted after the file is parsed, it seems).\n\n\t- added rudimentary documentation to t/README.\n\n\tInterdiff to follow.\n\n t/README | 6 ++++-\n t/test-lib.sh | 49 +++++++++++++++++++++++++++++++++++++++++++++-\n t/valgrind/.gitignore | 1 +\n t/valgrind/default.supp | 21 ++++++++++++++++++++\n t/valgrind/valgrind.sh | 12 +++++++++++\n 5 files changed, 86 insertions(+), 3 deletions(-)\n create mode 100644 t/valgrind/.gitignore\n create mode 100644 t/valgrind/default.supp\n create mode 100755 t/valgrind/valgrind.sh\n\ndiff --git a/t/README b/t/README\nindex 8f12d48..0cee429 100644\n--- a/t/README\n+++ b/t/README\n@@ -39,7 +39,8 @@ this:\n * passed all 3 test(s)\n \n You can pass --verbose (or -v), --debug (or -d), and --immediate\n-(or -i) command line argument to the test.\n+(or -i) command line argument to the test, or by setting GIT_TEST_OPTS\n+appropriately before running \"make\".\n \n --verbose::\n \tThis makes the test more verbose. Specifically, the\n@@ -58,6 +59,9 @@ You can pass --verbose (or -v), --debug (or -d), and --immediate\n \tThis causes additional long-running tests to be run (where\n \tavailable), for more exhaustive testing.\n \n+--valgrind::\n+\tExecute all Git binaries with valgrind and stop on errors (the\n+\texit code will be 126).\n \n Skipping Tests\n --------------\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 79f69de..f031905 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -94,6 +94,8 @@ do\n \t--no-python)\n \t\t# noop now...\n \t\tshift ;;\n+\t--va|--val|--valg|--valgr|--valgri|--valgrin|--valgrind)\n+\t\tvalgrind=t; shift ;;\n \t*)\n \t\tbreak ;;\n \tesac\n@@ -480,8 +482,51 @@ 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-PATH=$TEST_DIRECTORY/..:$PATH\n-GIT_EXEC_PATH=$(pwd)/..\n+if test -z \"$valgrind\"\n+then\n+\tPATH=$TEST_DIRECTORY/..:$PATH\n+\tGIT_EXEC_PATH=$TEST_DIRECTORY/..\n+else\n+\t# override all git executables in PATH and TEST_DIRECTORY/..\n+\tGIT_VALGRIND=$TEST_DIRECTORY/valgrind/bin\n+\tmkdir -p \"$GIT_VALGRIND\"\n+\tOLDIFS=$IFS\n+\tIFS=:\n+\tfor path in $PATH $TEST_DIRECTORY/..\n+\tdo\n+\t\tls \"$path\"/git \"$path\"/git-* 2> /dev/null |\n+\t\twhile read file\n+\t\tdo\n+\t\t\t# handle only executables\n+\t\t\ttest -x \"$file\" && test ! -d \"$file\" || continue\n+\n+\t\t\tbase=$(basename \"$file\")\n+\t\t\tsymlink_target=$TEST_DIRECTORY/../$base\n+\t\t\t# do not override scripts\n+\t\t\tif test -x \"$symlink_target\" &&\n+\t\t\t test \"#!\" != \"$(head -c 2 < \"$symlink_target\")\"\n+\t\t\tthen\n+\t\t\t\tsymlink_target=../valgrind.sh\n+\t\t\tfi\n+\t\t\tcase \"$base\" in\n+\t\t\t*.sh|*.perl)\n+\t\t\t\tsymlink_target=../unprocessed-script\n+\t\t\tesac\n+\t\t\t# create the link, or replace it if it is out of date\n+\t\t\tif test ! -h \"$GIT_VALGRIND\"/\"$base\" ||\n+\t\t\t test \"$symlink_target\" != \\\n+\t\t\t\t\t\"$(readlink \"$GIT_VALGRIND\"/\"$base\")\"\n+\t\t\tthen\n+\t\t\t\trm -f \"$GIT_VALGRIND\"/\"$base\" || exit\n+\t\t\t\tln -s \"$symlink_target\" \"$GIT_VALGRIND\"/\"$base\"\n+\t\t\tfi\n+\t\tdone\n+\tdone\n+\tIFS=$OLDIFS\n+\tPATH=$GIT_VALGRIND:$PATH\n+\tGIT_EXEC_PATH=$GIT_VALGRIND\n+\texport GIT_VALGRIND\n+fi\n GIT_TEMPLATE_DIR=$(pwd)/../templates/blt\n unset GIT_CONFIG\n GIT_CONFIG_NOSYSTEM=1\ndiff --git a/t/valgrind/.gitignore b/t/valgrind/.gitignore\nnew file mode 100644\nindex 0000000..ae3c172\n--- /dev/null\n+++ b/t/valgrind/.gitignore\n@@ -0,0 +1 @@\n+/bin/\ndiff --git a/t/valgrind/default.supp b/t/valgrind/default.supp\nnew file mode 100644\nindex 0000000..2482b3b\n--- /dev/null\n+++ b/t/valgrind/default.supp\n@@ -0,0 +1,21 @@\n+{\n+\tignore-zlib-errors-cond\n+\tMemcheck:Cond\n+\tobj:*libz.so*\n+}\n+\n+{\n+\tignore-zlib-errors-value4\n+\tMemcheck:Value4\n+\tobj:*libz.so*\n+}\n+\n+{\n+\twriting-data-from-zlib-triggers-errors\n+\tMemcheck:Param\n+\twrite(buf)\n+\tobj:/lib/ld-*.so\n+\tfun:write_in_full\n+\tfun:write_buffer\n+\tfun:write_loose_object\n+}\ndiff --git a/t/valgrind/valgrind.sh b/t/valgrind/valgrind.sh\nnew file mode 100755\nindex 0000000..2c4b54b\n--- /dev/null\n+++ b/t/valgrind/valgrind.sh\n@@ -0,0 +1,12 @@\n+#!/bin/sh\n+\n+base=$(basename \"$0\")\n+\n+exec valgrind -q --error-exitcode=126 \\\n+\t--leak-check=no \\\n+\t--suppressions=\"$GIT_VALGRIND/../default.supp\" \\\n+\t--gen-suppressions=all \\\n+\t--log-fd=4 \\\n+\t--input-fd=4 \\\n+\t$GIT_VALGRIND_OPTIONS \\\n+\t\"$GIT_VALGRIND\"/../../../\"$base\" \"$@\"\n-- \n1.6.1.243.g6c8bb35\n"},{"id":"101330","messageId":"alpine.DEB.1.00.0901210211060.19014@racer","threadId":"17248","inReplyTo":"alpine.DEB.1.00.0901210209580.19014@racer","subject":"[INTERDIFF of PATCH 1/2 v2] Add valgrind support in test scripts","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-21T01:11:37Z","receivedAt":"2009-01-21T01:11:37Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\n t/README | 6 +++++-\n t/test-lib.sh | 32 +++++++++++++++++++++-----------\n t/valgrind/.gitignore | 3 +--\n t/valgrind/valgrind.sh | 4 ++--\n 4 files changed, 29 insertions(+), 16 deletions(-)\n\ndiff --git a/t/README b/t/README\nindex 8f12d48..0cee429 100644\n--- a/t/README\n+++ b/t/README\n@@ -39,7 +39,8 @@ this:\n * passed all 3 test(s)\n \n You can pass --verbose (or -v), --debug (or -d), and --immediate\n-(or -i) command line argument to the test.\n+(or -i) command line argument to the test, or by setting GIT_TEST_OPTS\n+appropriately before running \"make\".\n \n --verbose::\n \tThis makes the test more verbose. Specifically, the\n@@ -58,6 +59,9 @@ You can pass --verbose (or -v), --debug (or -d), and --immediate\n \tThis causes additional long-running tests to be run (where\n \tavailable), for more exhaustive testing.\n \n+--valgrind::\n+\tExecute all Git binaries with valgrind and stop on errors (the\n+\texit code will be 126).\n \n Skipping Tests\n --------------\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 6bd893d..f031905 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -488,27 +488,37 @@ then\n \tGIT_EXEC_PATH=$TEST_DIRECTORY/..\n else\n \t# override all git executables in PATH and TEST_DIRECTORY/..\n-\tGIT_VALGRIND=$TEST_DIRECTORY/valgrind\n+\tGIT_VALGRIND=$TEST_DIRECTORY/valgrind/bin\n \tmkdir -p \"$GIT_VALGRIND\"\n \tOLDIFS=$IFS\n \tIFS=:\n-\tfor path in $PATH:$TEST_DIRECTORY/..\n+\tfor path in $PATH $TEST_DIRECTORY/..\n \tdo\n-\t\tls \"$TEST_DIRECTORY\"/../git \"$path\"/git-* 2> /dev/null |\n+\t\tls \"$path\"/git \"$path\"/git-* 2> /dev/null |\n \t\twhile read file\n \t\tdo\n \t\t\t# handle only executables\n-\t\t\ttest -x \"$file\" || continue\n+\t\t\ttest -x \"$file\" && test ! -d \"$file\" || continue\n \n \t\t\tbase=$(basename \"$file\")\n-\t\t\ttest ! -h \"$GIT_VALGRIND\"/\"$base\" || continue\n-\n-\t\t\tif test \"#!\" = \"$(head -c 2 < \"$file\")\"\n+\t\t\tsymlink_target=$TEST_DIRECTORY/../$base\n+\t\t\t# do not override scripts\n+\t\t\tif test -x \"$symlink_target\" &&\n+\t\t\t test \"#!\" != \"$(head -c 2 < \"$symlink_target\")\"\n+\t\t\tthen\n+\t\t\t\tsymlink_target=../valgrind.sh\n+\t\t\tfi\n+\t\t\tcase \"$base\" in\n+\t\t\t*.sh|*.perl)\n+\t\t\t\tsymlink_target=../unprocessed-script\n+\t\t\tesac\n+\t\t\t# create the link, or replace it if it is out of date\n+\t\t\tif test ! -h \"$GIT_VALGRIND\"/\"$base\" ||\n+\t\t\t test \"$symlink_target\" != \\\n+\t\t\t\t\t\"$(readlink \"$GIT_VALGRIND\"/\"$base\")\"\n \t\t\tthen\n-\t\t\t\t# do not override scripts\n-\t\t\t\tln -s ../../\"$base\" \"$GIT_VALGRIND\"/\"$base\"\n-\t\t\telse\n-\t\t\t\tln -s valgrind.sh \"$GIT_VALGRIND\"/\"$base\"\n+\t\t\t\trm -f \"$GIT_VALGRIND\"/\"$base\" || exit\n+\t\t\t\tln -s \"$symlink_target\" \"$GIT_VALGRIND\"/\"$base\"\n \t\t\tfi\n \t\tdone\n \tdone\ndiff --git a/t/valgrind/.gitignore b/t/valgrind/.gitignore\nindex d781a63..ae3c172 100644\n--- a/t/valgrind/.gitignore\n+++ b/t/valgrind/.gitignore\n@@ -1,2 +1 @@\n-/git\n-/git-*\n+/bin/\ndiff --git a/t/valgrind/valgrind.sh b/t/valgrind/valgrind.sh\nindex 24f3a4e..2c4b54b 100755\n--- a/t/valgrind/valgrind.sh\n+++ b/t/valgrind/valgrind.sh\n@@ -4,9 +4,9 @@ base=$(basename \"$0\")\n \n exec valgrind -q --error-exitcode=126 \\\n \t--leak-check=no \\\n-\t--suppressions=\"$GIT_VALGRIND/default.supp\" \\\n+\t--suppressions=\"$GIT_VALGRIND/../default.supp\" \\\n \t--gen-suppressions=all \\\n \t--log-fd=4 \\\n \t--input-fd=4 \\\n \t$GIT_VALGRIND_OPTIONS \\\n-\t\"$GIT_VALGRIND\"/../../\"$base\" \"$@\"\n+\t\"$GIT_VALGRIND\"/../../../\"$base\" \"$@\"\n"},{"id":"101331","messageId":"alpine.DEB.1.00.0901210216440.19014@racer","threadId":"17248","inReplyTo":"20090121003739.GA18373@coredump.intra.peff.net","subject":"Re: valgrind patches, was Re: What's cooking in git.git (Jan 2009, #04; Mon, 19)","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-21T01:26:56Z","receivedAt":"2009-01-21T01:26:56Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 20 Jan 2009, Jeff King wrote:\n\n> On Wed, Jan 21, 2009 at 01:28:01AM +0100, Johannes Schindelin wrote:\n> \n> > Actually, I test first if it is there, and only if it is not, try to \n> > create the symlink.\n> > \n> > Now, there is still a very minor chance for a race, namely if two \n> > processes happen to test the existence of the missing symlink at exactly \n> > the same time, and both do not find it, so both processes will try to \n> > create it.\n> \n> Yep. Though I find \"minor chance\" when it comes to races to really mean\n> \"annoying to debug\". But...\n\nWell, in this case, you will find that the \"bug\" is _at most_ some \nbinaries not being found.\n\nAnd really, the chance is so small as to be forgotten in the clutter: \nafter the valgrind setup, there are so many other things which are done \nthat by the time we actually use the Git binaries, everything should be \nokay.\n\nAnd keep in mind, this _only_ matters if you do make -j _and_ you haven't \nrun --valgrind _ever_. Once the symlinks are there, they are there.\n\n(Actually, with my new patch, the may be replaced, but _only_ if \nnecessary, and the same thing would apply as I said earlier: the binary \nwould not be found, or a binary from the PATH would be run without \nvalgrind; but the next runs will not have the problem.)\n\n> > However, the symlink creation is not checked for success, so the \n> > processes will still both run just fine.\n> \n> Yes, so there is no race in what is there currently. It's just sad that \n> we can't detect any actual errors.\n\nNow we can. I actually check for the correct link target now (which means \nI also check for a link), and if it is incorrect, the link is recreated \n(and the deletion is checked for errors).\n\n> > There is a very subtle problem, though. If you screw with your \n> > configuration, replacing a link in t/valgrind/ by a script, my code \n> > will not try to undo it. However, I think that's really asking for \n> > trouble, and you can get out of the mess by \"rm -r t/valgrind/git*\".\n> \n> I think we can safely ignore such mucking about in the valgrind\n> directory as craziness.\n\nYou'll find that v2 copes with that, too.\n\n> > Another problem which is potentially much more troublesome is this: \n> > when there was a script by a certain name, my code would symlink it to \n> > $GIT_DIR/$BASENAME (actually a relative path, but you get the idea). \n> > If that script is turned into a builtin -- this list has certainly \n> > known a certain person to push for that kind of conversion :-) -- that \n> > fact is not picked up.\n> \n> Yes. One way around this is to generate a \"want\" and a \"have\" list, and\n> then just operate on the differences. Something like (totally untested):\n> \n> (cd $GIT_VALGRIND && ls) | sort >have\n> (cd $TEST_DIRECTORY/.. && ls git git-*) | sort >want\n> comm -23 have want | xargs -r rm -v\n> comm -13 have want | while read f; do ln -s ../../$f $GIT_VALGRIND/$f; done\n> \n> and then you are also cleaning every time you create.\n\nThe script will now pick up on those changes, and recreate the symlink \ncorrectly.\n\nWe don't need cleaning, as we only link to $TEST_DIRECTORY/.. (at least \nvia valgrind.sh), and if the binary does not exist there, well, it does \nnot exist there, and the script will error out, saying so.\n\nCiao,\nDscho\n"},{"id":"101333","messageId":"alpine.DEB.1.00.0901210236320.19014@racer","threadId":"17248","inReplyTo":"alpine.DEB.1.00.0901210216440.19014@racer","subject":"[PATCH 2/2 v2] valgrind: ignore ldso errors","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-21T01:36:40Z","receivedAt":"2009-01-21T01:36:40Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nOn some Linux systems, we get a host of Cond and Addr errors\nfrom calls to dlopen that are caused by nss modules. We\nshould be able to safely ignore anything happening in\nld-*.so as \"not our problem.\"\n\nSigned-off-by: Jeff King \nSigned-off-by: Junio C Hamano \nSigned-off-by: Johannes Schindelin \n---\n\n\tOnly change vs v1: adds Addr4 suppression, so that ld.so \"errors\"\n\tare ignored on 32-bit, too.\n\n t/valgrind/default.supp | 18 ++++++++++++++++++\n 1 files changed, 18 insertions(+), 0 deletions(-)\n\ndiff --git a/t/valgrind/default.supp b/t/valgrind/default.supp\nindex 2482b3b..6061283 100644\n--- a/t/valgrind/default.supp\n+++ b/t/valgrind/default.supp\n@@ -11,6 +11,24 @@\n }\n \n {\n+\tignore-ldso-cond\n+\tMemcheck:Cond\n+\tobj:*ld-*.so\n+}\n+\n+{\n+\tignore-ldso-addr8\n+\tMemcheck:Addr8\n+\tobj:*ld-*.so\n+}\n+\n+{\n+\tignore-ldso-addr4\n+\tMemcheck:Addr4\n+\tobj:*ld-*.so\n+}\n+\n+{\n \twriting-data-from-zlib-triggers-errors\n \tMemcheck:Param\n \twrite(buf)\n-- \n1.6.1.243.g6c8bb35\n"},{"id":"101358","messageId":"7vskndgi3c.fsf@gitster.siamese.dyndns.org","threadId":"17248","inReplyTo":"alpine.DEB.1.00.0901210209580.19014@racer","subject":"Re: [PATCH 1/2 v2] Add valgrind support in test scripts","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-01-21T08:48:39Z","receivedAt":"2009-01-21T08:48:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> This patch adds the ability to use valgrind's memcheck tool to\n> diagnose memory problems in git while running the test scripts....\n\nHmmm, why do I haf to suffer with these new warnings from the tests?\n\n $ sh t2012-checkout-last.sh --valgrind -v -i\n warning: templates not found /git/git.git/t/valgrind/bin/templates/blt/\n Initialized empty Git repository in /git/git.git/t/trash directory.t2012-checkout-last/.git/\n mv: cannot stat `.git/hooks': No such file or directory\n * expecting success:\n echo hello >world &&\n\nAm I using the patch incorrectly somehow?\n"},{"id":"101377","messageId":"alpine.DEB.1.00.0901211319010.3586@pacific.mpi-cbg.de","threadId":"17248","inReplyTo":"7vskndgi3c.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 1/2 v2] Add valgrind support in test scripts","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-21T12:21:07Z","receivedAt":"2009-01-21T12:21:07Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 21 Jan 2009, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n> \n> > This patch adds the ability to use valgrind's memcheck tool to\n> > diagnose memory problems in git while running the test scripts....\n> \n> Hmmm, why do I haf to suffer with these new warnings from the tests?\n> \n> $ sh t2012-checkout-last.sh --valgrind -v -i\n> warning: templates not found /git/git.git/t/valgrind/bin/templates/blt/\n> Initialized empty Git repository in /git/git.git/t/trash directory.t2012-checkout-last/.git/\n> mv: cannot stat `.git/hooks': No such file or directory\n> * expecting success:\n> echo hello >world &&\n> \n> Am I using the patch incorrectly somehow?\n\nNope, I overlooked that GIT_EXEC_PATH was used by test-lib also to \ndetermine the location of the templates. Will squash this in (which \nmakes a function out of the symlink business, and also fixes the error \nthat git-gui/ was tested if it is a script; \"head\" complained that it is \nnot a file):\n\n-- snipsnap --\n t/test-lib.sh | 22 ++++++++++++++--------\n 1 files changed, 14 insertions(+), 8 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex f031905..6acc6e0 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -487,6 +487,14 @@ then\n \tPATH=$TEST_DIRECTORY/..:$PATH\n \tGIT_EXEC_PATH=$TEST_DIRECTORY/..\n else\n+\tmake_symlink () {\n+\t\ttest -h \"$2\" &&\n+\t\ttest \"$1\" = \"$(readlink \"$2\")\" || {\n+\t\t\trm -f \"$2\" &&\n+\t\t\tln -s \"$1\" \"$2\"\n+\t\t}\n+\t}\n+\n \t# override all git executables in PATH and TEST_DIRECTORY/..\n \tGIT_VALGRIND=$TEST_DIRECTORY/valgrind/bin\n \tmkdir -p \"$GIT_VALGRIND\"\n@@ -498,12 +506,13 @@ else\n \t\twhile read file\n \t\tdo\n \t\t\t# handle only executables\n-\t\t\ttest -x \"$file\" && test ! -d \"$file\" || continue\n+\t\t\ttest -x \"$file\" || continue\n \n \t\t\tbase=$(basename \"$file\")\n \t\t\tsymlink_target=$TEST_DIRECTORY/../$base\n \t\t\t# do not override scripts\n \t\t\tif test -x \"$symlink_target\" &&\n+\t\t\t test ! -d \"$symlink_target\" &&\n \t\t\t test \"#!\" != \"$(head -c 2 < \"$symlink_target\")\"\n \t\t\tthen\n \t\t\t\tsymlink_target=../valgrind.sh\n@@ -513,19 +522,16 @@ else\n \t\t\t\tsymlink_target=../unprocessed-script\n \t\t\tesac\n \t\t\t# create the link, or replace it if it is out of date\n-\t\t\tif test ! -h \"$GIT_VALGRIND\"/\"$base\" ||\n-\t\t\t test \"$symlink_target\" != \\\n-\t\t\t\t\t\"$(readlink \"$GIT_VALGRIND\"/\"$base\")\"\n-\t\t\tthen\n-\t\t\t\trm -f \"$GIT_VALGRIND\"/\"$base\" || exit\n-\t\t\t\tln -s \"$symlink_target\" \"$GIT_VALGRIND\"/\"$base\"\n-\t\t\tfi\n+\t\t\tmake_symlink \"$symlink_target\" \"$GIT_VALGRIND/$base\" ||\n+\t\t\texit\n \t\tdone\n \tdone\n \tIFS=$OLDIFS\n \tPATH=$GIT_VALGRIND:$PATH\n \tGIT_EXEC_PATH=$GIT_VALGRIND\n \texport GIT_VALGRIND\n+\n+\tmake_symlink ../../../templates \"$GIT_VALGRIND\"/templates || exit\n fi\n GIT_TEMPLATE_DIR=$(pwd)/../templates/blt\n unset GIT_CONFIG\n-- \n1.6.1.442.g38a50\n"},{"id":"101419","messageId":"20090121190201.GA21686@coredump.intra.peff.net","threadId":"17248","inReplyTo":"alpine.DEB.1.00.0901210209580.19014@racer","subject":"Re: [PATCH 1/2 v2] Add valgrind support in test scripts","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-01-21T19:02:01Z","receivedAt":"2009-01-21T19:02:01Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jan 21, 2009 at 02:10:17AM +0100, Johannes Schindelin wrote:\n\n> \t- symbolic links are inspected for correct targets now, and if they\n> \t point somewhere else than expected, they are removed (this can\n> \t error out if the file could not be removed) and recreated.\n\nNow you _do_ have a race on this, and triggering it will cause you to\nrun a random version of git from your PATH, not using valgrind (instead\nof running the version from the repo using valgrind). Something like:\n\n A: execvp(\"git-foo\")\n B: oops, \"git-foo\" is out of date\n B: rm $GIT_VALGRIND/git-foo\n A: look for $GIT_VALGRIND/git-foo; not there\n A: look for $PATH[1]/git-foo; ok, there it is\n B: ln -s ../../git-valgrind $GIT_VALGRIND/git-foo\n\n> +--valgrind::\n> +\tExecute all Git binaries with valgrind and stop on errors (the\n> +\texit code will be 126).\n\nIt doesn't necessarily stop: it just causes the command to fail, which\ncauses the test to fail. Which _will_ stop if you have \"-i\".\n\nAlso, you might want to mention that valgrind errors go to stderr, so\nusing \"-v\" is helpful.\n\n> +\t# override all git executables in PATH and TEST_DIRECTORY/..\n> +\tGIT_VALGRIND=$TEST_DIRECTORY/valgrind/bin\n\nI think you should leave GIT_VALGRIND pointing to the main valgrind\ndirectory. That way it is more convenient for people using\nGIT_VALGRIND_OPTIONS to make use of GIT_VALGRIND without having to \"..\"\neverything (for example, they may want to pick and choose suppressions\nto load for their platform).\n\n> +\t\t\tcase \"$base\" in\n> +\t\t\t*.sh|*.perl)\n> +\t\t\t\tsymlink_target=../unprocessed-script\n> +\t\t\tesac\n\nAFAIK, this triggers an error if I try to call \"git-foo.perl\" directly.\nWhat does this have to do with valgrind? Why does this error checking\nhappen when I run --valgrind, but _not_ otherwise?\n\nAnd yes, I know the answer is \"because it's easy to do here, since\n--valgrind is munging the PATH anyway\". But my point is that that is an\n_implementation_ detail, and the external behavior to a user is\nnonsensical.\n\nThe fact that there are other uses for munging the PATH than valgrind\nimplies to me that we should _always_ be munging the PATH like this to\ncatch these sorts of errors. And then \"--valgrind\" can just change the\nway we munge.\n\n> +\t\t\t# create the link, or replace it if it is out of date\n> +\t\t\tif test ! -h \"$GIT_VALGRIND\"/\"$base\" ||\n> +\t\t\t test \"$symlink_target\" != \\\n> +\t\t\t\t\t\"$(readlink \"$GIT_VALGRIND\"/\"$base\")\"\n> +\t\t\tthen\n\nreadlink is not portable; it's part of GNU coreutils. Right now valgrind\nbasically only runs on Linux, which I think generally means that\nreadlink will be available (though I have no idea if there are\ndistributions that vary in this). However, there is an experimental\nvalgrind port to FreeBSD and NetBSD, which are unlikely to have\nreadlink.\n\n-Peff\n"},{"id":"101421","messageId":"20090121190757.GB21686@coredump.intra.peff.net","threadId":"17248","inReplyTo":"alpine.DEB.1.00.0901210216440.19014@racer","subject":"Re: valgrind patches, was Re: What's cooking in git.git (Jan 2009, #04; Mon, 19)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-01-21T19:07:57Z","receivedAt":"2009-01-21T19:07:57Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jan 21, 2009 at 02:26:56AM +0100, Johannes Schindelin wrote:\n\n> Well, in this case, you will find that the \"bug\" is _at most_ some \n> binaries not being found.\n> [...]\n> (Actually, with my new patch, the may be replaced, but _only_ if \n> necessary, and the same thing would apply as I said earlier: the binary \n> would not be found, or a binary from the PATH would be run without \n> valgrind; but the next runs will not have the problem.)\n\nYou can run a random binary from the PATH. So I have asked git to test\nthe version in the repository using valgrind, and to report success only\nif both the git command itself succeeds and valgrind reports zero\nerrors. But it might run some other random version of git, not using\nvalgrind, and if _that_ succeeds, report success. And you don't think\nthat is a bug?\n\nI'll grant it is an unlikely race to lose. I guess I just prefer my\nraces to be non-existent.\n\n-Peff\n"},{"id":"101422","messageId":"20090121190921.GC21686@coredump.intra.peff.net","threadId":"17248","inReplyTo":"alpine.DEB.1.00.0901210236320.19014@racer","subject":"Re: [PATCH 2/2 v2] valgrind: ignore ldso errors","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-01-21T19:09:21Z","receivedAt":"2009-01-21T19:09:21Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jan 21, 2009 at 02:36:40AM +0100, Johannes Schindelin wrote:\n\n> \tOnly change vs v1: adds Addr4 suppression, so that ld.so \"errors\"\n> \tare ignored on 32-bit, too.\n\nI don't think it is wrong to add the extra suppression, but out of\ncuriosity, did you actually trigger it? I tested the original on both\n32- and 64-bit platforms, and that was what made me create the original\n(i.e., for some reason my 32-bit platform did not need the same ld.so\nsuppression).\n\n-Peff\n"},{"id":"101442","messageId":"alpine.DEB.1.00.0901212137130.3586@pacific.mpi-cbg.de","threadId":"17248","inReplyTo":"20090121190201.GA21686@coredump.intra.peff.net","subject":"Re: [PATCH 1/2 v2] Add valgrind support in test scripts","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-21T20:49:14Z","receivedAt":"2009-01-21T20:49:14Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 21 Jan 2009, Jeff King wrote:\n\n> On Wed, Jan 21, 2009 at 02:10:17AM +0100, Johannes Schindelin wrote:\n> \n> > \t- symbolic links are inspected for correct targets now, and if they\n> > \t point somewhere else than expected, they are removed (this can\n> > \t error out if the file could not be removed) and recreated.\n> \n> Now you _do_ have a race on this, and triggering it will cause you to\n> run a random version of git from your PATH, not using valgrind (instead\n> of running the version from the repo using valgrind). Something like:\n> \n> A: execvp(\"git-foo\")\n> B: oops, \"git-foo\" is out of date\n> B: rm $GIT_VALGRIND/git-foo\n> A: look for $GIT_VALGRIND/git-foo; not there\n> A: look for $PATH[1]/git-foo; ok, there it is\n> B: ln -s ../../git-valgrind $GIT_VALGRIND/git-foo\n\nExcept that A had to check the link first, and it was out-of-date already \n-- except if you changed a script into a builtin _and_ run make while a \nvalgrinded test is called _and_ you're unlucky.\n\n> > +--valgrind::\n> > +\tExecute all Git binaries with valgrind and stop on errors (the\n> > +\texit code will be 126).\n> \n> It doesn't necessarily stop: it just causes the command to fail, which\n> causes the test to fail. Which _will_ stop if you have \"-i\".\n> \n> Also, you might want to mention that valgrind errors go to stderr, so\n> using \"-v\" is helpful.\n\nOkay.\n\n> > +\t# override all git executables in PATH and TEST_DIRECTORY/..\n> > +\tGIT_VALGRIND=$TEST_DIRECTORY/valgrind/bin\n> \n> I think you should leave GIT_VALGRIND pointing to the main valgrind\n> directory. That way it is more convenient for people using\n> GIT_VALGRIND_OPTIONS to make use of GIT_VALGRIND without having to \"..\"\n> everything (for example, they may want to pick and choose suppressions\n> to load for their platform).\n\nOkay.\n\n> > +\t\t\tcase \"$base\" in\n> > +\t\t\t*.sh|*.perl)\n> > +\t\t\t\tsymlink_target=../unprocessed-script\n> > +\t\t\tesac\n> \n> AFAIK, this triggers an error if I try to call \"git-foo.perl\" directly.\n\nYep.\n\n> What does this have to do with valgrind?\n\nNothing, except that the infrastructure is there now.\n\n> Why does this error checking happen when I run --valgrind, but _not_ \n> otherwise?\n\nBecause we can only check for that kind of mistake in our scripts (which \nthe author would not realize is a mistake when running on a system where \nGIT_SHELL=/bin/sh) when we redirect GIT_EXEC_PATH.\n\nSo basically, it would take a tremendous effort otherwise, but here, it is \njust easy.\n\n> And yes, I know the answer is \"because it's easy to do here, since\n> --valgrind is munging the PATH anyway\". But my point is that that is an\n> _implementation_ detail, and the external behavior to a user is\n> nonsensical.\n> \n> The fact that there are other uses for munging the PATH than valgrind\n> implies to me that we should _always_ be munging the PATH like this to\n> catch these sorts of errors. And then \"--valgrind\" can just change the\n> way we munge.\n\nHmm. Maybe.\n\n> > +\t\t\t# create the link, or replace it if it is out of date\n> > +\t\t\tif test ! -h \"$GIT_VALGRIND\"/\"$base\" ||\n> > +\t\t\t test \"$symlink_target\" != \\\n> > +\t\t\t\t\t\"$(readlink \"$GIT_VALGRIND\"/\"$base\")\"\n> > +\t\t\tthen\n> \n> readlink is not portable; it's part of GNU coreutils. Right now valgrind\n> basically only runs on Linux, which I think generally means that\n> readlink will be available (though I have no idea if there are\n> distributions that vary in this). However, there is an experimental\n> valgrind port to FreeBSD and NetBSD, which are unlikely to have\n> readlink.\n\nAs I mentioned earlier: let's bridge this bridge when we face it \n(probably it involves making a test-readlink).\n\nOr are you insisting that the patch should be reworked _now_ so that \nGIT_EXEC_PATH _always_ points somewhere else?\n\nI hope not, because then you break Windows.\n\nCiao,\nDscho\n"},{"id":"101443","messageId":"alpine.DEB.1.00.0901212150131.3586@pacific.mpi-cbg.de","threadId":"17248","inReplyTo":"20090121190921.GC21686@coredump.intra.peff.net","subject":"Re: [PATCH 2/2 v2] valgrind: ignore ldso errors","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-21T20:51:05Z","receivedAt":"2009-01-21T20:51:05Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 21 Jan 2009, Jeff King wrote:\n\n> On Wed, Jan 21, 2009 at 02:36:40AM +0100, Johannes Schindelin wrote:\n> \n> > \tOnly change vs v1: adds Addr4 suppression, so that ld.so \"errors\"\n> > \tare ignored on 32-bit, too.\n> \n> I don't think it is wrong to add the extra suppression, but out of\n> curiosity, did you actually trigger it?\n\nYes. I wouldn't have touched the file if I hadn't triggered it.\n\nCiao,\nDscho\n"},{"id":"101462","messageId":"20090121215318.GA9107@sigill.intra.peff.net","threadId":"17248","inReplyTo":"alpine.DEB.1.00.0901212137130.3586@pacific.mpi-cbg.de","subject":"Re: [PATCH 1/2 v2] Add valgrind support in test scripts","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-01-21T21:53:18Z","receivedAt":"2009-01-21T21:53:18Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jan 21, 2009 at 09:49:14PM +0100, Johannes Schindelin wrote:\n\n> > A: execvp(\"git-foo\")\n> > B: oops, \"git-foo\" is out of date\n> > B: rm $GIT_VALGRIND/git-foo\n> > A: look for $GIT_VALGRIND/git-foo; not there\n> > A: look for $PATH[1]/git-foo; ok, there it is\n> > B: ln -s ../../git-valgrind $GIT_VALGRIND/git-foo\n> \n> Except that A had to check the link first, and it was out-of-date already \n> -- except if you changed a script into a builtin _and_ run make while a \n> valgrinded test is called _and_ you're unlucky.\n\nHrm, true. I consider running \"make\" in the middle of tests and\nexpecting them to work properly to be a bit crazy, so I guess this is\nnot a problem in practice.\n\nI'll stop bugging you about race conditions for now, then. :)\n\n> > readlink is not portable; it's part of GNU coreutils. Right now valgrind\n> > basically only runs on Linux, which I think generally means that\n> > readlink will be available (though I have no idea if there are\n> > distributions that vary in this). However, there is an experimental\n> > valgrind port to FreeBSD and NetBSD, which are unlikely to have\n> > readlink.\n> \n> As I mentioned earlier: let's bridge this bridge when we face it \n> (probably it involves making a test-readlink).\n\nActually, I am wrong. There is a stripped-down readlink that has\nshipped with FreeBSD (since 4.10) and NetBSD (since 1.6). So while\nreadlink isn't portable, I think it should generally work on platforms\nsupported by valgrind.\n\n> Or are you insisting that the patch should be reworked _now_ so that \n> GIT_EXEC_PATH _always_ points somewhere else?\n\nNo, I'm not insisting. It was merely a suggestion that the patch be\nsplit into two parts so non-valgrind invocations can benefit from this\ntype of bug checking (and by this type I mean general PATH issues -- I\nthink we had some problems in the past with invoking dashed forms of\ncommands which were supposed to be available only via exec-path).\n\n> I hope not, because then you break Windows.\n\nOnly if you use the same symlink technique.\n\n-Peff\n"},{"id":"101465","messageId":"alpine.DEB.1.00.0901212259420.3586@pacific.mpi-cbg.de","threadId":"17248","inReplyTo":"20090121190757.GB21686@coredump.intra.peff.net","subject":"Re: valgrind patches, was Re: What's cooking in git.git (Jan 2009, #04; Mon, 19)","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-21T22:17:35Z","receivedAt":"2009-01-21T22:17:35Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 21 Jan 2009, Jeff King wrote:\n\n> On Wed, Jan 21, 2009 at 02:26:56AM +0100, Johannes Schindelin wrote:\n> \n> > Well, in this case, you will find that the \"bug\" is _at most_ some \n> > binaries not being found.\n> > [...]\n> > (Actually, with my new patch, the may be replaced, but _only_ if \n> > necessary, and the same thing would apply as I said earlier: the binary \n> > would not be found, or a binary from the PATH would be run without \n> > valgrind; but the next runs will not have the problem.)\n> \n> You can run a random binary from the PATH.\n\nNo. You seem to assume that a test script can run all kinds of Git \ncommands while another, is replacing the symlinks in $GIT_VALGRIND/bin/ at \nthe same time.\n\nFact is: every test script will check $GIT_VALGRIND/bin/ for \nup-to-dateness first. Before running any Git command.\n\nDuring that time, races are possible, but non-fatal, because they all try \nto do the same thing.\n\nExcept, of course, if you replace a script by a builtin _while your test \nis running the up-to-date check of $GIT_VALGRIND/bin/_! But I would have \nno word of consolation for you in that case.\n\nSo, can we agree that every test script tries to keep $GIT_VALGRIND/bin/ \nup-to-date before the first Git command is called?\n\nNow, you might assume that it is possible that one test-script symlinked \nthe Git command while another removed it.\n\nBut the script that removed the symlink will recreate it right away.\n\nGranted, during that time, the other script could have gone off to call a \nGit command in that very brief time span, but keep in mind: it does not \ntake a long time from rm to ln -s, _and_ the other script would have to go \non to call a Git command _right through that time_.\n\nAnd you know which command that might be?\n\nExactly. git init. Which takes a long, long, long time, and where I \nreally could not care less if it is called from the PATH or not.\n\nNote: this would be only possible if both scripts checked the same name at \nthe very same time, coming to the very same result that the name needs \nsymlinking. Unlikely.\n\nNote, too: such a replacing/creating could only take place the very first \ntime you run valgrind, or when a script was replaced by a builtin. IOW \nvery, very rarely to begin with.\n\nNow the big question: is this highly, highly unlikely issue relevant?\n\nAnd I say: no. Because even in that highly, highly, highly unlikely \nevent, all that will happen is that a git init (which is tested later, \nanyway) is not valgrinded.\n\nBesides, if that race would happen _and_ you would see any issues, you'd \nrun the test again, without parallelization, because you would not be able \nto discern what messages belong together from the output of \"make -j50 \ntest\" anyway.\n\nAnd the whole issue goes away, because that call will again try to \nmake GIT_VALGRIND/bin up-to-date, and there will be no chance for a race \nthis time.\n\nPhew. A lot of time, a lot of braincycles, and a lot of keystrokes wasted \non that subject, don't you think?\n\nCiao,\nDscho\n"},{"id":"101466","messageId":"alpine.DEB.1.00.0901212331280.3586@pacific.mpi-cbg.de","threadId":"17248","inReplyTo":"alpine.DEB.1.00.0901212137130.3586@pacific.mpi-cbg.de","subject":"[PATCH] valgrind tests: be super-super paranoid when creating symlinks","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-21T22:31:58Z","receivedAt":"2009-01-21T22:31:58Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nEven if there is only a faint, almost neglible chance that two parallel\ntests create the symlinks needed for the valgrind test at the same time,\nPeff wrote more than just a couple mails about the issue.\n\nTo get rid of that threat^Wthread, use a locking mechanism to make\nsure a symlink is only created by one test invocation, and the other\nhas to wait.\n\nPeff, do you see how much I like you?\n\nSigned-off-by: Johannes Schindelin \n---\n t/test-lib.sh | 15 +++++++++++++--\n 1 files changed, 13 insertions(+), 2 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 6acc6e0..07e657e 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -490,8 +490,19 @@ else\n \tmake_symlink () {\n \t\ttest -h \"$2\" &&\n \t\ttest \"$1\" = \"$(readlink \"$2\")\" || {\n-\t\t\trm -f \"$2\" &&\n-\t\t\tln -s \"$1\" \"$2\"\n+\t\t\t# be super paranoid\n+\t\t\tif mkdir \"$2\".lock\n+\t\t\tthen\n+\t\t\t\trm -f \"$2\" &&\n+\t\t\t\tln -s \"$1\" \"$2\" &&\n+\t\t\t\trm -r \"$2\".lock\n+\t\t\telse\n+\t\t\t\twhile test -d \"$2\".lock\n+\t\t\t\tdo\n+\t\t\t\t\tsay \"Waiting for lock on $2.\"\n+\t\t\t\t\tsleep 1\n+\t\t\t\tdone\n+\t\t\tfi\n \t\t}\n \t}\n \n-- \n1.6.1.442.g112f5\n"},{"id":"101469","messageId":"alpine.DEB.1.00.0901212332030.3586@pacific.mpi-cbg.de","threadId":"17248","inReplyTo":"20090121215318.GA9107@sigill.intra.peff.net","subject":"Re: [PATCH 1/2 v2] Add valgrind support in test scripts","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-21T22:38:13Z","receivedAt":"2009-01-21T22:38:13Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 21 Jan 2009, Jeff King wrote:\n\n> Actually, I am wrong. There is a stripped-down readlink that has shipped \n> with FreeBSD (since 4.10) and NetBSD (since 1.6). So while readlink \n> isn't portable, I think it should generally work on platforms supported \n> by valgrind.\n\nA pity. I was already working on this patch:\n\n-- snipsnap --\n[PATCH] valgrind tests: provide a \"readlink\" function for systems which lack it\n\nSigned-off-by: Johannes Schindelin \n---\n t/test-lib.sh | 17 +++++++++++++++++\n 1 files changed, 17 insertions(+), 0 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 07e657e..c2199e7 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -487,7 +487,24 @@ then\n \tPATH=$TEST_DIRECTORY/..:$PATH\n \tGIT_EXEC_PATH=$TEST_DIRECTORY/..\n else\n+\treadlink -h 2> /dev/null\n+\tif test $? = 127\n+\tthen\n+\t\treadlink () {\n+\t\t\tls -l \"$1\" |\n+\t\t\tsed -e \"s/-> \\(.*\\)$/\\1/g\"\n+\t\t\t# cannot use s/.* -> //, because of\n+\t\t\t# ln -s \"a -> b\" \"c -> d\"\n+\t\t}\n+\tfi\n+\n \tmake_symlink () {\n+\t\tcase \"$1\" in\n+\t\t*\" -> \"*)\n+\t\t\tdie \"You must be kidding me ($1).\"\n+\t\t;;\n+\t\tesac\n+\n \t\ttest -h \"$2\" &&\n \t\ttest \"$1\" = \"$(readlink \"$2\")\" || {\n \t\t\t# be super paranoid\n-- \n1.6.1.442.g112f5\n"},{"id":"101481","messageId":"20090121235757.GA9668@sigill.intra.peff.net","threadId":"17248","inReplyTo":"alpine.DEB.1.00.0901212259420.3586@pacific.mpi-cbg.de","subject":"Re: valgrind patches, was Re: What's cooking in git.git (Jan 2009, #04; Mon, 19)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-01-21T23:57:57Z","receivedAt":"2009-01-21T23:57:57Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jan 21, 2009 at 11:17:35PM +0100, Johannes Schindelin wrote:\n\n> Phew. A lot of time, a lot of braincycles, and a lot of keystrokes wasted \n> on that subject, don't you think?\n\nYes, especially considering my other email that said I had dropped the\nsubject. ;P\n\nBut thank you for discussing it. There is still some part of me that\nsays \"if you have no races, you don't have to worry about analyzing\nthem.\" But I think your analysis is correct, and I am willing to let it\ngo in the name of practicality.\n\nAs for braincycles, I don't think they were necessarily wasted. The\npoint of review is to double-check, and the discussion is how we resolve\n(even if we resolve that it is OK as-is). Of course there is such a\nthing as useless, annoying pedantry, but I hope this didn't count... :)\n\n-Peff\n"},{"id":"101487","messageId":"7vr62wb28h.fsf@gitster.siamese.dyndns.org","threadId":"17248","inReplyTo":"alpine.DEB.1.00.0901212259420.3586@pacific.mpi-cbg.de","subject":"Re: valgrind patches, was Re: What's cooking in git.git (Jan 2009, #04; Mon, 19)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-01-22T00:42:22Z","receivedAt":"2009-01-22T00:42:22Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> Fact is: every test script will check $GIT_VALGRIND/bin/ for \n> up-to-dateness first. Before running any Git command.\n\nHmm, is that a good thing in general? Can't makefile rules be arranged in\nsuch a way that one \"valgrind-prep\" target runs before all the potentially\nparallel executions of actual tests begin?\n\nIndependent from the above, I suspect that some of the existing tests\ncannot run in parallel; I haven't really looked at any of them, but a\nserver-ish tests to open a local port and test interaction with client\nobviously need to either use different ports or serialize. Perhaps we\nneed a way to mark some tests that cannot be run in parallel even under\n\"make -j\"?\n"},{"id":"101488","messageId":"20090122005901.GA10826@sigill.intra.peff.net","threadId":"17248","inReplyTo":"7vr62wb28h.fsf@gitster.siamese.dyndns.org","subject":"Re: valgrind patches, was Re: What's cooking in git.git (Jan 2009, #04; Mon, 19)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-01-22T00:59:01Z","receivedAt":"2009-01-22T00:59:01Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jan 21, 2009 at 04:42:22PM -0800, Junio C Hamano wrote:\n\n> > Fact is: every test script will check $GIT_VALGRIND/bin/ for \n> > up-to-dateness first. Before running any Git command.\n> \n> Hmm, is that a good thing in general? Can't makefile rules be arranged in\n> such a way that one \"valgrind-prep\" target runs before all the potentially\n> parallel executions of actual tests begin?\n\nYou have to choose either \"everybody does this setup, whether they want\n--valgrind or not\" which is what my original patch did, or doing it\ninside test-lib.sh. Because we don't know we want --valgrind until we\nget into the individual scripts.\n\nI suppose one could try parsing GIT_TEST_OPTS in the Makefile, but that\nseems a bit hack-ish.\n\nBut I like putting it into test-lib.sh; yes, it is a little more CPU\ntime for each script, but it is negligible compared to running the\nactual tests (especially since you only pay when running with\n--valgrind, which makes the actual tests very expensive). But it is much\neasier to be sure it is _correct_ when you run the test, especially if\nyou tend to run the test script directly.\n\n> Independent from the above, I suspect that some of the existing tests\n> cannot run in parallel; I haven't really looked at any of them, but a\n> server-ish tests to open a local port and test interaction with client\n> obviously need to either use different ports or serialize. Perhaps we\n> need a way to mark some tests that cannot be run in parallel even under\n> \"make -j\"?\n\nI think the only culprits are http-push and a few SVN tests. The\nhttp-push test starts a server on a specific port, but because it is the\nonly script which uses that port, it is fine. It looks like a few\ndifferent SVN tests start an httpd server (9115, 9118, and 9120), which\ncould potentially interact badly. I've never had a problem running with\n\"-j4\", but I don't have svn installed, so I always end up skipping those\ntests.\n\nIt looks like both the http-push and svn tests are set up to take an\narbitrary port as input. Perhaps the simplest thing would be for each of\nthe svn tests to pick a different port so that they can be run\nsimultaneously.\n\n-Peff\n"},{"id":"101496","messageId":"alpine.DEB.1.00.0901220601500.3586@pacific.mpi-cbg.de","threadId":"17248","inReplyTo":"20090122005901.GA10826@sigill.intra.peff.net","subject":"Re: valgrind patches, was Re: What's cooking in git.git (Jan 2009, #04; Mon, 19)","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-22T05:02:51Z","receivedAt":"2009-01-22T05:02:51Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 21 Jan 2009, Jeff King wrote:\n\n> On Wed, Jan 21, 2009 at 04:42:22PM -0800, Junio C Hamano wrote:\n> \n> > Independent from the above, I suspect that some of the existing tests \n> > cannot run in parallel; I haven't really looked at any of them, but a \n> > server-ish tests to open a local port and test interaction with client \n> > obviously need to either use different ports or serialize. Perhaps we \n> > need a way to mark some tests that cannot be run in parallel even \n> > under \"make -j\"?\n> \n> I think the only culprits are http-push and a few SVN tests. The \n> http-push test starts a server on a specific port, but because it is the \n> only script which uses that port, it is fine. It looks like a few \n> different SVN tests start an httpd server (9115, 9118, and 9120), which \n> could potentially interact badly. I've never had a problem running with \n> \"-j4\", but I don't have svn installed, so I always end up skipping those \n> tests.\n> \n> It looks like both the http-push and svn tests are set up to take an \n> arbitrary port as input. Perhaps the simplest thing would be for each of \n> the svn tests to pick a different port so that they can be run \n> simultaneously.\n\nI _suspect_ that the svn tests already use different ports (or can work \nwith the same httpd), as I have subversion installed and run with -j50 \nregularly.\n\nCiao,\nDscho\n"},{"id":"101500","messageId":"20090122053935.GA15762@coredump.intra.peff.net","threadId":"17248","inReplyTo":"alpine.DEB.1.00.0901220601500.3586@pacific.mpi-cbg.de","subject":"Re: valgrind patches, was Re: What's cooking in git.git (Jan 2009, #04; Mon, 19)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-01-22T05:39:35Z","receivedAt":"2009-01-22T05:39:35Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jan 22, 2009 at 06:02:51AM +0100, Johannes Schindelin wrote:\n\n> I _suspect_ that the svn tests already use different ports (or can work \n> with the same httpd), as I have subversion installed and run with -j50 \n> regularly.\n\nI think you're just not running them; it looks like they bail if\nSVN_HTTPD_PORT isn't set by the user.\n\n-Peff\n"},{"id":"101895","messageId":"alpine.DEB.1.00.0901260014470.14855@racer","threadId":"17248","inReplyTo":"alpine.DEB.1.00.0901212332030.3586@pacific.mpi-cbg.de","subject":"[PATCH 0/3] Valgrind support","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-25T23:18:10Z","receivedAt":"2009-01-25T23:18:10Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nI finally decided to give in on both the lock (let's see how many races\nwe encounter in reality...) and the searching the PATH and handling .sh\nand .perl scripts, too. The latter issue is handled by 3/3, which is up\nfor discussion.\n\nOh, and BTW, this is vs 'next', and according to my tests, valgrind finds\nat least one issue.\n\nJeff King (1):\n valgrind: ignore ldso and more libz errors\n\nJohannes Schindelin (2):\n Add valgrind support in test scripts\n Valgrind support: check for more than just programming errors\n\n t/README | 8 +++++-\n t/test-lib.sh | 66 +++++++++++++++++++++++++++++++++++++++++++++-\n t/valgrind/.gitignore | 1 +\n t/valgrind/default.supp | 45 ++++++++++++++++++++++++++++++++\n t/valgrind/valgrind.sh | 12 ++++++++\n 5 files changed, 129 insertions(+), 3 deletions(-)\n create mode 100644 t/valgrind/.gitignore\n create mode 100644 t/valgrind/default.supp\n create mode 100755 t/valgrind/valgrind.sh\n"},{"id":"101896","messageId":"alpine.DEB.1.00.0901260018340.14855@racer","threadId":"17248","inReplyTo":"alpine.DEB.1.00.0901260014470.14855@racer","subject":"[PATCH v3 1/3] Add valgrind support in test scripts","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-25T23:18:50Z","receivedAt":"2009-01-25T23:18:50Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nThis patch adds the ability to use valgrind's memcheck tool to\ndiagnose memory problems in Git while running the test scripts.\n\nIt works by creating symlinks to a valgrind script, which have the same\nname as our Git binaries, and then putting that directory in front of\nthe test script's PATH as well as set GIT_EXEC_PATH to that directory.\n\nGit scripts are symlinked from that directory directly. That way, Git\nbinaries called by Git scripts are valgrinded, too.\n\nValgrind can be used by specifying \"GIT_TEST_OPTS=--valgrind\" in the\nmake invocation. Any invocation of git that finds any errors under\nvalgrind will exit with failure code 126. Any valgrind output will go\nto the usual stderr channel for tests (i.e., /dev/null, unless -v has\nbeen specified).\n\nIf you need to pass options to valgrind -- you might want to run\nanother tool than memcheck, for example -- you can set the environment\nvariable GIT_VALGRIND_OPTIONS.\n\nA few default suppressions are included, since libz seems to trigger\nquite a few false positives. We'll assume that libz works and that we\ncan ignore any errors which are reported there.\n\nNote: it is safe to run the valgrind tests in parallel, as the links in\nt/valgrind/bin/ are created using proper locking.\n\nInitial patch and all the hard work by Jeff King.\n\nSigned-off-by: Johannes Schindelin \n---\n t/README | 8 ++++++-\n t/test-lib.sh | 54 +++++++++++++++++++++++++++++++++++++++++++++-\n t/valgrind/.gitignore | 1 +\n t/valgrind/default.supp | 21 ++++++++++++++++++\n t/valgrind/valgrind.sh | 12 ++++++++++\n 5 files changed, 93 insertions(+), 3 deletions(-)\n create mode 100644 t/valgrind/.gitignore\n create mode 100644 t/valgrind/default.supp\n create mode 100755 t/valgrind/valgrind.sh\n\ndiff --git a/t/README b/t/README\nindex 8f12d48..811bc0d 100644\n--- a/t/README\n+++ b/t/README\n@@ -39,7 +39,8 @@ this:\n * passed all 3 test(s)\n \n You can pass --verbose (or -v), --debug (or -d), and --immediate\n-(or -i) command line argument to the test.\n+(or -i) command line argument to the test, or by setting GIT_TEST_OPTS\n+appropriately before running \"make\".\n \n --verbose::\n \tThis makes the test more verbose. Specifically, the\n@@ -58,6 +59,11 @@ You can pass --verbose (or -v), --debug (or -d), and --immediate\n \tThis causes additional long-running tests to be run (where\n \tavailable), for more exhaustive testing.\n \n+--valgrind::\n+\tExecute all Git binaries with valgrind and exit with status\n+\t126 on errors (just like regular tests, this will only stop\n+\tthe test script when running under -i). Valgrind errors\n+\tgo to stderr, so you might want to pass the -v option, too.\n \n Skipping Tests\n --------------\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 41d5a59..67d7883 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -94,6 +94,8 @@ do\n \t--no-python)\n \t\t# noop now...\n \t\tshift ;;\n+\t--va|--val|--valg|--valgr|--valgri|--valgrin|--valgrind)\n+\t\tvalgrind=t; shift ;;\n \t*)\n \t\tbreak ;;\n \tesac\n@@ -467,8 +469,56 @@ 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-PATH=$TEST_DIRECTORY/..:$PATH\n-GIT_EXEC_PATH=$(pwd)/..\n+if test -z \"$valgrind\"\n+then\n+\tPATH=$TEST_DIRECTORY/..:$PATH\n+\tGIT_EXEC_PATH=$TEST_DIRECTORY/..\n+else\n+\tmake_symlink () {\n+\t\ttest -h \"$2\" &&\n+\t\ttest \"$1\" = \"$(readlink \"$2\")\" || {\n+\t\t\t# be super paranoid\n+\t\t\tif mkdir \"$2\".lock\n+\t\t\tthen\n+\t\t\t\trm -f \"$2\" &&\n+\t\t\t\tln -s \"$1\" \"$2\" &&\n+\t\t\t\trm -r \"$2\".lock\n+\t\t\telse\n+\t\t\t\twhile test -d \"$2\".lock\n+\t\t\t\tdo\n+\t\t\t\t\tsay \"Waiting for lock on $2.\"\n+\t\t\t\t\tsleep 1\n+\t\t\t\tdone\n+\t\t\tfi\n+\t\t}\n+\t}\n+\n+\t# override all git executables in TEST_DIRECTORY/..\n+\tGIT_VALGRIND=$TEST_DIRECTORY/valgrind\n+\tmkdir -p \"$GIT_VALGRIND\"/bin\n+\tls $TEST_DIRECTORY/../git* 2> /dev/null |\n+\twhile read symlink_target\n+\tdo\n+\t\t# handle only executables\n+\t\ttest -x \"$symlink_target\" || continue\n+\n+\t\tbase=$(basename \"$symlink_target\")\n+\t\t# do not override scripts\n+\t\tif test ! -d \"$symlink_target\" &&\n+\t\t test \"#!\" != \"$(head -c 2 < \"$symlink_target\")\"\n+\t\tthen\n+\t\t\tsymlink_target=../valgrind.sh\n+\t\tfi\n+\t\t# create the link, or replace it if it is out of date\n+\t\tmake_symlink \"$symlink_target\" \\\n+\t\t\t\"$GIT_VALGRIND/bin/$base\" || exit\n+\tdone\n+\tPATH=$GIT_VALGRIND/bin:$PATH\n+\tGIT_EXEC_PATH=$GIT_VALGRIND/bin\n+\texport GIT_VALGRIND\n+\n+\tmake_symlink ../../templates \"$GIT_VALGRIND\"/templates || exit\n+fi\n GIT_TEMPLATE_DIR=$(pwd)/../templates/blt\n unset GIT_CONFIG\n GIT_CONFIG_NOSYSTEM=1\ndiff --git a/t/valgrind/.gitignore b/t/valgrind/.gitignore\nnew file mode 100644\nindex 0000000..ae3c172\n--- /dev/null\n+++ b/t/valgrind/.gitignore\n@@ -0,0 +1 @@\n+/bin/\ndiff --git a/t/valgrind/default.supp b/t/valgrind/default.supp\nnew file mode 100644\nindex 0000000..2482b3b\n--- /dev/null\n+++ b/t/valgrind/default.supp\n@@ -0,0 +1,21 @@\n+{\n+\tignore-zlib-errors-cond\n+\tMemcheck:Cond\n+\tobj:*libz.so*\n+}\n+\n+{\n+\tignore-zlib-errors-value4\n+\tMemcheck:Value4\n+\tobj:*libz.so*\n+}\n+\n+{\n+\twriting-data-from-zlib-triggers-errors\n+\tMemcheck:Param\n+\twrite(buf)\n+\tobj:/lib/ld-*.so\n+\tfun:write_in_full\n+\tfun:write_buffer\n+\tfun:write_loose_object\n+}\ndiff --git a/t/valgrind/valgrind.sh b/t/valgrind/valgrind.sh\nnew file mode 100755\nindex 0000000..24f3a4e\n--- /dev/null\n+++ b/t/valgrind/valgrind.sh\n@@ -0,0 +1,12 @@\n+#!/bin/sh\n+\n+base=$(basename \"$0\")\n+\n+exec valgrind -q --error-exitcode=126 \\\n+\t--leak-check=no \\\n+\t--suppressions=\"$GIT_VALGRIND/default.supp\" \\\n+\t--gen-suppressions=all \\\n+\t--log-fd=4 \\\n+\t--input-fd=4 \\\n+\t$GIT_VALGRIND_OPTIONS \\\n+\t\"$GIT_VALGRIND\"/../../\"$base\" \"$@\"\n-- \n1.6.1.482.g7d54be\n"},{"id":"101897","messageId":"alpine.DEB.1.00.0901260019000.14855@racer","threadId":"17248","inReplyTo":"alpine.DEB.1.00.0901260014470.14855@racer","subject":"[PATCH v3 2/3] valgrind: ignore ldso and more libz errors","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-25T23:19:12Z","receivedAt":"2009-01-25T23:19:12Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nOn some Linux systems, we get a host of Cond and Addr errors\nfrom calls to dlopen that are caused by nss modules. We\nshould be able to safely ignore anything happening in\nld-*.so as \"not our problem.\"\n\n[Johannes: I added some more...]\n\nSigned-off-by: Jeff King \nSigned-off-by: Johannes Schindelin \n---\n t/valgrind/default.supp | 24 ++++++++++++++++++++++++\n 1 files changed, 24 insertions(+), 0 deletions(-)\n\ndiff --git a/t/valgrind/default.supp b/t/valgrind/default.supp\nindex 2482b3b..b2da4fd 100644\n--- a/t/valgrind/default.supp\n+++ b/t/valgrind/default.supp\n@@ -5,12 +5,36 @@\n }\n \n {\n+\tignore-zlib-errors-value8\n+\tMemcheck:Value8\n+\tobj:*libz.so*\n+}\n+\n+{\n \tignore-zlib-errors-value4\n \tMemcheck:Value4\n \tobj:*libz.so*\n }\n \n {\n+\tignore-ldso-cond\n+\tMemcheck:Cond\n+\tobj:*ld-*.so\n+}\n+\n+{\n+\tignore-ldso-addr8\n+\tMemcheck:Addr8\n+\tobj:*ld-*.so\n+}\n+\n+{\n+\tignore-ldso-addr4\n+\tMemcheck:Addr4\n+\tobj:*ld-*.so\n+}\n+\n+{\n \twriting-data-from-zlib-triggers-errors\n \tMemcheck:Param\n \twrite(buf)\n-- \n1.6.1.482.g7d54be\n"},{"id":"101898","messageId":"alpine.DEB.1.00.0901260019160.14855@racer","threadId":"17248","inReplyTo":"alpine.DEB.1.00.0901260014470.14855@racer","subject":"[PATCH 3/3] Valgrind support: check for more than just programming errors","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-25T23:20:21Z","receivedAt":"2009-01-25T23:20:21Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nThis patch makes --valgrind try to override _all_ Git binaries in the\nPATH, and it will make calling *.sh and *.perl scripts directly an\nerror.\n\nWhile it is not strictly necessary to look through the whole PATH to\nfind git binaries to override, it is in line with running an expensive\ntest (which valgrind is) to make extra sure that no binary is tested\nthat actually comes from the git.git checkout.\n\nIn the same spirit, we can test that neither our test suite nor our\nscripts try to run the *.sh or *.perl scripts directly.\n\nIt's more like a \"because we can\" than a \"this is tightly connected\nto valgrind\", but in the author's opinion \"because we can\" is \"so we\nshould\" in this case.\n\nSigned-off-by: Johannes Schindelin \n---\n\n\tAs I said, I vividly remember chasing a bug which turned out to be \n\ta Git program that was installed, but no longer in git.git, yet \n\tthe test suite used it.\n\n\tThis would catch it.\n\n t/test-lib.sh | 42 +++++++++++++++++++++++++++---------------\n 1 files changed, 27 insertions(+), 15 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 67d7883..bdfb30f 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -496,23 +496,35 @@ else\n \t# override all git executables in TEST_DIRECTORY/..\n \tGIT_VALGRIND=$TEST_DIRECTORY/valgrind\n \tmkdir -p \"$GIT_VALGRIND\"/bin\n-\tls $TEST_DIRECTORY/../git* 2> /dev/null |\n-\twhile read symlink_target\n+\tOLDIFS=$IFS\n+\tIFS=:\n+\tfor path in $PATH $TEST_DIRECTORY/..\n \tdo\n-\t\t# handle only executables\n-\t\ttest -x \"$symlink_target\" || continue\n-\n-\t\tbase=$(basename \"$symlink_target\")\n-\t\t# do not override scripts\n-\t\tif test ! -d \"$symlink_target\" &&\n-\t\t test \"#!\" != \"$(head -c 2 < \"$symlink_target\")\"\n-\t\tthen\n-\t\t\tsymlink_target=../valgrind.sh\n-\t\tfi\n-\t\t# create the link, or replace it if it is out of date\n-\t\tmake_symlink \"$symlink_target\" \\\n-\t\t\t\"$GIT_VALGRIND/bin/$base\" || exit\n+\t\tls \"$path\"/git \"$path\"/git-* 2> /dev/null |\n+\t\twhile read file\n+\t\tdo\n+\t\t\t# handle only executables\n+\t\t\ttest -x \"$file\" || continue\n+\n+\t\t\tbase=$(basename \"$file\")\n+\t\t\tsymlink_target=$TEST_DIRECTORY/../$base\n+\t\t\t# do not override scripts\n+\t\t\tif test -x \"$symlink_target\" &&\n+\t\t\t test ! -d \"$symlink_target\" &&\n+\t\t\t test \"#!\" != \"$(head -c 2 < \"$symlink_target\")\"\n+\t\t\tthen\n+\t\t\t\tsymlink_target=../valgrind.sh\n+\t\t\tfi\n+\t\t\tcase \"$base\" in\n+\t\t\t*.sh|*.perl)\n+\t\t\t\tsymlink_target=../unprocessed-script\n+\t\t\tesac\n+\t\t\t# create the link, or replace it if it is out of date\n+\t\t\tmake_symlink \"$symlink_target\" \\\n+\t\t\t\t\"$GIT_VALGRIND/bin/$base\" || exit\n+\t\tdone\n \tdone\n+\tIFS=$OLDIFS\n \tPATH=$GIT_VALGRIND/bin:$PATH\n \tGIT_EXEC_PATH=$GIT_VALGRIND/bin\n \texport GIT_VALGRIND\n-- \n1.6.1.482.g7d54be\n"},{"id":"101903","messageId":"20090125232954.GC19099@coredump.intra.peff.net","threadId":"17248","inReplyTo":"alpine.DEB.1.00.0901260018340.14855@racer","subject":"Re: [PATCH v3 1/3] Add valgrind support in test scripts","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-01-25T23:29:55Z","receivedAt":"2009-01-25T23:29:55Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jan 26, 2009 at 12:18:50AM +0100, Johannes Schindelin wrote:\n\n> Note: it is safe to run the valgrind tests in parallel, as the links in\n> t/valgrind/bin/ are created using proper locking.\n\nI actually kind of liked the original atomic version over the one with\nlocking. But I find this one acceptable.\n\n> Initial patch and all the hard work by Jeff King.\n> \n> Signed-off-by: Johannes Schindelin \n\nI don't know that there is much of my work left in here, but feel free\nto add:\n\n Signed-off-by: Jeff King \n\n-Peff\n"},{"id":"101907","messageId":"20090125233243.GD19099@coredump.intra.peff.net","threadId":"17248","inReplyTo":"alpine.DEB.1.00.0901260019000.14855@racer","subject":"Re: [PATCH v3 2/3] valgrind: ignore ldso and more libz errors","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-01-25T23:32:43Z","receivedAt":"2009-01-25T23:32:43Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jan 26, 2009 at 12:19:12AM +0100, Johannes Schindelin wrote:\n\n> \n> On some Linux systems, we get a host of Cond and Addr errors\n> from calls to dlopen that are caused by nss modules. We\n> should be able to safely ignore anything happening in\n> ld-*.so as \"not our problem.\"\n> \n> [Johannes: I added some more...]\n> \n> Signed-off-by: Jeff King \n> Signed-off-by: Johannes Schindelin \n\nYour 0/3 cover letter lists this me as the author of this patch, but\nthere is no \"From:\" line at the top of this email. I don't particularly\ncare one way or the other for this patch, but I wanted to point it out\nas a potential issue with your patch-sending workflow.\n\n-Peff\n"},{"id":"101908","messageId":"alpine.DEB.1.00.0901260034520.14855@racer","threadId":"17248","inReplyTo":"20090125232954.GC19099@coredump.intra.peff.net","subject":"Re: [PATCH v3 1/3] Add valgrind support in test scripts","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-25T23:35:31Z","receivedAt":"2009-01-25T23:35:31Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 25 Jan 2009, Jeff King wrote:\n\n> On Mon, Jan 26, 2009 at 12:18:50AM +0100, Johannes Schindelin wrote:\n> \n> > Note: it is safe to run the valgrind tests in parallel, as the links \n> > in t/valgrind/bin/ are created using proper locking.\n> \n> I actually kind of liked the original atomic version over the one with\n> locking. But I find this one acceptable.\n\nThe locking is only in there because of you...\n\n> > Initial patch and all the hard work by Jeff King.\n> > \n> > Signed-off-by: Johannes Schindelin \n> \n> I don't know that there is much of my work left in here, but feel free \n> to add:\n> \n> Signed-off-by: Jeff King \n\nWill do!\n\nCiao,\nDscho\n"},{"id":"101910","messageId":"20090125234204.GA19202@coredump.intra.peff.net","threadId":"17248","inReplyTo":"alpine.DEB.1.00.0901260034520.14855@racer","subject":"Re: [PATCH v3 1/3] Add valgrind support in test scripts","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-01-25T23:42:04Z","receivedAt":"2009-01-25T23:42:04Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jan 26, 2009 at 12:35:31AM +0100, Johannes Schindelin wrote:\n\n> > I actually kind of liked the original atomic version over the one with\n> > locking. But I find this one acceptable.\n> \n> The locking is only in there because of you...\n\nI know it came out of our discussion, but I thought it was going a bit\nfar. That is, what should ideally be a little chunk of code to make some\nlinks keeps getting more and more complex. And as your locking patch\ncame after my \"OK, I guess this is fine\" comments, I thought you\nrealized I was accepting it as-is.\n\nSo sorry to make you to go to extra work (and please don't go to extra\nwork ripping it out on my account -- I just wanted to make clear that I\ndecided your analysis was sane, and that I am OK with any of the\niterations you posted).\n\n-Peff\n"},{"id":"101909","messageId":"20090125234249.GE19099@coredump.intra.peff.net","threadId":"17248","inReplyTo":"alpine.DEB.1.00.0901260019160.14855@racer","subject":"Re: [PATCH 3/3] Valgrind support: check for more than just programming errors","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-01-25T23:42:49Z","receivedAt":"2009-01-25T23:42:49Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jan 26, 2009 at 12:20:21AM +0100, Johannes Schindelin wrote:\n\n> While it is not strictly necessary to look through the whole PATH to\n> find git binaries to override, it is in line with running an expensive\n> test (which valgrind is) to make extra sure that no binary is tested\n> that actually comes from the git.git checkout.\n\nShould this be \"...no binary is tested that _doesn't_ actually come from\nthe git.git checkout\"?\n\n-Peff\n"},{"id":"101917","messageId":"alpine.DEB.1.00.0901260101030.14855@racer","threadId":"17248","inReplyTo":"20090125233243.GD19099@coredump.intra.peff.net","subject":"Re: [PATCH v3 2/3] valgrind: ignore ldso and more libz errors","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-26T00:02:24Z","receivedAt":"2009-01-26T00:02:24Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 25 Jan 2009, Jeff King wrote:\n\n> On Mon, Jan 26, 2009 at 12:19:12AM +0100, Johannes Schindelin wrote:\n> \n> > On some Linux systems, we get a host of Cond and Addr errors from \n> > calls to dlopen that are caused by nss modules. We should be able to \n> > safely ignore anything happening in ld-*.so as \"not our problem.\"\n> > \n> > [Johannes: I added some more...]\n> > \n> > Signed-off-by: Jeff King \n> > Signed-off-by: Johannes Schindelin \n> \n> Your 0/3 cover letter lists this me as the author of this patch, but \n> there is no \"From:\" line at the top of this email. I don't particularly \n> care one way or the other for this patch, but I wanted to point it out \n> as a potential issue with your patch-sending workflow.\n\nYep, sorry. I would not touch send-email with lead-protected gloves, so \nwhat I do is to edit all patches I send. And in this case, I missed the \nfact that there was another \"From:\". I am sorry.\n\nCiao,\nDscho \"who is burning midnight oil again\"\n"},{"id":"101919","messageId":"20090126001451.GA20256@coredump.intra.peff.net","threadId":"17248","inReplyTo":"alpine.DEB.1.00.0901260101030.14855@racer","subject":"Re: [PATCH v3 2/3] valgrind: ignore ldso and more libz errors","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-01-26T00:14:52Z","receivedAt":"2009-01-26T00:14:52Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jan 26, 2009 at 01:02:24AM +0100, Johannes Schindelin wrote:\n\n> > Your 0/3 cover letter lists this me as the author of this patch, but \n> > there is no \"From:\" line at the top of this email. I don't particularly \n> > care one way or the other for this patch, but I wanted to point it out \n> > as a potential issue with your patch-sending workflow.\n> \n> Yep, sorry. I would not touch send-email with lead-protected gloves, so \n> what I do is to edit all patches I send. And in this case, I missed the \n> fact that there was another \"From:\". I am sorry.\n\nHeh. I certainly can't blame you for that; I don't use send-email\nmyself.\n\nIt might be convenient for format-patch to have a mode where it uses the\ncommitter as the rfc822 \"From:\" and then adds a \"From:\" for the author\nin the body if it is not the same as the committer.\n\nIt certainly shouldn't be the default, since that would confuse things\nlike rebase. But it makes sense if you are just going to throw away the\nFrom header anyway when you import into your MUA.\n\n-Peff\n"},{"id":"101922","messageId":"alpine.DEB.1.00.0901260142100.14855@racer","threadId":"17248","inReplyTo":"20090125234249.GE19099@coredump.intra.peff.net","subject":"Re: [PATCH 3/3] Valgrind support: check for more than just programming errors","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-26T00:43:13Z","receivedAt":"2009-01-26T00:43:13Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 25 Jan 2009, Jeff King wrote:\n\n> On Mon, Jan 26, 2009 at 12:20:21AM +0100, Johannes Schindelin wrote:\n> \n> > While it is not strictly necessary to look through the whole PATH to\n> > find git binaries to override, it is in line with running an expensive\n> > test (which valgrind is) to make extra sure that no binary is tested\n> > that actually comes from the git.git checkout.\n> \n> Should this be \"...no binary is tested that _doesn't_ actually come from\n> the git.git checkout\"?\n\nYep, that was half the change to \"that only binaries are tested...\".\n\nThanks,\nDscho\n"},{"id":"102073","messageId":"alpine.DEB.1.00.0901270327200.26199@intel-tinevez-2-302","threadId":"17248","inReplyTo":"alpine.DEB.1.00.0901212259420.3586@pacific.mpi-cbg.de","subject":"Valgrind updates","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-27T02:50:48Z","receivedAt":"2009-01-27T02:50:48Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nit is real late now, so I am uncomfortable sending off a new patch series \n(I _know_ that I'll just introduce a stupid bug or forget to write a \ncommit message or whatever). In case you are interested in the current \nprogress, you know where my branches are.\n\nThe changes I made:\n\n- added t/valgrind/templates to t/.gitignore, too,\n\n- split out the valgrind-unrelated parts that Peff complained about,\n\n- added some more suppressions I needed,\n\n- added a mode whereby the tests' results are written to test-results/,\n\n- provided a Makefile target for further convenience,\n\n- added a script to coalesce the valgrind results by backtrace,\n\n- split out a patch that lets --valgrind imply --verbose, and\n\n- ran the scripts several times, which is a PITA because one run takes 5.5 \n hours (and the first time I forgot to redirect stderr, ouch, thus the \n test-results/ patch).\n\nI have an output from a previous full run, albeit it was done with an \nearlier version of the valgrind patch series I was not comfortable with, \nso I will not send it here. Besides, it is 300K (bzip2 -9 reduces that to \n20K), and I am sure you don't want to have it.\n\nJust that much, most of the backtraces are pretty repetitive. In fact, I \nthink most if not all of them touch xwrite.c (I got other errors from my \npatches, as I expected).\n\n==valgrind== Syscall param write(buf) points to uninitialised byte(s)\n==valgrind== at 0x5609E40: __write_nocancel (in /lib/libpthread-2.6.1.so)\n==valgrind== by 0x4D0380: xwrite (wrapper.c:129)\n==valgrind== by 0x4D046E: write_in_full (wrapper.c:159)\n==valgrind== by 0x4C0697: write_buffer (sha1_file.c:2275)\n==valgrind== by 0x4C0B1C: write_loose_object (sha1_file.c:2387)\n==valgrind== by 0x4C0C4F: write_sha1_file (sha1_file.c:2418)\n==valgrind== by 0x46DBB8: update_one (cache-tree.c:348)\n==valgrind== by 0x46D8CF: update_one (cache-tree.c:282)\n==valgrind== by 0x46DCCA: cache_tree_update (cache-tree.c:373)\n==valgrind== by 0x46E2B5: write_cache_as_tree (cache-tree.c:562)\n==valgrind== by 0x4662D4: cmd_write_tree (builtin-write-tree.c:36)\n==valgrind== by 0x404F37: run_command (git.c:243)\n==valgrind== Address 0x713dc23 is 51 bytes inside a block of size 195 alloc'd\n==valgrind== at 0x4C2273B: malloc (in /usr/local/lib/valgrind/amd64-linux/vgpreload_memcheck.so)\n==valgrind== by 0x4CFFCC: xmalloc (wrapper.c:20)\n==valgrind== by 0x4C0A33: write_loose_object (sha1_file.c:2362)\n==valgrind== by 0x4C0C4F: write_sha1_file (sha1_file.c:2418)\n==valgrind== by 0x46DBB8: update_one (cache-tree.c:348)\n==valgrind== by 0x46D8CF: update_one (cache-tree.c:282)\n==valgrind== by 0x46DCCA: cache_tree_update (cache-tree.c:373)\n==valgrind== by 0x46E2B5: write_cache_as_tree (cache-tree.c:562)\n==valgrind== by 0x4662D4: cmd_write_tree (builtin-write-tree.c:36)\n==valgrind== by 0x404F37: run_command (git.c:243)\n==valgrind== by 0x4050E4: handle_internal_command (git.c:387)\n==valgrind== by 0x4051CA: run_argv (git.c:425)\n\nwhich can be reproduced by running t0000-basic.out in valgrind mode.\n\nGood night, Vietnam,\nDscho\n"},{"id":"102075","messageId":"alpine.LFD.2.00.0901261934450.3123@localhost.localdomain","threadId":"17248","inReplyTo":"alpine.DEB.1.00.0901270327200.26199@intel-tinevez-2-302","subject":"Re: Valgrind updates","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2009-01-27T03:38:56Z","receivedAt":"2009-01-27T03:38:56Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 27 Jan 2009, Johannes Schindelin wrote:\n> \n> Just that much, most of the backtraces are pretty repetitive. In fact, I \n> think most if not all of them touch xwrite.c (I got other errors from my \n> patches, as I expected).\n> \n> ==valgrind== Syscall param write(buf) points to uninitialised byte(s)\n> ==valgrind== at 0x5609E40: __write_nocancel (in /lib/libpthread-2.6.1.so)\n> ==valgrind== by 0x4D0380: xwrite (wrapper.c:129)\n> ==valgrind== by 0x4D046E: write_in_full (wrapper.c:159)\n> ==valgrind== by 0x4C0697: write_buffer (sha1_file.c:2275)\n> ==valgrind== by 0x4C0B1C: write_loose_object (sha1_file.c:2387)\n\nLooks entirely bogus.\n\nI suspect that valgrind for some reason doesn't see the writes made by \nzlib as being initialization, possibly due to some incorrect valgrind \nannotations on deflate(). We've just totally initialized that whole \nbuffer with deflate().\n\nIt definitely does not look like a git bug, but a valgrind run issue.\n\n\t\tLinus\n"},{"id":"102078","messageId":"alpine.DEB.1.00.0901270512171.14855@racer","threadId":"17248","inReplyTo":"alpine.LFD.2.00.0901261934450.3123@localhost.localdomain","subject":"Re: Valgrind updates","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-27T04:26:34Z","receivedAt":"2009-01-27T04:26:34Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 26 Jan 2009, Linus Torvalds wrote:\n\n> On Tue, 27 Jan 2009, Johannes Schindelin wrote:\n> > \n> > Just that much, most of the backtraces are pretty repetitive. In \n> > fact, I think most if not all of them touch xwrite.c (I got other \n> > errors from my patches, as I expected).\n> > \n> > ==valgrind== Syscall param write(buf) points to uninitialised byte(s)\n> > ==valgrind== at 0x5609E40: __write_nocancel (in /lib/libpthread-2.6.1.so)\n> > ==valgrind== by 0x4D0380: xwrite (wrapper.c:129)\n> > ==valgrind== by 0x4D046E: write_in_full (wrapper.c:159)\n> > ==valgrind== by 0x4C0697: write_buffer (sha1_file.c:2275)\n> > ==valgrind== by 0x4C0B1C: write_loose_object (sha1_file.c:2387)\n> \n> Looks entirely bogus.\n\nAnd it gets worse.\n\nI suspected that zlib does something \"cute\" with alignments, i.e. that it \nwrites a possibly odd number of bytes, but then rounds up the buffer to \nthe next multiple of two of four bytes.\n\nYet, the buffer in question is 195 bytes, stream.total_count (which \ntotally agrees with size - stream.avail_out) says it is 58 bytes, and \nvalgrind says that the byte with offset 51 is uninitialized.\n\nSo it is definitely a zlib error. And a strange one at that. Even \nallowing for a header, if we have 51 valid bytes in the buffer (remember: \nthe 52nd byte is reported uninitialized by valgrind), even on a 64-bit \nmachine, it should not be rounded up to 58 bytes reported by zlib. And \nthe address of the buffer seems to be even 16-byte aligned (that's \nprobably valgrind's doing).\n\nJust for bullocks, I let valgrind check if offset 51 is the only \nuninitialized byte (who knows what zlib is thinking that it's doing?), and \nhere's the rub: offset 51 is indeed the _only_ one which valgrind thinks \nis uninitialized!\n\nWasn't there some zlib wizard in the kernel community? We could throw \nthat thing at him, to see why it behaves so strangely...\n\nOf course, it could also be a valgrind issue, as you suggested. Hmpf.\n\nCiao,\nDscho\n"},{"id":"102079","messageId":"alpine.DEB.1.00.0901270544450.14855@racer","threadId":"17248","inReplyTo":"alpine.DEB.1.00.0901270512171.14855@racer","subject":"Re: Valgrind updates","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-27T04:46:01Z","receivedAt":"2009-01-27T04:46:01Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 27 Jan 2009, Johannes Schindelin wrote:\n\n> On Mon, 26 Jan 2009, Linus Torvalds wrote:\n> \n> > On Tue, 27 Jan 2009, Johannes Schindelin wrote:\n> > > \n> > > Just that much, most of the backtraces are pretty repetitive. In \n> > > fact, I think most if not all of them touch xwrite.c (I got other \n> > > errors from my patches, as I expected).\n> > > \n> > > ==valgrind== Syscall param write(buf) points to uninitialised byte(s)\n> > > ==valgrind== at 0x5609E40: __write_nocancel (in /lib/libpthread-2.6.1.so)\n> > > ==valgrind== by 0x4D0380: xwrite (wrapper.c:129)\n> > > ==valgrind== by 0x4D046E: write_in_full (wrapper.c:159)\n> > > ==valgrind== by 0x4C0697: write_buffer (sha1_file.c:2275)\n> > > ==valgrind== by 0x4C0B1C: write_loose_object (sha1_file.c:2387)\n> > \n> > Looks entirely bogus.\n> \n> And it gets worse.\n> \n> I suspected that zlib does something \"cute\" with alignments, i.e. that \n> it writes a possibly odd number of bytes, but then rounds up the buffer \n> to the next multiple of two of four bytes.\n> \n> Yet, the buffer in question is 195 bytes, stream.total_count (which \n> totally agrees with size - stream.avail_out) says it is 58 bytes, and \n> valgrind says that the byte with offset 51 is uninitialized.\n> \n> So it is definitely a zlib error. And a strange one at that. Even \n> allowing for a header, if we have 51 valid bytes in the buffer \n> (remember: the 52nd byte is reported uninitialized by valgrind), even on \n> a 64-bit machine, it should not be rounded up to 58 bytes reported by \n> zlib. And the address of the buffer seems to be even 16-byte aligned \n> (that's probably valgrind's doing).\n> \n> Just for bullocks, I let valgrind check if offset 51 is the only \n> uninitialized byte (who knows what zlib is thinking that it's doing?), \n> and here's the rub: offset 51 is indeed the _only_ one which valgrind \n> thinks is uninitialized!\n> \n> Wasn't there some zlib wizard in the kernel community? We could throw \n> that thing at him, to see why it behaves so strangely...\n> \n> Of course, it could also be a valgrind issue, as you suggested. Hmpf.\n\nFWIW this test was done with 3.4.0.SVN.\n\nJust to be sure, I upgraded to 3.5.0.SVN, the very newest update (well, as \nnew as I could make my git svn mirror of valgrind and VEX deliver). Still \nthere.\n\nOff to bed,\nDscho\n"},{"id":"102080","messageId":"20090127044838.GA735@coredump.intra.peff.net","threadId":"17248","inReplyTo":"alpine.LFD.2.00.0901261934450.3123@localhost.localdomain","subject":"Re: Valgrind updates","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-01-27T04:48:38Z","receivedAt":"2009-01-27T04:48:38Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jan 26, 2009 at 07:38:56PM -0800, Linus Torvalds wrote:\n\n> > ==valgrind== Syscall param write(buf) points to uninitialised byte(s)\n> > ==valgrind== at 0x5609E40: __write_nocancel (in /lib/libpthread-2.6.1.so)\n> > ==valgrind== by 0x4D0380: xwrite (wrapper.c:129)\n> > ==valgrind== by 0x4D046E: write_in_full (wrapper.c:159)\n> > ==valgrind== by 0x4C0697: write_buffer (sha1_file.c:2275)\n> > ==valgrind== by 0x4C0B1C: write_loose_object (sha1_file.c:2387)\n> \n> Looks entirely bogus.\n> \n> I suspect that valgrind for some reason doesn't see the writes made by \n> zlib as being initialization, possibly due to some incorrect valgrind \n> annotations on deflate(). We've just totally initialized that whole \n> buffer with deflate().\n> \n> It definitely does not look like a git bug, but a valgrind run issue.\n\nYes, this is exactly the issue I ran into when doing the valgrind stuff\na few months ago. I spent several hours looking carefully at the code\nand came to the same conclusion. Anything zlib touches needs to be\nmanually suppressed for uninitialized writes (which I _thought_ was\ncovered in the suppressions I sent out originally, but maybe they need\nto be tweaked for Dscho's system).\n\n-Peff\n"},{"id":"102099","messageId":"alpine.DEB.1.00.0901271030000.14855@racer","threadId":"17248","inReplyTo":"20090127044838.GA735@coredump.intra.peff.net","subject":"Re: Valgrind updates","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-27T09:31:05Z","receivedAt":"2009-01-27T09:31:05Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 26 Jan 2009, Jeff King wrote:\n\n> On Mon, Jan 26, 2009 at 07:38:56PM -0800, Linus Torvalds wrote:\n> \n> > > ==valgrind== Syscall param write(buf) points to uninitialised byte(s)\n> > > ==valgrind== at 0x5609E40: __write_nocancel (in /lib/libpthread-2.6.1.so)\n> > > ==valgrind== by 0x4D0380: xwrite (wrapper.c:129)\n> > > ==valgrind== by 0x4D046E: write_in_full (wrapper.c:159)\n> > > ==valgrind== by 0x4C0697: write_buffer (sha1_file.c:2275)\n> > > ==valgrind== by 0x4C0B1C: write_loose_object (sha1_file.c:2387)\n> > \n> > Looks entirely bogus.\n> > \n> > I suspect that valgrind for some reason doesn't see the writes made by \n> > zlib as being initialization, possibly due to some incorrect valgrind \n> > annotations on deflate(). We've just totally initialized that whole \n> > buffer with deflate().\n> > \n> > It definitely does not look like a git bug, but a valgrind run issue.\n> \n> Yes, this is exactly the issue I ran into when doing the valgrind stuff\n> a few months ago. I spent several hours looking carefully at the code\n> and came to the same conclusion. Anything zlib touches needs to be\n> manually suppressed for uninitialized writes (which I _thought_ was\n> covered in the suppressions I sent out originally, but maybe they need\n> to be tweaked for Dscho's system).\n\nIndeed. I used the \"...\" wildcard to account for slight differences in \nGit's code calling path.\n\nSorry for the noise,\nDscho\n"},{"id":"102116","messageId":"20090127131404.GA11870@sirena.org.uk","threadId":"17248","inReplyTo":"alpine.DEB.1.00.0901270512171.14855@racer","subject":"Re: Valgrind updates","fromName":"Mark Brown","fromEmail":"broonie@sirena.org.uk","sentAt":"2009-01-27T13:14:04Z","receivedAt":"2009-01-27T13:14:04Z","isPatch":false,"sender":{"key":"broonie@sirena.org.uk","avatar":"https://gravatar.com/avatar/9e798c729a4a709279df497d9608ad68422755c9670e92434a5436f7e607cf86?d=mp&s=160"},"body":"On Tue, Jan 27, 2009 at 05:26:34AM +0100, Johannes Schindelin wrote:\n\n> I suspected that zlib does something \"cute\" with alignments, i.e. that it \n> writes a possibly odd number of bytes, but then rounds up the buffer to \n> the next multiple of two of four bytes.\n\nI don't recall anything along those lines in zlib but it does generate\nwarnings with valgrind which require overrides - it has at least one\nunrolled loop which roll on beyond initialised memory (but keep within\nmemory that zlib knows it has allocated). It rolls back the results of\nthe loop before producing output, but it's possible that some unused\nbits in the stream may be derived from the results.\n"},{"id":"102135","messageId":"alpine.DEB.1.00.0901271742430.3586@pacific.mpi-cbg.de","threadId":"17248","inReplyTo":"20090127131404.GA11870@sirena.org.uk","subject":"Re: Valgrind updates","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-27T16:54:56Z","receivedAt":"2009-01-27T16:54:56Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 27 Jan 2009, Mark Brown wrote:\n\n> On Tue, Jan 27, 2009 at 05:26:34AM +0100, Johannes Schindelin wrote:\n> \n> > I suspected that zlib does something \"cute\" with alignments, i.e. that \n> > it writes a possibly odd number of bytes, but then rounds up the \n> > buffer to the next multiple of two of four bytes.\n> \n> I don't recall anything along those lines in zlib but it does generate \n> warnings with valgrind which require overrides - it has at least one \n> unrolled loop which roll on beyond initialised memory (but keep within \n> memory that zlib knows it has allocated). It rolls back the results of \n> the loop before producing output, but it's possible that some unused \n> bits in the stream may be derived from the results.\n\nThat is what I suspected, but the data contradict this:\n\n- accesses to all offsets between 0 and 50 and 52 and 58 (one _more_ than \n indicated as valid by stream.total_count!) do not trigger any message in \n valgrind.\n\n- access to offset 51, which is well _within_ the boundaries, and even \n well outside the range of a stray alignment issue, _does_ trigger a \n valgrind message.\n\nSo either valgrind gets it wrong (which I find rather unlikely), or zlib \nreally does not write to that offset.\n\nOr, and I think that makes most sense so far, valgrind has not really \nignored the initialization of byte number 52 in that buffer which partly \ndepended on an uninitialized value (but does not matter, maybe due to \nHuffman cutoff or something similar).\n\nCome to think of it, the word \"suppression\" is probably a good indicator \nthat valgrind never claimed it would mark the zlib buffer as properly \ninitialized.\n\nSorry for the noise, then,\nDscho\n"},{"id":"102156","messageId":"alpine.LFD.2.00.0901271006060.3123@localhost.localdomain","threadId":"17248","inReplyTo":"alpine.DEB.1.00.0901271742430.3586@pacific.mpi-cbg.de","subject":"Re: Valgrind updates","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2009-01-27T18:55:40Z","receivedAt":"2009-01-27T18:55:40Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 27 Jan 2009, Johannes Schindelin wrote:\n> \n> Come to think of it, the word \"suppression\" is probably a good indicator \n> that valgrind never claimed it would mark the zlib buffer as properly \n> initialized.\n\nHmm. The zlib faq has a note about zlib doing a conditional on \nuninitialized memory that doesn't matter, and that is what the suppression \nshould be about (to avoid a warning about \"Conditional jump or move \ndepends on uninitialised value\").\n\nBut that one is documented to not matter for the actual output (zlib \nFAQ#36).\n\nIt's possible that zlib really does leave padding bytes around that \nliterally don't matter, and that don't get initialized. That really would \nbe bad, because it means that the output of git wouldn't be repeatable. \nBut I doubt this is the case - original git used to actually do the SHA1 \nover the _compressed_ data, which was admittedly a totally and utterly \nbroken design (and we fixed it), but it did work. Maybe it worked by luck, \nbut I somehow doubt it.\n\nSome googling did find this:\n\n\thttp://mailman.few.vu.nl/pipermail/sysprog/2008-October/000298.html\n\nwhich looks very similar: an uninitialized byte in the middle of a \ndeflate() packet.\n\nAnyway, I'm just going to Cc 'zlib@gzip.org', since this definitely is \n_not_ the same issue as in the FAQ, and we're not the only ones seeing it. \nFor the zlib people: the code is literally this:\n\n /* Set it up */\n memset(&stream, 0, sizeof(stream));\n deflateInit(&stream, zlib_compression_level);\n size = 8 + deflateBound(&stream, len+hdrlen);\n compressed = xmalloc(size);\n\n /* Compress it */\n stream.next_out = compressed;\n stream.avail_out = size;\n\n /* First header.. */\n stream.next_in = (unsigned char *)hdr;\n stream.avail_in = hdrlen;\n while (deflate(&stream, 0) == Z_OK)\n /* nothing */;\n\n /* Then the data itself.. */\n stream.next_in = buf;\n stream.avail_in = len;\n ret = deflate(&stream, Z_FINISH);\n if (ret != Z_STREAM_END)\n die(\"unable to deflate new object %s (%d)\", sha1_to_hex(sha1), ret);\n\n ret = deflateEnd(&stream);\n if (ret != Z_OK)\n die(\"deflateEnd on object %s failed (%d)\", sha1_to_hex(sha1), ret);\n\n size = stream.total_out;\n\n if (write_buffer(fd, compressed, size) < 0)\n die(\"unable to write sha1 file\");\n\nand valgrind complains that the \"write_buffer()\" call will touch an \nuninitialized byte (just one byte, and in the _middle_ of the buffer, no \nless):\n\n> Yet, the buffer in question is 195 bytes, stream.total_count (which \n> totally agrees with size - stream.avail_out) says it is 58 bytes, and \n> valgrind says that the byte with offset 51 is uninitialized.\n\nThe thing to note here is that what we are passing in to \"write_buffer()\" \nis _exactly_ what zlib deflated for us:\n\n - 'compressed' is the allocation, and is what we used to initialize \n 'stream.next_out' with (at the top of the code sequence above)\n\n - 'size' is gotten from 'stream.total_out' at the end of the compression.\n\nMaybe the zlib people can tell us that we're idiots and the above is \nbuggy, but maybe there is a real bug in zlib. Maybe it's triggered by our \nuse of using two different input buffers to deflate() (ie we compress the \nheader first, and then the body of the actual data, and put it all in one \nsingle output buffer), which may be unusual usage of zlib routines and may \nbe why there aren't tons of reports of this.\n\n(Our use of just depending on deflate() returning Z_BUF_ERROR after \nconsuming all of the header data is probably also \"unusual\", but the \nmanual explicitly says that it's not fatal and that deflate can be called \nagain with more buffers).\n\nOh Gods of zlib, please hear our plea for clarification..\n\n\t\t\tLinus\n"},{"id":"102164","messageId":"alpine.DEB.1.00.0901272241250.14855@racer","threadId":"17248","inReplyTo":"alpine.LFD.2.00.0901271006060.3123@localhost.localdomain","subject":"Re: Valgrind updates","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-27T21:52:39Z","receivedAt":"2009-01-27T21:52:39Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\n[Cc'ed the valgrind-users list, maybe the valgrind Gods can see that our \n case is pretty strange, and tell us what we do wrong.]\n\nNote to valgrind experts: this is _not_ about the Conditional thing in \nzlib, but about an uninitialized byte _in the middle_ of the zlib output \nbuffer.\n\n On Tue, 27 Jan 2009, Linus Torvalds wrote:\n\n> Hmm. The zlib faq has a note about zlib doing a conditional on \n> uninitialized memory that doesn't matter, and that is what the \n> suppression should be about (to avoid a warning about \"Conditional jump \n> or move depends on uninitialised value\").\n> \n> But that one is documented to not matter for the actual output (zlib \n> FAQ#36).\n> \n> It's possible that zlib really does leave padding bytes around that \n> literally don't matter, and that don't get initialized. That really \n> would be bad, because it means that the output of git wouldn't be \n> repeatable. But I doubt this is the case - original git used to actually \n> do the SHA1 over the _compressed_ data, which was admittedly a totally \n> and utterly broken design (and we fixed it), but it did work. Maybe it \n> worked by luck, but I somehow doubt it.\n> \n> Some googling did find this:\n> \n> \thttp://mailman.few.vu.nl/pipermail/sysprog/2008-October/000298.html\n> \n> which looks very similar: an uninitialized byte in the middle of a \n> deflate() packet.\n> \n> Anyway, I'm just going to Cc 'zlib@gzip.org', since this definitely is \n> _not_ the same issue as in the FAQ, and we're not the only ones seeing it.\n>\n> [...]\n>\n> Dscho wrote:\n>\n> > Yet, the buffer in question is 195 bytes, stream.total_count (which \n> > totally agrees with size - stream.avail_out) says it is 58 bytes, and \n> > valgrind says that the byte with offset 51 is uninitialized.\n> \n> The thing to note here is that what we are passing in to \"write_buffer()\" \n> is _exactly_ what zlib deflated for us:\n> \n> - 'compressed' is the allocation, and is what we used to initialize \n> 'stream.next_out' with (at the top of the code sequence above)\n> \n> - 'size' is gotten from 'stream.total_out' at the end of the compression.\n> \n> Oh Gods of zlib, please hear our plea for clarification..\n\nTo help ye Gods, I put together this almost minimal C program:\n\n-- snip --\n#include \n#include \n#include \n#include \n\nint main(int argc, char **argv)\n{\n\tconst char hdr[] = {\n\t\t0x74, 0x72, 0x65, 0x65, 0x20, 0x31, 0x36, 0x35,\n\t\t0x00,\n\t};\n\tint hdrlen = sizeof(hdr);\n\tconst char buf[] = {\n\t\t0x31, 0x30, 0x30, 0x36, 0x34, 0x34, 0x20, 0x66,\n\t\t0x69, 0x6c, 0x65, 0x31, 0x00, 0x10, 0x00, 0x00,\n\t\t0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,\n\t\t0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,\n\t\t0x00, 0x31, 0x30, 0x30, 0x36, 0x34, 0x34, 0x20,\n\t\t0x66, 0x69, 0x6c, 0x65, 0x32, 0x00, 0x20, 0x00,\n\t\t0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,\n\t\t0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,\n\t\t0x00, 0x00, 0x31, 0x30, 0x30, 0x36, 0x34, 0x34,\n\t\t0x20, 0x66, 0x69, 0x6c, 0x65, 0x33, 0x00, 0x30,\n\t\t0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,\n\t\t0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,\n\t\t0x00, 0x00, 0x00, 0x31, 0x30, 0x30, 0x36, 0x34,\n\t\t0x34, 0x20, 0x66, 0x69, 0x6c, 0x65, 0x34, 0x00,\n\t\t0x40, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,\n\t\t0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,\n\t\t0x00, 0x00, 0x00, 0x00, 0x31, 0x30, 0x30, 0x36,\n\t\t0x34, 0x34, 0x20, 0x66, 0x69, 0x6c, 0x65, 0x35,\n\t\t0x00, 0x50, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,\n\t\t0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,\n\t\t0x00, 0x00, 0x00, 0x00, 0x00,\n\t};\n\tint len = sizeof(buf);\n\tz_stream stream;\n\tunsigned char *compressed;\n\tint size, ret, i;\n\tFILE *out;\n\n\tmemset(&stream, 0, sizeof(stream));\n\tdeflateInit(&stream, Z_BEST_SPEED);\n\tsize = 8 + deflateBound(&stream, len+hdrlen);\n\tcompressed = malloc(size);\n\tif (!compressed)\n\t\treturn 1;\n\n\tstream.next_out = compressed;\n\tstream.avail_out = size;\n\n\tstream.next_in = (unsigned char *)hdr;\n\tstream.avail_in = hdrlen;\n\twhile ((ret = deflate(&stream, 0)) == Z_OK)\n\t\t/* nothing */;\n\t/* deflate() returns Z_BUF_ERROR at this point */\n\n\tstream.next_in = (unsigned char *)buf;\n\tstream.avail_in = len;\n\tret = deflate(&stream, Z_FINISH);\n\tif (ret != Z_STREAM_END)\n\t\treturn 1;\n\n\tif (deflateEnd(&stream) != Z_OK)\n\t\treturn 1;\n\n\tout = fopen(\"/dev/null\", \"w\");\n\tfwrite(compressed + 51, 51, 1, out);\n\tfwrite(compressed + 51, 1, 1, stderr);\n\tfflush(out);\n\tfclose(out);\n\n\tfree(compressed);\n\treturn 0;\n}\n-- snap --\n\n... which produces this output...\n\n-- snip --\n==6348== Memcheck, a memory error detector.\n==6348== Copyright (C) 2002-2008, and GNU GPL'd, by Julian Seward et al.\n==6348== Using LibVEX rev exported, a library for dynamic binary translation.\n==6348== Copyright (C) 2004-2008, and GNU GPL'd, by OpenWorks LLP.\n==6348== Using valgrind-3.5.0.SVN, a dynamic binary instrumentation framework.\n==6348== Copyright (C) 2000-2008, and GNU GPL'd, by Julian Seward et al.\n==6348== For more details, rerun with: -v\n==6348== \n==6348== Use of uninitialised value of size 8\n==6348== at 0x4E2FC5B: (within /usr/lib/libz.so.1.2.3.3)\n==6348== by 0x4E317B6: (within /usr/lib/libz.so.1.2.3.3)\n==6348== by 0x4E2DF9C: (within /usr/lib/libz.so.1.2.3.3)\n==6348== by 0x4E2E654: deflate (in /usr/lib/libz.so.1.2.3.3)\n==6348== by 0x400957: main (valgrind-testcase.c:60)\n==6348== \n==6348== Syscall param write(buf) points to uninitialised byte(s)\n==6348== at 0x5103D50: write (in /lib/libc-2.6.1.so)\n==6348== by 0x50A9AE2: _IO_file_write (in /lib/libc-2.6.1.so)\n==6348== by 0x50A9748: (within /lib/libc-2.6.1.so)\n==6348== by 0x50A9A4B: _IO_file_xsputn (in /lib/libc-2.6.1.so)\n==6348== by 0x509FDBA: fwrite (in /lib/libc-2.6.1.so)\n==6348== by 0x4009D7: main (valgrind-testcase.c:69)\n==6348== Address 0x53da87b is 51 bytes inside a block of size 195 alloc'd\n==6348== at 0x4C222CB: malloc (in /usr/local/lib/valgrind/amd64-linux/vgpreload_memcheck.so)\n==6348== by 0x4008D7: main (valgrind-testcase.c:45)\n,==6348== \n==6348== Syscall param write(buf) points to uninitialised byte(s)\n==6348== at 0x5103D50: write (in /lib/libc-2.6.1.so)\n==6348== by 0x50A9AE2: _IO_file_write (in /lib/libc-2.6.1.so)\n==6348== by 0x50A9748: (within /lib/libc-2.6.1.so)\n==6348== by 0x50A9A83: _IO_do_write (in /lib/libc-2.6.1.so)\n==6348== by 0x50AA048: _IO_file_sync (in /lib/libc-2.6.1.so)\n==6348== by 0x509EDB9: fflush (in /lib/libc-2.6.1.so)\n==6348== by 0x4009E0: main (valgrind-testcase.c:70)\n==6348== Address 0x4020000 is not stack'd, malloc'd or (recently) free'd\n==6348== \n==6348== ERROR SUMMARY: 3 errors from 3 contexts (suppressed: 15 from 4)\n==6348== malloc/free: in use at exit: 0 bytes in 0 blocks.\n==6348== malloc/free: 7 allocs, 7 frees, 268,835 bytes allocated.\n==6348== For counts of detected errors, rerun with: -v\n==6348== Use --track-origins=yes to see where uninitialised values come from\n==6348== All heap blocks were freed -- no leaks are possible.\n-- snap --\n\nNote that the error only occurs when fwrite()ing to stderr, not \nany other file.\n\nThis is with valgrind compiled from a git-svn mirror updated today, i.e. \nvalgrind-3.5.0.SVN.\n\n\nCiao,\nDscho\n"},{"id":"102369","messageId":"69A01114-27BB-4239-8FD8-C35D1306CE25@alumni.caltech.edu","threadId":"17248","inReplyTo":"alpine.LFD.2.00.0901271006060.3123@localhost.localdomain","subject":"Re: Valgrind updates","fromName":"Mark Adler","fromEmail":"madler@alumni.caltech.edu","sentAt":"2009-01-28T23:06:44Z","receivedAt":"2009-01-28T23:06:44Z","isPatch":false,"sender":{"key":"madler@alumni.caltech.edu","avatar":null},"body":"On Jan 27, 2009, at 10:55 AM, Linus Torvalds wrote:\n> and valgrind complains that the \"write_buffer()\" call will touch an\n> uninitialized byte (just one byte, and in the _middle_ of the \n> buffer, no\n> less):\n\nLinus,\n\nThat is definitely not deflate's intentional use of uninitialized \nbytes that is noted in the zlib FAQ. This is something else.\n\n> Maybe the zlib people can tell us that we're idiots and the above is\n> buggy, but maybe there is a real bug in zlib.\n\nI can't speak to the idiot part, but your usage of deflate is not \nbuggy. (At least assuming that NULL is all zeros for the compiler in \nuse.)\n\nIf this is all correct, it sounds like a serious bug in deflate. If \nso, it would have to be a very sneaky bug to not have been discovered \nover the last decade or so of deflate usage on who knows how many \nzettabytes of data. The deflate code has remained largely unchanged \nin that time, and there really isn't anything unusual about your usage.\n\nI have some questions:\n\n1. Is this problem reproducible on more than one machine?\n\n2. Can someone send me the input and the 58 bytes of output from this \ncase?\n\n3. Did you try decompressing the 58 bytes?\n\n4. For the detection of an \"uninitialized byte\", if for example an \nuninitialized byte is copied to another location, is that location \nthen also considered uninitialized? Or does uninitialized mean that \nthat location has really never been written to?\n\n5. Would the access of uninitialized bytes by deflate have been \ndetected? Since I don't see a mention of uninitialized access before \nthe write_buffer(), does that mean that deflate never did such a thing \nitself?\n\nMark\n"},{"id":"102370","messageId":"alpine.DEB.1.00.0901290024290.3586@pacific.mpi-cbg.de","threadId":"17248","inReplyTo":"69A01114-27BB-4239-8FD8-C35D1306CE25@alumni.caltech.edu","subject":"Re: Valgrind updates","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-28T23:27:31Z","receivedAt":"2009-01-28T23:27:31Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 28 Jan 2009, Mark Adler wrote:\n\n> 2. Can someone send me the input and the 58 bytes of output from this \n> case?\n\nI did better than that already... \nhttp://article.gmane.org/gmane.comp.version-control.git/107391\n\nMaybe it did not go through correctly.\n\nUnfortunately, I was sick today and could not do any proper work, so I \ncould not even test the suggestions Julian gave me.\n\nThe easiest test, though, should be to set the byte at offset 51 to \nsomething bogus and see if inflate() still groks it.\n\nCiao,\nDscho\n"},{"id":"102376","messageId":"4D595705-7935-4AC2-91F4-1DAB3C6C7D27@alumni.caltech.edu","threadId":"17248","inReplyTo":"alpine.DEB.1.00.0901290024290.3586@pacific.mpi-cbg.de","subject":"Re: Valgrind updates","fromName":"Mark Adler","fromEmail":"madler@alumni.caltech.edu","sentAt":"2009-01-29T00:15:44Z","receivedAt":"2009-01-29T00:15:44Z","isPatch":false,"sender":{"key":"madler@alumni.caltech.edu","avatar":null},"body":"On Jan 28, 2009, at 3:27 PM, Johannes Schindelin wrote:\n> On Wed, 28 Jan 2009, Mark Adler wrote:\n>> 2. Can someone send me the input and the 58 bytes of output from \n>> this\n>> case?\n>\n> I did better than that already...\n> http://article.gmane.org/gmane.comp.version-control.git/107391\n\nJohannes,\n\nThanks for the input and code. When I run it, the byte in question at \noffset 51 is 0x2c. The output decompresses fine and the result \nmatches the input. If I change the 0x2c to anything else, \ndecompression fails. The 58 bytes are below.\n\nCan you also send me the 58 bytes of output that you get when you run \nit? Thanks.\n\nMark\n\n\n\n78 01 2b 29 4a 4d 55 30 34 33 65 30 34 30 30 33\n31 51 48 cb cc 49 35 64 10 60 c0 04 48 0a 8c 18\n14 30 e5 91 4d 30 66 30 c0 af c0 84 c1 01 bf 02\n53 86 00 2c 0a 00 86 79 13 07\n"},{"id":"102384","messageId":"alpine.LFD.2.00.0901281751580.3123@localhost.localdomain","threadId":"17248","inReplyTo":"alpine.DEB.1.00.0901272241250.14855@racer","subject":"Re: Valgrind updates","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2009-01-29T01:56:05Z","receivedAt":"2009-01-29T01:56:05Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 27 Jan 2009, Johannes Schindelin wrote:\n> \n> To help ye Gods, I put together this almost minimal C program:\n\nThis one is buggy.\n\n> \tout = fopen(\"/dev/null\", \"w\");\n> \tfwrite(compressed + 51, 51, 1, out);\n> \tfwrite(compressed + 51, 1, 1, stderr);\n> \tfflush(out);\n> \tfclose(out);\n\nThe problem is that the first argument to that first \"fwrite()\" is simply \nwrong. It shouldn't be \"compressed + 51\", it should be just \"compressed\". \nAs it is, you're writing 51 bytes, starting at 51 bytes in, and that's \nobviously not correct (you only got 58 bytes from deflate()).\n\nSo valgrind does complain about it, but for a perfectly valid reason.\n\nSo I think your minimal C program isn't actually showing what you wanted \nto show, and isn't showing the behaviour you see in git.\n\n\t\t\tLinus\n"},{"id":"102454","messageId":"alpine.DEB.1.00.0901291510520.3586@pacific.mpi-cbg.de","threadId":"17248","inReplyTo":"4D595705-7935-4AC2-91F4-1DAB3C6C7D27@alumni.caltech.edu","subject":"Re: Valgrind updates","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-29T14:14:17Z","receivedAt":"2009-01-29T14:14:17Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 28 Jan 2009, Mark Adler wrote:\n\n> On Jan 28, 2009, at 3:27 PM, Johannes Schindelin wrote:\n> >On Wed, 28 Jan 2009, Mark Adler wrote:\n> > >2. Can someone send me the input and the 58 bytes of output from this\n> > > case?\n> >\n> >I did better than that already...\n> >http://article.gmane.org/gmane.comp.version-control.git/107391\n> \n> Johannes,\n> \n> Thanks for the input and code. When I run it, the byte in question at \n> offset 51 is 0x2c. The output decompresses fine and the result matches \n> the input. If I change the 0x2c to anything else, decompression fails. \n> The 58 bytes are below.\n> \n> Can you also send me the 58 bytes of output that you get when you run it?\n\nI get exactly the same 58 bytes. Together with the fact that the 52nd \nbyte is actually required to be 0x2c, I think that maybe valgrind is \nhaving problems to track that this byte was correctly initialized.\n\nBTW did you have any chance to test the code with valgrind on your \nmachine? It might be related to this here platform (x86_64).\n\nCiao,\nDscho\n"},{"id":"102456","messageId":"alpine.DEB.1.00.0901291514240.3586@pacific.mpi-cbg.de","threadId":"17248","inReplyTo":"alpine.LFD.2.00.0901281751580.3123@localhost.localdomain","subject":"Re: Valgrind updates","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-29T14:22:59Z","receivedAt":"2009-01-29T14:22:59Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 28 Jan 2009, Linus Torvalds wrote:\n\n> On Tue, 27 Jan 2009, Johannes Schindelin wrote:\n> > \n> > To help ye Gods, I put together this almost minimal C program:\n> \n> This one is buggy.\n\nNot exactly buggy. Underexplained.\n\n> > \tout = fopen(\"/dev/null\", \"w\");\n> > \tfwrite(compressed + 51, 51, 1, out);\n> > \tfwrite(compressed + 51, 1, 1, stderr);\n> > \tfflush(out);\n> > \tfclose(out);\n> \n> The problem is that the first argument to that first \"fwrite()\" is simply \n> wrong. It shouldn't be \"compressed + 51\", it should be just \"compressed\". \n\nNope. It should be \"compressed + 51\" to narrow down the issue, as \nvalgrind does not complain about _any other_ offset.\n\nNot even when that is _well_ after the 58 bytes deflate() says are \navailable.\n\n> As it is, you're writing 51 bytes, starting at 51 bytes in, and that's \n> obviously not correct (you only got 58 bytes from deflate()).\n\nIt is not, granted. But I left it in for a purpose: to show that valgrind \ndoes not even bother to mention bytes we think should be invalid.\n\nI thought that there might be a shortcut for /dev/null, so I changed the \noutfile to a real file, and it _still_ does not complain.\n\n> So valgrind does complain about it, but for a perfectly valid reason.\n\nOnly it does not. It complains about the write of 1 byte, not the write \nof 51.\n\nBut I know why: \"out\" is opened buffered, so it shows the error (well \ndelayed, I might add, and not in a helpful manner) when fflush() is \ncalled.\n\nThe real issue, namely that an access of offset 51 triggers a valgrind \nerror, is demonstrated by my small test case.\n\nCiao,\nDscho\n"},{"id":"102463","messageId":"alpine.DEB.1.00.0901291547510.3586@pacific.mpi-cbg.de","threadId":"17248","inReplyTo":"alpine.DEB.1.00.0901291510520.3586@pacific.mpi-cbg.de","subject":"Re: Valgrind updates","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-29T14:54:11Z","receivedAt":"2009-01-29T14:54:11Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 29 Jan 2009, Johannes Schindelin wrote:\n\n> On Wed, 28 Jan 2009, Mark Adler wrote:\n> \n> > On Jan 28, 2009, at 3:27 PM, Johannes Schindelin wrote:\n> > >On Wed, 28 Jan 2009, Mark Adler wrote:\n> > > >2. Can someone send me the input and the 58 bytes of output from this\n> > > > case?\n> > >\n> > >I did better than that already...\n> > >http://article.gmane.org/gmane.comp.version-control.git/107391\n> > \n> > Johannes,\n> > \n> > Thanks for the input and code. When I run it, the byte in question at \n> > offset 51 is 0x2c. The output decompresses fine and the result matches \n> > the input. If I change the 0x2c to anything else, decompression fails. \n> > The 58 bytes are below.\n> > \n> > Can you also send me the 58 bytes of output that you get when you run it?\n> \n> I get exactly the same 58 bytes. Together with the fact that the 52nd \n> byte is actually required to be 0x2c, I think that maybe valgrind is \n> having problems to track that this byte was correctly initialized.\n> \n> BTW did you have any chance to test the code with valgrind on your \n> machine? It might be related to this here platform (x86_64).\n\nNow, things get interesting.\n\nOf course, I made sure that I had the newest zlib installed before \nmentioning publically that I found a strange valgrind issue.\n\nBut I did not build it from source myself; I installed what Ubuntu Gutsy \nGibbon had to offer me.\n\nNow that I tried to investigate further by compiling zlib from source, \ninstrumenting it with various valgrind-specific code to find out what is \nactually happening, I cannot reproduce anymore!\n\nSo I searched for the sources that Ubuntu provides, and I _still_ cannot \nreproduce.\n\nSo I'll just go for the easy solution, install plain straightforward \nzlib-1.2.3 (as opposed to zlib_1.2.3.3.dfsg-12ubuntu1), and apologise to \ny'all for all the bruhaha.\n\nCiao,\nDscho\n\nP.S.: Note that there is still something fishy going on, as Ubuntu's zlib \ngenerates the deflated stream correctly. But that will have to be \ninvestigated by someone with substantially more time on her hands than me.\n"}]}