{"thread":{"id":"40702","subject":"git.git as of tonight","startedAt":"2015-11-02T02:58:51Z","lastAt":"2015-11-05T06:58:01Z","messageCount":18,"participants":["Junio C Hamano","Johannes Sixt","Stefan Beller","Torsten Bögershausen","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"272716","messageId":"xmqqpoztf6ok.fsf@gitster.mtv.corp.google.com","threadId":"40702","inReplyTo":null,"subject":"git.git as of tonight","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-11-02T02:58:51Z","receivedAt":"2015-11-02T02:58:51Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"I've merged a handful of topics to 'next', and hoping that many of\nthem can be merged to 'master' before I'll go offline for a few\nweeks starting on the week #7 of the cycle (starting Nov 9th).\n\nAs I hate sending out the whole text back to back (the last one was\nissued on the last Friday), here is an abbreviated update for the\n\"What's cooking\" report, highlighting those topics that I want to\nsee in 'master' by the end of the week #6 (Nov 8th).\n\nThanks.\n\n\n* kn/for-each-branch (2015-10-30) 1 commit\n  (merged to 'next' on 2015-11-01 at 4249dc9)\n + ref-filter: fallback on alphabetical comparison\n\n Using the timestamp based criteria in \"git branch --sort\" did not\n tiebreak branches that point at commits with the same timestamp (or\n the same commit), making the resulting output unstable.\n\n Will merge to 'master'.\n\n\n* jc/mailinfo-lib (2015-11-01) 1 commit\n  (merged to 'next' on 2015-11-01 at 3ecaa28)\n + mailinfo: fix passing wrong address to git_mailinfo_config\n\n Hotfix for a topic already in 'master'.\n\n Will merge to 'master'.\n\n\n* sb/submodule-parallel-fetch (2015-10-21) 14 commits\n  (merged to 'next' on 2015-10-23 at 8f04bbd)\n + run-command: fix missing output from late callbacks\n + test-run-command: increase test coverage\n + test-run-command: test for gracefully aborting\n + run-command: initialize the shutdown flag\n + run-command: clear leftover state from child_process structure\n + run-command: fix early shutdown\n  (merged to 'next' on 2015-10-15 at df63590)\n + submodules: allow parallel fetching, add tests and documentation\n + fetch_populated_submodules: use new parallel job processing\n + run-command: add an asynchronous parallel child processor\n + sigchain: add command to pop all common signals\n + strbuf: add strbuf_read_once to read without blocking\n + xread_nonblock: add functionality to read from fds without blocking\n + xread: poll on non blocking fds\n + submodule.c: write \"Fetching submodule <foo>\" to stderr\n (this branch is used by rs/daemon-leak-fix and sb/submodule-parallel-update.)\n\n Add a framework to spawn a group of processes in parallel, and use\n it to run \"git fetch --recurse-submodules\" in parallel.\n\n Will merge to 'master'.\n\n\n* rs/daemon-leak-fix (2015-10-31) 3 commits\n  (merged to 'next' on 2015-11-01 at 9b6c8f9)\n + daemon: plug memory leak\n + run-command: export child_process_clear()\n + run-command: name the cleanup function child_process_clear()\n (this branch is used by sb/submodule-parallel-update; uses sb/submodule-parallel-fetch.)\n\n \"git daemon\" uses \"run_command()\" without \"finish_command()\", so it\n needs to release resources itself, which it forgot to do.\n\n Will merge to 'master'.\n\n\n* rs/wt-status-detached-branch-fix (2015-11-01) 5 commits\n  (merged to 'next' on 2015-11-01 at cb23615)\n + wt-status: use skip_prefix() to get rid of magic string length constants\n + wt-status: don't skip a magical number of characters blindly\n + wt-status: avoid building bogus branch name with detached HEAD\n + wt-status: exit early using goto in wt_shortstatus_print_tracking()\n + t7060: add test for status --branch on a detached HEAD\n\n \"git status --branch --short\" accessed beyond the constant string\n \"HEAD\", which has been corrected.\n\n Will merge to 'master'.\n\n\n* da/difftool (2015-10-29) 1 commit\n  (merged to 'next' on 2015-11-01 at 4e5ab33)\n + difftool: ignore symbolic links in use_wt_file\n\n The code to prepare the working tree side of temporary directory\n for the \"dir-diff\" feature forgot that symbolic links need not be\n copied (or symlinked) to the temporary area, as the code already\n special cases and overwrites them.  Besides, it was wrong to try\n computing the object name of the target of symbolic link, which may\n not even exist or may be a directory.\n\n Will merge to 'master'.\n\n\n* jk/initialization-fix-to-add-submodule-odb (2015-10-28) 1 commit\n  (merged to 'next' on 2015-11-01 at da94b97)\n + add_submodule_odb: initialize alt_odb list earlier\n\n We peek objects from submodule's object store by linking it to the\n list of alternate object databases, but the code to do so forgot to\n correctly initialize the list.\n\n Will merge to 'master'.\n"},{"id":"272757","messageId":"5637D266.4060904@kdbg.org","threadId":"40702","inReplyTo":"xmqqpoztf6ok.fsf@gitster.mtv.corp.google.com","subject":"Re: git.git as of tonight","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2015-11-02T21:15:18Z","receivedAt":"2015-11-02T21:15:18Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 02.11.2015 um 03:58 schrieb Junio C Hamano:\n> * sb/submodule-parallel-fetch (2015-10-21) 14 commits\n>    (merged to 'next' on 2015-10-23 at 8f04bbd)\n>   + run-command: fix missing output from late callbacks\n>   + test-run-command: increase test coverage\n>   + test-run-command: test for gracefully aborting\n>   + run-command: initialize the shutdown flag\n>   + run-command: clear leftover state from child_process structure\n>   + run-command: fix early shutdown\n>    (merged to 'next' on 2015-10-15 at df63590)\n>   + submodules: allow parallel fetching, add tests and documentation\n>   + fetch_populated_submodules: use new parallel job processing\n>   + run-command: add an asynchronous parallel child processor\n>   + sigchain: add command to pop all common signals\n>   + strbuf: add strbuf_read_once to read without blocking\n>   + xread_nonblock: add functionality to read from fds without blocking\n>   + xread: poll on non blocking fds\n>   + submodule.c: write \"Fetching submodule <foo>\" to stderr\n>   (this branch is used by rs/daemon-leak-fix and sb/submodule-parallel-update.)\n> \n>   Add a framework to spawn a group of processes in parallel, and use\n>   it to run \"git fetch --recurse-submodules\" in parallel.\n> \n>   Will merge to 'master'.\n\nPlease don't, yet. This series does not build on Windows:\n\nrun-command.c: In function 'set_nonblocking':\nrun-command.c:1011: error: 'F_GETFL' undeclared (first use in this function)\nrun-command.c:1011: error: (Each undeclared identifier is reported only once\nrun-command.c:1011: error: for each function it appears in.)\nrun-command.c:1015: error: 'F_SETFL' undeclared (first use in this function)\nrun-command.c:1015: error: 'O_NONBLOCK' undeclared (first use in this function)\nmake: *** [run-command.o] Error 1\n\nI have to investigate whether we can have some sort of Posixy\nnon-blocking IO on Windows or whether we have to opt-out from this\nparallel-process facility. Any help from Windows experts would be\nappreciated.\n\n-- Hannes\n"},{"id":"272766","messageId":"xmqqlhagdp2l.fsf@gitster.mtv.corp.google.com","threadId":"40702","inReplyTo":"5637D266.4060904@kdbg.org","subject":"Re: git.git as of tonight","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-11-02T22:16:50Z","receivedAt":"2015-11-02T22:16:50Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt <j6t@kdbg.org> writes:\n\n> Am 02.11.2015 um 03:58 schrieb Junio C Hamano:\n>> * sb/submodule-parallel-fetch (2015-10-21) 14 commits\n>>    (merged to 'next' on 2015-10-23 at 8f04bbd)\n>>   + run-command: fix missing output from late callbacks\n>>...\n>>   + submodule.c: write \"Fetching submodule <foo>\" to stderr\n>>   (this branch is used by rs/daemon-leak-fix and sb/submodule-parallel-update.)\n>> \n>>   Add a framework to spawn a group of processes in parallel, and use\n>>   it to run \"git fetch --recurse-submodules\" in parallel.\n>> \n>>   Will merge to 'master'.\n>\n> Please don't, yet. This series does not build on Windows:\n\nThe only reason the series is listed here is because the cycle is\nstill young and I was hoping that any fallout will be addressed by\nthe time we tag -rc0; if the extent of required fixups is too great,\nthat obviously would not work well.\n\nI'll try to see if I can untangle rs/daemon-leak-fix topic so that\nit does not depend on this thing and have it graduate separately.\n\nThanks for stopping me.\n"},{"id":"272772","messageId":"CAGZ79kbx7UJOOkhiXiUJfUS25gMGx5BUq=Qxa9205H0Z78ijbQ@mail.gmail.com","threadId":"40702","inReplyTo":"5637D266.4060904@kdbg.org","subject":"Re: git.git as of tonight","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-11-02T23:06:44Z","receivedAt":"2015-11-02T23:06:44Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Mon, Nov 2, 2015 at 1:15 PM, Johannes Sixt <j6t@kdbg.org> wrote:\n> Am 02.11.2015 um 03:58 schrieb Junio C Hamano:\n>> * sb/submodule-parallel-fetch (2015-10-21) 14 commits\n>>    (merged to 'next' on 2015-10-23 at 8f04bbd)\n>>   + run-command: fix missing output from late callbacks\n>>   + test-run-command: increase test coverage\n>>   + test-run-command: test for gracefully aborting\n>>   + run-command: initialize the shutdown flag\n>>   + run-command: clear leftover state from child_process structure\n>>   + run-command: fix early shutdown\n>>    (merged to 'next' on 2015-10-15 at df63590)\n>>   + submodules: allow parallel fetching, add tests and documentation\n>>   + fetch_populated_submodules: use new parallel job processing\n>>   + run-command: add an asynchronous parallel child processor\n>>   + sigchain: add command to pop all common signals\n>>   + strbuf: add strbuf_read_once to read without blocking\n>>   + xread_nonblock: add functionality to read from fds without blocking\n>>   + xread: poll on non blocking fds\n>>   + submodule.c: write \"Fetching submodule <foo>\" to stderr\n>>   (this branch is used by rs/daemon-leak-fix and sb/submodule-parallel-update.)\n>>\n>>   Add a framework to spawn a group of processes in parallel, and use\n>>   it to run \"git fetch --recurse-submodules\" in parallel.\n>>\n>>   Will merge to 'master'.\n>\n> Please don't, yet. This series does not build on Windows:\n>\n> run-command.c: In function 'set_nonblocking':\n> run-command.c:1011: error: 'F_GETFL' undeclared (first use in this function)\n> run-command.c:1011: error: (Each undeclared identifier is reported only once\n> run-command.c:1011: error: for each function it appears in.)\n> run-command.c:1015: error: 'F_SETFL' undeclared (first use in this function)\n> run-command.c:1015: error: 'O_NONBLOCK' undeclared (first use in this function)\n> make: *** [run-command.o] Error 1\n\nGoing by a quick search http://stackoverflow.com/a/22756664\nI'd hope we only need to modify the set_nonblocking function using #ifdefs ?\n\n>\n> I have to investigate whether we can have some sort of Posixy\n> non-blocking IO on Windows or whether we have to opt-out from this\n> parallel-process facility. Any help from Windows experts would be\n> appreciated.\n>\n> -- Hannes\n>\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n"},{"id":"272786","messageId":"56385587.1010601@kdbg.org","threadId":"40702","inReplyTo":"CAGZ79kbx7UJOOkhiXiUJfUS25gMGx5BUq=Qxa9205H0Z78ijbQ@mail.gmail.com","subject":"Re: git.git as of tonight","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2015-11-03T06:34:47Z","receivedAt":"2015-11-03T06:34:47Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 03.11.2015 um 00:06 schrieb Stefan Beller:\n> On Mon, Nov 2, 2015 at 1:15 PM, Johannes Sixt <j6t@kdbg.org> wrote:\n>> run-command.c: In function 'set_nonblocking':\n>> run-command.c:1011: error: 'F_GETFL' undeclared (first use in this function)\n>> run-command.c:1011: error: (Each undeclared identifier is reported only once\n>> run-command.c:1011: error: for each function it appears in.)\n>> run-command.c:1015: error: 'F_SETFL' undeclared (first use in this function)\n>> run-command.c:1015: error: 'O_NONBLOCK' undeclared (first use in this function)\n>> make: *** [run-command.o] Error 1\n>\n> Going by a quick search http://stackoverflow.com/a/22756664\n> I'd hope we only need to modify the set_nonblocking function using #ifdefs ?\n\nUnfortunately, the solutions outlined in that post do not work for us. \nOn Windows, the FIONBIO option is only available for sockets. \n\"Overlapped IO\" can be used only when the file descriptor is set in this \nmode right from the beginning (by open/CreateFile), and if it were so, \nit would have to be used in a totally different way than in Posix code.\n\nMy findings so far are negative. The only short-term and mid-term \nsolution I see so far is to opt-out from the framework during build-time.\n\n-- Hannes\n"},{"id":"272837","messageId":"xmqq37wndndp.fsf@gitster.mtv.corp.google.com","threadId":"40702","inReplyTo":"56385587.1010601@kdbg.org","subject":"Re: git.git as of tonight","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-11-03T17:05:38Z","receivedAt":"2015-11-03T17:05:38Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt <j6t@kdbg.org> writes:\n\n> My findings so far are negative. The only short-term and mid-term\n> solution I see so far is to opt-out from the framework during\n> build-time.\n\nNow, from where I sit, it seems that the way forward would be\n\n 1. Make this an optional feature so that platforms can compile it\n    out, if it is not already done.  My preference, even if we go\n    that route, would be to see if we can find a way to preserve the\n    overall code structure (e.g. instead of spawning multiple\n    workers, which is why the code needs NONBLOCK to avoid getting\n    stuck on reading from one while others are working, perhaps we\n    can spawn only one and not do a nonblock read?).\n\n 2. After that is done, the feature could graduate to 'master'.  As\n    this is a bigger framework change than others, however, we do\n    not necessarily want to rush it.  On the other hand, because\n    this only affects submodules, which means it has fewer users and\n    testers that would give us feedback while it is on 'next', we\n    may want to push it to 'master' sooner to give it a larger\n    exposure.  I dunno, and I do not want to decide this myself the\n    week before I'll go offline for a few weeks (i.e. today).\n\n 3. Then we would enlist help from folks who are more familiar with\n    Windows platform (like you) to see how the \"run parallel workers\n    and collect from them\" can be (re)done with a nice level of\n    abstraction.  I am hoping that we can continue the tradition of\n    the evolution of run-command.c API (I am specifically impressed\n    by what you did for \"async\" that allows the callers not to worry\n    about threads and processes) aroundt this area.  That is\n    obviously a mid- to longer term goal.\n\nThanks for working together well, you two.\n"},{"id":"272841","messageId":"CAGZ79kbAj-tZS3X1EwFw_gGh1=YU8M0OmEX91W8f++fqs6TiBA@mail.gmail.com","threadId":"40702","inReplyTo":"xmqq37wndndp.fsf@gitster.mtv.corp.google.com","subject":"Re: git.git as of tonight","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-11-03T18:18:40Z","receivedAt":"2015-11-03T18:18:40Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Nov 3, 2015 at 9:05 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Johannes Sixt <j6t@kdbg.org> writes:\n>\n>> My findings so far are negative. The only short-term and mid-term\n>> solution I see so far is to opt-out from the framework during\n>> build-time.\n\nSo I started reading up on that[1].\nAs far as I understand, we don't need to mark a file descriptor\nto be non blocking, but rather we could use ReadFileEx[2] with\na flag set for \"overlapped\" operation.\n\nSo that said, we can make set_nonblocking a noop and\nprovide another implementation for strbuf_read_once\ndepending on NO_PTHREADS being set.\nMaybe not even strbuf_read_once, but rather the underlying\nxread_nonblock ?\n\n\n\n[1] http://tinyclouds.org/iocp-links.html\n[2] https://msdn.microsoft.com/en-us/library/aa365468(v=VS.85).aspx\n\n>\n> Now, from where I sit, it seems that the way forward would be\n>\n>  1. Make this an optional feature so that platforms can compile it\n>     out, if it is not already done.  My preference, even if we go\n>     that route, would be to see if we can find a way to preserve the\n>     overall code structure (e.g. instead of spawning multiple\n>     workers, which is why the code needs NONBLOCK to avoid getting\n>     stuck on reading from one while others are working, perhaps we\n>     can spawn only one and not do a nonblock read?).\n\nYeah that would be my understanding as well. If we don't come up with\na good solution for parallelism in Windows now, we'd need to make it at\nleast working in the jobs=1 case as well as it worked before.\n\n>\n>  2. After that is done, the feature could graduate to 'master'.  As\n>     this is a bigger framework change than others, however, we do\n>     not necessarily want to rush it.  On the other hand, because\n>     this only affects submodules, which means it has fewer users and\n>     testers that would give us feedback while it is on 'next', we\n>     may want to push it to 'master' sooner to give it a larger\n>     exposure.  I dunno, and I do not want to decide this myself the\n>     week before I'll go offline for a few weeks (i.e. today).\n\nYeah I guess cooking this well done has its benefits.\n\n>\n>  3. Then we would enlist help from folks who are more familiar with\n>     Windows platform (like you) to see how the \"run parallel workers\n>     and collect from them\" can be (re)done with a nice level of\n>     abstraction.  I am hoping that we can continue the tradition of\n>     the evolution of run-command.c API (I am specifically impressed\n>     by what you did for \"async\" that allows the callers not to worry\n>     about threads and processes) aroundt this area.  That is\n>     obviously a mid- to longer term goal.\n\nI just wonder if we can skip step 1) and 2) by having the discussion\nnow how to change the framework to work well without posix file\ndescriptors here.\n\n>\n> Thanks for working together well, you two.\n"},{"id":"272850","messageId":"56392106.1010401@kdbg.org","threadId":"40702","inReplyTo":"CAGZ79kbAj-tZS3X1EwFw_gGh1=YU8M0OmEX91W8f++fqs6TiBA@mail.gmail.com","subject":"Re: git.git as of tonight","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2015-11-03T21:03:02Z","receivedAt":"2015-11-03T21:03:02Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 03.11.2015 um 19:18 schrieb Stefan Beller:\n> ... ReadFileEx ... \"overlapped\" operation.\n\nLet's not go there just yet.\n\n>>   1. Make this an optional feature so that platforms can compile it\n>>      out, if it is not already done.  My preference, even if we go\n>>      that route, would be to see if we can find a way to preserve the\n>>      overall code structure (e.g. instead of spawning multiple\n>>      workers, which is why the code needs NONBLOCK to avoid getting\n>>      stuck on reading from one while others are working, perhaps we\n>>      can spawn only one and not do a nonblock read?).\n>\n> Yeah that would be my understanding as well. If we don't come up with\n> a good solution for parallelism in Windows now, we'd need to make it at\n> least working in the jobs=1 case as well as it worked before.\n\nThat should be possible. I discovered today that we have this function:\n\nstatic void set_nonblocking(int fd)\n{\n\tint flags = fcntl(fd, F_GETFL);\n\tif (flags < 0)\n\t\twarning(\"Could not get file status flags, \"\n\t\t\t\"output will be degraded\");\n\telse if (fcntl(fd, F_SETFL, flags | O_NONBLOCK))\n\t\twarning(\"Could not set file status flags, \"\n\t\t\t\"output will be degraded\");\n}\n\nNotice that it is not a fatal condition if O_NONBLOCK cannot be \nestablished. (BTW, did you ever test this condition?) If we add two \nlines (which remove the stuff that does not work on Windows) like this:\n\nstatic void set_nonblocking(int fd)\n{\n#ifndef GIT_WINDOWS_NATIVE\n\tint flags = fcntl(fd, F_GETFL);\n\tif (flags < 0)\n\t\twarning(\"Could not get file status flags, \"\n\t\t\t\"output will be degraded\");\n\telse if (fcntl(fd, F_SETFL, flags | O_NONBLOCK))\n#endif\n\t\twarning(\"Could not set file status flags, \"\n\t\t\t\"output will be degraded\");\n}\n\nwe should get something that works, theoretically. We still need a more \ncomplete waitpid emulation, but that does not look like rocket science. \nI'll investigate further in this direction.\n\n-- Hannes\n"},{"id":"272864","messageId":"CAGZ79kZM-Q2oxVkMO9=v=tAdJkpWWOTVkaSMRDKZSZia2MY5Ng@mail.gmail.com","threadId":"40702","inReplyTo":"56392106.1010401@kdbg.org","subject":"Re: git.git as of tonight","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-11-03T23:00:24Z","receivedAt":"2015-11-03T23:00:24Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Nov 3, 2015 at 1:03 PM, Johannes Sixt <j6t@kdbg.org> wrote:\n> Am 03.11.2015 um 19:18 schrieb Stefan Beller:\n>>\n>> ... ReadFileEx ... \"overlapped\" operation.\n>\n>\n> Let's not go there just yet.\n>\n>>>   1. Make this an optional feature so that platforms can compile it\n>>>      out, if it is not already done.  My preference, even if we go\n>>>      that route, would be to see if we can find a way to preserve the\n>>>      overall code structure (e.g. instead of spawning multiple\n>>>      workers, which is why the code needs NONBLOCK to avoid getting\n>>>      stuck on reading from one while others are working, perhaps we\n>>>      can spawn only one and not do a nonblock read?).\n>>\n>>\n>> Yeah that would be my understanding as well. If we don't come up with\n>> a good solution for parallelism in Windows now, we'd need to make it at\n>> least working in the jobs=1 case as well as it worked before.\n>\n>\n> That should be possible. I discovered today that we have this function:\n>\n> static void set_nonblocking(int fd)\n> {\n>         int flags = fcntl(fd, F_GETFL);\n>         if (flags < 0)\n>                 warning(\"Could not get file status flags, \"\n>                         \"output will be degraded\");\n>         else if (fcntl(fd, F_SETFL, flags | O_NONBLOCK))\n>                 warning(\"Could not set file status flags, \"\n>                         \"output will be degraded\");\n> }\n>\n> Notice that it is not a fatal condition if O_NONBLOCK cannot be established.\n> (BTW, did you ever test this condition?)\n\nNo, as I viewed it more like a severe problem, not to be happen in the\nnear future.\nBut if it were to happen we would still need to finish the command\ninstead of giving\nup because of degraded output. (I would not know how to test this\nsystem call to fail,\nso maybe I am just making up excuses)\n\nI added an #ifdef just as you proposed below and the output itself\ndoesn't look too bad\nexcept for the warning message themselves. If we'd just remove them it\nwould look\nbetter to me.\n\nSo maybe we could just go with\n\n    static void set_nonblocking(int fd)\n    {\n    #ifndef GIT_WINDOWS_NATIVE\n        int flags = fcntl(fd, F_GETFL);\n        if (!(flags < 0))\n            fcntl(fd, F_SETFL, flags | O_NONBLOCK)\n    #endif\n    }\n\nand see how people react to the output then?\n\n\n> If we add two lines (which remove\n> the stuff that does not work on Windows) like this:\n>\n> static void set_nonblocking(int fd)\n> {\n> #ifndef GIT_WINDOWS_NATIVE\n>         int flags = fcntl(fd, F_GETFL);\n>         if (flags < 0)\n>                 warning(\"Could not get file status flags, \"\n>                         \"output will be degraded\");\n>         else if (fcntl(fd, F_SETFL, flags | O_NONBLOCK))\n> #endif\n>                 warning(\"Could not set file status flags, \"\n>                         \"output will be degraded\");\n> }\n>\n> we should get something that works, theoretically. We still need a more\n> complete waitpid emulation, but that does not look like rocket science. I'll\n> investigate further in this direction.\n>\n> -- Hannes\n>\n"},{"id":"272909","messageId":"563A63BA.9020407@web.de","threadId":"40702","inReplyTo":"CAGZ79kZM-Q2oxVkMO9=v=tAdJkpWWOTVkaSMRDKZSZia2MY5Ng@mail.gmail.com","subject":"Re: O_NONBLOCK under Windows (was: git.git as of tonight)","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2015-11-04T19:59:54Z","receivedAt":"2015-11-04T19:59:54Z","isPatch":false,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 11/04/2015 12:00 AM, Stefan Beller wrote:\n> On Tue, Nov 3, 2015 at 1:03 PM, Johannes Sixt <j6t@kdbg.org> wrote:\n>\nThe #ifdef assumes that Windows never will have O_NONBLOCK/F_GETFL)\n\nDoes the following make more sense ?\n#if defined (O_NONBLOCK) && defined (F_GETFL)\n\nOr may be:\n#ifndef NO_O_NONBLOCK\n>> #ifndef GIT_WINDOWS_NATIVE\n>>         int flags = fcntl(fd, F_GETFL);\n>>         if (flags < 0)\n>>                 warning(\"Could not get file status flags, \"\n>>                         \"output will be degraded\");\n>>         else if (fcntl(fd, F_SETFL, flags | O_NONBLOCK))\n>> #endif\n>>                 warning(\"Could not set file status flags, \"\n>>                         \"output will be degraded\");\n>> }\n>>\n>>\n>>\n>>   \n>>\n"},{"id":"272911","messageId":"CAGZ79kZk-oAcdVNHsFe_cRhR+TuGi8RqLYigHajC9KJu7ATJJw@mail.gmail.com","threadId":"40702","inReplyTo":"563A63BA.9020407@web.de","subject":"Re: O_NONBLOCK under Windows (was: git.git as of tonight)","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-11-04T20:07:25Z","receivedAt":"2015-11-04T20:07:25Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Nov 4, 2015 at 11:59 AM, Torsten Bögershausen <tboegi@web.de> wrote:\n> On 11/04/2015 12:00 AM, Stefan Beller wrote:\n>> On Tue, Nov 3, 2015 at 1:03 PM, Johannes Sixt <j6t@kdbg.org> wrote:\n>>\n> The #ifdef assumes that Windows never will have O_NONBLOCK/F_GETFL)\n>\n> Does the following make more sense ?\n> #if defined (O_NONBLOCK) && defined (F_GETFL)\n>\n> Or may be:\n> #ifndef NO_O_NONBLOCK\n>>> #ifndef GIT_WINDOWS_NATIVE\n>>>         int flags = fcntl(fd, F_GETFL);\n>>>         if (flags < 0)\n>>>                 warning(\"Could not get file status flags, \"\n>>>                         \"output will be degraded\");\n>>>         else if (fcntl(fd, F_SETFL, flags | O_NONBLOCK))\n>>> #endif\n>>>                 warning(\"Could not set file status flags, \"\n>>>                         \"output will be degraded\");\n>>> }\n>>>\n\nReading Junios answer to the resent patch[1], I am currently debating\nif this is the right way to go anyway. As Junio points out, this is\nnot a warning\nbut rather a critical issue such that we'd maybe rather die(...) than\njust warning(...),\nwhich would make the discussion about the correct condition in the #ifdef moot.\n\n[1] [PATCHv3 02/11] run-command: report failure for degraded output just once\n"},{"id":"272923","messageId":"1446677029-10745-1-git-send-email-sbeller@google.com","threadId":"40702","inReplyTo":"563A63BA.9020407@web.de","subject":"[PATCH 0/2] Missing O_NONBLOCK under Windows (was: git.git as of tonight)","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-11-04T22:43:47Z","receivedAt":"2015-11-04T22:43:47Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"The first patch is a general fixup as per discussion,\nthe second patch will make Git compile in Windows again (hopefully, not tested)\n\nThe number of #ifdefs seems acceptable to me, opinions on that?\n\nThis has been developed on top of d075d2604c0f9 (Merge branch 'rs/daemon-plug-child-leak'\ninto sb/submodule-parallel-update), but should also apply on top of origin/sb/submodule-parallel-fetch\n\nThanks,\nStefan\n\nStefan Beller (2):\n  run-parallel: rename set_nonblocking to set_nonblocking_or_die\n  run-parallel: Run sequential if nonblocking I/O is unavailable\n\n run-command.c | 25 ++++++++++++++++++-------\n 1 file changed, 18 insertions(+), 7 deletions(-)\n\n-- \n2.6.1.247.ge8f2a41.dirty\n"},{"id":"272924","messageId":"1446677029-10745-2-git-send-email-sbeller@google.com","threadId":"40702","inReplyTo":"1446677029-10745-1-git-send-email-sbeller@google.com","subject":"[PATCH 1/2] run-parallel: rename set_nonblocking to set_nonblocking_or_die","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-11-04T22:43:48Z","receivedAt":"2015-11-04T22:43:48Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Discussion turned out a warning is not enough, but we need to die in case\nof blocking output as we can lockup the child processes.\n\nJunio wrote:\n> Imagine that we are running two things A and B at the same time.  We\n> ask poll(2) and it says both A and B have some data ready to be\n> read, and we try to read from A.  strbuf_read_once() would try to\n> read up to 8K, relying on the fact that you earlier set the IO to be\n> nonblock.  It will get stuck reading from A without allowing output\n> from B to drain.  B's write may get stuck because we are not reading\n> from it, and would cause B to stop making progress.\n\n> What if the other sides of the connection from A and B are talking\n> with each other, and B's non-progress caused the processing for A on\n> the other side of the connection to block, causing it not to produce\n> more output to allow us to make progress reading from A (so that\n> eventually we can give B a chance to drain its output)?  Imagine A\n> and B are pushes to the same remote, B may be pushing a change to a\n> submodule while A may be pushing a matching change to its\n> superproject, and the server may be trying to make sure that the\n> submodule update completes and updates the ref before making the\n> superproject's tree that binds that updated submodule's commit\n> availble, for example?  Can we make any progress from that point?\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n run-command.c | 10 ++++------\n 1 file changed, 4 insertions(+), 6 deletions(-)\n\ndiff --git a/run-command.c b/run-command.c\nindex 0a3c24e..86fbe50 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -1006,15 +1006,13 @@ static void pp_cleanup(struct parallel_processes *pp)\n \tsigchain_pop_common();\n }\n \n-static void set_nonblocking(int fd)\n+static void set_nonblocking_or_die(int fd)\n {\n \tint flags = fcntl(fd, F_GETFL);\n \tif (flags < 0)\n-\t\twarning(\"Could not get file status flags, \"\n-\t\t\t\"output will be degraded\");\n+\t\tdie(\"Could not get file status flags\");\n \telse if (fcntl(fd, F_SETFL, flags | O_NONBLOCK))\n-\t\twarning(\"Could not set file status flags, \"\n-\t\t\t\"output will be degraded\");\n+\t\tdie(\"Could not set file status flags\");\n }\n \n /* returns\n@@ -1052,7 +1050,7 @@ static int pp_start_one(struct parallel_processes *pp)\n \t\treturn code ? -1 : 1;\n \t}\n \n-\tset_nonblocking(pp->children[i].process.err);\n+\tset_nonblocking_or_die(pp->children[i].process.err);\n \n \tpp->nr_processes++;\n \tpp->children[i].in_use = 1;\n-- \n2.6.1.247.ge8f2a41.dirty\n"},{"id":"272925","messageId":"1446677029-10745-3-git-send-email-sbeller@google.com","threadId":"40702","inReplyTo":"1446677029-10745-1-git-send-email-sbeller@google.com","subject":"[PATCH 2/2] run-parallel: Run sequential if nonblocking I/O is unavailable","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-11-04T22:43:49Z","receivedAt":"2015-11-04T22:43:49Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Windows doesn't have O_NONBLOCK nor F_GETFL defined, so we need cannot run\nin parallel there. Instead the children will output directly to our stderr\nand we run one child at a time.\n\nBonus: We are setting process.err = -1; which we previously expected the\nget_next_task callback to do. It is easy to forget that part in the callback\nleading to hard to debug errors.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n run-command.c | 15 ++++++++++++++-\n 1 file changed, 14 insertions(+), 1 deletion(-)\n\ndiff --git a/run-command.c b/run-command.c\nindex 86fbe50..19de253 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -958,6 +958,10 @@ static struct parallel_processes *pp_init(int n,\n \tif (n < 1)\n \t\tn = online_cpus();\n \n+#if !(defined (O_NONBLOCK) && defined (F_GETFL))\n+\tn = 1;\n+#endif\n+\n \tpp->max_processes = n;\n \tpp->data = data;\n \tif (!get_next_task)\n@@ -1006,6 +1010,7 @@ static void pp_cleanup(struct parallel_processes *pp)\n \tsigchain_pop_common();\n }\n \n+#if defined (O_NONBLOCK) && defined (F_GETFL)\n static void set_nonblocking_or_die(int fd)\n {\n \tint flags = fcntl(fd, F_GETFL);\n@@ -1014,6 +1019,7 @@ static void set_nonblocking_or_die(int fd)\n \telse if (fcntl(fd, F_SETFL, flags | O_NONBLOCK))\n \t\tdie(\"Could not set file status flags\");\n }\n+#endif\n \n /* returns\n  *  0 if a new task was started.\n@@ -1031,6 +1037,12 @@ static int pp_start_one(struct parallel_processes *pp)\n \tif (i == pp->max_processes)\n \t\tdie(\"BUG: bookkeeping is hard\");\n \n+#if defined (O_NONBLOCK) && defined (F_GETFL)\n+\tpp->children[i].process.err = -1;\n+#else\n+\tpp->children[i].process.err = 2;\n+#endif\n+\n \tif (!pp->get_next_task(&pp->children[i].data,\n \t\t\t       &pp->children[i].process,\n \t\t\t       &pp->children[i].err,\n@@ -1049,8 +1061,9 @@ static int pp_start_one(struct parallel_processes *pp)\n \t\tstrbuf_reset(&pp->children[i].err);\n \t\treturn code ? -1 : 1;\n \t}\n-\n+#if defined (O_NONBLOCK) && defined (F_GETFL)\n \tset_nonblocking_or_die(pp->children[i].process.err);\n+#endif\n \n \tpp->nr_processes++;\n \tpp->children[i].in_use = 1;\n-- \n2.6.1.247.ge8f2a41.dirty\n"},{"id":"272932","messageId":"563AF229.1060302@web.de","threadId":"40702","inReplyTo":"1446677029-10745-2-git-send-email-sbeller@google.com","subject":"Re: [PATCH 1/2] run-parallel: rename set_nonblocking to set_nonblocking_or_die","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2015-11-05T06:07:37Z","receivedAt":"2015-11-05T06:07:37Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 11/04/2015 11:43 PM, Stefan Beller wrote:\n> Discussion turned out a warning is not enough, but we need to die in case\n> of blocking output as we can lockup the child processes.\n>\n> Junio wrote:\n>> Imagine that we are running two things A and B at the same time.  We\n>> ask poll(2) and it says both A and B have some data ready to be\n>> read, and we try to read from A.  strbuf_read_once() would try to\n>> read up to 8K, relying on the fact that you earlier set the IO to be\n>> nonblock.\n(Jumping into an old discussion, I may be off topic)\n\nWhen we want to make really sure, not to get stucked, we can do like this:\n\nssize_t xread_nonblock(int fd, void *buf, size_t len)\n{\n     ssize_t nr;\n     if (len > MAX_IO_SIZE)\n         len = MAX_IO_SIZE;\n     nr = read(fd, buf, len);\n     if (nr < 0) {\n        if (errno == EWOULDBLOCK)\n             errno = EAGAIN;\n      return nr;\n}\n\nOnce poll() returns POLLIN as set on a fd, we can safely call read() once \nwithout getting stucked.\n\"Data can be read without blocking\".\n\nAnd this work regardless if the fd blocking or not, so from that point of view,\nthe set_nonblocking() is not needed at all.\n\n\nThe major question is, if the poll() works under Windows, (and I haven't found \ntime to dig further)\n"},{"id":"272939","messageId":"xmqqfv0l7z2r.fsf@gitster.mtv.corp.google.com","threadId":"40702","inReplyTo":"563AF229.1060302@web.de","subject":"Re: [PATCH 1/2] run-parallel: rename set_nonblocking to set_nonblocking_or_die","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-11-05T06:14:04Z","receivedAt":"2015-11-05T06:14:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Torsten Bögershausen <tboegi@web.de> writes:\n\n> (Jumping into an old discussion, I may be off topic)\n\nI think this is exactly the latest \"I wonder\" from Peff, to which I\nsaid \"well, perhaps we didn't need nonblock after all from the\nbeginning\".\n\n> And this work regardless if the fd blocking or not, so from that point of view,\n> the set_nonblocking() is not needed at all.\n>\n> The major question is, if the poll() works under Windows, (and I\n> haven't found time to dig further)\n\n;-)\n"},{"id":"272940","messageId":"xmqq8u6d7ytc.fsf@gitster.mtv.corp.google.com","threadId":"40702","inReplyTo":"xmqqfv0l7z2r.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 1/2] run-parallel: rename set_nonblocking to set_nonblocking_or_die","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-11-05T06:19:43Z","receivedAt":"2015-11-05T06:19:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Torsten Bögershausen <tboegi@web.de> writes:\n>\n>> (Jumping into an old discussion, I may be off topic)\n>\n> I think this is exactly the latest \"I wonder\" from Peff, to which I\n> said \"well, perhaps we didn't need nonblock after all from the\n> beginning\".\n>\n>> And this work regardless if the fd blocking or not, so from that point of view,\n>> the set_nonblocking() is not needed at all.\n>>\n>> The major question is, if the poll() works under Windows, (and I\n>> haven't found time to dig further)\n>\n> ;-)\n\nHaving read your message, I notice these disturbing passages in my\ncopy of manual pages.\n\npoll(2) has\n\n    BUGS\n           See the discussion of spurious readiness notifications under\n           the BUGS section of select(2).\n\nand then select(2) has\n\n       Under Linux, select() may report a socket file descriptor as\n       \"ready for read‐ ing\", while nevertheless a subsequent read\n       blocks.  This could for example happen when data has arrived\n       but upon examination has wrong checksum and is discarded.\n       There may be other circumstances in which a file descriptor\n       is spuriously reported as ready.  Thus it may be safer to use\n       O_NONBLOCK on sock‐ ets that should not block.\n\nThe world is not all Linux, so there may be systems that we do not\nhave to worry about the bug described here, but there still are some\nLinux systems in use in the world, so we cannot assume the bug\ndescribed above will not matter, either.\n\nSo I am not convinced that set_nonblocking() is unnecessary X-<.\n"},{"id":"272943","messageId":"20151105065801.GB4725@sigill.intra.peff.net","threadId":"40702","inReplyTo":"xmqq8u6d7ytc.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 1/2] run-parallel: rename set_nonblocking to set_nonblocking_or_die","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-11-05T06:58:01Z","receivedAt":"2015-11-05T06:58:01Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 04, 2015 at 10:19:43PM -0800, Junio C Hamano wrote:\n\n> Having read your message, I notice these disturbing passages in my\n> copy of manual pages.\n> \n> poll(2) has\n> \n>     BUGS\n>            See the discussion of spurious readiness notifications under\n>            the BUGS section of select(2).\n> \n> and then select(2) has\n> \n>        Under Linux, select() may report a socket file descriptor as\n>        \"ready for read‐ ing\", while nevertheless a subsequent read\n>        blocks.  This could for example happen when data has arrived\n>        but upon examination has wrong checksum and is discarded.\n>        There may be other circumstances in which a file descriptor\n>        is spuriously reported as ready.  Thus it may be safer to use\n>        O_NONBLOCK on sock‐ ets that should not block.\n> \n> The world is not all Linux, so there may be systems that we do not\n> have to worry about the bug described here, but there still are some\n> Linux systems in use in the world, so we cannot assume the bug\n> described above will not matter, either.\n> \n> So I am not convinced that set_nonblocking() is unnecessary X-<.\n\nYes, I've heard of this bug, and I wouldn't be surprised if it is\npresent on many systems. But notice that it is talking about sockets,\nwhereas here I think we are dealing only with pipes. Pipes generally\nhave much simpler and more predictable behavior, and their behaviors are\nmore closely mandated by POSIX.\n\nOf course that's all hearsay, and I can't quite you chapter and verse of\nPOSIX (as if that would be enough, anyway) to prove it. But personally,\nI would be comfortable dropping the non-blocking bits completely from\nStefan's series and adding them back in later if anybody can actually\nfind a real-world example that doesn't conform.\n\nOf course, simply omitting the O_NONBLOCK bits on Windows would also\nwork if we can guarantee the Windows behavior. It means we're carrying\naround extra code that may or may not be doing anything useful, but it's\nnot _that_ much code.\n\n-Peff\n"}]}