{"thread":{"id":"44218","subject":"Regression: git no longer works with musl libc's regex impl","startedAt":"2016-10-04T15:08:56Z","lastAt":"2016-10-07T11:31:21Z","messageCount":28,"participants":["Rich Felker","Jeff King","Johannes Schindelin","Ray Donnelly","James B","Junio C Hamano","Szabolcs Nagy","Jakub Narębski","Ævar Arnfjörð Bjarmason","Ramsay Jones"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"303241","messageId":"20161004150848.GA7949@brightrain.aerifal.cx","threadId":"44218","inReplyTo":null,"subject":"Regression: git no longer works with musl libc's regex impl","fromName":"Rich Felker","fromEmail":"dalias@libc.org","sentAt":"2016-10-04T15:08:48Z","receivedAt":"2016-10-04T15:08:56Z","isPatch":false,"sender":{"key":"dalias@libc.org","avatar":null},"body":"This commit broke support for using git with musl libc:\n\nhttps://github.com/git/git/commit/2f8952250a84313b74f96abb7b035874854cf202\n\nRather than depending on non-portable GNU regex extensions, there is a\nsimple portable fix for the issue this code was added to work around:\nWhen a text file is being mmapped for use with string functions which\ndepend on null termination, if the file size:\n\n1. is nonzero mod page size, it just works; the remainder of the last\n   page reads as zero bytes when mmapped.\n\n2. if an exact multiple of the page size, then instead of directly\n   mmapping the file, first mmap a mapping 1 byte (thus 1 page) larger\n   with MAP_ANON, then use MAP_FIXED to map the file over top of all\n   but the last page. Now the mmapped buffer can safely be used as a C\n   string.\n\nIf such a solution is acceptable I can try to prepare a patch.\n\nRich\n"},{"id":"303242","messageId":"20161004152722.ex2nox43oj5ak4yi@sigill.intra.peff.net","threadId":"44218","inReplyTo":"20161004150848.GA7949@brightrain.aerifal.cx","subject":"Re: Regression: git no longer works with musl libc's regex impl","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-10-04T15:27:22Z","receivedAt":"2016-10-04T15:27:33Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Oct 04, 2016 at 11:08:48AM -0400, Rich Felker wrote:\n\n> This commit broke support for using git with musl libc:\n> \n> https://github.com/git/git/commit/2f8952250a84313b74f96abb7b035874854cf202\n\nYep. The idea is that you would compile git with NO_REGEX=1, and it\nwould use the included compat routines.\n\nIs there something in particular you want to get out of using musl's\nregex that is not supported in the compat library?\n\n> Rather than depending on non-portable GNU regex extensions, there is a\n> simple portable fix for the issue this code was added to work around:\n> When a text file is being mmapped for use with string functions which\n> depend on null termination, if the file size:\n> \n> 1. is nonzero mod page size, it just works; the remainder of the last\n>    page reads as zero bytes when mmapped.\n\nIs that a portable assumption?\n\n> 2. if an exact multiple of the page size, then instead of directly\n>    mmapping the file, first mmap a mapping 1 byte (thus 1 page) larger\n>    with MAP_ANON, then use MAP_FIXED to map the file over top of all\n>    but the last page. Now the mmapped buffer can safely be used as a C\n>    string.\n\nI'm not sure whether all of our compat layers for mmap would be happy\nwith that (e.g., see compat/win32mmap.c).\n\nSo it seems like any mmap-related solutions would have to be\nconditional, too. And then regexec_buf() would have to become something\nlike:\n\n  int regexec_buf(...)\n  {\n  #if defined(REG_STARTEND)\n\t... set up match ...\n\treturn regexec(..., REG_STARTEND);\n  #elif defined(MMAP_ALWAYS_HAS_NUL)\n\t/*\n\t * We assume that every buffer we see is always NUL-terminated\n\t * eventually, either because it comes from xmallocz() or our\n\t * mmap layer always ensures an extra NUL.\n\t */\n\t return regexec(...);\n  #else\n  #error \"Nope, you need either NO_REGEX or USE_MMAP_NUL\"\n  #endif\n  }\n\nThe assumption in the middle case feels pretty hacky, though. It fails\nif we get a buffer from somewhere besides those two sources. It fails if\nsomebody calls regexec_buf() on a subset of a string.\n\nIt also doesn't handle matching past embedded NULs in the string. That's\nnot something we're relying on yet, but it would be nice to support\nconsistently in the long run.\n\nIf there's a compelling reason, it might be worth making that tradeoff.\nBut I am not sure what the compelling reason is to use musl's regex\n(aside from the obvious of \"less code in the resulting executable\").\n\n-Peff\n"},{"id":"303245","messageId":"20161004154045.GT19318@brightrain.aerifal.cx","threadId":"44218","inReplyTo":"20161004152722.ex2nox43oj5ak4yi@sigill.intra.peff.net","subject":"Re: Regression: git no longer works with musl libc's regex impl","fromName":"Rich Felker","fromEmail":"dalias@libc.org","sentAt":"2016-10-04T15:40:45Z","receivedAt":"2016-10-04T15:40:55Z","isPatch":false,"sender":{"key":"dalias@libc.org","avatar":null},"body":"On Tue, Oct 04, 2016 at 11:27:22AM -0400, Jeff King wrote:\n> On Tue, Oct 04, 2016 at 11:08:48AM -0400, Rich Felker wrote:\n> \n> > This commit broke support for using git with musl libc:\n> > \n> > https://github.com/git/git/commit/2f8952250a84313b74f96abb7b035874854cf202\n> \n> Yep. The idea is that you would compile git with NO_REGEX=1, and it\n> would use the included compat routines.\n\nThis is really obnoxious to have to do manually. Why can't it simply\nbe auto-detected based on non-definition of REG_STARDEND?\n\n> Is there something in particular you want to get out of using musl's\n> regex that is not supported in the compat library?\n\nIt's always nice not to link extra implementations of the same thing.\nThis comes up all the time with gratuitous gnulib replacement\nfunctions too.\n\n> > Rather than depending on non-portable GNU regex extensions, there is a\n> > simple portable fix for the issue this code was added to work around:\n> > When a text file is being mmapped for use with string functions which\n> > depend on null termination, if the file size:\n> > \n> > 1. is nonzero mod page size, it just works; the remainder of the last\n> >    page reads as zero bytes when mmapped.\n> \n> Is that a portable assumption?\n\nYes. Per POSIX:\n\n\"The system shall always zero-fill any partial page at the end of an\nobject. Further, the system shall never write out any modified\nportions of the last page of an object which are beyond its end.\nReferences within the address range starting at pa and continuing for\nlen bytes to whole pages following the end of an object shall result\nin delivery of a SIGBUS signal.\"\n\nSource: http://pubs.opengroup.org/onlinepubs/9699919799/functions/mmap.html\n\nThe same or similar text appears at least back to SUSv2; I did not\nlook further back.\n\n> > 2. if an exact multiple of the page size, then instead of directly\n> >    mmapping the file, first mmap a mapping 1 byte (thus 1 page) larger\n> >    with MAP_ANON, then use MAP_FIXED to map the file over top of all\n> >    but the last page. Now the mmapped buffer can safely be used as a C\n> >    string.\n> \n> I'm not sure whether all of our compat layers for mmap would be happy\n> with that (e.g., see compat/win32mmap.c).\n\nThe nice thing about this approach is that if the MAP_FIXED fails due\nto a broken system you can just read() into the anonymous memory.\n\n> So it seems like any mmap-related solutions would have to be\n> conditional, too. And then regexec_buf() would have to become something\n> like:\n> \n>   int regexec_buf(...)\n>   {\n>   #if defined(REG_STARTEND)\n> \t... set up match ...\n> \treturn regexec(..., REG_STARTEND);\n>   #elif defined(MMAP_ALWAYS_HAS_NUL)\n> \t/*\n> \t * We assume that every buffer we see is always NUL-terminated\n> \t * eventually, either because it comes from xmallocz() or our\n> \t * mmap layer always ensures an extra NUL.\n> \t */\n> \t return regexec(...);\n>   #else\n>   #error \"Nope, you need either NO_REGEX or USE_MMAP_NUL\"\n>   #endif\n>   }\n> \n> The assumption in the middle case feels pretty hacky, though. It fails\n\nI agree. I would prefer just falling back to read() after MAP_ANON if\nthe MAP_FIXED fails. This would work for all systems.\n\n> if we get a buffer from somewhere besides those two sources. It fails if\n> somebody calls regexec_buf() on a subset of a string.\n> \n> It also doesn't handle matching past embedded NULs in the string. That's\n> not something we're relying on yet, but it would be nice to support\n> consistently in the long run.\n\nThis is going to be even more non-portable and\nimplementation-specific. There's not even a good spec for how embedded\nnuls should be treated in regex.\n\n> If there's a compelling reason, it might be worth making that tradeoff.\n> But I am not sure what the compelling reason is to use musl's regex\n> (aside from the obvious of \"less code in the resulting executable\").\n\nThe compelling reason is just being portable by default rather than\nrequiring manual overrides to use a replacement library with weird\nextensions for something that could have been done portably to begin\nwith.\n\nRich\n"},{"id":"303247","messageId":"alpine.DEB.2.20.1610041801130.35196@virtualbox","threadId":"44218","inReplyTo":"20161004152722.ex2nox43oj5ak4yi@sigill.intra.peff.net","subject":"Re: Regression: git no longer works with musl libc's regex impl","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-10-04T16:01:25Z","receivedAt":"2016-10-04T16:02:02Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 4 Oct 2016, Jeff King wrote:\n\n> On Tue, Oct 04, 2016 at 11:08:48AM -0400, Rich Felker wrote:\n> \n> > 1. is nonzero mod page size, it just works; the remainder of the last\n> >    page reads as zero bytes when mmapped.\n> \n> Is that a portable assumption?\n\nNo.\n\nCiao,\nDscho\n"},{"id":"303248","messageId":"alpine.DEB.2.20.1610041802310.35196@virtualbox","threadId":"44218","inReplyTo":"20161004154045.GT19318@brightrain.aerifal.cx","subject":"Re: Regression: git no longer works with musl libc's regex impl","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-10-04T16:08:33Z","receivedAt":"2016-10-04T16:09:01Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Rich,\n\nOn Tue, 4 Oct 2016, Rich Felker wrote:\n\n> On Tue, Oct 04, 2016 at 11:27:22AM -0400, Jeff King wrote:\n> > On Tue, Oct 04, 2016 at 11:08:48AM -0400, Rich Felker wrote:\n> > \n> > > 1. is nonzero mod page size, it just works; the remainder of the last\n> > >    page reads as zero bytes when mmapped.\n> > \n> > Is that a portable assumption?\n> \n> Yes.\n\nNo, it is not. You quote POSIX, but the matter of the fact is that we use\na subset of POSIX in order to be able to keep things running on Windows.\n\nAnd quite honestly, there are lots of reasons to keep things running on\nWindows, and even to favor Windows support over musl support. Over four\nmillion reasons: the Git for Windows users.\n\nSo rather than getting into an ideological discussion about \"broken\"\nsystems, it would be good to keep things practical, realizing that those\nusers make up a very real chunk of all of Git's users.\n\nAs to making NO_REGEX conditional on REG_STARTEND: you are talking about\napples and oranges here. NO_REGEX is a Makefile flag, while REG_STARTEND\nis a C preprocessor macro.\n\nUnless you can convince the rest of the Git developers (you would not\nconvince me) to simulate autoconf by compiling an executable every time\n`make` is run, to determine whether REG_STARTEND is defined, this is a\nno-go.\n\nHowever, you *can* use autoconf directly, and come up with a patch to our\nconfigure.ac that detects the absence of REG_STARTEND and sets NO_REGEX=1.\n\nAlternatively, you can set NO_REGEX=1 in your config.mak.\n\nOr, if you use one of the auto-detected cases in config.mak.uname, you\ncould patch it to set NO_REGEX=1.\n\nAnd lastly, the best alternative would be to teach musl about\nREG_STARTEND, as it is rather useful a feature.\n\nCiao,\nJohannes\n"},{"id":"303249","messageId":"20161004161130.GX19318@brightrain.aerifal.cx","threadId":"44218","inReplyTo":"alpine.DEB.2.20.1610041802310.35196@virtualbox","subject":"Re: Regression: git no longer works with musl libc's regex impl","fromName":"Rich Felker","fromEmail":"dalias@libc.org","sentAt":"2016-10-04T16:11:30Z","receivedAt":"2016-10-04T16:11:47Z","isPatch":false,"sender":{"key":"dalias@libc.org","avatar":null},"body":"On Tue, Oct 04, 2016 at 06:08:33PM +0200, Johannes Schindelin wrote:\n> Hi Rich,\n> \n> On Tue, 4 Oct 2016, Rich Felker wrote:\n> \n> > On Tue, Oct 04, 2016 at 11:27:22AM -0400, Jeff King wrote:\n> > > On Tue, Oct 04, 2016 at 11:08:48AM -0400, Rich Felker wrote:\n> > > \n> > > > 1. is nonzero mod page size, it just works; the remainder of the last\n> > > >    page reads as zero bytes when mmapped.\n> > > \n> > > Is that a portable assumption?\n> > \n> > Yes.\n> \n> No, it is not. You quote POSIX, but the matter of the fact is that we use\n> a subset of POSIX in order to be able to keep things running on Windows.\n> \n> And quite honestly, there are lots of reasons to keep things running on\n> Windows, and even to favor Windows support over musl support. Over four\n> million reasons: the Git for Windows users.\n\nI would hope that in the future, git-for-windows users will be using\nmusl, via midipix, rather than the painfully slow and awful version\nthey're stuck with now...\n\nRich\n"},{"id":"303260","messageId":"alpine.DEB.2.20.1610041915320.35196@virtualbox","threadId":"44218","inReplyTo":"20161004161130.GX19318@brightrain.aerifal.cx","subject":"Re: Regression: git no longer works with musl libc's regex impl","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-10-04T17:16:04Z","receivedAt":"2016-10-04T17:16:37Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Rich,\n\nOn Tue, 4 Oct 2016, Rich Felker wrote:\n\n> On Tue, Oct 04, 2016 at 06:08:33PM +0200, Johannes Schindelin wrote:\n> > Hi Rich,\n> > \n> > On Tue, 4 Oct 2016, Rich Felker wrote:\n> > \n> > > On Tue, Oct 04, 2016 at 11:27:22AM -0400, Jeff King wrote:\n> > > > On Tue, Oct 04, 2016 at 11:08:48AM -0400, Rich Felker wrote:\n> > > > \n> > > > > 1. is nonzero mod page size, it just works; the remainder of the last\n> > > > >    page reads as zero bytes when mmapped.\n> > > > \n> > > > Is that a portable assumption?\n> > > \n> > > Yes.\n> > \n> > No, it is not. You quote POSIX, but the matter of the fact is that we use\n> > a subset of POSIX in order to be able to keep things running on Windows.\n> > \n> > And quite honestly, there are lots of reasons to keep things running on\n> > Windows, and even to favor Windows support over musl support. Over four\n> > million reasons: the Git for Windows users.\n> \n> I would hope that in the future, git-for-windows users will be using\n> musl, via midipix, rather than the painfully slow and awful version\n> they're stuck with now...\n\nGit for Windows actually uses the MSVC runtime, which is blazing fast.\n\nYou are probably confusing Git for Windows with Cygwin Git.\n\nCiao,\nJohannes\n"},{"id":"303268","messageId":"20161004173926.GA19318@brightrain.aerifal.cx","threadId":"44218","inReplyTo":"alpine.DEB.2.20.1610041802310.35196@virtualbox","subject":"Re: [musl] Re: Regression: git no longer works with musl libc's regex impl","fromName":"Rich Felker","fromEmail":"dalias@libc.org","sentAt":"2016-10-04T17:39:26Z","receivedAt":"2016-10-04T17:39:40Z","isPatch":false,"sender":{"key":"dalias@libc.org","avatar":null},"body":"On Tue, Oct 04, 2016 at 06:08:33PM +0200, Johannes Schindelin wrote:\n> Hi Rich,\n> \n> On Tue, 4 Oct 2016, Rich Felker wrote:\n> \n> > On Tue, Oct 04, 2016 at 11:27:22AM -0400, Jeff King wrote:\n> > > On Tue, Oct 04, 2016 at 11:08:48AM -0400, Rich Felker wrote:\n> > > \n> > > > 1. is nonzero mod page size, it just works; the remainder of the last\n> > > >    page reads as zero bytes when mmapped.\n> > > \n> > > Is that a portable assumption?\n> > \n> > Yes.\n> \n> No, it is not. You quote POSIX, but the matter of the fact is that we use\n> a subset of POSIX in order to be able to keep things running on Windows.\n> \n> And quite honestly, there are lots of reasons to keep things running on\n> Windows, and even to favor Windows support over musl support. Over four\n> million reasons: the Git for Windows users.\n> \n> So rather than getting into an ideological discussion about \"broken\"\n> systems, it would be good to keep things practical, realizing that those\n> users make up a very real chunk of all of Git's users.\n> \n> As to making NO_REGEX conditional on REG_STARTEND: you are talking about\n> apples and oranges here. NO_REGEX is a Makefile flag, while REG_STARTEND\n> is a C preprocessor macro.\n\nIt seems like you could just always compile the source file, and just\nhave it all inside #if defined(NO_REGEX) || !defined(REG_STARTEND) or\nsimilar.\n\n> And lastly, the best alternative would be to teach musl about\n> REG_STARTEND, as it is rather useful a feature.\n\nMaybe, but it seems fundamentally costly to support -- it's extra\nstate in the inner loops that imposes costly spill/reload on archs\nwith too few registers (x86). I'll look at doing this when we\noverhaul/replace the regex implementation, and I'm happy to do some\nperformance-regression tests for adding it now if someone has a simple\npatch (as was mentioned on the musl list).\n\nRich\n"},{"id":"303274","messageId":"CAOYw7duf6VbtB3eL-z2b6TL+Wc7PRuARpfi-nwANnF9N0-2NBA@mail.gmail.com","threadId":"44218","inReplyTo":"alpine.DEB.2.20.1610041915320.35196@virtualbox","subject":"Re: Regression: git no longer works with musl libc's regex impl","fromName":"Ray Donnelly","fromEmail":"mingw.android@gmail.com","sentAt":"2016-10-04T18:00:30Z","receivedAt":"2016-10-04T18:00:36Z","isPatch":false,"sender":{"key":"mingw.android@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1042804?v=4"},"body":"On Tue, Oct 4, 2016 at 6:16 PM, Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n> Hi Rich,\n>\n> On Tue, 4 Oct 2016, Rich Felker wrote:\n>\n>> On Tue, Oct 04, 2016 at 06:08:33PM +0200, Johannes Schindelin wrote:\n>> > Hi Rich,\n>> >\n>> > On Tue, 4 Oct 2016, Rich Felker wrote:\n>> >\n>> > > On Tue, Oct 04, 2016 at 11:27:22AM -0400, Jeff King wrote:\n>> > > > On Tue, Oct 04, 2016 at 11:08:48AM -0400, Rich Felker wrote:\n>> > > >\n>> > > > > 1. is nonzero mod page size, it just works; the remainder of the last\n>> > > > >    page reads as zero bytes when mmapped.\n>> > > >\n>> > > > Is that a portable assumption?\n>> > >\n>> > > Yes.\n>> >\n>> > No, it is not. You quote POSIX, but the matter of the fact is that we use\n>> > a subset of POSIX in order to be able to keep things running on Windows.\n>> >\n>> > And quite honestly, there are lots of reasons to keep things running on\n>> > Windows, and even to favor Windows support over musl support. Over four\n>> > million reasons: the Git for Windows users.\n>>\n>> I would hope that in the future, git-for-windows users will be using\n>> musl, via midipix, rather than the painfully slow and awful version\n>> they're stuck with now...\n>\n> Git for Windows actually uses the MSVC runtime, which is blazing fast.\n>\n> You are probably confusing Git for Windows with Cygwin Git.\n\nTo be fair, Cygwin Git isn't *that* slow, though I look forward to the\nday when MSYS2 can use the native-Windows/GfW version instead\n(including your rebase-in-C changes)\n\n>\n> Ciao,\n> Johannes\n"},{"id":"303322","messageId":"20161005090625.683fdbbfac8164125dee6469@gmail.com","threadId":"44218","inReplyTo":"alpine.DEB.2.20.1610041802310.35196@virtualbox","subject":"Re: [musl] Re: Regression: git no longer works with musl libc's regex impl","fromName":"James B","fromEmail":"jamesbond3142@gmail.com","sentAt":"2016-10-04T22:06:25Z","receivedAt":"2016-10-04T22:11:34Z","isPatch":false,"sender":{"key":"jamesbond3142@gmail.com","avatar":null},"body":"On Tue, 4 Oct 2016 18:08:33 +0200 (CEST)\nJohannes Schindelin <Johannes.Schindelin@gmx.de> wrote:\n\n> \n> No, it is not. You quote POSIX, but the matter of the fact is that we use\n> a subset of POSIX in order to be able to keep things running on Windows.\n> \n> And quite honestly, there are lots of reasons to keep things running on\n> Windows, and even to favor Windows support over musl support. Over four\n> million reasons: the Git for Windows users.\n> \n\nWow, I don't know that Windows is a git's first-tier platform now, and Linux/POSIX second. Are we talking about the same git that was originally written in Linus Torvalds, and is used to manage Linux kernel? Are you by any chance employed by Redmond, directly or indirectly?\n\nSorry - can't help it.\n"},{"id":"303324","messageId":"20161004223322.GE19318@brightrain.aerifal.cx","threadId":"44218","inReplyTo":"20161005090625.683fdbbfac8164125dee6469@gmail.com","subject":"Re: [musl] Re: Regression: git no longer works with musl libc's regex impl","fromName":"Rich Felker","fromEmail":"dalias@libc.org","sentAt":"2016-10-04T22:33:22Z","receivedAt":"2016-10-04T22:33:49Z","isPatch":false,"sender":{"key":"dalias@libc.org","avatar":null},"body":"On Wed, Oct 05, 2016 at 09:06:25AM +1100, James B wrote:\n> On Tue, 4 Oct 2016 18:08:33 +0200 (CEST)\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:\n> \n> > \n> > No, it is not. You quote POSIX, but the matter of the fact is that we use\n> > a subset of POSIX in order to be able to keep things running on Windows.\n> > \n> > And quite honestly, there are lots of reasons to keep things running on\n> > Windows, and even to favor Windows support over musl support. Over four\n> > million reasons: the Git for Windows users.\n> > \n> \n> Wow, I don't know that Windows is a git's first-tier platform now,\n> and Linux/POSIX second. Are we talking about the same git that was\n> originally written in Linus Torvalds, and is used to manage Linux\n> kernel? Are you by any chance employed by Redmond, directly or\n> indirectly?\n> \n> Sorry - can't help it.\n\nI don't think the hostility and sarcasm are really needed here. But\nwhat this does speak to is that users don't like feeling like their\nplatform is being treated as a second-class target, which is what it\nfeels like when you have to manually flip a switch to make git build.\nThis is especially unfriendly when the semantics of the switch come\nacross, at least to some users, as \"your system regex is incomplete\"\nrather than \"git can't use it because git depends on nonstandard\nextensions\".\n\nRich\n"},{"id":"303326","messageId":"xmqq37kbspbu.fsf@gitster.mtv.corp.google.com","threadId":"44218","inReplyTo":"20161004223322.GE19318@brightrain.aerifal.cx","subject":"Re: [musl] Re: Regression: git no longer works with musl libc's regex impl","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-10-04T22:48:37Z","receivedAt":"2016-10-04T22:48:46Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Rich Felker <dalias@libc.org> writes:\n\n> This is especially unfriendly when the semantics of the switch come\n> across, at least to some users, as \"your system regex is incomplete\"\n> rather than \"git can't use it because git depends on nonstandard\n> extensions\".\n\nThe latter is exactly what Makefile patch that brought this change\nsays, I think.\n\n    # Define NO_REGEX if your C library lacks regex support with REG_STARTEND\n    # feature.\n\nBefore the series updated the message to the above, we used to say:\n\n    # Define NO_REGEX if you have no or inferior regex support in your C library.\n\nwhich _was_ unfair to those who needed to set NO_REGEX for whatever\nreason.  It was totally unclear \"inferior\" relative to what standard\nthe message was passing its harsh judgement on your C library.\n"},{"id":"303346","messageId":"alpine.DEB.2.20.1610051231390.35196@virtualbox","threadId":"44218","inReplyTo":"20161005090625.683fdbbfac8164125dee6469@gmail.com","subject":"Re: [musl] Re: Regression: git no longer works with musl libc's regex impl","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-10-05T10:41:50Z","receivedAt":"2016-10-05T10:42:24Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi James,\n\nOn Wed, 5 Oct 2016, James B wrote:\n\n> On Tue, 4 Oct 2016 18:08:33 +0200 (CEST)\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:\n> \n> > No, it is not. You quote POSIX, but the matter of the fact is that we\n> > use a subset of POSIX in order to be able to keep things running on\n> > Windows.\n> > \n> > And quite honestly, there are lots of reasons to keep things running\n> > on Windows, and even to favor Windows support over musl support. Over\n> > four million reasons: the Git for Windows users.\n> \n> Wow, I don't know that Windows is a git's first-tier platform now,\n\nIt is. Git for Windows is maintained by me, and I make as certain as I can\nthat it works fine. And yes, we have download numbers to support my claim.\nThe latest release is less than 24h old, but I can point you to Git for\nWindows 2.8.1 whose 32-bit installer was downloaded 397,273 times, and\nwhose 64-bit installer was downloaded 3,780,079 times.\n\n> and Linux/POSIX second.\n\nThis is not at all what I said, so please be careful of what you accuse\nme.\n\nWhat I said is that we never exploited the full POSIX standard, but that\nwe made certain to use a subset of POSIX in Git which would be relatively\neasy to emulate using Windows' API.\n\n> Are we talking about the same git that was originally written in Linus\n> Torvalds, and is used to manage Linux kernel?\n\nIt was originally written by (not in) Linus Torvalds, and yes, the Linux\nkernel is one of its many users.\n\nGit is not used only for the Linux kernel, though, and I am certain that\nLinus agrees that it should not cater only to the Linux folks. Git is used\nvery widely in OSS as well as in the industry. So we, the Git developers,\nkind of have an obligation to make things work in a much broader\nperspective than you suggested.\n\n> Are you by any chance employed by Redmond, directly or indirectly?\n\nI am not exactly employed by Redmond, but by Microsoft (this is what you\nmeant, I guess).\n\nI maintained Git for Windows in my spare time, next to a very demanding\nposition in science, for eight years. In 2015, I joined Microsoft and part\nof my role is to maintain Git for Windows, allowing me to do a much better\njob at it.\n\nOf course, I do not only improve Git's Windows support, but contribute\nother patches, too. You might also appreciate the fact that some of my\ncolleagues started contributing patches to Git that benefit all Git users.\n\n> Sorry - can't help it.\n\nI do not know why you are sorry, and I do not believe that you have to be.\n\nCiao,\nJohannes\n"},{"id":"303349","messageId":"alpine.DEB.2.20.1610051250080.35196@virtualbox","threadId":"44218","inReplyTo":"20161004173926.GA19318@brightrain.aerifal.cx","subject":"Re: [musl] Re: Regression: git no longer works with musl libc's regex impl","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-10-05T11:17:49Z","receivedAt":"2016-10-05T11:18:20Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Rich,\n\nOn Tue, 4 Oct 2016, Rich Felker wrote:\n\n> On Tue, Oct 04, 2016 at 06:08:33PM +0200, Johannes Schindelin wrote:\n>\n> > And lastly, the best alternative would be to teach musl about\n> > REG_STARTEND, as it is rather useful a feature.\n> \n> Maybe, but it seems fundamentally costly to support -- it's extra\n> state in the inner loops that imposes costly spill/reload on archs\n> with too few registers (x86).\n\nIt is true that it could cause that.\n\nI had a brief look at the source code (you use backtracking... hopefully\nnobody uses musl to parse regular expressions from untrusted, or\ninexperienced, sources [*1*]), and it seems that the regex code might\nspill unnecessarily already (I see, for example, that the reg_notbol,\nreg_noteol and reg_newline flags all use up complete int registers, not\nmerely bits of a single one).\n\nIt seems, specifically, that the *match_end_ofs parameter of the two\nregexec backends is always set to point to eo, which is so far not\ninitialized. You could initialize it to -1 and set it to pmatch[0].rm_eo\nif the REG_STARTEND flag is set. The GET_NEXT_WCHAR() macro would then\nneed to test something like\n\n\tif (str_byte >= string + *match_end_ofs) {\n\t\tret = REG_NOMATCH; goto error_exit;\n\t}\n\nThis does not handle non-zero pmatch[0].rm_so, though. I would probably\ntry to pass another input parameter for that, but I have not verified yet\nthat a \"^\" would be handled properly (if pmatch[0].rm_so > 0 and\nREG_STARTEND is set, \"^\" should *not* match).\n\n> I'll look at doing this when we overhaul/replace the regex\n> implementation, and I'm happy to do some performance-regression tests\n> for adding it now if someone has a simple patch (as was mentioned on the\n> musl list).\n\nI'd be interested to be kept in the loop, if you do not mind Cc:ing me.\n\nCiao,\nJohannes\n\nFootnote *1*:\nhttp://stackstatus.net/post/147710624694/outage-postmortem-july-20-2016\n"},{"id":"303355","messageId":"20161005225934.770d73b7d491d4bf4816411d@gmail.com","threadId":"44218","inReplyTo":"alpine.DEB.2.20.1610051231390.35196@virtualbox","subject":"Re: [musl] Re: Regression: git no longer works with musl libc's regex impl","fromName":"James B","fromEmail":"jamesbond3142@gmail.com","sentAt":"2016-10-05T11:59:34Z","receivedAt":"2016-10-05T12:04:47Z","isPatch":false,"sender":{"key":"jamesbond3142@gmail.com","avatar":null},"body":"On Wed, 5 Oct 2016 12:41:50 +0200 (CEST)\nJohannes Schindelin <Johannes.Schindelin@gmx.de> wrote:\n\n> > \n> > Wow, I don't know that Windows is a git's first-tier platform now,\n> \n> It is. Git for Windows is maintained by me, and I make as certain as I can\n> that it works fine. \n> And yes, we have download numbers to support my claim.\n> The latest release is less than 24h old, but I can point you to Git for\n> Windows 2.8.1 whose 32-bit installer was downloaded 397,273 times, and\n> whose 64-bit installer was downloaded 3,780,079 times.\n\nNumber downloads does not make first-tier platform. You know that as well as everyone else.\n\nFirst-tier support is the decision made by the maintainers that the entire features of the software must be available on those first tier platforms. So if Windows is indeed first-tier platform for git, it means any features that don't work on git version of Windows must not be used/developed or even castrated. That's a scary thought.\n\nBeing a maintainer for \"Git for Windows\" does not make one automatically as the maintainer for \"git\", although that can happen.\n\nSo this decision that \"Windows is now a first-tier platform for git\" - is your own opinion, or is this the collective opinion of *all* the git maintainers? \n\n\n> \n> > and Linux/POSIX second.\n> \n> This is not at all what I said, so please be careful of what you accuse\n> me.\n\nYes, you did not say that. I said that. And I will say more. Git has Linux/POSIX roots. Attempting to \"not use common POSIX features because they're not available on Windows\" *does* make Linux/POSIX feels like second class platform for git. The way I see it, it should be *the other way around*.\n\nIt's a very sad day for a tool that was developed originally to maintain Linux kernel, by the Linux kernel author, now is restricted to avoid use/optimise on Linux/POSIX features *because* it has to run on another Windows ...\n\n> \n> What I said is that we never exploited the full POSIX standard, but that\n> we made certain to use a subset of POSIX in Git which would be relatively\n> easy to emulate using Windows' API.\n\nAll this just proves my point above.\n\nAnd - I notice you use the pronoun \"we\" - is that a \"royal we\" (which means the entire point is your own or your cohorts position), or is it the official position of all git maintainers?\n\n> \n> > Are we talking about the same git that was originally written in Linus\n> > Torvalds, and is used to manage Linux kernel?\n> \n> It was originally written by (not in) Linus Torvalds, and yes, the Linux\n> kernel is one of its many users.\n\nThat was a rhetorical question.\n\n> \n> > Are you by any chance employed by Redmond, directly or indirectly?\n> \n> I am not exactly employed by Redmond, but by Microsoft (this is what you\n> meant, I guess).\n> \n> I maintained Git for Windows in my spare time, next to a very demanding\n> position in science, for eight years. In 2015, I joined Microsoft and part\n> of my role is to maintain Git for Windows, allowing me to do a much better\n> job at it.\n\n\nWell thank you for being honest. I can see now why you responded the way you did (and still do). By being employed by Microsoft, and especially paid to work on Git for Windows, you have all the incentives to make it work best on Windows, and to make it as its first-tier platform within the limitation of Windows.\n\nThat in itself is not a problem - it only starts to become a problem when you try to cut down support for other platforms or stifle improvements on other platforms because \"hey it makes it too hard to do those things in Windows\".\n\n> \n> Of course, I do not only improve Git's Windows support, but contribute\n> other patches, too. You might also appreciate the fact that some of my\n> colleagues started contributing patches to Git that benefit all Git users.\n> \n\nBy \"colleagues\" I assume other Microsoft employees? \nI don't have a problem with that - thank you and your colleagues for making git better.\n\nBut that does not give the right to you to control that \"if it doesn't work on Windows we shouldn't do it\". If you do, and at the same time claim that musl-libc (which is Linux-only) support is unimportant compared to your 4 million Git-for-Windows downloads really don't do well for your or your employer's image. I don't have to say why - everyone outside Microsoft knows why.\n\nIn conclusion, I certainly hope that your view is not shared by the other git maintainers.\n\nPS: Rich, sorry for the distraction. I have said what I want to say, so I'll bow out from this thread.\n\ncheers,\nJames\n"},{"id":"303358","messageId":"20161005130130.GM1280@port70.net","threadId":"44218","inReplyTo":"alpine.DEB.2.20.1610051250080.35196@virtualbox","subject":"Re: [musl] Re: Regression: git no longer works with musl libc's regex impl","fromName":"Szabolcs Nagy","fromEmail":"nsz@port70.net","sentAt":"2016-10-05T13:01:31Z","receivedAt":"2016-10-05T13:07:22Z","isPatch":false,"sender":{"key":"nsz@port70.net","avatar":null},"body":"* Johannes Schindelin <Johannes.Schindelin@gmx.de> [2016-10-05 13:17:49 +0200]:\n> I had a brief look at the source code (you use backtracking... hopefully\n> nobody uses musl to parse regular expressions from untrusted, or\n> inexperienced, sources [*1*]), and it seems that the regex code might\n\ndoes git use BRE?\n\na conforming BRE implementation has to use back tracking\nif the pattern has back references.\n\nusually ERE implementations may also use back tracking\nsince they support back references as an extension.\n\nmusl does not support this extension (and many others) so\nit never uses back tracking for ERE matches, note however\nthat match complexity and memory usage of a conforming\nERE implementation is still exponential in pattern\nlength because of repetition counts.\n"},{"id":"303360","messageId":"bc3da1a4-4b99-737f-050e-54ef5844c402@gmail.com","threadId":"44218","inReplyTo":"20161004223322.GE19318@brightrain.aerifal.cx","subject":"Re: Regression: git no longer works with musl libc's regex impl","fromName":"Jakub Narębski","fromEmail":"jnareb@gmail.com","sentAt":"2016-10-05T13:11:05Z","receivedAt":"2016-10-05T13:11:29Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"W dniu 05.10.2016 o 00:33, Rich Felker pisze:\n> On Wed, Oct 05, 2016 at 09:06:25AM +1100, James B wrote:\n>> On Tue, 4 Oct 2016 18:08:33 +0200 (CEST)\n>> Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:\n>>>\n>>> No, it is not. You quote POSIX, but the matter of the fact is that we use\n>>> a subset of POSIX in order to be able to keep things running on Windows.\n>>>\n>>> And quite honestly, there are lots of reasons to keep things running on\n>>> Windows, and even to favor Windows support over musl support. Over four\n>>> million reasons: the Git for Windows users.\n>>\n>> Wow, I don't know that Windows is a git's first-tier platform now,\n>> and Linux/POSIX second. Are we talking about the same git that was\n>> originally written in Linus Torvalds, and is used to manage Linux\n>> kernel? Are you by any chance employed by Redmond, directly or\n>> indirectly?\n>>\n>> Sorry - can't help it.\n\nWindows is one of the major platforms, yes.  I think there much, much\nmore people using Git on Windows, than using Git with musl.  More\nusers = more important.\n\nAlso, working with some inconvenience (requiring compilation with\nNO_REGEX=1) is better than not working at all.\n\nIn CodingGuidelines we say:\n\n - Most importantly, we never say \"It's in POSIX; we'll happily\n   ignore your needs should your system not conform to it.\"\n   We live in the real world.\n\n - However, we often say \"Let's stay away from that construct,\n   it's not even in POSIX\".\n\n - In spite of the above two rules, we sometimes say \"Although\n   this is not in POSIX, it (is so convenient | makes the code\n   much more readable | has other good characteristics) and\n   practically all the platforms we care about support it, so\n   let's use it\".\n\nThe REG_STARTEND is 3rd point, mmap shenningans looks like 1st...\n\n...on the other hand midipix <writeonce@midipix.org> wrote in\nhttp://public-inbox.org/git/20161004200057.dc30d64f61e5ec441c34ffd4f788e58e.efa66ead67.wbe@email15.godaddy.com/\nthat the proposed fix should work on all Windows version we are\ninterested in (I think).  Test program included / attached.\n\nThe above-mentioned email also explains that the problem was\ncaught on MS Windows; it triggers if file end falls on the mmapped\npage boundary, which is more likely to happen with 4096 mod size\non Windows rather than 65536 mod size on Linux.\n\n\nOn the other hand, while the proposed solution of \"add padding as\nto not end at page boundary, if necessary\" doesn't have the\nperformance impact of \"memcpy into NUL-terminated buffer\" that\nwas originally proposed in patch series, it is still extra code\nto maintain.\n\n> \n> I don't think the hostility and sarcasm are really needed here. But\n> what this does speak to is that users don't like feeling like their\n> platform is being treated as a second-class target, which is what it\n> feels like when you have to manually flip a switch to make git build.\n\nYou are welcome to send a patch adding to configure.ac detection\nof REG_STARTEND support in standard library - setting NO_REGEX if\nneeded, and/or adding to Makefile uname-based defaults setting\nNO_REGEX for compiling with musl.\n\n> This is especially unfriendly when the semantics of the switch come\n> across, at least to some users, as \"your system regex is incomplete\"\n> rather than \"git can't use it because git depends on nonstandard\n> extensions\".\n\nNonstandard but common extension.  As 2f8952250a commit message says\nhttps://github.com/git/git/commit/2f8952250a84313b74f96abb7b035874854cf202\n\n  Happily, there is an extension to regexec() introduced by the NetBSD\n  project and present in all major regex implementation including\n  Linux', MacOSX' and the one Git includes in compat/regex/: [...]\n\n  [...]\n\n  Since support for REG_STARTEND is so widespread by now, let's just\n  introduce a helper function that always uses it, and tell people\n  on a platform whose regex library does not support it to use the\n  one from our compat/regex/ directory.\n\nAlso, as Junio said, the description of NO_REGEX option in the\nMakefile now explicitly says:\n\n    # Define NO_REGEX if your C library lacks regex support with REG_STARTEND\n    # feature.\n\nBest,\n-- \nJakub Narębski\n\n"},{"id":"303361","messageId":"20161005131559.GG19318@brightrain.aerifal.cx","threadId":"44218","inReplyTo":"alpine.DEB.2.20.1610051250080.35196@virtualbox","subject":"Re: [musl] Re: Regression: git no longer works with musl libc's regex impl","fromName":"Rich Felker","fromEmail":"dalias@libc.org","sentAt":"2016-10-05T13:15:59Z","receivedAt":"2016-10-05T13:16:17Z","isPatch":false,"sender":{"key":"dalias@libc.org","avatar":null},"body":"On Wed, Oct 05, 2016 at 01:17:49PM +0200, Johannes Schindelin wrote:\n> Hi Rich,\n> \n> On Tue, 4 Oct 2016, Rich Felker wrote:\n> \n> > On Tue, Oct 04, 2016 at 06:08:33PM +0200, Johannes Schindelin wrote:\n> >\n> > > And lastly, the best alternative would be to teach musl about\n> > > REG_STARTEND, as it is rather useful a feature.\n> > \n> > Maybe, but it seems fundamentally costly to support -- it's extra\n> > state in the inner loops that imposes costly spill/reload on archs\n> > with too few registers (x86).\n> \n> It is true that it could cause that.\n> \n> I had a brief look at the source code (you use backtracking...\n\nWhere did you get that idea? Backtracking is the most utterly\nincompetent way to implement regex -- it throws away the whole\nproperty that makes regex useful, being regular. Unfortunately, POSIX\nBRE is not regular, as it contains backreferences, so any\nimplementation of regcomp/regexec requires at least a minimal\nbacktracking code path for BREs that contain backreferences.\n\n> hopefully\n> nobody uses musl to parse regular expressions from untrusted, or\n\nOn the contrary, musl's is the only system reccomp/regexec I'm aware\nof that actually attempts to be safe with untrusted input -- when\nusing REG_EXTENDED (ERE). Other implementations provide backreferences\nin ERE as an extension, making ERE unsafe just like BRE. musl\nintentionally disallows them as a feature.\n\nAt least until recently, glibc also crashed on malloc failures in\nregcomp, making it unsafe on untrusted input for that reason too.\n\nRich\n"},{"id":"303379","messageId":"20161005161158.62o7qmpwxdgf6zzk@sigill.intra.peff.net","threadId":"44218","inReplyTo":"20161005225934.770d73b7d491d4bf4816411d@gmail.com","subject":"Re: [musl] Re: Regression: git no longer works with musl libc's regex impl","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-10-05T16:11:58Z","receivedAt":"2016-10-05T16:12:11Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Oct 05, 2016 at 10:59:34PM +1100, James B wrote:\n\n> Number downloads does not make first-tier platform. You know that as\n> well as everyone else.\n> \n> First-tier support is the decision made by the maintainers that the\n> entire features of the software must be available on those first tier\n> platforms. So if Windows is indeed first-tier platform for git, it\n> means any features that don't work on git version of Windows must not\n> be used/developed or even castrated. That's a scary thought.\n\nPrepare to be scared, then, I guess. Ever since the msysgit project\nstarted years ago, we have made concessions in the code to work both\nwith POSIX-ish systems and with the msys layer. E.g., see how git-daemon\ndoes not fork(), but actually re-spawns itself to handle connections.\n\nWhen possible we try to put our abstractions at a level where they can\nbe implemented in a performant way on all platforms (the git-daemon\nthings is probably the _most_ ugly in that respect; I think nobody has\nreally cared about the performance enough to add back in a forking code\npath for POSIX systems).\n\n> So this decision that \"Windows is now a first-tier platform for git\" -\n> is your own opinion, or is this the collective opinion of *all* the\n> git maintainers?\n\nThere is only one maintainer of git: Junio. However, you'll note that I\nalso used \"we\" in the paragraphs above. And that is because the approach\nI am talking about is something that has been done over the course of\nmany years by many members of the development community.\n\nYou may disagree with that approach, but it is nothing new. The msysgit\nproject started in 2007.\n\n> Well thank you for being honest. I can see now why you responded the\n> way you did (and still do). By being employed by Microsoft, and\n> especially paid to work on Git for Windows, you have all the\n> incentives to make it work best on Windows, and to make it as its\n> first-tier platform within the limitation of Windows.\n\nPlease don't insinuate that Johannes is a Microsoft shill. He has been\nworking on the Windows port of Git for over 9 years, and was only\nemployed by Microsoft this year. Furthermore, his original REG_STARTEND\npatch actually did a run-time fallback of NUL-terminating the input\nbuffers. It was _I_ who suggested that we should simply push people\ntowards our compat/regex routines instead. So if you want to be mad at\nsomebody, be mad at me.\n\n-Peff\n"},{"id":"303383","messageId":"20161005161531.GH19318@brightrain.aerifal.cx","threadId":"44218","inReplyTo":"bc3da1a4-4b99-737f-050e-54ef5844c402@gmail.com","subject":"Re: [musl] Re: Regression: git no longer works with musl libc's regex impl","fromName":"Rich Felker","fromEmail":"dalias@libc.org","sentAt":"2016-10-05T16:15:31Z","receivedAt":"2016-10-05T16:16:37Z","isPatch":false,"sender":{"key":"dalias@libc.org","avatar":null},"body":"On Wed, Oct 05, 2016 at 03:11:05PM +0200, Jakub Narębski wrote:\n> W dniu 05.10.2016 o 00:33, Rich Felker pisze:\n> > On Wed, Oct 05, 2016 at 09:06:25AM +1100, James B wrote:\n> >> On Tue, 4 Oct 2016 18:08:33 +0200 (CEST)\n> >> Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:\n> >>>\n> >>> No, it is not. You quote POSIX, but the matter of the fact is that we use\n> >>> a subset of POSIX in order to be able to keep things running on Windows.\n> >>>\n> >>> And quite honestly, there are lots of reasons to keep things running on\n> >>> Windows, and even to favor Windows support over musl support. Over four\n> >>> million reasons: the Git for Windows users.\n> >>\n> >> Wow, I don't know that Windows is a git's first-tier platform now,\n> >> and Linux/POSIX second. Are we talking about the same git that was\n> >> originally written in Linus Torvalds, and is used to manage Linux\n> >> kernel? Are you by any chance employed by Redmond, directly or\n> >> indirectly?\n> >>\n> >> Sorry - can't help it.\n> \n> Windows is one of the major platforms, yes.  I think there much, much\n> more people using Git on Windows, than using Git with musl.  More\n> users = more important.\n> \n> Also, working with some inconvenience (requiring compilation with\n> NO_REGEX=1) is better than not working at all.\n> \n> In CodingGuidelines we say:\n> \n>  - Most importantly, we never say \"It's in POSIX; we'll happily\n>    ignore your needs should your system not conform to it.\"\n>    We live in the real world.\n> \n>  - However, we often say \"Let's stay away from that construct,\n>    it's not even in POSIX\".\n\nI agree wholeheartedly with these points.\n\n> \n>  - In spite of the above two rules, we sometimes say \"Although\n>    this is not in POSIX, it (is so convenient | makes the code\n>    much more readable | has other good characteristics) and\n>    practically all the platforms we care about support it, so\n>    let's use it\".\n> \n> The REG_STARTEND is 3rd point,\n\nTo begin with I wasn't clear that REG_STARDEND being nonstandard was\neven noticed or compatibility considered when adding the dependency on\nit, but it seems such discussion did take place and most targets have\nit. Perhaps this means it should be proposed for standardization in\nthe next issue of POSIX.\n\n> mmap shenningans looks like 1st...\n> \n> ....on the other hand midipix <writeonce@midipix.org> wrote in\n> http://public-inbox.org/git/20161004200057.dc30d64f61e5ec441c34ffd4f788e58e.efa66ead67.wbe@email15.godaddy.com/\n> that the proposed fix should work on all Windows version we are\n> interested in (I think).  Test program included / attached.\n> \n> The above-mentioned email also explains that the problem was\n> caught on MS Windows; it triggers if file end falls on the mmapped\n> page boundary, which is more likely to happen with 4096 mod size\n> on Windows rather than 65536 mod size on Linux.\n\nOn Linux page-size (mmap granularity) varies by arch but it's 4k on\nbasically all archs that people care about. I think midipix's author\nwas talking about real page size on Windows (4k) vs the minimum\nlogical page size (mmap granularity) that can be used to get\nPOSIX-matching semantics in midipix (which is 64k due to some\ntechnical reasons I forget, which he could probably remind me of).\n\n> On the other hand, while the proposed solution of \"add padding as\n> to not end at page boundary, if necessary\" doesn't have the\n> performance impact of \"memcpy into NUL-terminated buffer\" that\n> was originally proposed in patch series, it is still extra code\n> to maintain.\n\n*nod*\n\nRich\n"},{"id":"303386","messageId":"20161005162727.GK19318@brightrain.aerifal.cx","threadId":"44218","inReplyTo":"20161005161158.62o7qmpwxdgf6zzk@sigill.intra.peff.net","subject":"Re: [musl] Re: Regression: git no longer works with musl libc's regex impl","fromName":"Rich Felker","fromEmail":"dalias@libc.org","sentAt":"2016-10-05T16:27:27Z","receivedAt":"2016-10-05T16:28:16Z","isPatch":false,"sender":{"key":"dalias@libc.org","avatar":null},"body":"On Wed, Oct 05, 2016 at 12:11:58PM -0400, Jeff King wrote:\n> On Wed, Oct 05, 2016 at 10:59:34PM +1100, James B wrote:\n> \n> > Number downloads does not make first-tier platform. You know that as\n> > well as everyone else.\n> > \n> > First-tier support is the decision made by the maintainers that the\n> > entire features of the software must be available on those first tier\n> > platforms. So if Windows is indeed first-tier platform for git, it\n> > means any features that don't work on git version of Windows must not\n> > be used/developed or even castrated. That's a scary thought.\n> \n> Prepare to be scared, then, I guess. Ever since the msysgit project\n> started years ago, we have made concessions in the code to work both\n> with POSIX-ish systems and with the msys layer. E.g., see how git-daemon\n> does not fork(), but actually re-spawns itself to handle connections.\n> \n> When possible we try to put our abstractions at a level where they can\n> be implemented in a performant way on all platforms (the git-daemon\n> things is probably the _most_ ugly in that respect; I think nobody has\n> really cared about the performance enough to add back in a forking code\n> path for POSIX systems).\n> \n> > So this decision that \"Windows is now a first-tier platform for git\" -\n> > is your own opinion, or is this the collective opinion of *all* the\n> > git maintainers?\n> \n> There is only one maintainer of git: Junio. However, you'll note that I\n> also used \"we\" in the paragraphs above. And that is because the approach\n> I am talking about is something that has been done over the course of\n> many years by many members of the development community.\n> \n> You may disagree with that approach, but it is nothing new. The msysgit\n> project started in 2007.\n\nThe goal of the midipix project is to make the need for FOSS projects\nsupporting Windows to do hacks like this obsolete. It still has a\nlittle ways to go to be ready for mainstream use, but it's already\nrunning a lot, and I hope you'll consider it for the future since it\nsimplifies things A LOT when you can just write to POSIX instead of\nhaving to come up with abstraction layers that cater to Windows'\nbrokenness.\n\n> > Well thank you for being honest. I can see now why you responded the\n> > way you did (and still do). By being employed by Microsoft, and\n> > especially paid to work on Git for Windows, you have all the\n> > incentives to make it work best on Windows, and to make it as its\n> > first-tier platform within the limitation of Windows.\n> \n> Please don't insinuate that Johannes is a Microsoft shill. He has been\n> working on the Windows port of Git for over 9 years, and was only\n> employed by Microsoft this year. Furthermore, his original REG_STARTEND\n> patch actually did a run-time fallback of NUL-terminating the input\n> buffers. It was _I_ who suggested that we should simply push people\n> towards our compat/regex routines instead. So if you want to be mad at\n> somebody, be mad at me.\n\nI hope we can get this thread away from accusing and attacking people\nand on to doing productive things to make the software better.\n\nRich\n"},{"id":"303476","messageId":"alpine.DEB.2.20.1610061239480.35196@virtualbox","threadId":"44218","inReplyTo":"20161005225934.770d73b7d491d4bf4816411d@gmail.com","subject":"Re: [musl] Re: Regression: git no longer works with musl libc's regex impl","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-10-06T10:44:37Z","receivedAt":"2016-10-06T10:45:44Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi James,\n\nOn Wed, 5 Oct 2016, James B wrote:\n\n> On Wed, 5 Oct 2016 12:41:50 +0200 (CEST)\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:\n> \n> It's a very sad day for a tool that was developed originally to maintain\n> Linux kernel, by the Linux kernel author, now is restricted to avoid\n> use/optimise on Linux/POSIX features *because* it has to run on another\n> Windows ...\n\nPlease note that this thread started because Git tried to shed the\nrestrictions of POSIX by using REG_STARTEND. In other words, we tried very\nmuch *not* to be restricted.\n\nAnd pragmatically, I must add that the REG_STARTEND feature is a very,\nvery useful one.\n\nIf you want to turn your sadness into something productive (you will allow\nme that little prod after all you said in your mail), why don't you\ndig back through the Git mailing list, fish out my original patch that\nfell back to the \"malloc();memcpy();/*add NUL*/\" strategy, and contribute\nthat as a patch to the Git project, making a case that musl requires it?\n\nCiao,\nJohannes\n"},{"id":"303517","messageId":"CACBZZX4XPqZauD_M_ieOwVauT1fi3MQb4+6taELQaRG9M-Kz_w@mail.gmail.com","threadId":"44218","inReplyTo":"alpine.DEB.2.20.1610041802310.35196@virtualbox","subject":"Re: Regression: git no longer works with musl libc's regex impl","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2016-10-06T19:18:29Z","receivedAt":"2016-10-06T19:18:57Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Tue, Oct 4, 2016 at 6:08 PM, Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n> As to making NO_REGEX conditional on REG_STARTEND: you are talking about\n> apples and oranges here. NO_REGEX is a Makefile flag, while REG_STARTEND\n> is a C preprocessor macro.\n>\n> Unless you can convince the rest of the Git developers (you would not\n> convince me) to simulate autoconf by compiling an executable every time\n> `make` is run, to determine whether REG_STARTEND is defined, this is a\n> no-go.\n\nBut just to clarify, does anyone have any objection to making our\nconfigure.ac compile a C program to check for this sort of thing?\nBecause that seems like the easiest solution to this class of problem.\n"},{"id":"303519","messageId":"20161006192339.3yddgxxk7jn7zfqx@sigill.intra.peff.net","threadId":"44218","inReplyTo":"CACBZZX4XPqZauD_M_ieOwVauT1fi3MQb4+6taELQaRG9M-Kz_w@mail.gmail.com","subject":"Re: Regression: git no longer works with musl libc's regex impl","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-10-06T19:23:40Z","receivedAt":"2016-10-06T19:24:36Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 06, 2016 at 09:18:29PM +0200, Ævar Arnfjörð Bjarmason wrote:\n\n> On Tue, Oct 4, 2016 at 6:08 PM, Johannes Schindelin\n> <Johannes.Schindelin@gmx.de> wrote:\n> > As to making NO_REGEX conditional on REG_STARTEND: you are talking about\n> > apples and oranges here. NO_REGEX is a Makefile flag, while REG_STARTEND\n> > is a C preprocessor macro.\n> >\n> > Unless you can convince the rest of the Git developers (you would not\n> > convince me) to simulate autoconf by compiling an executable every time\n> > `make` is run, to determine whether REG_STARTEND is defined, this is a\n> > no-go.\n> \n> But just to clarify, does anyone have any objection to making our\n> configure.ac compile a C program to check for this sort of thing?\n> Because that seems like the easiest solution to this class of problem.\n\nNo, I think that is the exact purpose of configure.ac and autoconf.\n\nIt would be neat if we could auto-fallback during the build. Rich\nsuggested always compiling compat/regex.c, and just having it be a noop\nat the preprocessor level. I'm not sure if that would work, though,\nbecause we'd have to include the system \"regex.h\" to know if we have\nREG_STARTEND, at which point it is potentially too late to compile our\nown regex routines (we're potentially going to conflict with the system\ndeclarations).\n\n-Peff\n"},{"id":"303521","messageId":"20161006192500.GS19318@brightrain.aerifal.cx","threadId":"44218","inReplyTo":"20161006192339.3yddgxxk7jn7zfqx@sigill.intra.peff.net","subject":"Re: Regression: git no longer works with musl libc's regex impl","fromName":"Rich Felker","fromEmail":"dalias@libc.org","sentAt":"2016-10-06T19:25:00Z","receivedAt":"2016-10-06T19:25:21Z","isPatch":false,"sender":{"key":"dalias@libc.org","avatar":null},"body":"On Thu, Oct 06, 2016 at 03:23:40PM -0400, Jeff King wrote:\n> On Thu, Oct 06, 2016 at 09:18:29PM +0200, Ævar Arnfjörð Bjarmason wrote:\n> \n> > On Tue, Oct 4, 2016 at 6:08 PM, Johannes Schindelin\n> > <Johannes.Schindelin@gmx.de> wrote:\n> > > As to making NO_REGEX conditional on REG_STARTEND: you are talking about\n> > > apples and oranges here. NO_REGEX is a Makefile flag, while REG_STARTEND\n> > > is a C preprocessor macro.\n> > >\n> > > Unless you can convince the rest of the Git developers (you would not\n> > > convince me) to simulate autoconf by compiling an executable every time\n> > > `make` is run, to determine whether REG_STARTEND is defined, this is a\n> > > no-go.\n> > \n> > But just to clarify, does anyone have any objection to making our\n> > configure.ac compile a C program to check for this sort of thing?\n> > Because that seems like the easiest solution to this class of problem.\n> \n> No, I think that is the exact purpose of configure.ac and autoconf.\n> \n> It would be neat if we could auto-fallback during the build. Rich\n> suggested always compiling compat/regex.c, and just having it be a noop\n> at the preprocessor level. I'm not sure if that would work, though,\n> because we'd have to include the system \"regex.h\" to know if we have\n> REG_STARTEND, at which point it is potentially too late to compile our\n> own regex routines (we're potentially going to conflict with the system\n> declarations).\n\nIf you have autoconf testing for REG_STARTEND at configure time then\ncompat/regex.c can #include \"config.h\" and test for HAVE_REG_STARTEND\nrather than for REG_STARTEND, or something like that.\n\nRich\n"},{"id":"303522","messageId":"20161006192811.xy5iahvczigfgapa@sigill.intra.peff.net","threadId":"44218","inReplyTo":"20161006192500.GS19318@brightrain.aerifal.cx","subject":"Re: Regression: git no longer works with musl libc's regex impl","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-10-06T19:28:11Z","receivedAt":"2016-10-06T19:28:17Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 06, 2016 at 03:25:00PM -0400, Rich Felker wrote:\n\n> > No, I think that is the exact purpose of configure.ac and autoconf.\n> > \n> > It would be neat if we could auto-fallback during the build. Rich\n> > suggested always compiling compat/regex.c, and just having it be a noop\n> > at the preprocessor level. I'm not sure if that would work, though,\n> > because we'd have to include the system \"regex.h\" to know if we have\n> > REG_STARTEND, at which point it is potentially too late to compile our\n> > own regex routines (we're potentially going to conflict with the system\n> > declarations).\n> \n> If you have autoconf testing for REG_STARTEND at configure time then\n> compat/regex.c can #include \"config.h\" and test for HAVE_REG_STARTEND\n> rather than for REG_STARTEND, or something like that.\n\nRight, that part is easy; we do not even have to touch compat/regex.c,\nbecause we already have such a knob in the Makefile (NO_REGEX), and\nautoconf just needs to tweak that knob.\n\nMy question was whether we could do it without running a separate\ncompile (via autoconf or via the Makefile), and I think the answer is\n\"no\".\n\n-Peff\n"},{"id":"303538","messageId":"20336ac7-a494-d725-f928-834b1b3194fe@ramsayjones.plus.com","threadId":"44218","inReplyTo":"CACBZZX4XPqZauD_M_ieOwVauT1fi3MQb4+6taELQaRG9M-Kz_w@mail.gmail.com","subject":"Re: Regression: git no longer works with musl libc's regex impl","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2016-10-06T22:42:01Z","receivedAt":"2016-10-06T22:42:22Z","isPatch":false,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 06/10/16 20:18, Ævar Arnfjörð Bjarmason wrote:\n> On Tue, Oct 4, 2016 at 6:08 PM, Johannes Schindelin\n> <Johannes.Schindelin@gmx.de> wrote:\n>> As to making NO_REGEX conditional on REG_STARTEND: you are talking about\n>> apples and oranges here. NO_REGEX is a Makefile flag, while REG_STARTEND\n>> is a C preprocessor macro.\n>>\n>> Unless you can convince the rest of the Git developers (you would not\n>> convince me) to simulate autoconf by compiling an executable every time\n>> `make` is run, to determine whether REG_STARTEND is defined, this is a\n>> no-go.\n> \n> But just to clarify, does anyone have any objection to making our\n> configure.ac compile a C program to check for this sort of thing?\n> Because that seems like the easiest solution to this class of problem.\n\nErr, you do know that we already do that, right?\n\n[see commit a1e3b669 (\"autoconf: don't use platform regex if it lacks REG_STARTEND\", 17-08-2010)]\n\nIn fact, if you run the auto tools on cygwin, you get a different setting\nfor NO_REGEX than via config.mak.uname. Which is why I don't run configure\non cygwin. :-D\n\n[The issue is exposed by t7008-grep-binary.sh, where the cygwin native\nregex library matches '.' in a pattern with the NUL character. ie the\ntest_expect_failure test passes.]\n\nATB,\nRamsay Jones\n\n\n"},{"id":"303553","messageId":"4615e3c4-9793-3dce-c500-9091a9056379@gmail.com","threadId":"44218","inReplyTo":"20336ac7-a494-d725-f928-834b1b3194fe@ramsayjones.plus.com","subject":"Re: Regression: git no longer works with musl libc's regex impl","fromName":"Jakub Narębski","fromEmail":"jnareb@gmail.com","sentAt":"2016-10-07T11:30:52Z","receivedAt":"2016-10-07T11:31:21Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"W dniu 07.10.2016 o 00:42, Ramsay Jones pisze: \n> On 06/10/16 20:18, Ævar Arnfjörð Bjarmason wrote:\n[...]\n>> But just to clarify, does anyone have any objection to making our\n>> configure.ac compile a C program to check for this sort of thing?\n>> Because that seems like the easiest solution to this class of problem.\n> \n> Err, you do know that we already do that, right?\n> \n> [see commit a1e3b669 (\"autoconf: don't use platform regex if it lacks REG_STARTEND\", 17-08-2010)]\n> \n> In fact, if you run the auto tools on cygwin, you get a different setting\n> for NO_REGEX than via config.mak.uname. Which is why I don't run configure\n> on cygwin. :-D\n> \n> [The issue is exposed by t7008-grep-binary.sh, where the cygwin native\n> regex library matches '.' in a pattern with the NUL character. ie the\n> test_expect_failure test passes.]\n\nHuh.  So we have NO_REGEX support in ./configure, and people using\nGit on untypical architectures and systems *can* make use of it.\n\nIt was just described wrongly, so in turn to have the more neutral\ndescription, the same as in Makefile, let's do this:\n\n-------- >8 ---------- >8 ------------- >8 ---------- >8 ----------\nSubject: [PATCH] configure.ac: Improve description of NO_REGEX test\n\nThe commit 2f8952250a changed description of NO_REGEX build config\nvariable to be more neutral, and actually say that it is about\nsupport for REG_STARTEND.  Change description in configure.ac to\nbe the same.\n\nChange also the test message and variable name to match.  The test\njust checks that REG_STARTEND is #defined.\n\nIssue-found-by: Ramsay Jones <ramsay@ramsayjones.plus.com>\nSigned-off-by: Jakub Narębski <jnareb@gmail.com>\n---\n configure.ac | 13 +++++++------\n 1 file changed, 7 insertions(+), 6 deletions(-)\n\ndiff --git a/configure.ac b/configure.ac\nindex aa9c91d..7f39fd0 100644\n--- a/configure.ac\n+++ b/configure.ac\n@@ -835,9 +835,10 @@ AC_CHECK_TYPE([struct addrinfo],[\n ])\n GIT_CONF_SUBST([NO_IPV6])\n #\n-# Define NO_REGEX if you have no or inferior regex support in your C library.\n-AC_CACHE_CHECK([whether the platform regex can handle null bytes],\n- [ac_cv_c_excellent_regex], [\n+# Define NO_REGEX if your C library lacks regex support with REG_STARTEND\n+# feature.\n+AC_CACHE_CHECK([whether the platform regex supports REG_STARTEND],\n+ [ac_cv_c_regex_with_reg_startend], [\n AC_EGREP_CPP(yippeeyeswehaveit,\n \tAC_LANG_PROGRAM([AC_INCLUDES_DEFAULT\n #include <regex.h>\n@@ -846,10 +847,10 @@ AC_EGREP_CPP(yippeeyeswehaveit,\n yippeeyeswehaveit\n #endif\n ]),\n-\t[ac_cv_c_excellent_regex=yes],\n-\t[ac_cv_c_excellent_regex=no])\n+\t[ac_cv_c_regex_with_reg_startend=yes],\n+\t[ac_cv_c_regex_with_reg_startend=no])\n ])\n-if test $ac_cv_c_excellent_regex = yes; then\n+if test $ac_cv_c_regex_with_reg_startend = yes; then\n \tNO_REGEX=\n else\n \tNO_REGEX=YesPlease\n-- \n2.10.0\n\n\n\n"}]}