{"thread":{"id":"46666","subject":"cat-file timing window on Cygwin","startedAt":"2017-08-25T11:25:41Z","lastAt":"2017-09-08T20:02:55Z","messageCount":11,"participants":["Adam Dinwoodie","Jeff King","Ramsay Jones","Jason Pyeron"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"327182","messageId":"20170825112529.GA10378@dinwoodie.org","threadId":"46666","inReplyTo":null,"subject":"cat-file timing window on Cygwin","fromName":"Adam Dinwoodie","fromEmail":"adam@dinwoodie.org","sentAt":"2017-08-25T11:25:29Z","receivedAt":"2017-08-25T11:25:41Z","isPatch":false,"sender":{"key":"adam@dinwoodie.org","avatar":"https://avatars.githubusercontent.com/u/1397507?v=4"},"body":"As of v2.10.0-rc1-4-g321459439 (\"cat-file: support --textconv/--filters\nin batch mode\"), t8010-cat-file-filters.sh has been failing on Cygwin.\nDigging into this, the test looks to expose a timing window: it appears\nthat if `git cat-file --textconv --batch` receives input on stdin too\nquickly, it fails to parse some of that input.\n\nCompare the following output, run from the t8010 trash directory after a\nfailed test run, where adding a `sleep` between the two lines of input\nchanges the output:\n\n    $ { echo $sha1 hello.txt ; echo $sha1 hello; } | git -c diff.txt.textconv='tr A-Za-z N-ZA-Mn-za-m <' cat-file --textconv --batch\n    ce013625030ba8dba906f756967f9e9ca394464a blob 6\n    uryyb\n\n    $ { echo $sha1 hello.txt ; sleep 1; echo $sha1 hello; } | git -c diff.txt.textconv='tr A-Za-z N-ZA-Mn-za-m <' cat-file --textconv --batch\n    ce013625030ba8dba906f756967f9e9ca394464a blob 6\n    uryyb\n\n    ce013625030ba8dba906f756967f9e9ca394464a blob 6\n    hello\n\nSimilarly, I can get t8010 to pass with the following patch:\n\ndiff --git a/t/t8010-cat-file-filters.sh b/t/t8010-cat-file-filters.sh\nindex d8242e467..3aa1385ad 100755\n--- a/t/t8010-cat-file-filters.sh\n+++ b/t/t8010-cat-file-filters.sh\n@@ -54,7 +54,7 @@\n test_expect_success 'cat-file --textconv --batch works' '\n \tsha1=$(git rev-parse -q --verify HEAD:world.txt) &&\n \ttest_config diff.txt.textconv \"tr A-Za-z N-ZA-Mn-za-m <\" &&\n-\tprintf \"%s hello.txt\\n%s hello\\n\" $sha1 $sha1 |\n+\t{ printf \"%s hello.txt\\n\" $sha1 && sleep 1 && printf \"%s hello\\n\" $sha1; } |\n \tgit cat-file --textconv --batch >actual &&\n \tprintf \"%s blob 6\\nuryyb\\r\\n\\n%s blob 6\\nhello\\n\\n\" \\\n \t\t$sha1 $sha1 >expect &&\n\nI don't think blindly applying the patch is the solution here, however.\nThe correct solution is presumably to work out what is causing cat-file\nto discard some of its input and to get it to stop doing that.\n\n(For reference, to get the output above the test was run on the current\nmaster branch, specifically v2.14.1-326-g3dc57ebfb, while the local\ninstalled Git version was v2.14.0, although this behaviour seems to be\nconsistent since the originating commit.)\n"},{"id":"327186","messageId":"20170825150819.agxvbjytom7ao6n6@sigill.intra.peff.net","threadId":"46666","inReplyTo":"20170825112529.GA10378@dinwoodie.org","subject":"Re: cat-file timing window on Cygwin","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-08-25T15:08:20Z","receivedAt":"2017-08-25T15:08:28Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Aug 25, 2017 at 12:25:29PM +0100, Adam Dinwoodie wrote:\n\n> As of v2.10.0-rc1-4-g321459439 (\"cat-file: support --textconv/--filters\n> in batch mode\"), t8010-cat-file-filters.sh has been failing on Cygwin.\n> Digging into this, the test looks to expose a timing window: it appears\n> that if `git cat-file --textconv --batch` receives input on stdin too\n> quickly, it fails to parse some of that input.\n\nHmm. That commit doesn't seem to actually touch the stdin loop. That\nhappens via strbuf_getline(), which should be pretty robust to timing.\nBut here are a few random thoughts:\n\n  1. Do you build with HAVE_GETDELIM? Does toggling it have any effect?\n\n  2. Does the problem happen only with --textconv?\n\n     If so, I wonder if spawning the child process is somehow draining\n     stdin. I don't see how the child would accidentally read from our\n     stdin; we replace its stdin with a pipe so it shouldn't get a\n     chance to see our descriptor at all.\n\n     If we accidentally called fflush(stdin) that would discard buffered\n     input and give the results you're seeing. We don't do that\n     directly, of course, but we do call fflush(NULL) in run-command\n     before spawning the child. That _should_ just flush output handles,\n     but it's possible that there's a cygwin bug, I guess.\n\nI can't reproduce here on Linux. But does the patch below have any\nimpact?\n\ndiff --git a/run-command.c b/run-command.c\nindex 98621faca8..064ebd1995 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -641,7 +641,6 @@ int start_command(struct child_process *cmd)\n \t}\n \n \ttrace_argv_printf(cmd->argv, \"trace: run_command:\");\n-\tfflush(NULL);\n \n #ifndef GIT_WINDOWS_NATIVE\n {\n\n-Peff\n"},{"id":"327227","messageId":"253bf111-4e8a-54b7-6e39-2908270aa357@ramsayjones.plus.com","threadId":"46666","inReplyTo":"20170825150819.agxvbjytom7ao6n6@sigill.intra.peff.net","subject":"Re: cat-file timing window on Cygwin","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2017-08-26T00:57:18Z","receivedAt":"2017-08-26T01:00:28Z","isPatch":false,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 25/08/17 16:08, Jeff King wrote:\n> On Fri, Aug 25, 2017 at 12:25:29PM +0100, Adam Dinwoodie wrote:\n> \n>> As of v2.10.0-rc1-4-g321459439 (\"cat-file: support --textconv/--filters\n>> in batch mode\"), t8010-cat-file-filters.sh has been failing on Cygwin.\n>> Digging into this, the test looks to expose a timing window: it appears\n>> that if `git cat-file --textconv --batch` receives input on stdin too\n>> quickly, it fails to parse some of that input.\n\nThis has not been failing since the above commit (when this\nfeature was first introduced), but (for me) when testing the\nv2.13.0-rc0 build. Please refer to my original email:\n\nhttps://public-inbox.org/git/bf7db655-d90f-e711-afc8-6565b71373d2@ramsayjones.plus.com/\n\n... where I was reasonably sure this was caused by a change in\nthe cygwin dll while upgrading from 2.7.0-1 -> 2.8.0-1.\n\n> Hmm. That commit doesn't seem to actually touch the stdin loop. That\n> happens via strbuf_getline(), which should be pretty robust to timing.\n> But here are a few random thoughts:\n> \n>   1. Do you build with HAVE_GETDELIM? Does toggling it have any effect?\n\nIf Adam builds using the configure script, then yes, else no. ;-)\nI never use configure, so I don't have HAVE_GETDELIM set.\n\n>   2. Does the problem happen only with --textconv?\n> \n>      If so, I wonder if spawning the child process is somehow draining\n>      stdin. I don't see how the child would accidentally read from our\n>      stdin; we replace its stdin with a pipe so it shouldn't get a\n>      chance to see our descriptor at all.\n\nAs I said in my previous email, \"the loop in batch_objects()\n(builtin/cat-file.c lines #489-505) only reads one line from\nstdin, then gets EOF on the stream.\"\n\n>      If we accidentally called fflush(stdin) that would discard buffered\n>      input and give the results you're seeing. We don't do that\n>      directly, of course, but we do call fflush(NULL) in run-command\n>      before spawning the child. That _should_ just flush output handles,\n>      but it's possible that there's a cygwin bug, I guess.\n\nI suspect so, see previous email ...\n\n> I can't reproduce here on Linux. But does the patch below have any\n> impact?\n> \n> diff --git a/run-command.c b/run-command.c\n> index 98621faca8..064ebd1995 100644\n> --- a/run-command.c\n> +++ b/run-command.c\n> @@ -641,7 +641,6 @@ int start_command(struct child_process *cmd)\n>  \t}\n>  \n>  \ttrace_argv_printf(cmd->argv, \"trace: run_command:\");\n> -\tfflush(NULL);\n>  \n>  #ifndef GIT_WINDOWS_NATIVE\n>  {\n\nI suspect not, but I can give it a try ...\n\n... oh, wow, that works! Ahem. (Hmm, so it's flushing stdin?!)\n\nAlso, t5526-fetch-submodules.sh still works as well (see commit\n13af8cbd6a \"start_command: flush buffers in the WIN32 code path\nas well\", 04-02-2011).\n\nATB,\nRamsay Jones\n\n\n"},{"id":"327246","messageId":"20170826154306.GB10378@dinwoodie.org","threadId":"46666","inReplyTo":"253bf111-4e8a-54b7-6e39-2908270aa357@ramsayjones.plus.com","subject":"Re: cat-file timing window on Cygwin","fromName":"Adam Dinwoodie","fromEmail":"adam@dinwoodie.org","sentAt":"2017-08-26T15:43:06Z","receivedAt":"2017-08-26T15:43:15Z","isPatch":false,"sender":{"key":"adam@dinwoodie.org","avatar":"https://avatars.githubusercontent.com/u/1397507?v=4"},"body":"On Sat, Aug 26, 2017 at 01:57:18AM +0100, Ramsay Jones wrote:\n> On 25/08/17 16:08, Jeff King wrote:\n> > On Fri, Aug 25, 2017 at 12:25:29PM +0100, Adam Dinwoodie wrote:\n> > \n> >> As of v2.10.0-rc1-4-g321459439 (\"cat-file: support --textconv/--filters\n> >> in batch mode\"), t8010-cat-file-filters.sh has been failing on Cygwin.\n> >> Digging into this, the test looks to expose a timing window: it appears\n> >> that if `git cat-file --textconv --batch` receives input on stdin too\n> >> quickly, it fails to parse some of that input.\n> \n> This has not been failing since the above commit (when this\n> feature was first introduced), but (for me) when testing the\n> v2.13.0-rc0 build. Please refer to my original email:\n> \n> https://public-inbox.org/git/bf7db655-d90f-e711-afc8-6565b71373d2@ramsayjones.plus.com/\n> \n> ... where I was reasonably sure this was caused by a change in\n> the cygwin dll while upgrading from 2.7.0-1 -> 2.8.0-1.\n\nThe failure started at some point when I wasn't paying much attention to\nthe state of Cygwin or Git due to personal health stuff.  Now I'm\ncatching up with the backlog from that period, I bisected the Git builds\nto get to the commit referenced above, but it's entirely plausible the\nbehaviour change is in the Cygwin DLL and is merely exposed by the test\nthat commit added.\n\n> > Hmm. That commit doesn't seem to actually touch the stdin loop. That\n> > happens via strbuf_getline(), which should be pretty robust to timing.\n> > But here are a few random thoughts:\n> > \n> >   1. Do you build with HAVE_GETDELIM? Does toggling it have any effect?\n> \n> If Adam builds using the configure script, then yes, else no. ;-)\n> I never use configure, so I don't have HAVE_GETDELIM set.\n\nI do use the configure script and do run with HAVE_GETDELIM set.  As you\nmight expect given that Ramsay doesn't and they see the same behaviour,\ntoggling it makes no difference to whether t8010.8 passes or fails.\n\n> >   2. Does the problem happen only with --textconv?\n> > \n> >      If so, I wonder if spawning the child process is somehow draining\n> >      stdin. I don't see how the child would accidentally read from our\n> >      stdin; we replace its stdin with a pipe so it shouldn't get a\n> >      chance to see our descriptor at all.\n> \n> As I said in my previous email, \"the loop in batch_objects()\n> (builtin/cat-file.c lines #489-505) only reads one line from\n> stdin, then gets EOF on the stream.\"\n\nTesting now, removing --textconv does seem to make a difference.  With\nthe following diff, t8010 passes:\n\ndiff --git a/t/t8010-cat-file-filters.sh b/t/t8010-cat-file-filters.sh\nindex d8242e467..7c85bef9b 100755\n--- a/t/t8010-cat-file-filters.sh\n+++ b/t/t8010-cat-file-filters.sh\n@@ -54,9 +54,9 @@\n test_expect_success 'cat-file --textconv --batch works' '\n \tsha1=$(git rev-parse -q --verify HEAD:world.txt) &&\n \ttest_config diff.txt.textconv \"tr A-Za-z N-ZA-Mn-za-m <\" &&\n-\tprintf \"%s hello.txt\\n%s hello\\n\" $sha1 $sha1 |\n-\tgit cat-file --textconv --batch >actual &&\n-\tprintf \"%s blob 6\\nuryyb\\r\\n\\n%s blob 6\\nhello\\n\\n\" \\\n+\tprintf \"%s\\n%s\\n\" $sha1 $sha1 |\n+\tgit cat-file --batch >actual &&\n+\tprintf \"%s blob 6\\nhello\\n\\n%s blob 6\\nhello\\n\\n\" \\\n \t\t$sha1 $sha1 >expect &&\n \ttest_cmp expect actual\n '\n\n(I am mildly baffled by the necessary change in line endings in the\n`expect` file, but that's the output that was being produced, and I\nactually think the single CRLF line ending in the file that otherwise\nuses LF in the original test is the oddity here.)\n\nA similar test using my live version of Git also works as expected:\n\n    $ { echo $sha1 ; echo $sha1 ; } | git cat-file --batch\n    ce013625030ba8dba906f756967f9e9ca394464a blob 6\n    hello\n    \n    ce013625030ba8dba906f756967f9e9ca394464a blob 6\n    hello\n\n> >      If we accidentally called fflush(stdin) that would discard buffered\n> >      input and give the results you're seeing. We don't do that\n> >      directly, of course, but we do call fflush(NULL) in run-command\n> >      before spawning the child. That _should_ just flush output handles,\n> >      but it's possible that there's a cygwin bug, I guess.\n> \n> I suspect so, see previous email ...\n> \n> > I can't reproduce here on Linux. But does the patch below have any\n> > impact?\n> > \n> > diff --git a/run-command.c b/run-command.c\n> > index 98621faca8..064ebd1995 100644\n> > --- a/run-command.c\n> > +++ b/run-command.c\n> > @@ -641,7 +641,6 @@ int start_command(struct child_process *cmd)\n> >  \t}\n> >  \n> >  \ttrace_argv_printf(cmd->argv, \"trace: run_command:\");\n> > -\tfflush(NULL);\n> >  \n> >  #ifndef GIT_WINDOWS_NATIVE\n> >  {\n> \n> I suspect not, but I can give it a try ...\n> \n> ... oh, wow, that works! Ahem. (Hmm, so it's flushing stdin?!)\n> \n> Also, t5526-fetch-submodules.sh still works as well (see commit\n> 13af8cbd6a \"start_command: flush buffers in the WIN32 code path\n> as well\", 04-02-2011).\n\nLikewise, this gets t8010 passing for me.  I've run through the entire\ntest suite and there are no other new breakages, too.\n"},{"id":"327248","messageId":"20170826185337.hrcswjuunon54kgj@sigill.intra.peff.net","threadId":"46666","inReplyTo":"253bf111-4e8a-54b7-6e39-2908270aa357@ramsayjones.plus.com","subject":"Re: cat-file timing window on Cygwin","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-08-26T18:53:37Z","receivedAt":"2017-08-26T18:53:44Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Aug 26, 2017 at 01:57:18AM +0100, Ramsay Jones wrote:\n\n> > diff --git a/run-command.c b/run-command.c\n> > index 98621faca8..064ebd1995 100644\n> > --- a/run-command.c\n> > +++ b/run-command.c\n> > @@ -641,7 +641,6 @@ int start_command(struct child_process *cmd)\n> >  \t}\n> >  \n> >  \ttrace_argv_printf(cmd->argv, \"trace: run_command:\");\n> > -\tfflush(NULL);\n> >  \n> >  #ifndef GIT_WINDOWS_NATIVE\n> >  {\n> \n> I suspect not, but I can give it a try ...\n> \n> ... oh, wow, that works! Ahem. (Hmm, so it's flushing stdin?!)\n\nInteresting. I find it a little hard to believe there's so obvious a bug\nas \"fflush(NULL) flushes stdin\", but well...that's what it seems like.\n\nIf that's truly what it is, this is the minimal reproduction I came up\nwith:\n\n-- >8 --\n#include <stdio.h>\n\nint main(void)\n{\n\tchar buf[256];\n\twhile (fgets(buf, sizeof(buf), stdin)) {\n\t\tfprintf(stdout, \"got: %s\", buf);\n\t\tfflush(NULL);\n\t}\n\treturn 0;\n}\n-- 8< --\n\nIf this really is the bug, then doing something like \"seq 10 | ./a.out\"\nwould drop some of the input lines.\n\n-Peff\n"},{"id":"327250","messageId":"20170826211104.GC10378@dinwoodie.org","threadId":"46666","inReplyTo":"20170826185337.hrcswjuunon54kgj@sigill.intra.peff.net","subject":"Re: cat-file timing window on Cygwin","fromName":"Adam Dinwoodie","fromEmail":"adam@dinwoodie.org","sentAt":"2017-08-26T21:11:04Z","receivedAt":"2017-08-26T21:11:13Z","isPatch":false,"sender":{"key":"adam@dinwoodie.org","avatar":"https://avatars.githubusercontent.com/u/1397507?v=4"},"body":"On Sat, Aug 26, 2017 at 11:53:37AM -0700, Jeff King wrote:\n> On Sat, Aug 26, 2017 at 01:57:18AM +0100, Ramsay Jones wrote:\n> \n> > > diff --git a/run-command.c b/run-command.c\n> > > index 98621faca8..064ebd1995 100644\n> > > --- a/run-command.c\n> > > +++ b/run-command.c\n> > > @@ -641,7 +641,6 @@ int start_command(struct child_process *cmd)\n> > >  \t}\n> > >  \n> > >  \ttrace_argv_printf(cmd->argv, \"trace: run_command:\");\n> > > -\tfflush(NULL);\n> > >  \n> > >  #ifndef GIT_WINDOWS_NATIVE\n> > >  {\n> > \n> > I suspect not, but I can give it a try ...\n> > \n> > ... oh, wow, that works! Ahem. (Hmm, so it's flushing stdin?!)\n> \n> Interesting. I find it a little hard to believe there's so obvious a bug\n> as \"fflush(NULL) flushes stdin\", but well...that's what it seems like.\n> \n> If that's truly what it is, this is the minimal reproduction I came up\n> with:\n> \n> -- >8 --\n> #include <stdio.h>\n> \n> int main(void)\n> {\n> \tchar buf[256];\n> \twhile (fgets(buf, sizeof(buf), stdin)) {\n> \t\tfprintf(stdout, \"got: %s\", buf);\n> \t\tfflush(NULL);\n> \t}\n> \treturn 0;\n> }\n> -- 8< --\n> \n> If this really is the bug, then doing something like \"seq 10 | ./a.out\"\n> would drop some of the input lines.\n\n...yep.  It does.  Specifically, I consistently only get the firsts\nline:\n\n    $ seq 10 | ./a.exe\n    got: 1\n    \n    $ \n\nIf I introduce a delay between the lines of stdin (which I tested by\njust typing stdin from the keyboard), it works as expected.\n\nLooks like this one will need to go to the Cygwin mailing list; I'll\ntake it there shortly.  Thank you all for your help getting this far!\n"},{"id":"327252","messageId":"c5f3fe2d-6d76-7c08-2a50-fb10c66adc48@ramsayjones.plus.com","threadId":"46666","inReplyTo":"20170826211104.GC10378@dinwoodie.org","subject":"Re: cat-file timing window on Cygwin","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2017-08-27T02:06:31Z","receivedAt":"2017-08-27T02:06:39Z","isPatch":false,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 26/08/17 22:11, Adam Dinwoodie wrote:\n> On Sat, Aug 26, 2017 at 11:53:37AM -0700, Jeff King wrote:\n>> On Sat, Aug 26, 2017 at 01:57:18AM +0100, Ramsay Jones wrote:\n>>\n>>>> diff --git a/run-command.c b/run-command.c\n>>>> index 98621faca8..064ebd1995 100644\n>>>> --- a/run-command.c\n>>>> +++ b/run-command.c\n>>>> @@ -641,7 +641,6 @@ int start_command(struct child_process *cmd)\n>>>>  \t}\n>>>>  \n>>>>  \ttrace_argv_printf(cmd->argv, \"trace: run_command:\");\n>>>> -\tfflush(NULL);\n>>>>  \n>>>>  #ifndef GIT_WINDOWS_NATIVE\n>>>>  {\n>>>\n>>> I suspect not, but I can give it a try ...\n>>>\n>>> ... oh, wow, that works! Ahem. (Hmm, so it's flushing stdin?!)\n>>\n>> Interesting. I find it a little hard to believe there's so obvious a bug\n>> as \"fflush(NULL) flushes stdin\", but well...that's what it seems like.\n>>\n>> If that's truly what it is, this is the minimal reproduction I came up\n>> with:\n>>\n>> -- >8 --\n>> #include <stdio.h>\n>>\n>> int main(void)\n>> {\n>> \tchar buf[256];\n>> \twhile (fgets(buf, sizeof(buf), stdin)) {\n>> \t\tfprintf(stdout, \"got: %s\", buf);\n>> \t\tfflush(NULL);\n>> \t}\n>> \treturn 0;\n>> }\n>> -- 8< --\n>>\n>> If this really is the bug, then doing something like \"seq 10 | ./a.out\"\n>> would drop some of the input lines.\n> \n> ...yep.  It does.  Specifically, I consistently only get the firsts\n> line:\n> \n>     $ seq 10 | ./a.exe\n>     got: 1\n>     \n>     $ \n> \n> If I introduce a delay between the lines of stdin (which I tested by\n> just typing stdin from the keyboard), it works as expected.\n> \n> Looks like this one will need to go to the Cygwin mailing list; I'll\n> take it there shortly.  Thank you all for your help getting this far!\n\nThis is apparently fixed in cygwin v2.8.3 [see commit 78ade082fe,\n('Revert \"errno: Stop using _impure_ptr->_errno completely\"', 19-07-2017),\ncommit 9cc89b0438 (\"cygwin: Use errno instead of _impure_ptr->_errno\", 19-07-2017), commit a674199fc9 (\"cygwin: Bump DLL version to 2.8.3\",\n19-07-2017) and commit d2ae2f00b8 (\"cygwin: add fflush fix to release\nnotes\", 19-07-2017)].\n\nI haven't had a chance to try v2.8.3 yet (it's 3am and I'm about to\ngo get some sleep).\n\nATB,\nRamsay Jones\n\n\n"},{"id":"327257","messageId":"20170827113358.GF10378@dinwoodie.org","threadId":"46666","inReplyTo":"c5f3fe2d-6d76-7c08-2a50-fb10c66adc48@ramsayjones.plus.com","subject":"Re: cat-file timing window on Cygwin","fromName":"Adam Dinwoodie","fromEmail":"adam@dinwoodie.org","sentAt":"2017-08-27T11:33:58Z","receivedAt":"2017-08-27T11:34:08Z","isPatch":false,"sender":{"key":"adam@dinwoodie.org","avatar":"https://avatars.githubusercontent.com/u/1397507?v=4"},"body":"On Sun, Aug 27, 2017 at 03:06:31AM +0100, Ramsay Jones wrote:\n> On 26/08/17 22:11, Adam Dinwoodie wrote:\n> > On Sat, Aug 26, 2017 at 11:53:37AM -0700, Jeff King wrote:\n> >> Interesting. I find it a little hard to believe there's so obvious a bug\n> >> as \"fflush(NULL) flushes stdin\", but well...that's what it seems like.\n> >>\n> >> If that's truly what it is, this is the minimal reproduction I came up\n> >> with:\n> >>\n> >> -- >8 --\n> >> #include <stdio.h>\n> >>\n> >> int main(void)\n> >> {\n> >> \tchar buf[256];\n> >> \twhile (fgets(buf, sizeof(buf), stdin)) {\n> >> \t\tfprintf(stdout, \"got: %s\", buf);\n> >> \t\tfflush(NULL);\n> >> \t}\n> >> \treturn 0;\n> >> }\n> >> -- 8< --\n> >>\n> >> If this really is the bug, then doing something like \"seq 10 | ./a.out\"\n> >> would drop some of the input lines.\n> > \n> > ...yep.  It does.  Specifically, I consistently only get the firsts\n> > line:\n> > \n> >     $ seq 10 | ./a.exe\n> >     got: 1\n> >     \n> >     $ \n> > \n> > If I introduce a delay between the lines of stdin (which I tested by\n> > just typing stdin from the keyboard), it works as expected.\n> > \n> > Looks like this one will need to go to the Cygwin mailing list; I'll\n> > take it there shortly.  Thank you all for your help getting this far!\n> \n> This is apparently fixed in cygwin v2.8.3 [see commit 78ade082fe,\n> ('Revert \"errno: Stop using _impure_ptr->_errno completely\"', 19-07-2017),\n> commit 9cc89b0438 (\"cygwin: Use errno instead of _impure_ptr->_errno\", 19-07-2017), commit a674199fc9 (\"cygwin: Bump DLL version to 2.8.3\",\n> 19-07-2017) and commit d2ae2f00b8 (\"cygwin: add fflush fix to release\n> notes\", 19-07-2017)].\n> \n> I haven't had a chance to try v2.8.3 yet (it's 3am and I'm about to\n> go get some sleep).\n\nCygwin 2.8.3 hasn't been released yet, but I've just tested the latest\ndevelopment snapshot with Jeff's simple test case, and it works as\nexpected, so I'm going to assume the Git test will start passing once\nthat version of the Cygwin DLL is released too.\n"},{"id":"327260","messageId":"0822303f-63b1-13e2-be43-8e2e4dab478e@ramsayjones.plus.com","threadId":"46666","inReplyTo":"20170827113358.GF10378@dinwoodie.org","subject":"Re: cat-file timing window on Cygwin","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2017-08-27T15:47:45Z","receivedAt":"2017-08-27T15:47:53Z","isPatch":false,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 27/08/17 12:33, Adam Dinwoodie wrote:\n> On Sun, Aug 27, 2017 at 03:06:31AM +0100, Ramsay Jones wrote:\n>> On 26/08/17 22:11, Adam Dinwoodie wrote:\n>>> On Sat, Aug 26, 2017 at 11:53:37AM -0700, Jeff King wrote:\n>>>> Interesting. I find it a little hard to believe there's so obvious a bug\n>>>> as \"fflush(NULL) flushes stdin\", but well...that's what it seems like.\n>>>>\n>>>> If that's truly what it is, this is the minimal reproduction I came up\n>>>> with:\n>>>>\n>>>> -- >8 --\n>>>> #include <stdio.h>\n>>>>\n>>>> int main(void)\n>>>> {\n>>>> \tchar buf[256];\n>>>> \twhile (fgets(buf, sizeof(buf), stdin)) {\n>>>> \t\tfprintf(stdout, \"got: %s\", buf);\n>>>> \t\tfflush(NULL);\n>>>> \t}\n>>>> \treturn 0;\n>>>> }\n>>>> -- 8< --\n>>>>\n>>>> If this really is the bug, then doing something like \"seq 10 | ./a.out\"\n>>>> would drop some of the input lines.\n>>>\n>>> ...yep.  It does.  Specifically, I consistently only get the firsts\n>>> line:\n>>>\n>>>     $ seq 10 | ./a.exe\n>>>     got: 1\n>>>     \n>>>     $ \n>>>\n>>> If I introduce a delay between the lines of stdin (which I tested by\n>>> just typing stdin from the keyboard), it works as expected.\n>>>\n>>> Looks like this one will need to go to the Cygwin mailing list; I'll\n>>> take it there shortly.  Thank you all for your help getting this far!\n>>\n>> This is apparently fixed in cygwin v2.8.3 [see commit 78ade082fe,\n>> ('Revert \"errno: Stop using _impure_ptr->_errno completely\"', 19-07-2017),\n>> commit 9cc89b0438 (\"cygwin: Use errno instead of _impure_ptr->_errno\", 19-07-2017), commit a674199fc9 (\"cygwin: Bump DLL version to 2.8.3\",\n>> 19-07-2017) and commit d2ae2f00b8 (\"cygwin: add fflush fix to release\n>> notes\", 19-07-2017)].\n>>\n>> I haven't had a chance to try v2.8.3 yet (it's 3am and I'm about to\n>> go get some sleep).\n> \n> Cygwin 2.8.3 hasn't been released yet, \n\nHeh, yes, I found that out myself this afternoon. ;-)\n\n>                                         but I've just tested the latest\n> development snapshot with Jeff's simple test case, and it works as\n> expected, so I'm going to assume the Git test will start passing once\n> that version of the Cygwin DLL is released too.\n\nHmm, I'm not keen on installing \"snapshot\"(s), so I think I will\nwait for the release to test it. (However, as a matter of interest,\nhow would I obtain/install/test this snapshot release - is it a\n'low-risk' exercise?)\n\nATB,\nRamsay Jones\n\n\n"},{"id":"327264","messageId":"C7EA6E1174C3400B98A9F64FF0E8DEC4@blackfat","threadId":"46666","inReplyTo":"0822303f-63b1-13e2-be43-8e2e4dab478e@ramsayjones.plus.com","subject":"RE: cat-file timing window on Cygwin","fromName":"Jason Pyeron","fromEmail":"jpyeron@pdinc.us","sentAt":"2017-08-27T18:53:27Z","receivedAt":"2017-08-27T18:53:44Z","isPatch":false,"sender":{"key":"jpyeron@pdinc.us","avatar":"https://gravatar.com/avatar/c2e53452caa53d940768a1ffc9cf76196d851b9b534b7a39cd39852a70a0508f?d=mp&s=160"},"body":"> -----Original Message-----\n> From: Ramsay Jones\n> Sent: Sunday, August 27, 2017 11:48 AM\n> To: Adam Dinwoodie\n> Cc: Jeff King; git@vger.kernel.org; Johannes Schindelin\n> Subject: Re: cat-file timing window on Cygwin\n> \n> \n> \n> On 27/08/17 12:33, Adam Dinwoodie wrote:\n> > On Sun, Aug 27, 2017 at 03:06:31AM +0100, Ramsay Jones wrote:\n> >> On 26/08/17 22:11, Adam Dinwoodie wrote:\n> >>> On Sat, Aug 26, 2017 at 11:53:37AM -0700, Jeff King wrote:\n> >>>> Interesting. I find it a little hard to believe there's \n> so obvious \n> >>>> a bug as \"fflush(NULL) flushes stdin\", but well...that's \n> what it seems like.\n> >>>>\n> >>>> If that's truly what it is, this is the minimal \n> reproduction I came \n> >>>> up\n> >>>> with:\n> >>>>\n> >>>> -- >8 --\n> >>>> #include <stdio.h>\n> >>>>\n> >>>> int main(void)\n> >>>> {\n> >>>> \tchar buf[256];\n> >>>> \twhile (fgets(buf, sizeof(buf), stdin)) {\n> >>>> \t\tfprintf(stdout, \"got: %s\", buf);\n> >>>> \t\tfflush(NULL);\n> >>>> \t}\n> >>>> \treturn 0;\n> >>>> }\n> >>>> -- 8< --\n> >>>>\n> >>>> If this really is the bug, then doing something like \n> \"seq 10 | ./a.out\"\n\nTests good on latest snapshot. Fails on \n\nCygwin DLL version info:\n        DLL version: 2.8.2\n        DLL epoch: 19\n        DLL old termios: 5\n        DLL malloc env: 28\n        Cygwin conv: 181\n        API major: 0\n        API minor: 313\n        Shared data: 5\n        DLL identifier: cygwin1\n        Mount registry: 3\n        Cygwin registry name: Cygwin\n        Installations name: Installations\n        Cygdrive default prefix:\n        Build date:\n        Shared id: cygwin1S5\n\n\n> >>>> would drop some of the input lines.\n> >>>\n> >>> ...yep.  It does.  Specifically, I consistently only get \n> the firsts\n> >>> line:\n> >>>\n> >>>     $ seq 10 | ./a.exe\n> >>>     got: 1\n> >>>     \n> >>>     $\n> >>>\n> >>> If I introduce a delay between the lines of stdin (which \n> I tested by \n> >>> just typing stdin from the keyboard), it works as expected.\n> >>>\n> >>> Looks like this one will need to go to the Cygwin mailing \n> list; I'll \n> >>> take it there shortly.  Thank you all for your help \n> getting this far!\n> >>\n> >> This is apparently fixed in cygwin v2.8.3 [see commit 78ade082fe, \n> >> ('Revert \"errno: Stop using _impure_ptr->_errno completely\"', \n> >> 19-07-2017), commit 9cc89b0438 (\"cygwin: Use errno instead of \n> >> _impure_ptr->_errno\", 19-07-2017), commit a674199fc9 \n> (\"cygwin: Bump \n> >> DLL version to 2.8.3\",\n> >> 19-07-2017) and commit d2ae2f00b8 (\"cygwin: add fflush fix \n> to release \n> >> notes\", 19-07-2017)].\n> >>\n> >> I haven't had a chance to try v2.8.3 yet (it's 3am and I'm \n> about to \n> >> go get some sleep).\n> > \n> > Cygwin 2.8.3 hasn't been released yet,\n> \n> Heh, yes, I found that out myself this afternoon. ;-)\n> \n> >                                         but I've just tested the \n> > latest development snapshot with Jeff's simple test case, \n> and it works \n> > as expected, so I'm going to assume the Git test will start passing \n> > once that version of the Cygwin DLL is released too.\n> \n> Hmm, I'm not keen on installing \"snapshot\"(s), so I think I \n> will wait for the release to test it. (However, as a matter \n> of interest, how would I obtain/install/test this snapshot \n> release - is it a 'low-risk' exercise?)\n\nUsing https://cygwin.com/snapshots/x86_64/cygwin-20170823.tar.xz\n\nD:\\inst\\cygwin\\cygwin-20170823>usr\\bin\\bash.exe\nbash-4.4$ seq 10 | ./a.exe\ngot: 1\ngot: 2\ngot: 3\ngot: 4\ngot: 5\ngot: 6\ngot: 7\ngot: 8\ngot: 9\ngot: 10\nbash-4.4$ cat cygcheck.out\n\nCygwin Configuration Diagnostics\nCurrent System Time: Sun Aug 27 14:35:25 2017\n\nWindows 10 Professional Ver 10.0 Build 14393\n\n    Cygwin DLL version info:\n        DLL version: 2.9.0\n        DLL epoch: 19\n        DLL old termios: 5\n        DLL malloc env: 28\n        Cygwin conv: 181\n        API major: 0\n        API minor: 317\n        Shared data: 5\n        DLL identifier: cygwin1\n        Mount registry: 3\n        Cygwin registry name: Cygwin\n        Installations name: Installations\n        Cygdrive default prefix:\n        Build date:\n        Snapshot date: 20170823-15:44:28\n        Shared id: cygwin1S5\n\nbash-4.4$\n\nv/r,\n\nJason Pyeron\n\n--\n-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-\n-                                                               -\n- Jason Pyeron                      PD Inc. http://www.pdinc.us -\n- Principal Consultant              10 West 24th Street #100    -\n- +1 (443) 269-1555 x333            Baltimore, Maryland 21218   -\n-                                                               -\n-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=- \n\n"},{"id":"327794","messageId":"8f7e5a78-a90b-68b8-712f-25367819f46c@ramsayjones.plus.com","threadId":"46666","inReplyTo":"0822303f-63b1-13e2-be43-8e2e4dab478e@ramsayjones.plus.com","subject":"Re: Re: cat-file timing window on Cygwin","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2017-09-08T20:02:24Z","receivedAt":"2017-09-08T20:02:55Z","isPatch":false,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 27/08/17 16:47, Ramsay Jones wrote:\n> On 27/08/17 12:33, Adam Dinwoodie wrote:\n[snip]\n\n>> Cygwin 2.8.3 hasn't been released yet, \n> \n> Heh, yes, I found that out myself this afternoon. ;-)\n> \n>>                                         but I've just tested the latest\n>> development snapshot with Jeff's simple test case, and it works as\n>> expected, so I'm going to assume the Git test will start passing once\n>> that version of the Cygwin DLL is released too.\n\nI noticed that cygwin 2.9.0 has been released, so I updated and\nran test t8010-cat-file-filters.sh which, as expected, now passes.\n\nSince the above failure was caused, in part, by an errant errno\nvalue, I decided to try reverting the fix for t0301-credential-cache.sh.\n(ie. commit 1f180e5eb9, \"credential-cache: interpret an ECONNRESET\nas an EOF\", 27-07-2017). However, that re-introduces the failure! ;-)\n\n[I haven't done a complete test-suite run yet, but I don't expect\nto have any failures - famous last words!]\n\nATB,\nRamsay Jones\n\n"}]}