{"thread":{"id":"20152","subject":"[test failure] t4114 binary file becomes symlink","startedAt":"2009-07-18T13:45:51Z","lastAt":"2009-07-23T16:22:34Z","messageCount":32,"participants":["Nicolas Sebrecht","Jeff King","Junio C Hamano","Johannes Sixt","Linus Torvalds","Jakub Narebski","Eric Blake","Brandon Casey","Paolo Bonzini"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"118231","messageId":"20090718134551.GC16708@vidovic","threadId":"20152","inReplyTo":null,"subject":"[test failure] t4114 binary file becomes symlink","fromName":"Nicolas Sebrecht","fromEmail":"nicolas.s.dev@gmx.fr","sentAt":"2009-07-18T13:45:51Z","receivedAt":"2009-07-18T13:45:51Z","isPatch":false,"sender":{"key":"nicolas.s.dev@gmx.fr","avatar":null},"body":"\nHi,\n\nRunning 'sh t4114-apply-typechange.sh --verbose --debug' fails since its\nintroduction by b67b9612e1a90ae093445abeaeff930e9f4cf936 with this\noutput:\n\n\n   * expecting success: \n   \tgit checkout -f foo-becomes-binary &&\n   \tgit diff-tree -p --binary HEAD foo-symlinked-to-bar > patch &&\n   \tgit apply --index < patch\n   \t\n   ./test-lib.sh: line 234: 26816 Segmentation fault      git checkout -f\n   foo-becomes-binary\n   * FAIL 8: binary file becomes symlink\n   \t\n   \t\tgit checkout -f foo-becomes-binary &&\n   \t\tgit diff-tree -p --binary HEAD foo-symlinked-to-bar > patch &&\n   \t\tgit apply --index < patch\n   \t\t\n   \n   diff --git a/foo b/foo\n   deleted file mode 120000\n   index ba0e162..0000000\n   --- a/foo\n   +++ /dev/null\n   @@ -1 +0,0 @@\n   -bar\n   \\ No newline at end of file\n   diff --git a/foo b/foo\n   new file mode 100644\n   index 0000000..ab26de5\n   --- /dev/null\n   +++ b/foo\n   @@ -0,0 +1 @@\n   +how far is the sun?\n\n\nThe filesystem used is ext3 (2.6.26-gentoo-r4-v7).\n\nI guess it's not really expected. So, I'll start my own research but I\ndon't know this code.\n\nI can provide more tests if needed.\n\n-- \nNicolas Sebrecht\n"},{"id":"118232","messageId":"20090718135649.GA6759@sigill.intra.peff.net","threadId":"20152","inReplyTo":"20090718134551.GC16708@vidovic","subject":"Re: [test failure] t4114 binary file becomes symlink","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-07-18T13:56:50Z","receivedAt":"2009-07-18T13:56:50Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Jul 18, 2009 at 03:45:51PM +0200, Nicolas Sebrecht wrote:\n\n> Running 'sh t4114-apply-typechange.sh --verbose --debug' fails since its\n> introduction by b67b9612e1a90ae093445abeaeff930e9f4cf936 with this\n> output:\n> \n> \n>    * expecting success: \n>    \tgit checkout -f foo-becomes-binary &&\n>    \tgit diff-tree -p --binary HEAD foo-symlinked-to-bar > patch &&\n>    \tgit apply --index < patch\n>    \t\n>    ./test-lib.sh: line 234: 26816 Segmentation fault      git checkout -f\n>    foo-becomes-binary\n\nSorry, I can't reproduce here (I tried v1.6.3 and the current\n'next'). The tests pass just fine with --debug (which, IIRC, doesn't\nactually do much). What is the exact commit you're seeing it fail on?\n\nCan you try running it under gdb to get a stack trace? If you have\nvalgrind installed, can you run the test script with --valgrind?\n\n-Peff\n"},{"id":"118233","messageId":"20090718141658.GE16708@vidovic","threadId":"20152","inReplyTo":"20090718135649.GA6759@sigill.intra.peff.net","subject":"[test failure] Re: t4114 binary file becomes symlink","fromName":"Nicolas Sebrecht","fromEmail":"nicolas.s.dev@gmx.fr","sentAt":"2009-07-18T14:16:58Z","receivedAt":"2009-07-18T14:16:58Z","isPatch":false,"sender":{"key":"nicolas.s.dev@gmx.fr","avatar":null},"body":"The 18/07/09, Jeff King wrote:\n> On Sat, Jul 18, 2009 at 03:45:51PM +0200, Nicolas Sebrecht wrote:\n> \n> > Running 'sh t4114-apply-typechange.sh --verbose --debug' fails since its\n> > introduction by b67b9612e1a90ae093445abeaeff930e9f4cf936 with this\n> > output:\n> > \n> > \n> >    * expecting success: \n> >    \tgit checkout -f foo-becomes-binary &&\n> >    \tgit diff-tree -p --binary HEAD foo-symlinked-to-bar > patch &&\n> >    \tgit apply --index < patch\n> >    \t\n> >    ./test-lib.sh: line 234: 26816 Segmentation fault      git checkout -f\n> >    foo-becomes-binary\n> \n> Sorry, I can't reproduce here (I tried v1.6.3 and the current\n> 'next'). The tests pass just fine with --debug (which, IIRC, doesn't\n> actually do much). What is the exact commit you're seeing it fail on?\n\nIt fails on:\n  - next\n  - v1.6.3\n  - b67b9612e1a90ae093445abeaeff930e9f4cf936\n  - (other I don't remember, but does it really matter?)\n\n> Can you try running it under gdb to get a stack trace? If you have\n> valgrind installed, can you run the test script with --valgrind?\n\n$ sh t4114-apply-typechange.sh --valgrind\n\n<snip>\n\n* expecting success: \n\tgit checkout -f foo-becomes-binary &&\n\tgit diff-tree -p --binary HEAD foo-symlinked-to-bar > patch &&\n\tgit apply --index < patch\n\t\n==10807== Invalid read of size 1\n==10807==    at 0x4C22349: strlen (in /usr/lib64/valgrind/amd64-linux/vgpreload_memcheck.so)\n==10807==    by 0x5616ED6: vfprintf (in /lib64/libc-2.8.so)\n==10807==    by 0x563C159: vsnprintf (in /lib64/libc-2.8.so)\n==10807==    by 0x495E90: git_vsnprintf (snprintf.c:38)\n==10807==    by 0x48917B: strbuf_addf (strbuf.c:203)\n==10807==    by 0x412AA0: cmd_checkout (builtin-checkout.c:364)\n==10807==    by 0x404222: handle_internal_command (git.c:243)\n==10807==    by 0x404466: main (git.c:483)\n==10807==  Address 0x1 is not stack'd, malloc'd or (recently) free'd\n{\n   <insert a suppression name here>\n   Memcheck:Addr1\n   fun:strlen\n   fun:vfprintf\n   fun:vsnprintf\n   fun:git_vsnprintf\n   fun:strbuf_addf\n   fun:cmd_checkout\n   fun:handle_internal_command\n   fun:main\n}\n==10807== \n==10807== Process terminating with default action of signal 11 (SIGSEGV)\n==10807==  Access not within mapped region at address 0x1\n==10807==    at 0x4C22349: strlen (in /usr/lib64/valgrind/amd64-linux/vgpreload_memcheck.so)\n==10807==    by 0x5616ED6: vfprintf (in /lib64/libc-2.8.so)\n==10807==    by 0x563C159: vsnprintf (in /lib64/libc-2.8.so)\n==10807==    by 0x495E90: git_vsnprintf (snprintf.c:38)\n==10807==    by 0x48917B: strbuf_addf (strbuf.c:203)\n==10807==    by 0x412AA0: cmd_checkout (builtin-checkout.c:364)\n==10807==    by 0x404222: handle_internal_command (git.c:243)\n==10807==    by 0x404466: main (git.c:483)\n==10807==  If you believe this happened as a result of a stack overflow in your\n==10807==  program's main thread (unlikely but possible), you can try to increase\n==10807==  the size of the main thread stack using the --main-stacksize= flag.\n==10807==  The main thread stack size used in this run was 8388608.\n* FAIL 8: binary file becomes symlink\n\n<snip>\n\n$\n\n-- \nNicolas Sebrecht\n"},{"id":"118236","messageId":"20090718153148.GA9367@sigill.intra.peff.net","threadId":"20152","inReplyTo":"20090718141658.GE16708@vidovic","subject":"Re: [test failure] Re: t4114 binary file becomes symlink","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-07-18T15:31:49Z","receivedAt":"2009-07-18T15:31:49Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Jul 18, 2009 at 04:16:58PM +0200, Nicolas Sebrecht wrote:\n\n> It fails on:\n>   - next\n>   - v1.6.3\n>   - b67b9612e1a90ae093445abeaeff930e9f4cf936\n>   - (other I don't remember, but does it really matter?)\n\nHmm. So it is clearly reproducible on your system, but not on mine. I\nwonder what the difference could be.\n\nAre you compiling with any special options? I usually compile with just\n\"-g -Wall -Werror\", but I also tried with \"-O2\" and couldn't reproduce.\nMaybe compiler version? I'm using gcc 4.3.3.\n\n> ==10807== Invalid read of size 1\n> ==10807==    at 0x4C22349: strlen (in /usr/lib64/valgrind/amd64-linux/vgpreload_memcheck.so)\n> ==10807==    by 0x5616ED6: vfprintf (in /lib64/libc-2.8.so)\n> ==10807==    by 0x563C159: vsnprintf (in /lib64/libc-2.8.so)\n> ==10807==    by 0x495E90: git_vsnprintf (snprintf.c:38)\n> ==10807==    by 0x48917B: strbuf_addf (strbuf.c:203)\n> ==10807==    by 0x412AA0: cmd_checkout (builtin-checkout.c:364)\n> ==10807==    by 0x404222: handle_internal_command (git.c:243)\n> ==10807==    by 0x404466: main (git.c:483)\n> ==10807==  Address 0x1 is not stack'd, malloc'd or (recently) free'd\n\nLooking at that strbuf_addf call, we presumably have a bogus pointer\neither in old->name or new->name. Which is odd, since reading the code,\nboth get memset() to zero, and then assigned from something which should\nbe sane.\n\nAt this point, I would try either running it under gdb or putting in\nsome debugging printfs into update_refs_for_switch to try to isolate\nwhere the bogus value is coming from (valgrind sees it as cmd_checkout,\nbut presumably that is because it inlines the static\nupdate_refs_for_switch).\n\nCan you try that? Otherwise, I'm not sure how to proceed because I can't\nreproduce it on my box.\n\n-Peff\n"},{"id":"118242","messageId":"7veisd3k31.fsf@alter.siamese.dyndns.org","threadId":"20152","inReplyTo":"20090718153148.GA9367@sigill.intra.peff.net","subject":"Re: [test failure] Re: t4114 binary file becomes symlink","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-07-18T18:46:42Z","receivedAt":"2009-07-18T18:46:42Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Sat, Jul 18, 2009 at 04:16:58PM +0200, Nicolas Sebrecht wrote:\n>\n>> It fails on:\n>>   - next\n>>   - v1.6.3\n>>   - b67b9612e1a90ae093445abeaeff930e9f4cf936\n>>   - (other I don't remember, but does it really matter?)\n>\n> Hmm. So it is clearly reproducible on your system, but not on mine. I\n> wonder what the difference could be.\n>\n> Are you compiling with any special options? I usually compile with just\n> \"-g -Wall -Werror\", but I also tried with \"-O2\" and couldn't reproduce.\n\nMe neither, but I found an unrelated anomaly in the output from the test\nnearby.\n\n    Switched to branch 'foo-baz-renamed-from-foo'\n    *   ok 3: file renamed from foo/baz to foo\n\n    * expecting success:\n            git checkout -f foo-becomes-a-directory &&\n            git diff-tree -p HEAD initial > patch &&\n            git apply --index < patch\n\n    error: Invalid path ''\n    Switched to branch 'foo-becomes-a-directory'\n    *   ok 4: directory becomes file\n\nNotice the \"error\"?\n\nThis is coming from the oneway_merge() codepath in unpack-trees.c.\n\nWhen we switch branches with \"checkout -f\", unpack_trees() feeds two \ncache_entries to oneway_merge() function in its src[] array argument.  The\nzeroth entry comes from the current index, and the first entry represents\nwhat the merge result should be, taken from the tree recorded in the\ncommit we are switching to.\n\nWhen we have a blob (either regular file or a symlink) in the index and in\nthe work tree at path \"foo\", and the switched-to tree has \"foo/bar\",\ni.e. \"foo\" becomes a directory, src[0] is obviously that blob currently\nregistered at \"foo\".  Even though we do not have anything at \"foo\" in the\nswitched-to tree, src[1] is _not_ NULL.\n\nThe unpack_trees() machinery places a special marker df_conflict_entry\nto signal that no blob exists at \"foo\", but it will become a directory\nthat may have somthing underneath it, namely \"foo/bar\".\n\nPassing that df_conflict_entry marker to merged_entry() happens to remove\nthe \"foo\" in the end because the df_conflict_entry does not have any name\n(hence the \"error\" message) and its addition in add_index_entry() is\nrejected, but it is wrong.\n\n unpack-trees.c |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 48d862d..720f7a1 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -999,7 +999,7 @@ int oneway_merge(struct cache_entry **src, struct unpack_trees_options *o)\n \t\treturn error(\"Cannot do a oneway merge of %d trees\",\n \t\t\t     o->merge_size);\n \n-\tif (!a)\n+\tif (!a || a == o->df_conflict_entry)\n \t\treturn deleted_entry(old, old, o);\n \n \tif (old && same(old, a)) {\n"},{"id":"118243","messageId":"200907182106.06776.j6t@kdbg.org","threadId":"20152","inReplyTo":"20090718141658.GE16708@vidovic","subject":"Re: [test failure] Re: t4114 binary file becomes symlink","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2009-07-18T19:06:06Z","receivedAt":"2009-07-18T19:06:06Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"On Samstag, 18. Juli 2009, Nicolas Sebrecht wrote:\n> ==10807== Process terminating with default action of signal 11 (SIGSEGV)\n> ==10807==  Access not within mapped region at address 0x1\n> ==10807==    at 0x4C22349: strlen (in\n> /usr/lib64/valgrind/amd64-linux/vgpreload_memcheck.so) ==10807==    by\n> 0x5616ED6: vfprintf (in /lib64/libc-2.8.so)\n> ==10807==    by 0x563C159: vsnprintf (in /lib64/libc-2.8.so)\n> ==10807==    by 0x495E90: git_vsnprintf (snprintf.c:38)\n> ==10807==    by 0x48917B: strbuf_addf (strbuf.c:203)\n\namd64-linux, and you build with SNPRINTF_RETURNS_BOGUS? Why do you have this \noption set?\n\n-- Hannes\n"},{"id":"118246","messageId":"20090718201721.GF16708@vidovic","threadId":"20152","inReplyTo":"200907182106.06776.j6t@kdbg.org","subject":"[test failure] Re: t4114 binary file becomes symlink","fromName":"Nicolas Sebrecht","fromEmail":"nicolas.s.dev@gmx.fr","sentAt":"2009-07-18T20:17:21Z","receivedAt":"2009-07-18T20:17:21Z","isPatch":false,"sender":{"key":"nicolas.s.dev@gmx.fr","avatar":null},"body":"The 18/07/09, Johannes Sixt wrote:\n> On Samstag, 18. Juli 2009, Nicolas Sebrecht wrote:\n> > ==10807== Process terminating with default action of signal 11 (SIGSEGV)\n> > ==10807==  Access not within mapped region at address 0x1\n> > ==10807==    at 0x4C22349: strlen (in\n> > /usr/lib64/valgrind/amd64-linux/vgpreload_memcheck.so) ==10807==    by\n> > 0x5616ED6: vfprintf (in /lib64/libc-2.8.so)\n> > ==10807==    by 0x563C159: vsnprintf (in /lib64/libc-2.8.so)\n> > ==10807==    by 0x495E90: git_vsnprintf (snprintf.c:38)\n> > ==10807==    by 0x48917B: strbuf_addf (strbuf.c:203)\n> \n> amd64-linux, and you build with SNPRINTF_RETURNS_BOGUS? Why do you have this \n> option set?\n\nI don't tune any flags myself and\n\n  $ printf \"$CC\\n $CFLAGS\\n $LDFLAGS\\n $LIBS\\n $CPPFLAGS\\n $CPP\\n\"\n\nis empty. Is there another way to accidentally set a flag other than\nenvironment variables or options to 'make'?\n\nI've tried\n\n  make\n  make install\n\nand\n\n  ./configure --prefix=/home/nicolas/bin\n  make\n  make install\n\nI don't know if it changes anything but both failed with the same error.\n\n\n-- \nNicolas Sebrecht\n"},{"id":"118247","messageId":"20090718203922.GG16708@vidovic","threadId":"20152","inReplyTo":"7veisd3k31.fsf@alter.siamese.dyndns.org","subject":"[test failure] Re: t4114 binary file becomes symlink","fromName":"Nicolas Sebrecht","fromEmail":"nicolas.s.dev@gmx.fr","sentAt":"2009-07-18T20:39:22Z","receivedAt":"2009-07-18T20:39:22Z","isPatch":false,"sender":{"key":"nicolas.s.dev@gmx.fr","avatar":null},"body":"The 18/07/09, Junio C Hamano wrote:\n> Jeff King <peff@peff.net> writes:\n> \n> > Are you compiling with any special options?\n\nNormally, no (cf. my mail to Johannes Sixt).\n\n> >                                             I usually compile with just\n> > \"-g -Wall -Werror\", but I also tried with \"-O2\" and couldn't reproduce.\n\nThe Makefile have\n\n  215:CFLAGS = -g -O2 -Wall\n  217:ALL_CFLAGS = $(CFLAGS)\n\nand it should be OK. I know that -Werror will fail on\n\n  libgit.a(mkdtemp.o): In function `gitmkdtemp':\n    /home/nicolas/dev/official_packages/git/compat/mkdtemp.c:5: warning: the\n    use of `mktemp' is dangerous, better use `mkstemp'\n\nbut I think this is an unrelated problem.\n\n$ gcc --version\ngcc (GCC) 4.1.2 (Gentoo 4.1.2 p1.3)\n\n> Me neither, but I found an unrelated anomaly in the output from the test\n> nearby.\n> \n>     Switched to branch 'foo-baz-renamed-from-foo'\n>     *   ok 3: file renamed from foo/baz to foo\n> \n>     * expecting success:\n>             git checkout -f foo-becomes-a-directory &&\n>             git diff-tree -p HEAD initial > patch &&\n>             git apply --index < patch\n> \n>     error: Invalid path ''\n>     Switched to branch 'foo-becomes-a-directory'\n>     *   ok 4: directory becomes file\n> \n> Notice the \"error\"?\n\nI couldn't reproduce this error here with\n\n  sh t4114-apply-typechange.sh --verbose\nand\n  sh t4114-apply-typechange.sh --verbose --debug\n\non the current next.\n\nWith this patch, I still fail on the test.\n\n-- \nNicolas Sebrecht\n"},{"id":"118248","messageId":"20090718211345.GI16708@vidovic","threadId":"20152","inReplyTo":"20090718201721.GF16708@vidovic","subject":"[test failure] Re: t4114 binary file becomes symlink","fromName":"Nicolas Sebrecht","fromEmail":"nicolas.s.dev@gmx.fr","sentAt":"2009-07-18T21:13:45Z","receivedAt":"2009-07-18T21:13:45Z","isPatch":false,"sender":{"key":"nicolas.s.dev@gmx.fr","avatar":null},"body":"The 18/07/09, Nicolas Sebrecht wrote:\n> The 18/07/09, Johannes Sixt wrote:\n> > On Samstag, 18. Juli 2009, Nicolas Sebrecht wrote:\n> > > ==10807== Process terminating with default action of signal 11 (SIGSEGV)\n> > > ==10807==  Access not within mapped region at address 0x1\n> > > ==10807==    at 0x4C22349: strlen (in\n> > > /usr/lib64/valgrind/amd64-linux/vgpreload_memcheck.so) ==10807==    by\n> > > 0x5616ED6: vfprintf (in /lib64/libc-2.8.so)\n> > > ==10807==    by 0x563C159: vsnprintf (in /lib64/libc-2.8.so)\n> > > ==10807==    by 0x495E90: git_vsnprintf (snprintf.c:38)\n> > > ==10807==    by 0x48917B: strbuf_addf (strbuf.c:203)\n> > \n> > amd64-linux, and you build with SNPRINTF_RETURNS_BOGUS? Why do you have this \n> > option set?\n> \n> I don't tune any flags myself and\n> \n>   $ printf \"$CC\\n $CFLAGS\\n $LDFLAGS\\n $LIBS\\n $CPPFLAGS\\n $CPP\\n\"\n> \n> is empty. Is there another way to accidentally set a flag other than\n> environment variables or options to 'make'?\n\nHum, 'rm configure ; make configure ; ./configure' give\n\n checking whether snprintf() and/or vsnprintf() return bogus value... yes\n\nWTF?\n\n-- \nNicolas Sebrecht\n"},{"id":"118249","messageId":"200907190003.56137.j6t@kdbg.org","threadId":"20152","inReplyTo":"20090718201721.GF16708@vidovic","subject":"Re: [test failure] Re: t4114 binary file becomes symlink","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2009-07-18T22:03:55Z","receivedAt":"2009-07-18T22:03:55Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"On Samstag, 18. Juli 2009, Nicolas Sebrecht wrote:\n>   ./configure --prefix=/home/nicolas/bin\n>   make\n>   make install\n\nIf you are on Linux and only want a special prefix, you don't \nneed ./configure; just:\n\n   echo prefix=/home/nicolas/bin > config.mak\n   make && make install\n\n-- Hannes\n"},{"id":"118250","messageId":"20090718222947.GA31147@coredump.intra.peff.net","threadId":"20152","inReplyTo":"200907182106.06776.j6t@kdbg.org","subject":"Re: [test failure] Re: t4114 binary file becomes symlink","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-07-18T22:29:47Z","receivedAt":"2009-07-18T22:29:47Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Jul 18, 2009 at 09:06:06PM +0200, Johannes Sixt wrote:\n\n> On Samstag, 18. Juli 2009, Nicolas Sebrecht wrote:\n> > ==10807== Process terminating with default action of signal 11 (SIGSEGV)\n> > ==10807==  Access not within mapped region at address 0x1\n> > ==10807==    at 0x4C22349: strlen (in\n> > /usr/lib64/valgrind/amd64-linux/vgpreload_memcheck.so) ==10807==    by\n> > 0x5616ED6: vfprintf (in /lib64/libc-2.8.so)\n> > ==10807==    by 0x563C159: vsnprintf (in /lib64/libc-2.8.so)\n> > ==10807==    by 0x495E90: git_vsnprintf (snprintf.c:38)\n> > ==10807==    by 0x48917B: strbuf_addf (strbuf.c:203)\n> \n> amd64-linux, and you build with SNPRINTF_RETURNS_BOGUS? Why do you have this \n> option set?\n\nAh, that's what I was missing. I can reproduce it by setting\nSNPRINTF_RETURNS_BOGUS. I think the problem is in the git_vsnprintf\ncode, and it just by coincidence triggers in this test because of the\nexact string we are trying to format.\n\nLook at compat/snprintf.c. In git_vsnprintf, we are passed a \"va_list\nap\", which we then repeatedly call vsnprintf on, checking the return to\nmake sure we have enough space. But using a va_list repeatedly without a\nva_end and va_start in the middle invokes undefined behavior. So we need\nto va_copy it and use the copy.\n\nA patch is below, which fixes the problem for me. However, va_copy is\nC99, so we would generally try to avoid it. But I don't think there is a\nportable way of writing this function without it. And most systems\nshouldn't need to use our snprintf at all, so maybe it is portable\nenough. I dunno.\n\n---\ndiff --git a/compat/snprintf.c b/compat/snprintf.c\nindex 6c0fb05..e862db6 100644\n--- a/compat/snprintf.c\n+++ b/compat/snprintf.c\n@@ -18,9 +18,12 @@ int git_vsnprintf(char *str, size_t maxsize, const char *format, va_list ap)\n {\n \tchar *s;\n \tint ret = -1;\n+\tva_list cp;\n \n \tif (maxsize > 0) {\n-\t\tret = vsnprintf(str, maxsize-SNPRINTF_SIZE_CORR, format, ap);\n+\t\tva_copy(cp, ap);\n+\t\tret = vsnprintf(str, maxsize-SNPRINTF_SIZE_CORR, format, cp);\n+\t\tva_end(cp);\n \t\tif (ret == maxsize-1)\n \t\t\tret = -1;\n \t\t/* Windows does not NUL-terminate if result fills buffer */\n@@ -39,7 +42,9 @@ int git_vsnprintf(char *str, size_t maxsize, const char *format, va_list ap)\n \t\tif (! str)\n \t\t\tbreak;\n \t\ts = str;\n-\t\tret = vsnprintf(str, maxsize-SNPRINTF_SIZE_CORR, format, ap);\n+\t\tva_copy(cp, ap);\n+\t\tret = vsnprintf(str, maxsize-SNPRINTF_SIZE_CORR, format, cp);\n+\t\tva_end(cp);\n \t\tif (ret == maxsize-1)\n \t\t\tret = -1;\n \t}\n"},{"id":"118251","messageId":"20090718225135.GJ16708@vidovic","threadId":"20152","inReplyTo":"20090718222947.GA31147@coredump.intra.peff.net","subject":"[test failure] Re: t4114 binary file becomes symlink","fromName":"Nicolas Sebrecht","fromEmail":"nicolas.s.dev@gmx.fr","sentAt":"2009-07-18T22:51:35Z","receivedAt":"2009-07-18T22:51:35Z","isPatch":false,"sender":{"key":"nicolas.s.dev@gmx.fr","avatar":null},"body":"The 18/07/09, Jeff King wrote:\n> On Sat, Jul 18, 2009 at 09:06:06PM +0200, Johannes Sixt wrote:\n> \n> Ah, that's what I was missing. I can reproduce it by setting\n> SNPRINTF_RETURNS_BOGUS. I think the problem is in the git_vsnprintf\n> code, and it just by coincidence triggers in this test because of the\n> exact string we are trying to format.\n> \n> Look at compat/snprintf.c. In git_vsnprintf, we are passed a \"va_list\n> ap\", which we then repeatedly call vsnprintf on, checking the return to\n> make sure we have enough space. But using a va_list repeatedly without a\n> va_end and va_start in the middle invokes undefined behavior. So we need\n> to va_copy it and use the copy.\n> \n> A patch is below, which fixes the problem for me. However, va_copy is\n> C99, so we would generally try to avoid it. But I don't think there is a\n> portable way of writing this function without it. And most systems\n> shouldn't need to use our snprintf at all, so maybe it is portable\n> enough. I dunno.\n\nMy investigations made me realize I was building a 64-bits git version\nin a 32-bits userland (gentoo flag multilib set) which is not the best\nthing to do. So, another possible fix is to export CFLAGS with '-m32'.\nMixing 32 and 64-bits applications is bad. :-)\n\nI confirm this patch does fix the failure for the 64 bits version. Thank\nyou all.\n\nNow, I wonder if it is safe to run a 32-bits git version on repositories\nbuilt with a 64-bits version. It should be safe but do you think it\nactually is?\n\n-- \nNicolas Sebrecht\n"},{"id":"118253","messageId":"alpine.LFD.2.01.0907181612390.13838@localhost.localdomain","threadId":"20152","inReplyTo":"7veisd3k31.fsf@alter.siamese.dyndns.org","subject":"Re: [test failure] Re: t4114 binary file becomes symlink","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2009-07-18T23:18:45Z","receivedAt":"2009-07-18T23:18:45Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sat, 18 Jul 2009, Junio C Hamano wrote:\n> \n> The unpack_trees() machinery places a special marker df_conflict_entry\n> to signal that no blob exists at \"foo\", but it will become a directory\n> that may have somthing underneath it, namely \"foo/bar\".\n> \n> Passing that df_conflict_entry marker to merged_entry() happens to remove\n> the \"foo\" in the end because the df_conflict_entry does not have any name\n> (hence the \"error\" message) and its addition in add_index_entry() is\n> rejected, but it is wrong.\n\nAck. This looks like a really old bug.\n\nThat whole 'df_conflict_entry' always confused me.\n\n\t\tLinus\n"},{"id":"118259","messageId":"m34ot9c67t.fsf_-_@localhost.localdomain","threadId":"20152","inReplyTo":"20090718211345.GI16708@vidovic","subject":"./configure misdetects SNPRINTF_RETURNS_BOGUS (was: [test failure] t4114 binary file becomes symlink)","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-07-19T10:33:50Z","receivedAt":"2009-07-19T10:33:50Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Nicolas Sebrecht <nicolas.s.dev@gmx.fr> writes:\n \n> Hum, 'rm configure ; make configure ; ./configure' give\n> \n>  checking whether snprintf() and/or vsnprintf() return bogus value... yes\n> \n> WTF?\n\nIt looks like some recent bug in configure.ac, as I have run\n./configure without \"make configure\" and it had\n\n  NO_LIBGEN_H=@NO_LIBGEN_H@\n  NEEDS_RESOLV=@NEEDS_RESOLV@\n\n  SNPRINTF_RETURNS_BOGUS=\n\nand when I did \"rm configure ; make configure ; ./configure\"\nit gave me\n\n  NO_LIBGEN_H=\n  NEEDS_RESOLV=\n\n  SNPRINTF_RETURNS_BOGUS=UnfortunatelyYes\n\nI have tried to find which commit introduced this regression.\n\n $ git bisect start origin v1.6.3 v1.6.3.2 -- configure.ac config.mak.in\n $ git bisect run ~/git/test.sh\n\nfinds ecc395c (Makefile: add NEEDS_LIBGEN to optionally add -lgen to\ncompile arguments, 2009-07-10) as a first bad commit.  But I don't see\nhow it could have changed it... Strange...\n\nCC-ed Brandon Casey, author of blamed changeset, and David Syzdek who\noffered at some time help with maintaining autoconf.\n\n\nP.S. Perhaps it is time for creating MAINTAINERS file for git?\n\n-- >8 --\n#!/bin/bash\n\nrm -f configure &&\nmake configure  &&\n./configure -q  &&\ngrep -q \"^SNPRINTF_RETURNS_BOGUS=$\" config.mak.autogen\n\n# end of test.sh\n-- \nJakub Narebski\nPoland\nShadeHawk on #git\n"},{"id":"118260","messageId":"200907191301.15533.j6t@kdbg.org","threadId":"20152","inReplyTo":"20090718222947.GA31147@coredump.intra.peff.net","subject":"Re: [test failure] Re: t4114 binary file becomes symlink","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2009-07-19T11:01:15Z","receivedAt":"2009-07-19T11:01:15Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"On Sonntag, 19. Juli 2009, Jeff King wrote:\n> Look at compat/snprintf.c. In git_vsnprintf, we are passed a \"va_list\n> ap\", which we then repeatedly call vsnprintf on, checking the return to\n> make sure we have enough space. But using a va_list repeatedly without a\n> va_end and va_start in the middle invokes undefined behavior. So we need\n> to va_copy it and use the copy.\n>\n> A patch is below, which fixes the problem for me. However, va_copy is\n> C99, so we would generally try to avoid it. But I don't think there is a\n> portable way of writing this function without it. And most systems\n> shouldn't need to use our snprintf at all, so maybe it is portable\n> enough. I dunno.\n\nProblem is, snprintf was made for very old systems, which typically do not \nhave va_copy. (E.g. Windows, but there the situation *might* have changed \nwith the switch to gcc 4.)\n\nThe rationale not to use va_copy is that this function is to be used *only* if \nnecessary, i.e. portability is already lacking, and if it can be verified \nthat this function works as is. Portability and correct-by-the-law C code are \n*not* a goal here. If the function does not work as is, don't use it.\n\n-- Hannes\n"},{"id":"118261","messageId":"a52be8ba36206abc5ff5c91a759036a931e2658c.1248007036.git.nicolas.s.dev@gmx.fr","threadId":"20152","inReplyTo":"m34ot9c67t.fsf_-_@localhost.localdomain","subject":"[PATCH] configure: use AC_SEARCH_LIBS instead of AC_CHECK_LIB","fromName":"Nicolas Sebrecht","fromEmail":"nicolas.s.dev@gmx.fr","sentAt":"2009-07-19T12:48:21Z","receivedAt":"2009-07-19T12:48:21Z","isPatch":true,"sender":{"key":"nicolas.s.dev@gmx.fr","avatar":null},"body":"\n> It looks like some recent bug in configure.ac, as I have run\n> ./configure without \"make configure\" and it had\n> \n>   NO_LIBGEN_H=@NO_LIBGEN_H@\n>   NEEDS_RESOLV=@NEEDS_RESOLV@\n> \n>   SNPRINTF_RETURNS_BOGUS=\n> \n> and when I did \"rm configure ; make configure ; ./configure\"\n> it gave me\n> \n>   NO_LIBGEN_H=\n>   NEEDS_RESOLV=\n> \n>   SNPRINTF_RETURNS_BOGUS=UnfortunatelyYes\n> \n> I have tried to find which commit introduced this regression.\n> \n>  $ git bisect start origin v1.6.3 v1.6.3.2 -- configure.ac config.mak.in\n>  $ git bisect run ~/git/test.sh\n> \n> finds ecc395c (Makefile: add NEEDS_LIBGEN to optionally add -lgen to\n> compile arguments, 2009-07-10) as a first bad commit.  But I don't see\n> how it could have changed it... Strange...\n> \n> CC-ed Brandon Casey, author of blamed changeset, and David Syzdek who\n> offered at some time help with maintaining autoconf.\n\nThank you, I did the same investigation here. :-)\n\n\nAC_CHECK_LIB may fail to check the good libraries. Use\nAC_SEARCH_LIBS instead as pointed out by the autotool\ndocumentation.\n\nhttp://www.gnu.org/software/autoconf/manual/html_node/Libraries.html#Libraries\n\nSigned-off-by: Nicolas Sebrecht <nicolas.s.dev@gmx.fr>\n---\n\n configure.ac |   16 ++++++++--------\n 1 files changed, 8 insertions(+), 8 deletions(-)\n\ndiff --git a/configure.ac b/configure.ac\nindex 74d0af5..48dd5f3 100644\n--- a/configure.ac\n+++ b/configure.ac\n@@ -339,9 +339,9 @@ AC_MSG_NOTICE([CHECKS for libraries])\n \n GIT_STASH_FLAGS($OPENSSLDIR)\n \n-AC_CHECK_LIB([crypto], [SHA1_Init],\n+AC_SEARCH_LIBS([SHA1_Init], [crypto],\n [NEEDS_SSL_WITH_CRYPTO=],\n-[AC_CHECK_LIB([ssl], [SHA1_Init],\n+[AC_SEARCH_LIBS([SHA1_Init], [ssl],\n  [NEEDS_SSL_WITH_CRYPTO=YesPlease\n   NEEDS_SSL_WITH_CRYPTO=],\n  [NO_OPENSSL=YesPlease])])\n@@ -358,7 +358,7 @@ AC_SUBST(NO_OPENSSL)\n \n GIT_STASH_FLAGS($CURLDIR)\n \n-AC_CHECK_LIB([curl], [curl_global_init],\n+AC_SEARCH_LIBS([curl_global_init], [curl],\n [NO_CURL=],\n [NO_CURL=YesPlease])\n \n@@ -372,7 +372,7 @@ AC_SUBST(NO_CURL)\n \n GIT_STASH_FLAGS($EXPATDIR)\n \n-AC_CHECK_LIB([expat], [XML_ParserCreate],\n+AC_SEARCH_LIBS([XML_ParserCreate], [expat],\n [NO_EXPAT=],\n [NO_EXPAT=YesPlease])\n \n@@ -469,7 +469,7 @@ AC_SUBST(NO_DEFLATE_BOUND)\n #\n # Define NEEDS_SOCKET if linking with libc is not enough (SunOS,\n # Patrick Mauritz).\n-AC_CHECK_LIB([c], [socket],\n+AC_SEARCH_LIBS([socket], [c],\n [NEEDS_SOCKET=],\n [NEEDS_SOCKET=YesPlease])\n AC_SUBST(NEEDS_SOCKET)\n@@ -479,13 +479,13 @@ test -n \"$NEEDS_SOCKET\" && LIBS=\"$LIBS -lsocket\"\n # Define NEEDS_RESOLV if linking with -lnsl and/or -lsocket is not enough.\n # Notably on Solaris hstrerror resides in libresolv and on Solaris 7\n # inet_ntop and inet_pton additionally reside there.\n-AC_CHECK_LIB([resolv], [hstrerror],\n+AC_SEARCH_LIBS([hstrerror], [resolv],\n [NEEDS_RESOLV=],\n [NEEDS_RESOLV=YesPlease])\n AC_SUBST(NEEDS_RESOLV)\n test -n \"$NEEDS_RESOLV\" && LIBS=\"$LIBS -lresolv\"\n \n-AC_CHECK_LIB([gen], [basename],\n+AC_SEARCH_LIBS([basename], [gen],\n [NEEDS_LIBGEN=],\n [NEEDS_LIBGEN=YesPlease])\n AC_SUBST(NEEDS_LIBGEN)\n@@ -647,7 +647,7 @@ AC_SUBST(SNPRINTF_RETURNS_BOGUS)\n \n \n ## Checks for library functions.\n-## (in default C library and libraries checked by AC_CHECK_LIB)\n+## (in default C library and libraries checked by AC_SEARCH_LIBS)\n AC_MSG_NOTICE([CHECKS for library functions])\n #\n # Define NO_LIBGEN_H if you don't have libgen.h.\n-- \n1.6.4.rc1.170.ge727\n"},{"id":"118263","messageId":"20090719131404.GA11546@vidovic","threadId":"20152","inReplyTo":"a52be8ba36206abc5ff5c91a759036a931e2658c.1248007036.git.nicolas.s.dev@gmx.fr","subject":"[PATCH] Re: configure: use AC_SEARCH_LIBS instead of AC_CHECK_LIB","fromName":"Nicolas Sebrecht","fromEmail":"nicolas.s.dev@gmx.fr","sentAt":"2009-07-19T13:14:04Z","receivedAt":"2009-07-19T13:14:04Z","isPatch":true,"sender":{"key":"nicolas.s.dev@gmx.fr","avatar":null},"body":"The 19/07/09, Nicolas Sebrecht wrote:\n> \n> > and when I did \"rm configure ; make configure ; ./configure\"\n> > it gave me\n> > \n> >   NO_LIBGEN_H=\n> >   NEEDS_RESOLV=\n> > \n> >   SNPRINTF_RETURNS_BOGUS=UnfortunatelyYes\n> > \n> > I have tried to find which commit introduced this regression.\n> > \n> >  $ git bisect start origin v1.6.3 v1.6.3.2 -- configure.ac config.mak.in\n> >  $ git bisect run ~/git/test.sh\n> > \n> > finds ecc395c (Makefile: add NEEDS_LIBGEN to optionally add -lgen to\n> > compile arguments, 2009-07-10) as a first bad commit.  But I don't see\n> > how it could have changed it... Strange...\n\nThe wrong check of lib gen added a flag ' -lgen' to $LIBS (in configure).\nThis wrong flag then gave:\n\n [...]/bin/ld: cannot find -lgen collect2: ld returned 1 exit status\n\nwhich made wrongly fail the next compilation test (using $LIBS).\n\n> > CC-ed Brandon Casey, author of blamed changeset, and David Syzdek who\n> > offered at some time help with maintaining autoconf.\n> \n> Thank you, I did the same investigation here. :-)\n\nI wonder if we could have some feedbacks on this patch. The change on\nthe whole file may make more bad than good. Works as expected here on\nLinux (glibc 2.9).\n\nOtherwise, the following lines are sufficient to correct the original\nerror:\n\n>  test -n \"$NEEDS_RESOLV\" && LIBS=\"$LIBS -lresolv\"\n>  \n> -AC_CHECK_LIB([gen], [basename],\n> +AC_SEARCH_LIBS([basename], [gen],\n>  [NEEDS_LIBGEN=],\n>  [NEEDS_LIBGEN=YesPlease])\n>  AC_SUBST(NEEDS_LIBGEN)\n\n-- \nNicolas Sebrecht\n"},{"id":"118266","messageId":"7v3a8szm4q.fsf@alter.siamese.dyndns.org","threadId":"20152","inReplyTo":"20090719131404.GA11546@vidovic","subject":"Re: [PATCH] Re: configure: use AC_SEARCH_LIBS instead of AC_CHECK_LIB","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-07-19T16:13:41Z","receivedAt":"2009-07-19T16:13:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nicolas Sebrecht <nicolas.s.dev@gmx.fr> writes:\n\n> I wonder if we could have some feedbacks on this patch. The change on\n> the whole file may make more bad than good. Works as expected here on\n> Linux (glibc 2.9).\n>\n> Otherwise, the following lines are sufficient to correct the original\n> error:\n>\n>>  test -n \"$NEEDS_RESOLV\" && LIBS=\"$LIBS -lresolv\"\n>>  \n>> -AC_CHECK_LIB([gen], [basename],\n>> +AC_SEARCH_LIBS([basename], [gen],\n>>  [NEEDS_LIBGEN=],\n>>  [NEEDS_LIBGEN=YesPlease])\n>>  AC_SUBST(NEEDS_LIBGEN)\n\nThanks.\n\nThe autoconf documentation does not give much confidence either way:\n\n\tAC_CHECK_LIB requires some care in usage, and should be avoided in\n\tsome common cases.\n\nIt is unclear what these \"some\" are, and is sufficiently unclear for us to\ndecide if our situation falls into these \"some common cases.\"  The only\nproblem it cites is that alternative libraries may contain a variant\nimplementation of the same function found in a more common library, in\nwhich case you may not want to use such an alternative implementation, and\nI do not think \"basename() in -lgen\" falls into that category.\n\nIt does say\n\n\tThese days it is normally better to use AC_SEARCH_LIBS(...)\n\tinstead of AC_CHECK_LIB(...).\n\nbut without any further justification, which does not help building\nconfidence in the suggestion.\n\nIt is very tempting to revert the configure.ac part of commit ecc395c\nespecially in an -rc freeze, but if some autoconf experts can help us\ndecide by clarifying the situation, and explain why AC_SEARCH_LIBS() is\nindeed the better way these days (which I suspect it is), I'd say we can\napply your patch to favor AC_SEARCH_LIBS() over AC_CHECK_LIB() with more\nconfidence.\n\nIf autoconf people say AC_SEARCH_LIBS() should be preferred over\nAC_CHECK_LIB(), it probably is the right thing to do.  But I want to have\na warmer-and-fuzzier feeling that we understand why it went wrong, rather\nthan blindly doing what the documentation says because the documentation\ntells us to.\n"},{"id":"118274","messageId":"loom.20090719T225210-990@post.gmane.org","threadId":"20152","inReplyTo":"7v3a8szm4q.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Re: configure: use AC_SEARCH_LIBS instead of AC_CHECK_LIB","fromName":"Eric Blake","fromEmail":"ebb9@byu.net","sentAt":"2009-07-19T22:53:16Z","receivedAt":"2009-07-19T22:53:16Z","isPatch":true,"sender":{"key":"eblake@redhat.com","avatar":"https://avatars.githubusercontent.com/u/32933908?v=4"},"body":"Junio C Hamano <gitster <at> pobox.com> writes:\n\n> The autoconf documentation does not give much confidence either way:\n> \n> \tAC_CHECK_LIB requires some care in usage, and should be avoided in\n> \tsome common cases.\n> \n> It is unclear what these \"some\" are, and is sufficiently unclear for us to\n> decide if our situation falls into these \"some common cases.\"  The only\n> problem it cites is that alternative libraries may contain a variant\n> implementation of the same function found in a more common library, in\n> which case you may not want to use such an alternative implementation, and\n> I do not think \"basename() in -lgen\" falls into that category.\n\nPlease report that as a bug to the autoconf lists, then.\n\n> If autoconf people say AC_SEARCH_LIBS() should be preferred over\n> AC_CHECK_LIB(), it probably is the right thing to do.  But I want to have\n> a warmer-and-fuzzier feeling that we understand why it went wrong, rather\n> than blindly doing what the documentation says because the documentation\n> tells us to.\n\nAs autoconf maintainer (and hoping to release autoconf 2.64 soon), yes, I can\ndefinitively say that AC_SEARCH_LIBS is nicer than AC_CHECK_LIB.\n\n-- \nEric Blake\n"},{"id":"118293","messageId":"20090720090937.GA20412@sigill.intra.peff.net","threadId":"20152","inReplyTo":"200907191301.15533.j6t@kdbg.org","subject":"Re: [test failure] Re: t4114 binary file becomes symlink","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-07-20T09:09:38Z","receivedAt":"2009-07-20T09:09:38Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Jul 19, 2009 at 01:01:15PM +0200, Johannes Sixt wrote:\n\n> Problem is, snprintf was made for very old systems, which typically do\n> not have va_copy. (E.g. Windows, but there the situation *might* have\n> changed with the switch to gcc 4.)\n> \n> The rationale not to use va_copy is that this function is to be used\n> *only* if necessary, i.e. portability is already lacking, and if it\n> can be verified that this function works as is. Portability and\n> correct-by-the-law C code are *not* a goal here. If the function does\n> not work as is, don't use it.\n\nOK, I guess I can buy the \"don't use this unless you need it\" rationale.\nBut two questions:\n\n  1. _Are_ we sure it works under Windows?  That is, do we know for a\n     fact that using a va_list twice is OK there, or are we just going\n     on the fact that nobody has reported the bug?\n\n     If we're not sure, then you might want to try running the recipe\n     below which consistently produces a segfault for me on Linux amd64\n     (but not i386, which seems to use a different representation for\n     va_lists).\n\n  2. In this case, using SNPRINTF_RETURNS_BOGUS was a mistake. But\n     unfortunately using it erroneously doesn't simply cause the\n     compilation to barf, or to use a slower implementation; instead it\n     introduces a very subtle and hard to diagnose bug on some\n     platforms. Is there anything simple we can do to protect people\n     from that?\n\n     I can't really think of anything simple, because such a mechanism\n     would basically involve compiling a test program and seeing if it\n     segfaults.\n\nAnyway, bug-reproducing recipe is below.\n\n-- >8 --\ncat <<'EOF' >test-vsnprintf.c\n#define SNPRINTF_RETURNS_BOGUS\n#include \"git-compat-util.h\"\nint main() {\n        char buf[16];\n        /* this 8 may need to be tweaked depending on\n         * the system's vsnprintf return value; the goal\n         * is to get git_vsnprintf to have to look at\n         * it's va_list twice */\n        git_snprintf(buf, 8, \"%s %s\", \"foo\", \"bar\");\n        return 0;\n}\nEOF\nmake test-vsnprintf.o compat/snprintf.o\ngcc -o test-vsnprintf test-vsnprintf.o compat/snprintf.o\n./test-vsnprintf ;# or valgrind test-vsnprintf\n"},{"id":"118344","messageId":"200907202251.22490.j6t@kdbg.org","threadId":"20152","inReplyTo":"20090720090937.GA20412@sigill.intra.peff.net","subject":"Re: [test failure] Re: t4114 binary file becomes symlink","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2009-07-20T20:51:21Z","receivedAt":"2009-07-20T20:51:21Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"On Montag, 20. Juli 2009, Jeff King wrote:\n> On Sun, Jul 19, 2009 at 01:01:15PM +0200, Johannes Sixt wrote:\n> > Problem is, snprintf was made for very old systems, which typically do\n> > not have va_copy. (E.g. Windows, but there the situation *might* have\n> > changed with the switch to gcc 4.)\n> >\n> > The rationale not to use va_copy is that this function is to be used\n> > *only* if necessary, i.e. portability is already lacking, and if it\n> > can be verified that this function works as is. Portability and\n> > correct-by-the-law C code are *not* a goal here. If the function does\n> > not work as is, don't use it.\n>\n> OK, I guess I can buy the \"don't use this unless you need it\" rationale.\n> But two questions:\n>\n>   1. _Are_ we sure it works under Windows?  That is, do we know for a\n>      fact that using a va_list twice is OK there, or are we just going\n>      on the fact that nobody has reported the bug?\n\nWe are sure (well, I am sure ;) that it worked on Windows with gcc 3. It \ncertainly is a reasonable workaround. It remains to confirm that the \nworkaround works as expected on the other systems that use it (IRIX, \nHP/UX).  'git branch -v' is a test that calls the system provided vsnprintf \ntwice (as long as the branch head commits have moderatly long summary lines).\n\nOn Windows, however, today everybody who compiles git is most likely using \nmsysgit's gcc 4.4. This version has C99 vsnprintf, and the workaround should \nnot be used anymore, although it does not hurt.\n\n-- Hannes\n"},{"id":"118350","messageId":"alpine.LFD.2.01.0907201451150.19335@localhost.localdomain","threadId":"20152","inReplyTo":"200907202251.22490.j6t@kdbg.org","subject":"Re: [test failure] Re: t4114 binary file becomes symlink","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2009-07-20T21:56:32Z","receivedAt":"2009-07-20T21:56:32Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Mon, 20 Jul 2009, Johannes Sixt wrote:\n\n> On Montag, 20. Juli 2009, Jeff King wrote:\n> >\n> > OK, I guess I can buy the \"don't use this unless you need it\" rationale.\n> > But two questions:\n> >\n> >   1. _Are_ we sure it works under Windows?  That is, do we know for a\n> >      fact that using a va_list twice is OK there, or are we just going\n> >      on the fact that nobody has reported the bug?\n> \n> We are sure (well, I am sure ;) that it worked on Windows with gcc 3.\n\nNot only that, but it's literally how people used to historically do \nthings.\n\nSure, some places do require 'va_copy()', and there's a reason why it got \nadded in C99. But there's also a reason why it wasn't added earlier - \nbecause it didn't exist!\n\nSo I agree with Johannes: we should _not_ make our compat version of \n'snprintf()' use va_copy, for the simple reason that\n\n - modern C library versions will have a working snprintf already\n\n - old setups that don't have a working snprintf quite likely don't have a \n   va_copy either.\n\nSo the set of systems that need both the va_copy _and_ our compat version \nof snprintf is likely much smaller than the set of systems that would \nactively break compilation if they don't have va_copy.\n\nOf course, we could also add a new \"NEEDS_VA_COPY\" thing, and do\n\n\t#ifdef NEEDS_VA_COPY\n\t  #define va_copy(dst,src) (dst) = (src)\n\t#endif\n\nor something like that.\n\n\t\tLinus\n"},{"id":"118371","messageId":"6WnEB0at_uuAu9kicWjrHLsBbTv58WtCNOANkKzk-SqTgqjuWyh8WA@cipher.nrlssc.navy.mil","threadId":"20152","inReplyTo":"a52be8ba36206abc5ff5c91a759036a931e2658c.1248007036.git.nicolas.s.dev@gmx.fr","subject":"Re: [PATCH] configure: use AC_SEARCH_LIBS instead of AC_CHECK_LIB","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2009-07-21T15:04:14Z","receivedAt":"2009-07-21T15:04:14Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"\nAfter actually reading the autoconf documentation, don't some of the\ntests have the [action-if-found] and [action-if-not-found] actions\nbackwards?\n\nAC_CHECK_LIB has the form:\n\n   AC_CHECK_LIB (library, function, [action-if-found], [action-if-not-found], [other-libraries])\n\nThe test I added (which I blindly copied from the NEEDS_RESOLV test) looks like:\n\n   AC_CHECK_LIB([gen], [basename], [NEEDS_LIBGEN=], [NEEDS_LIBGEN=YesPlease])\n   AC_SUBST(NEEDS_LIBGEN)\n   test -n \"$NEEDS_LIBGEN\" && LIBS=\"$LIBS -lgen\"\n\nWon't this check whether a program which calls basename() successfully links with -lgen?\nIf it successfully links, then it will perform NEED_LIBGEN= and if not it will set\nNEEDS_LIBGEN=YesPlease, right?  Isn't that the opposite of what should be done?\n\nIf that is correct, then the NEEDS_RESOLV and NEEDS_LIBGEN tests are wrong and they\nmay still be wrong even if AC_SEARCH_LIBS is used instead of AC_CHECK_LIB.\n\n\nA question about AC_SEARCH_LIBS:\n\nWith AC_SEARCH_LIBS, which of [action-if-found] or [action-if-not-found]\nis executed if the function is found in the standard c library i.e. \"calling\n`AC_LINK_IFELSE([AC_LANG_CALL([], [function])])' first with no libraries\"?\nIs the answer neither?  If the answer is [action-if-found], won't the\nNEEDS_LIBGEN=YesPlease be set when the function is found in the c library?\nAssuming the NEEDS_LIBGEN test is made to look like this:\n\n   AC_SEARCH_LIBS([basename], [gen], [NEEDS_LIBGEN=YesPlease], [NEEDS_LIBGEN=])\n\nDepending on the answer to that question, we either will want to use\nAC_SEARCH_LIBS, or stick with AC_CHECK_LIB but correct the [action] fields,\nor maybe even stick with AC_CHECK_LIB but rework the NEEDS_RESOLV and\nNEEDS_LIBGEN tests to look like the NEEDS_SOCKET test.\n\n-brandon\n"},{"id":"118372","messageId":"ROnxPc_FlcxpQoCOZcOfoalyiRYK1R7qCAowbfiQsNLLI8qrrtJEUw@cipher.nrlssc.navy.mil","threadId":"20152","inReplyTo":"6WnEB0at_uuAu9kicWjrHLsBbTv58WtCNOANkKzk-SqTgqjuWyh8WA@cipher.nrlssc.navy.mil","subject":"Re: [PATCH] configure: use AC_SEARCH_LIBS instead of AC_CHECK_LIB","fromName":"Brandon Casey","fromEmail":"brandon.casey.ctr@nrlssc.navy.mil","sentAt":"2009-07-21T15:12:31Z","receivedAt":"2009-07-21T15:12:31Z","isPatch":true,"sender":{"key":"brandon.casey.ctr@nrlssc.navy.mil","avatar":null},"body":"Brandon Casey wrote:\n> A question about AC_SEARCH_LIBS:\n> \n> With AC_SEARCH_LIBS, which of [action-if-found] or [action-if-not-found]\n> is executed if the function is found in the standard c library i.e. \"calling\n> `AC_LINK_IFELSE([AC_LANG_CALL([], [function])])' first with no libraries\"?\n\nbtw, the \"`AC_LINK_IFELSE(...\" line is from the autoconf documentation.  It is\nthe first thing that AC_SEARCH_LIBS does.  It tries to link without any of the\nspecified libraries first, then it iterates trying to link using each specified\nlibrary, stopping when it is successful.\n\nIf you didn't recognize that that line was from the documentation, it may be\nconfusing.\n\n-brandon\n"},{"id":"118373","messageId":"4A65DCB9.3030207@gnu.org","threadId":"20152","inReplyTo":"6WnEB0at_uuAu9kicWjrHLsBbTv58WtCNOANkKzk-SqTgqjuWyh8WA@cipher.nrlssc.navy.mil","subject":"Re: [PATCH] configure: use AC_SEARCH_LIBS instead of AC_CHECK_LIB","fromName":"Paolo Bonzini","fromEmail":"bonzini@gnu.org","sentAt":"2009-07-21T15:20:25Z","receivedAt":"2009-07-21T15:20:25Z","isPatch":true,"sender":{"key":"bonzini@gnu.org","avatar":"https://avatars.githubusercontent.com/u/42082?v=4"},"body":"\n> With AC_SEARCH_LIBS, which of [action-if-found] or [action-if-not-found]\n> is executed if the function is found in the standard c library i.e. \"calling\n> `AC_LINK_IFELSE([AC_LANG_CALL([], [function])])' first with no libraries\"?\n> Is the answer neither?  If the answer is [action-if-found], won't the\n> NEEDS_LIBGEN=YesPlease be set when the function is found in the c library?\n\nIt evaluates the action-if-found and adds nothing to LIBS.  Instead, if \nit is found in a library, it evaluates the action-if-found after adding \n(actually prepending) -lBLAH to LIBS.\n\nPaolo\n"},{"id":"118378","messageId":"qSl_KXgcJD_1H47Nrg3FwRdtL-WxwLP1_aueDE8gN-By3M0uJOpw1w@cipher.nrlssc.navy.mil","threadId":"20152","inReplyTo":"4A65DCB9.3030207@gnu.org","subject":"Re: [PATCH] configure: use AC_SEARCH_LIBS instead of AC_CHECK_LIB","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2009-07-21T15:34:30Z","receivedAt":"2009-07-21T15:34:30Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"Paolo Bonzini wrote:\n> \n>> With AC_SEARCH_LIBS, which of [action-if-found] or [action-if-not-found]\n>> is executed if the function is found in the standard c library i.e.\n>> \"calling\n>> `AC_LINK_IFELSE([AC_LANG_CALL([], [function])])' first with no\n>> libraries\"?\n>> Is the answer neither?  If the answer is [action-if-found], won't the\n>> NEEDS_LIBGEN=YesPlease be set when the function is found in the c\n>> library?\n> \n> It evaluates the action-if-found and adds nothing to LIBS.  Instead, if\n> it is found in a library, it evaluates the action-if-found after adding\n> (actually prepending) -lBLAH to LIBS.\n\nThat's what I suspected.  It means we can't have NEEDS_SOMETHING=true in\nthe action-if-found parameter when using AC_SEARCH_LIBS since that may cause\nthe Makefile to append a library requirement when none is necessary.\n\n-brandon\n"},{"id":"118416","messageId":"N0R1rELOlDFDG8pOY2LYJHmWygM5WFD8q_fNZgfE1A2QOX1yg6OPCZW4RbX5i03hokq4Bx2zOjM@cipher.nrlssc.navy.mil","threadId":"20152","inReplyTo":"qSl_KXgcJD_1H47Nrg3FwRdtL-WxwLP1_aueDE8gN-By3M0uJOpw1w@cipher.nrlssc.navy.mil","subject":"[PATCH] configure.ac: rework/fix the NEEDS_RESOLV and NEEDS_LIBGEN tests","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2009-07-21T20:23:06Z","receivedAt":"2009-07-21T20:23:06Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"From: Brandon Casey <drafnel@gmail.com>\n\nThe \"action\" parameters for these two tests were supplied incorrectly for\nthe way the tests were implemented.  The tests check whether a program\nwhich calls hstrerror() or basename() successfully links when -lresolv or\n-lgen are used, respectively.  A successful linking would result in\nNEEDS_RESOLV or NEEDS_LIBGEN being unset, and failure would result in\nsetting the respective variable.\n\nAside from that issue, the tests did not handle the case where neither\nlibrary was necessary for accessing the functions in question.  So solve\nboth of these issues by re-working the two tests so that their form is like\nthe NEEDS_SOCKET test which attempts to link with just the c library, and\nif it fails then assumes that the additional library is necessary and sets\nthe appropriate variable.\n\nSigned-off-by: Brandon Casey <drafnel@gmail.com>\n---\n\n\nMaybe this is the appropriate thing to do?\n\n-brandon\n\n\n configure.ac |    4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/configure.ac b/configure.ac\nindex 74d0af5..ba44cf2 100644\n--- a/configure.ac\n+++ b/configure.ac\n@@ -479,13 +479,13 @@ test -n \"$NEEDS_SOCKET\" && LIBS=\"$LIBS -lsocket\"\n # Define NEEDS_RESOLV if linking with -lnsl and/or -lsocket is not enough.\n # Notably on Solaris hstrerror resides in libresolv and on Solaris 7\n # inet_ntop and inet_pton additionally reside there.\n-AC_CHECK_LIB([resolv], [hstrerror],\n+AC_CHECK_LIB([c], [hstrerror],\n [NEEDS_RESOLV=],\n [NEEDS_RESOLV=YesPlease])\n AC_SUBST(NEEDS_RESOLV)\n test -n \"$NEEDS_RESOLV\" && LIBS=\"$LIBS -lresolv\"\n \n-AC_CHECK_LIB([gen], [basename],\n+AC_CHECK_LIB([c], [basename],\n [NEEDS_LIBGEN=],\n [NEEDS_LIBGEN=YesPlease])\n AC_SUBST(NEEDS_LIBGEN)\n-- \n1.6.3.1.24.g152f4\n"},{"id":"118419","messageId":"7v3a8pdbdr.fsf@alter.siamese.dyndns.org","threadId":"20152","inReplyTo":"N0R1rELOlDFDG8pOY2LYJHmWygM5WFD8q_fNZgfE1A2QOX1yg6OPCZW4RbX5i03hokq4Bx2zOjM@cipher.nrlssc.navy.mil","subject":"Re: [PATCH] configure.ac: rework/fix the NEEDS_RESOLV and NEEDS_LIBGEN tests","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-07-21T20:33:36Z","receivedAt":"2009-07-21T20:33:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Brandon Casey <casey@nrlssc.navy.mil> writes:\n\n> Maybe this is the appropriate thing to do?\n\nThanks.\n\nInstead of saying that \"hstrerror not in -lc means we do have -lresolv and\nthe function will be found there\" blindly, we may want to have a nested\ncheck.\n\n\tAC_CHECK_LIB([c], [hstrerror], [NEEDS_RESOLV=],\n       \t    AC_CHECK_LIB([resolv], [hstrerror], [NEEDS_RESOLV=YesPlease]))\n\nBut we do not have any provision for the case where -lc does not have it\nand -lresolv does not have it either (or -lresolv does not exist) anyway,\nso we might as well go with your patch.\n\nI take it that swapping [if-found][if-not-found] parameters is what the\nautoconf documentation warns against?  That is, both -lc and -lresolv may\nhave it but -lresolv one may be a specialized one you would not normally\nwant.\n\n>  configure.ac |    4 ++--\n>  1 files changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/configure.ac b/configure.ac\n> index 74d0af5..ba44cf2 100644\n> --- a/configure.ac\n> +++ b/configure.ac\n> @@ -479,13 +479,13 @@ test -n \"$NEEDS_SOCKET\" && LIBS=\"$LIBS -lsocket\"\n>  # Define NEEDS_RESOLV if linking with -lnsl and/or -lsocket is not enough.\n>  # Notably on Solaris hstrerror resides in libresolv and on Solaris 7\n>  # inet_ntop and inet_pton additionally reside there.\n> -AC_CHECK_LIB([resolv], [hstrerror],\n> +AC_CHECK_LIB([c], [hstrerror],\n>  [NEEDS_RESOLV=],\n>  [NEEDS_RESOLV=YesPlease])\n>  AC_SUBST(NEEDS_RESOLV)\n>  test -n \"$NEEDS_RESOLV\" && LIBS=\"$LIBS -lresolv\"\n>  \n> -AC_CHECK_LIB([gen], [basename],\n> +AC_CHECK_LIB([c], [basename],\n>  [NEEDS_LIBGEN=],\n>  [NEEDS_LIBGEN=YesPlease])\n>  AC_SUBST(NEEDS_LIBGEN)\n> -- \n> 1.6.3.1.24.g152f4\n"},{"id":"118462","messageId":"Y0sTaUtyR0m1AxCoWzsTI2uclknZENRMzspLXWzGliyRILR9F2ffNg@cipher.nrlssc.navy.mil","threadId":"20152","inReplyTo":"7v3a8pdbdr.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] configure.ac: rework/fix the NEEDS_RESOLV and NEEDS_LIBGEN tests","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2009-07-22T14:59:18Z","receivedAt":"2009-07-22T14:59:18Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"Junio C Hamano wrote:\n\n> Instead of saying that \"hstrerror not in -lc means we do have -lresolv and\n> the function will be found there\" blindly, we may want to have a nested\n> check.\n> \n> \tAC_CHECK_LIB([c], [hstrerror], [NEEDS_RESOLV=],\n>        \t    AC_CHECK_LIB([resolv], [hstrerror], [NEEDS_RESOLV=YesPlease]))\n> \n> But we do not have any provision for the case where -lc does not have it\n> and -lresolv does not have it either (or -lresolv does not exist) anyway,\n> so we might as well go with your patch.\n\nYeah, I understood how the NEEDS_SSL_WITH_CRYPTO test worked like this, but\nI didn't make the tests for NEEDS_RESOLV and NEEDS_LIBGEN any more complex\nfor the same reason you state: we don't have any other provision...\n\n> I take it that swapping [if-found][if-not-found] parameters is what the\n> autoconf documentation warns against?  That is, both -lc and -lresolv may\n> have it but -lresolv one may be a specialized one you would not normally\n> want.\n\nThat's what I assume, and that's probably why AC_SEARCH_LIBS was invented which\nattempts to link without using any of the specified libraries first.  It just\ndoesn't seem to fit our needs in this case.\n\n-brandon\n"},{"id":"118510","messageId":"57XXtFYcpNU4ayC2onbxyZE72nxUH64HrjFA1KTX3lvwrSUaKMsqemp5CX0U7W460zAC_hc0GdA@cipher.nrlssc.navy.mil","threadId":"20152","inReplyTo":"N0R1rELOlDFDG8pOY2LYJHmWygM5WFD8q_fNZgfE1A2QOX1yg6OPCZW4RbX5i03hokq4Bx2zOjM@cipher.nrlssc.navy.mil","subject":"[PATCH] config.mak.in: continue fixing NEEDS_LIBGEN autoconfigure feature","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2009-07-22T22:15:18Z","receivedAt":"2009-07-22T22:15:18Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"From: Brandon Casey <drafnel@gmail.com>\n\nAn entry in the config.mak.in file is necessary for the NEEDS_LIBGEN variable\nto appear in the config.mak.autogen file with the value assigned by the\nconfigure script.\n\nSigned-off-by: Brandon Casey <drafnel@gmail.com>\n---\n\n\nJunio,\n\nYou probably want to apply or squash this somewhere.\n\nAs I noted in the original patch, the autoconf part was untested.\nI actually did some light testing on this one.  I created the configure file\non linux and ran it on Solaris and IRIX.  Both produce an error which looks\nsomething like:\n\n   configure[1234]: syntax error at line 4806 : `;' unexpected\n\nAnd line 4806 looks like:\n\n   for ac_lib in ; do\n      ...\n\nIt works with bash though, and it works with /bin/sh on Solaris 10.  On\nSolaris 10, the configure script correctly detects that hstrerror cannot be\nused without -lresolv, and basename can be used without -lgen.  In\nconfig.mak.autogen, NEEDS_RESOLV is set to 'YesPlease' and NEEDS_LIBGEN is\nunset.  On my Solaris 7, bash is not available, but the informational messages\nindicate the same results as for Solaris 10.  On IRIX, hstrerror() can be used\nwithout -lresolv and basename cannot be used without -lgen.  In\nconfig.mak.autogen, NEEDS_RESOLV is unset, and NEEDS_LIBGEN is set to\n'YesPlease'.\n\nSo, I think this patch should be the final one.\n\n-brandon\n\n\n config.mak.in |    1 +\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\ndiff --git a/config.mak.in b/config.mak.in\nindex dd60451..67b12f7 100644\n--- a/config.mak.in\n+++ b/config.mak.in\n@@ -34,6 +34,7 @@ NO_LIBGEN_H=@NO_LIBGEN_H@\n NEEDS_LIBICONV=@NEEDS_LIBICONV@\n NEEDS_SOCKET=@NEEDS_SOCKET@\n NEEDS_RESOLV=@NEEDS_RESOLV@\n+NEEDS_LIBGEN=@NEEDS_LIBGEN@\n NO_SYS_SELECT_H=@NO_SYS_SELECT_H@\n NO_D_INO_IN_DIRENT=@NO_D_INO_IN_DIRENT@\n NO_D_TYPE_IN_DIRENT=@NO_D_TYPE_IN_DIRENT@\n-- \n1.6.3.1.24.g152f4\n"},{"id":"118520","messageId":"7vprbsbb2c.fsf@alter.siamese.dyndns.org","threadId":"20152","inReplyTo":"57XXtFYcpNU4ayC2onbxyZE72nxUH64HrjFA1KTX3lvwrSUaKMsqemp5CX0U7W460zAC_hc0GdA@cipher.nrlssc.navy.mil","subject":"Re: [PATCH] config.mak.in: continue fixing NEEDS_LIBGEN autoconfigure feature","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-07-22T22:35:39Z","receivedAt":"2009-07-22T22:35:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Brandon Casey <casey@nrlssc.navy.mil> writes:\n\n> You probably want to apply or squash this somewhere.\n\nThanks.\n"},{"id":"118588","messageId":"pJGueCmuNgb226XrXo6E-MM775KWLC_yj40cI639W5tahKT6xRtS-g@cipher.nrlssc.navy.mil","threadId":"20152","inReplyTo":"57XXtFYcpNU4ayC2onbxyZE72nxUH64HrjFA1KTX3lvwrSUaKMsqemp5CX0U7W460zAC_hc0GdA@cipher.nrlssc.navy.mil","subject":"Re: [PATCH] config.mak.in: continue fixing NEEDS_LIBGEN autoconfigure feature","fromName":"Brandon Casey","fromEmail":"brandon.casey.ctr@nrlssc.navy.mil","sentAt":"2009-07-23T16:22:34Z","receivedAt":"2009-07-23T16:22:34Z","isPatch":true,"sender":{"key":"brandon.casey.ctr@nrlssc.navy.mil","avatar":null},"body":"\nJunio,\n\nI see you squashed this and applied it to master (a1142892), but\nsince my email was not clear, the commit message is not accurate\n(in an unimportant way).\n\nYou said\n\n    Without [the addition of NEEDS_LIBGEN to config.mak.in], the\n    generated shell script would contain a snippet like this:\n\n       for ac_lib in ; do\n         ...\n\n    which is incorrect.\n\nActually, the \"for ac_lib in ; do\" snippet is produced with or without\nmy patch.  I didn't mean to imply that there was a change in that\nrespect.  It's just that that snippet is what prevents the configure\nscript from completing successfully on IRIX and Solaris 7 when executing\nit with /bin/sh or /bin/ksh.  When bash is used, the configure script\ncan execute successfully, and then it can be verified that the patched\nconfigure.ac produces a configure script that operates correctly.\n\nSorry for the confusion.\n\n-brandon\n\n\nBrandon Casey wrote:\n> From: Brandon Casey <drafnel@gmail.com>\n> \n> An entry in the config.mak.in file is necessary for the NEEDS_LIBGEN variable\n> to appear in the config.mak.autogen file with the value assigned by the\n> configure script.\n> \n> Signed-off-by: Brandon Casey <drafnel@gmail.com>\n> ---\n> \n> \n> Junio,\n> \n> You probably want to apply or squash this somewhere.\n> \n> As I noted in the original patch, the autoconf part was untested.\n> I actually did some light testing on this one.  I created the configure file\n> on linux and ran it on Solaris and IRIX.  Both produce an error which looks\n> something like:\n> \n>    configure[1234]: syntax error at line 4806 : `;' unexpected\n> \n> And line 4806 looks like:\n> \n>    for ac_lib in ; do\n>       ...\n> \n> It works with bash though, and it works with /bin/sh on Solaris 10.  On\n> Solaris 10, the configure script correctly detects that hstrerror cannot be\n> used without -lresolv, and basename can be used without -lgen.  In\n> config.mak.autogen, NEEDS_RESOLV is set to 'YesPlease' and NEEDS_LIBGEN is\n> unset.  On my Solaris 7, bash is not available, but the informational messages\n> indicate the same results as for Solaris 10.  On IRIX, hstrerror() can be used\n> without -lresolv and basename cannot be used without -lgen.  In\n> config.mak.autogen, NEEDS_RESOLV is unset, and NEEDS_LIBGEN is set to\n> 'YesPlease'.\n> \n> So, I think this patch should be the final one.\n> \n> -brandon\n> \n> \n>  config.mak.in |    1 +\n>  1 files changed, 1 insertions(+), 0 deletions(-)\n> \n> diff --git a/config.mak.in b/config.mak.in\n> index dd60451..67b12f7 100644\n> --- a/config.mak.in\n> +++ b/config.mak.in\n> @@ -34,6 +34,7 @@ NO_LIBGEN_H=@NO_LIBGEN_H@\n>  NEEDS_LIBICONV=@NEEDS_LIBICONV@\n>  NEEDS_SOCKET=@NEEDS_SOCKET@\n>  NEEDS_RESOLV=@NEEDS_RESOLV@\n> +NEEDS_LIBGEN=@NEEDS_LIBGEN@\n>  NO_SYS_SELECT_H=@NO_SYS_SELECT_H@\n>  NO_D_INO_IN_DIRENT=@NO_D_INO_IN_DIRENT@\n>  NO_D_TYPE_IN_DIRENT=@NO_D_TYPE_IN_DIRENT@\n"}]}