[PATCH] config.mak.uname: enable OPEN_RETURNS_EINTR for macOS Big Sur
- From
Jeff King <peff@peff.net>
- Date
- Mar 1, 2021, 09:29 UTC
- Message-ID
- <YDy0C9sRvboGXQ7P@coredump.intra.peff.net>
- In-Reply-To
- <xmqq8s7ascby.fsf@gitster.g>
On Fri, Feb 26, 2021 at 02:17:53PM -0800, Junio C Hamano wrote:
Show 6 quoted lines
> > + /* > > + * Also O_TMPFILE would take a mode, but it isn't defined everywhere. > > + * And anyway, we don't use it in our code base. > > + */ > > That is being extra careful---I like it very much.
I wondered what would happen if my "anyway" above is wrong. We at least would not invoke undefined behavior (because we'd avoid looking at the mode parameter even though it exists), but would pass a "0" mode to the real open(). Presumably somebody would notice that. :)
Show 15 quoted lines
> > + if (flags & O_CREAT) {
> > + va_list ap;
> > + va_start(ap, flags);
> > + mode = va_arg(ap, int);
> > + va_end(ap);
> > + }
> > +
> > + do {
> > + ret = open(path, flags, mode);
> > + } while (ret < 0 && errno == EINTR);
> > +
> > + return ret;
> > +}
>
> Thanks.I got another off-list report of the problem. I think we probably want to do this on top:
-- >8 -- Subject: config.mak.uname: enable OPEN_RETURNS_EINTR for macOS Big Sur
We've had mixed reports on whether the latest release of macOS needs this Makefile knob set. In most reported cases, there's antivirus software running (which one might imagine could cause an open() call to be delayed). However, one of the (off-list) reports I've gotten indicated that it happened on an otherwise clean install of Big Sur.
Since the symptom is so bad (checkout randomly fails to write several fails when the progress meter kicks in), and since the workaround is so lightweight (if we don't see EINTR, it's just an extra conditional check), let's just turn it on by default.
Signed-off-by: Jeff King <peff@peff.net> --- Apparently Big Sur jumped from macOS 10.x to 11.x. But our "uname -r" check gives the "Darwin version", in which it is 20.x (following 19.x for the previous version). At least according to some sources I found online. :) So that is good, because otherwise all of our uname_R checks here would have been broken. I don't have a Big Sur machine handy to test with, but I believe this should do what we want.
config.mak.uname | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/config.mak.uname b/config.mak.uname index e22d4b6d67..d204c20a64 100644 --- a/config.mak.uname +++ b/config.mak.uname @@ -124,6 +124,9 @@ ifeq ($(uname_S),Darwin) ifeq ($(shell test "`expr "$(uname_R)" : '\([0-9][0-9]*\)\.'`" -ge 11 && echo 1),1) HAVE_GETDELIM = YesPlease endif + ifeq ($(shell test "`expr "$(uname_R)" : '\([0-9][0-9]*\)\.'`" -ge 20 && echo 1),1) + OPEN_RETURNS_EINTR = UnfortunatelyYes + endif NO_MEMMEM = YesPlease USE_ST_TIMESPEC = YesPlease HAVE_DEV_TTY = YesPlease
-- 2.31.0.rc0.521.g56be7fa5e1