{"thread":{"id":"29517","subject":"Breakage in master?","startedAt":"2012-02-02T12:14:19Z","lastAt":"2012-02-09T06:03:51Z","messageCount":12,"participants":["Erik Faye-Lund","Jeff King","Johannes Schindelin","Torsten Bögershausen","Johannes Sixt","Joel C. Salomon","svnpenn@gmail.com"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"183594","messageId":"CABPQNSbWu0r_gKGvCHk567pUtQiyDOCO8vFfrzPMFW1eUaj1nw@mail.gmail.com","threadId":"29517","inReplyTo":null,"subject":"Breakage in master?","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2012-02-02T12:14:19Z","receivedAt":"2012-02-02T12:14:19Z","isPatch":false,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"Something strange is going on in Junio's current 'master' branch\n(f3fb075). \"git show\" has started to error out on Windows with a\ncomplaint about our vsnprintf:\n---8<---\n\n$ git show\ncommit f3fb07509c2e0b21b12a598fcd0a19a92fc38a9d\nAuthor: Junio C Hamano \nDate: Tue Jan 31 22:31:35 2012 -0800\n\n Update draft release notes to 1.7.10\n\n Signed-off-by: Junio C Hamano \n\nfatal: BUG: your vsnprintf is broken (returned -1)\n---8<---\n\n\"git status\" is also behaving strange:\n---8<---\n$ git status\n# On branch master\n# Untracked files:\n# (use \"git add ...\" to include in what will be committed)\n#\n# ←[31mcompat/vcbuild/include/sys/resource.h←[m\n# ←[31mtemp.patch←[m\n# ←[31mtest.c←[m\n# ←[31mtest.patch←[m\nnothing added to commit but untracked files present (use \"git add\" to track)\n---8<---\n\nYeah, the ANSI color codes are being printed verbatim, even though\ncompat/winansi.c is supposed to convert these. \"git -p status\" works\nfine, as it pipes the ANSI codes directly through less.\n\nBut here's the REALLY puzzling part: If I add a simple, unused\nfunction to diff-lib.c, like this:\n\n---8<---\ndiff --git a/diff-lib.c b/diff-lib.c\nindex fc0dff3..914a224 100644\n--- a/diff-lib.c\n+++ b/diff-lib.c\n@@ -82,6 +82,11 @@ static int match_stat_with_submodule(struct\ndiff_options *diffopt,\n \treturn changed;\n }\n\n+static unsigned int foo(const char *a, unsigned int b)\n+{\n+\treturn b;\n+}\n+\n int run_diff_files(struct rev_info *revs, unsigned int option)\n {\n \tint entries, i;\n---8<---\n\n\"git status\" starts to error out with that same vsnprintf complaint!\n\n---8<---\n$ git status\n# On branch master\n# Changes not staged for commit:\n# (use \"git add ...\" to update what will be committed)\nfatal: BUG: your vsnprintf is broken (returned -1)\n---8<---\n\nHere's the stack-trace:\n---8<---\nBreakpoint 1, die (err=0x5268ec \"BUG: your vsnprintf is broken (returned %d)\")\n at usage.c:81\n81 void NORETURN die(const char *err, ...)\n(gdb) bt\n#0 die (err=0x5268ec \"BUG: your vsnprintf is broken (returned %d)\")\n at usage.c:81\n#1 0x00466314 in strbuf_vaddf (sb=0x28fb54,\n fmt=0x533ef4 \" (use \\\"git checkout -- ...\\\" to discard changes in wor\nking directory)\", ap=0x28fbac \"+\\025\\026\\002\") at strbuf.c:221\n#2 0x004cbb6f in status_vprintf (s=0x28fc38, at_bol=0,\n color=0x46c4f2 \"\\205└t\\n\\203─\\020[^╔├\\215v\",\n fmt=0x533ef4 \" (use \\\"git checkout -- ...\\\" to discard changes in wor\nking directory)\", ap=0x28fbac \"+\\025\\026\\002\", trail=0x533a89 \"\\n\")\n at wt-status.c:44\n#3 0x004cbe6b in status_printf_ln (s=0x28fc38, color=0x28fc70 \"\",\n fmt=0x533ef4 \" (use \\\"git checkout -- ...\\\" to discard changes in wor\nking directory)\") at wt-status.c:87\n#4 0x004cced1 in wt_status_print (s=0x28fc38) at wt-status.c:176\n#5 0x0041922e in cmd_status (argc=1, argv=0x319b4, prefix=0x0)\n at builtin/commit.c:1254\n#6 0x004019d6 in handle_internal_command (argc=,\n argv=) at git.c:308\n#7 0x00401c26 in main (argc=2, argv=0x319b0) at git.c:513\n(gdb)\n---8<---\n\nThis smells a bit like a smashed stack to me. Both issues happens in\nroughly the same area of the call stack, and when adding an unused\nfunction changes behavior, something really odd is going on ;)\n\nI've bisected the issues down to 5e9637c (i18n: add infrastructure for\ntranslating Git with gettext). Trying to apply my unused-function\npatch on top of this commit starts giving the same \"fatal: BUG: your\nvsnprintf is broken (returned -1)\" error. It's ancestor, bc1bbe0(Git\n1.7.8-rc2), does not yield any of the issues.\n\nI'm at a loss here. Does anyone have a hunch about what's going on?\n"},{"id":"183618","messageId":"20120202174601.GB30857@sigill.intra.peff.net","threadId":"29517","inReplyTo":"CABPQNSbWu0r_gKGvCHk567pUtQiyDOCO8vFfrzPMFW1eUaj1nw@mail.gmail.com","subject":"Re: Breakage in master?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-02-02T17:46:01Z","receivedAt":"2012-02-02T17:46:01Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Feb 02, 2012 at 01:14:19PM +0100, Erik Faye-Lund wrote:\n\n> But here's the REALLY puzzling part: If I add a simple, unused\n> function to diff-lib.c, like this:\n> [...]\n> \"git status\" starts to error out with that same vsnprintf complaint!\n> \n> ---8<---\n> $ git status\n> # On branch master\n> # Changes not staged for commit:\n> # (use \"git add ...\" to update what will be committed)\n> fatal: BUG: your vsnprintf is broken (returned -1)\n> ---8<---\n\nOK, that's definitely odd.\n\nAt the moment of the die() in strbuf_vaddf, what does errno say?\nvsnprintf should generally never be returning -1 (it should return the\nnumber of characters that would have been written). Since you're on\nWindows, I assume you're using the replacement version in\ncompat/snprintf.c.\n\nThat one will return -1 if realloc fails. So I'm curious if that is what\nis happening (you might also instrument the call to realloc in\nsnprintf.c to see if it is failing, and if so, at what maxsize). And/or\ncheck errno in git_vsnprintf after calling the native vsnprintf and\ngetting -1.\n\nHere's one possible sequence of events that seems plausible to me (and\nremember that this is a wild guess):\n\n 1. gettext somehow munges the format string in a way that Windows\n vsnprintf doesn't like, and it returns -1.\n\n 2. Our git_vsnprintf wrapper interprets this -1 as \"you didn't give me\n enough space to store the result\", and we grow our test-buffer to\n try again\n\n 3. Eventually the test buffer gets unreasonably large, and realloc\n fails. We have no choice but to return -1 from our wrapper.\n\n 4. strbuf_vaddf sees the -1 and thinks you are using a broken\n vsnprintf.\n\nAll of that would make sense to me, _except_ for your weird \"if I add a\nrandom function, the problem is more reproducible\" bit. Which does seem\nlike something is invoking undefined behavior (of course, it could be\nthat undefined behavior or stack-smashing that is causing vsnprintf to\nreport an error). Lacking any better leads, it might be worth pursuing.\n\n> I've bisected the issues down to 5e9637c (i18n: add infrastructure for\n> translating Git with gettext). Trying to apply my unused-function\n> patch on top of this commit starts giving the same \"fatal: BUG: your\n> vsnprintf is broken (returned -1)\" error. It's ancestor, bc1bbe0(Git\n> 1.7.8-rc2), does not yield any of the issues.\n\nI've looked at 5e9637c, and it really doesn't do anything that looks\nbad. I wonder if your gettext library is buggy. Does compiling with\nNO_GETTEXT help?\n\n-Peff\n"},{"id":"183630","messageId":"alpine.DEB.1.00.1202021956370.1249@s15462909.onlinehome-server.info","threadId":"29517","inReplyTo":"CABPQNSbWu0r_gKGvCHk567pUtQiyDOCO8vFfrzPMFW1eUaj1nw@mail.gmail.com","subject":"Re: [msysGit] Breakage in master?","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2012-02-02T18:57:39Z","receivedAt":"2012-02-02T18:57:39Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Erik,\n\nOn Thu, 2 Feb 2012, Erik Faye-Lund wrote:\n\n> Something strange is going on in Junio's current 'master' branch\n> (f3fb075). \"git show\" has started to error out on Windows with a\n> complaint about our vsnprintf:\n> ---8<---\n> \n> $ git show\n> commit f3fb07509c2e0b21b12a598fcd0a19a92fc38a9d\n> Author: Junio C Hamano \n> Date: Tue Jan 31 22:31:35 2012 -0800\n> \n> Update draft release notes to 1.7.10\n> \n> Signed-off-by: Junio C Hamano \n> \n> fatal: BUG: your vsnprintf is broken (returned -1)\n> ---8<---\n>\n> [...]\n>\n> I'm at a loss here. Does anyone have a hunch about what's going on?\n\nIt very much reminds me of 6ef404095bc1162031fc3cb43430b512e975bc6a...\n\nIs it possible that NO_GETTEXT is either not set, or ignored?\n\nCiao,\nDscho\n"},{"id":"183655","messageId":"4F2AEED2.30503@web.de","threadId":"29517","inReplyTo":"alpine.DEB.1.00.1202021956370.1249@s15462909.onlinehome-server.info","subject":"Re: [msysGit] Breakage in master?","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2012-02-02T20:15:14Z","receivedAt":"2012-02-02T20:15:14Z","isPatch":false,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 02.02.12 19:57, Johannes Schindelin wrote:\n> Hi Erik,\n> \n> On Thu, 2 Feb 2012, Erik Faye-Lund wrote:\n> \n>> Something strange is going on in Junio's current 'master' branch\n>> (f3fb075). \"git show\" has started to error out on Windows with a\n>> complaint about our vsnprintf:\n>> ---8<---\n>>\n>> $ git show\n>> commit f3fb07509c2e0b21b12a598fcd0a19a92fc38a9d\n>> Author: Junio C Hamano \n>> Date: Tue Jan 31 22:31:35 2012 -0800\n>>\n>> Update draft release notes to 1.7.10\n>>\n>> Signed-off-by: Junio C Hamano \n>>\n>> fatal: BUG: your vsnprintf is broken (returned -1)\n>> ---8<---\n>>\n>> [...]\n>>\n>> I'm at a loss here. Does anyone have a hunch about what's going on?\n> \n> It very much reminds me of 6ef404095bc1162031fc3cb43430b512e975bc6a...\n> \n> Is it possible that NO_GETTEXT is either not set, or ignored?\n> \n> Ciao,\n> Dscho\nHi,\nsame problem here, tested on Win XP.\n\nGood news (?):\nSetting \nNO_GETTEXT=YesPlease\nin the Makefile makes the problem go away for me.\n/Torsten\n"},{"id":"183662","messageId":"4F2AF5D6.4060609@kdbg.org","threadId":"29517","inReplyTo":"CABPQNSbWu0r_gKGvCHk567pUtQiyDOCO8vFfrzPMFW1eUaj1nw@mail.gmail.com","subject":"Re: [msysGit] Breakage in master?","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2012-02-02T20:45:10Z","receivedAt":"2012-02-02T20:45:10Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 02.02.2012 13:14, schrieb Erik Faye-Lund:\n> Something strange is going on in Junio's current 'master' branch\n> (f3fb075). \"git show\" has started to error out on Windows with a\n> complaint about our vsnprintf:\n> ---8<---\n> \n> $ git show\n> commit f3fb07509c2e0b21b12a598fcd0a19a92fc38a9d\n> Author: Junio C Hamano \n> Date: Tue Jan 31 22:31:35 2012 -0800\n> \n> Update draft release notes to 1.7.10\n> \n> Signed-off-by: Junio C Hamano \n> \n> fatal: BUG: your vsnprintf is broken (returned -1)\n\nFWIW, I run a version of master on Windows with almost no additional\npatches that touch compat/ (the few changes I do have are unsuspicious).\nBut I do not observe this behavior. I build with NO_GETTEXT and -O0.\n\n-- Hannes\n"},{"id":"183712","messageId":"CABPQNSZfKCTsuusPpHa2djEOeGVN9z5s_Fr+S3EaHiv7Q4Re9w@mail.gmail.com","threadId":"29517","inReplyTo":"20120202174601.GB30857@sigill.intra.peff.net","subject":"Re: Breakage in master?","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2012-02-03T12:28:29Z","receivedAt":"2012-02-03T12:28:29Z","isPatch":false,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Thu, Feb 2, 2012 at 6:46 PM, Jeff King wrote:\n> On Thu, Feb 02, 2012 at 01:14:19PM +0100, Erik Faye-Lund wrote:\n>\n>> But here's the REALLY puzzling part: If I add a simple, unused\n>> function to diff-lib.c, like this:\n>> [...]\n>> \"git status\" starts to error out with that same vsnprintf complaint!\n>>\n>> ---8<---\n>> $ git status\n>> # On branch master\n>> # Changes not staged for commit:\n>> #   (use \"git add ...\" to update what will be committed)\n>> fatal: BUG: your vsnprintf is broken (returned -1)\n>> ---8<---\n>\n> OK, that's definitely odd.\n>\n> At the moment of the die() in strbuf_vaddf, what does errno say?\n\nIf I apply this patch:\n---8<---\ndiff --git a/strbuf.c b/strbuf.c\nindex ff0b96b..52dfdd6 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -218,7 +218,7 @@ void strbuf_vaddf(struct strbuf *sb, const char\n*fmt, va_list ap)\n \tlen = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, cp);\n \tva_end(cp);\n \tif (len < 0)\n-\t\tdie(\"BUG: your vsnprintf is broken (returned %d)\", len);\n+\t\tdie_errno(\"BUG: your vsnprintf is broken (returned %d)\", len);\n \tif (len > strbuf_avail(sb)) {\n \t\tstrbuf_grow(sb, len);\n \t\tlen = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);\n---8<---\n\nThen I get \"fatal: BUG: your vsnprintf is broken (returned -1): Result\ntoo large\". This goes both for both failure cases I described. I\nassume this means errno=ERANGE.\n\n> vsnprintf should generally never be returning -1 (it should return the\n> number of characters that would have been written). Since you're on\n> Windows, I assume you're using the replacement version in\n> compat/snprintf.c.\n\nNo. SNPRINTF_RETURNS_BOGUS is only set for the MSVC target, not for\nthe MinGW target. I'm assuming that means MinGW-runtime has a sane\nvsnprintf implementation. But even if I enable SNPRINTF_RETURNS_BOGUS,\nthe problem occurs. And it's still \"Result too large\".\n\nSo I decided to do a bit of stepping, and it seems libintl takes over\nvsnprintf, directing us to libintl_vsnprintf instead. I guess this is\nso it can ensure we support reordering the parameters with $1 etc...\nAnd aparently this vsnprintf implementation calls the system vnsprintf\nif the format string does not contain '$', and it's using _vsnprintf\nrather than vsnprintf on Windows. _vsnprintf is the MSVCRT-version,\nand not the MinGW-runtime, which needs SNPRINTF_RETURNS_BOGUS.\n\nSo I guess I can patch libintl to call vsnprintf from MinGW-runtime instead.\n\n> All of that would make sense to me, _except_ for your weird \"if I add a\n> random function, the problem is more reproducible\" bit. Which does seem\n> like something is invoking undefined behavior (of course, it could be\n> that undefined behavior or stack-smashing that is causing vsnprintf to\n> report an error). Lacking any better leads, it might be worth pursuing.\n\nWell, now at least I have some better leads, but I'm still not able to\nexplain the \"if I add a random function, the problem is more\nreproducible\" bit. :(\n\n>> I've bisected the issues down to 5e9637c (i18n: add infrastructure for\n>> translating Git with gettext). Trying to apply my unused-function\n>> patch on top of this commit starts giving the same \"fatal: BUG: your\n>> vsnprintf is broken (returned -1)\" error. It's ancestor, bc1bbe0(Git\n>> 1.7.8-rc2), does not yield any of the issues.\n>\n> I've looked at 5e9637c, and it really doesn't do anything that looks\n> bad. I wonder if your gettext library is buggy. Does compiling with\n> NO_GETTEXT help?\n\nCompiling with NO_GETTEXT does make the symptoms go away, but that's\nnot curing the problem ;)\n\nBut, I have a lead now. I'll see if I can find out *why* libintl calls\n_vsnprintf on MinGW. I expect it's so the MSVC and the MinGW versions\nbehave similarly, MSVC doesn't have a sane vsnprintf. Perhaps I should\nback-port SNPRINTF_RETURNS_BOGUS-workaround to libintl, so our MSVC\nbuilds doesn't break also?\n"},{"id":"183732","messageId":"4F2BE759.4000902@gmail.com","threadId":"29517","inReplyTo":"CABPQNSZfKCTsuusPpHa2djEOeGVN9z5s_Fr+S3EaHiv7Q4Re9w@mail.gmail.com","subject":"Re: Breakage in master?","fromName":"Joel C. Salomon","fromEmail":"joelcsalomon@gmail.com","sentAt":"2012-02-03T13:55:37Z","receivedAt":"2012-02-03T13:55:37Z","isPatch":false,"sender":{"key":"joelcsalomon@gmail.com","avatar":"https://gravatar.com/avatar/271ed240fc850235728b19d5b2b9f38c41ebdf0209c5b663f8b07060950489e6?d=mp&s=160"},"body":"On 02/03/2012 07:28 AM, Erik Faye-Lund wrote:\n> On Thu, Feb 2, 2012 at 6:46 PM, Jeff King wrote:\n>> vsnprintf should generally never be returning -1 (it should return the\n>> number of characters that would have been written). Since you're on\n>> Windows, I assume you're using the replacement version in\n>> compat/snprintf.c.\n> \n> No. SNPRINTF_RETURNS_BOGUS is only set for the MSVC target, not for\n> the MinGW target. I'm assuming that means MinGW-runtime has a sane\n> vsnprintf implementation.\n\nThat doesn't sound right; MinGW defaults to linking to a fairly old\nversion of the Windows C library (MSVCRT.dll from Visual Studio 6,\nIIRC). According to there exists libmingwex\nwith some functions (especially those from ) replaced for\nStandard compatibility, but it's \"far from complete\". (Is msysGit using\nit anyway?)\n\n--Joel\n"},{"id":"183733","messageId":"CABPQNSbQTF1UiDuOZkX-KrTQ7oFyVvx6FxZ85c9uCF2FFUtTSg@mail.gmail.com","threadId":"29517","inReplyTo":"4F2BE759.4000902@gmail.com","subject":"Re: Breakage in master?","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2012-02-03T14:05:30Z","receivedAt":"2012-02-03T14:05:30Z","isPatch":false,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Fri, Feb 3, 2012 at 2:55 PM, Joel C. Salomon wrote:\n> On 02/03/2012 07:28 AM, Erik Faye-Lund wrote:\n>> On Thu, Feb 2, 2012 at 6:46 PM, Jeff King wrote:\n>>> vsnprintf should generally never be returning -1 (it should return the\n>>> number of characters that would have been written). Since you're on\n>>> Windows, I assume you're using the replacement version in\n>>> compat/snprintf.c.\n>>\n>> No. SNPRINTF_RETURNS_BOGUS is only set for the MSVC target, not for\n>> the MinGW target. I'm assuming that means MinGW-runtime has a sane\n>> vsnprintf implementation.\n>\n> That doesn't sound right; MinGW defaults to linking to a fairly old\n> version of the Windows C library (MSVCRT.dll from Visual Studio 6,\n> IIRC).  According to there exists libmingwex\n> with some functions (especially those from ) replaced for\n> Standard compatibility, but it's \"far from complete\".  (Is msysGit using\n> it anyway?)\n\nI'm not entirely sure what you are arguing. On MinGW, calling\nvsnprintf vs _vsnprintf leads to different implementations on MinGW.\nThis is documented in the release-notes:\nhttp://sourceforge.net/project/shownotes.php?release_id=24832\n\n\"As in previous releases, the MinGW implementations of snprintf() and\nvsnprintf() are the default for these two functions, with the MSVCRT\nalternatives being called as _snprintf() and _vsnprintf().\"\n\nI don't see how this is contradicted by your argument of a third,\nC99-ish implementation. I'm pretty sure the \"far from complete\"-part\nis about the C99-features anyway.\n"},{"id":"183735","messageId":"4F2BEDA3.8070308@gmail.com","threadId":"29517","inReplyTo":"CABPQNSbQTF1UiDuOZkX-KrTQ7oFyVvx6FxZ85c9uCF2FFUtTSg@mail.gmail.com","subject":"Re: Breakage in master?","fromName":"Joel C. Salomon","fromEmail":"joelcsalomon@gmail.com","sentAt":"2012-02-03T14:22:27Z","receivedAt":"2012-02-03T14:22:27Z","isPatch":false,"sender":{"key":"joelcsalomon@gmail.com","avatar":"https://gravatar.com/avatar/271ed240fc850235728b19d5b2b9f38c41ebdf0209c5b663f8b07060950489e6?d=mp&s=160"},"body":"On 02/03/2012 09:05 AM, Erik Faye-Lund wrote:\n> On Fri, Feb 3, 2012 at 2:55 PM, Joel C. Salomon wrote:\n>> That doesn't sound right; MinGW defaults to linking to a fairly old\n>> version of the Windows C library (MSVCRT.dll from Visual Studio 6,\n>> IIRC).\n> \n> I'm not entirely sure what you are arguing. On MinGW, calling\n> vsnprintf vs _vsnprintf leads to different implementations on MinGW.\n> This is documented in the release-notes:\n> http://sourceforge.net/project/shownotes.php?release_id=24832\n\nNever mind; I stand corrected. It's been some time since I actively\nfollowed MinGW development.\n\n--Joel\n"},{"id":"183871","messageId":"CABPQNSbj8QKqkdY49Y7tpAOQd53t+z6Gc5U-CS0-TZWyNz1WfQ@mail.gmail.com","threadId":"29517","inReplyTo":"CABPQNSZfKCTsuusPpHa2djEOeGVN9z5s_Fr+S3EaHiv7Q4Re9w@mail.gmail.com","subject":"Re: Breakage in master?","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2012-02-04T21:55:16Z","receivedAt":"2012-02-04T21:55:16Z","isPatch":false,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Fri, Feb 3, 2012 at 1:28 PM, Erik Faye-Lund wrote:\n> On Thu, Feb 2, 2012 at 6:46 PM, Jeff King wrote:\n>> On Thu, Feb 02, 2012 at 01:14:19PM +0100, Erik Faye-Lund wrote:\n>>\n>>> But here's the REALLY puzzling part: If I add a simple, unused\n>>> function to diff-lib.c, like this:\n>>> [...]\n>>> \"git status\" starts to error out with that same vsnprintf complaint!\n>>>\n>>> ---8<---\n>>> $ git status\n>>> # On branch master\n>>> # Changes not staged for commit:\n>>> #   (use \"git add ...\" to update what will be committed)\n>>> fatal: BUG: your vsnprintf is broken (returned -1)\n>>> ---8<---\n>>\n>> OK, that's definitely odd.\n>>\n>> At the moment of the die() in strbuf_vaddf, what does errno say?\n>\n> If I apply this patch:\n> ---8<---\n> diff --git a/strbuf.c b/strbuf.c\n> index ff0b96b..52dfdd6 100644\n> --- a/strbuf.c\n> +++ b/strbuf.c\n> @@ -218,7 +218,7 @@ void strbuf_vaddf(struct strbuf *sb, const char\n> *fmt, va_list ap)\n>        len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, cp);\n>        va_end(cp);\n>        if (len < 0)\n> -               die(\"BUG: your vsnprintf is broken (returned %d)\", len);\n> +               die_errno(\"BUG: your vsnprintf is broken (returned %d)\", len);\n>        if (len > strbuf_avail(sb)) {\n>                strbuf_grow(sb, len);\n>                len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);\n> ---8<---\n>\n> Then I get \"fatal: BUG: your vsnprintf is broken (returned -1): Result\n> too large\". This goes both for both failure cases I described. I\n> assume this means errno=ERANGE.\n>\n>> vsnprintf should generally never be returning -1 (it should return the\n>> number of characters that would have been written). Since you're on\n>> Windows, I assume you're using the replacement version in\n>> compat/snprintf.c.\n>\n> No. SNPRINTF_RETURNS_BOGUS is only set for the MSVC target, not for\n> the MinGW target. I'm assuming that means MinGW-runtime has a sane\n> vsnprintf implementation. But even if I enable SNPRINTF_RETURNS_BOGUS,\n> the problem occurs. And it's still \"Result too large\".\n>\n> So I decided to do a bit of stepping, and it seems libintl takes over\n> vsnprintf, directing us to libintl_vsnprintf instead. I guess this is\n> so it can ensure we support reordering the parameters with $1 etc...\n> And aparently this vsnprintf implementation calls the system vnsprintf\n> if the format string does not contain '$', and it's using _vsnprintf\n> rather than vsnprintf on Windows. _vsnprintf is the MSVCRT-version,\n> and not the MinGW-runtime, which needs SNPRINTF_RETURNS_BOGUS.\n>\n> So I guess I can patch libintl to call vsnprintf from MinGW-runtime instead.\n>\n\nIndeed, I just got around to testing this, and doing this on top of\ngettext seems to fix the problem for me. For the MSVC, a more\nelaborate fix is needed, as it doesn't have a sane vsnprintf.\n\n---\n\ndiff --git a/gettext-runtime/intl/printf.c b/gettext-runtime/intl/printf.c\nindex b7cdc5d..f55023e 100644\n--- a/gettext-runtime/intl/printf.c\n+++ b/gettext-runtime/intl/printf.c\n@@ -192,7 +192,7 @@ libintl_sprintf (char *resultbuf, const char *format, ...)\n\n #if HAVE_SNPRINTF\n\n-# if HAVE_DECL__SNPRINTF\n+# if HAVE_DECL__SNPRINTF && !defined(__MINGW32__)\n /* Windows. */\n # define system_vsnprintf _vsnprintf\n # else\n"},{"id":"183897","messageId":"CABPQNSbPA3jicuPiaHuSBY_qEy1Pmy1uWnLBWt=7O0dETbKZ3g@mail.gmail.com","threadId":"29517","inReplyTo":"CABPQNSbj8QKqkdY49Y7tpAOQd53t+z6Gc5U-CS0-TZWyNz1WfQ@mail.gmail.com","subject":"Fwd: Breakage in master?","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2012-02-05T14:46:04Z","receivedAt":"2012-02-05T14:46:04Z","isPatch":false,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"Git has recently switched to using gettext for translations, and I\nhave observed a breakage on Windows due to the way gettext handles\nvsnprintf.\n\nOn MinGW, vsnprintf and _vsnprintf are two different implementations;\nvsnprintf is from MinGW-runtime, and provides a reasonably sane\nimplementation. _vnsprintf on the other hand is from MSVCRT.dll, and\nhas some issues with it's return value. Before using gettext, the\nMinGW-built version of Git called the version from mingw-runtime, and\neverything worked fine. When built with MSVC, a shim was used to fixup\nthe bogus return value. This shim was injected through a define,\nsimilar to what gettext does.\n\nThe shim in gettext lead to issues for Git, both on MinGW and on MSVC.\n\nFor MinGW, the problem is that libintl_vsnprintf calls _vsnprintf\nrather than vsnprintf, giving us the same, broken return value that we\ntried to prevent. This means that our code intended to call the\nMinGW-runtime version, but gettext ended up calling the MSVCRT.dll\nversion. I don't find this very reasonable; a call to vsnprintf ends\nup as a call to _vsnprintf.\n\nOn MSVC the problem is a bit easier to spot; libgnuintl.h.in contains\nthe following:\n\n---8<---\n#if !(defined vsnprintf && defined _GL_STDIO_H) /* don't override gnulib */\n#undef vsnprintf\n#define vsnprintf libintl_vsnprintf\nextern int vsnprintf (char *, size_t, const char *, va_list);\n#endif\n---8<---\n\nUhm, what? Unless we're using Gnulib, our definition of vsnprintf\nshould simply be ignored?\n\nI'm not saying figuring out what to do here is exactly trivial; but I\nthink undefining any definitions of vsnprintf that aren't exactly\n\"_vsnprintf\" is dangerous. The forwarded mail below contains a\nquick-fix I did locally that seems to side-step the problem for me, by\nnot using _vsnprintf on MinGW. But perhaps there's something better we\ncan do?\n\n---------- Forwarded message ----------\nFrom: Erik Faye-Lund \nDate: Sat, Feb 4, 2012 at 10:55 PM\nSubject: Re: Breakage in master?\nTo: Jeff King \nCc: Git Mailing List , msysGit\n, Ævar Arnfjörð \n\n\nOn Fri, Feb 3, 2012 at 1:28 PM, Erik Faye-Lund wrote:\n> On Thu, Feb 2, 2012 at 6:46 PM, Jeff King wrote:\n>> On Thu, Feb 02, 2012 at 01:14:19PM +0100, Erik Faye-Lund wrote:\n>>\n>>> But here's the REALLY puzzling part: If I add a simple, unused\n>>> function to diff-lib.c, like this:\n>>> [...]\n>>> \"git status\" starts to error out with that same vsnprintf complaint!\n>>>\n>>> ---8<---\n>>> $ git status\n>>> # On branch master\n>>> # Changes not staged for commit:\n>>> #   (use \"git add ...\" to update what will be committed)\n>>> fatal: BUG: your vsnprintf is broken (returned -1)\n>>> ---8<---\n>>\n>> OK, that's definitely odd.\n>>\n>> At the moment of the die() in strbuf_vaddf, what does errno say?\n>\n> If I apply this patch:\n> ---8<---\n> diff --git a/strbuf.c b/strbuf.c\n> index ff0b96b..52dfdd6 100644\n> --- a/strbuf.c\n> +++ b/strbuf.c\n> @@ -218,7 +218,7 @@ void strbuf_vaddf(struct strbuf *sb, const char\n> *fmt, va_list ap)\n>        len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, cp);\n>        va_end(cp);\n>        if (len < 0)\n> -               die(\"BUG: your vsnprintf is broken (returned %d)\", len);\n> +               die_errno(\"BUG: your vsnprintf is broken (returned %d)\", len);\n>        if (len > strbuf_avail(sb)) {\n>                strbuf_grow(sb, len);\n>                len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);\n> ---8<---\n>\n> Then I get \"fatal: BUG: your vsnprintf is broken (returned -1): Result\n> too large\". This goes both for both failure cases I described. I\n> assume this means errno=ERANGE.\n>\n>> vsnprintf should generally never be returning -1 (it should return the\n>> number of characters that would have been written). Since you're on\n>> Windows, I assume you're using the replacement version in\n>> compat/snprintf.c.\n>\n> No. SNPRINTF_RETURNS_BOGUS is only set for the MSVC target, not for\n> the MinGW target. I'm assuming that means MinGW-runtime has a sane\n> vsnprintf implementation. But even if I enable SNPRINTF_RETURNS_BOGUS,\n> the problem occurs. And it's still \"Result too large\".\n>\n> So I decided to do a bit of stepping, and it seems libintl takes over\n> vsnprintf, directing us to libintl_vsnprintf instead. I guess this is\n> so it can ensure we support reordering the parameters with $1 etc...\n> And aparently this vsnprintf implementation calls the system vnsprintf\n> if the format string does not contain '$', and it's using _vsnprintf\n> rather than vsnprintf on Windows. _vsnprintf is the MSVCRT-version,\n> and not the MinGW-runtime, which needs SNPRINTF_RETURNS_BOGUS.\n>\n> So I guess I can patch libintl to call vsnprintf from MinGW-runtime instead.\n>\n\nIndeed, I just got around to testing this, and doing this on top of\ngettext seems to fix the problem for me. For the MSVC, a more\nelaborate fix is needed, as it doesn't have a sane vsnprintf.\n\n---\n\ndiff --git a/gettext-runtime/intl/printf.c b/gettext-runtime/intl/printf.c\nindex b7cdc5d..f55023e 100644\n--- a/gettext-runtime/intl/printf.c\n+++ b/gettext-runtime/intl/printf.c\n@@ -192,7 +192,7 @@ libintl_sprintf (char *resultbuf, const char *format, ...)\n\n #if HAVE_SNPRINTF\n\n-# if HAVE_DECL__SNPRINTF\n+# if HAVE_DECL__SNPRINTF && !defined(__MINGW32__)\n   /* Windows.  */\n #  define system_vsnprintf _vsnprintf\n # else\n"},{"id":"184221","messageId":"1303990.135.1328767431554.JavaMail.geo-discussion-forums@vbnh4","threadId":"29517","inReplyTo":"CABPQNSbWu0r_gKGvCHk567pUtQiyDOCO8vFfrzPMFW1eUaj1nw@mail.gmail.com","subject":"Re: Breakage in master?","fromName":"","fromEmail":"svnpenn@gmail.com","sentAt":"2012-02-09T06:03:51Z","receivedAt":"2012-02-09T06:03:51Z","isPatch":false,"sender":{"key":"svnpenn@gmail.com","avatar":"https://avatars.githubusercontent.com/u/60531334?v=4"},"body":"When I use NO_GETTEXT=YesPlease I lose text coloring. I assume this is expected. \n\nIs there a workaround that doesnt lose text coloring?\n"}]}