Re: [PATCH] config.mak.uname: enable OPEN_RETURNS_EINTR for macOS Big Sur

From: Jeff King <peff@peff.net>
Date: Mar 3, 2021, 13:41 UTC
Message-ID: <YD+SCtzmtWgFArwW@coredump.intra.peff.net>
In-Reply-To: <xmqqczwimnps.fsf@gitster.c.googlers.com>

On Mon, Mar 01, 2021 at 03:57:35PM -0800, Junio C Hamano wrote:

Show 6 quoted lines

> Jeff King <peff@peff.net> writes:
> 
> > I got another off-list report of the problem. I think we probably want
> > to do this on top:
> 
> Queued and pushed out.

Thanks. I guess we're a bit late to make it into the upcoming release. Certainly we have survived for many years without this particular bugfix, so in that sense it is not urgent. But I do wonder if we will see more reports as more people start using the new macOS release. So it might be good to keep in mind for maint, if we cut a minor release.

Or alternatively, we could include _just_ the first patch. That's low risk, since you have to enable to knob yourself, but it gives people an option if they run into the symptom. But even that is probably not that urgent. People can also cherry-pick the patch, after all (and a distributor like homebrew can probably include the patch in their recipe if need be).

Show 11 quoted lines

> I wonder if these hits for SA_RESTART in config.mak.uname would be a
> good way to guide us.
> 
>     [6c109904 (Port to HP NonStop, 2012-09-19)]
>             # Not detected (nor checked for) by './configure'.
>             # We don't have SA_RESTART on NonStop, unfortunalety.
>             COMPAT_CFLAGS += -DSA_RESTART=0
> 
>     [40036bed (Port to QNX, 2012-12-18)]
>     ifeq ($(uname_S),QNX)
>             COMPAT_CFLAGS += -DSA_RESTART=0

I'm inclined to leave them alone until somebody with access to such a system can look further into it. After all, if you do not have SA_RESTART, you might not even have EINTR in the first place.

> One caveat is that we do not know if their system headers hide the
> real implementation of open(2) behind a C preprocessor macro, in
> whcih case OPEN_RETURNS_EINTR wrapper may not work correctly.

Yeah. I didn't think about that when I did the original "just do it everywhere" patch. But that is exactly what caused the problem on Windows (not a system macro, but in fact our own!). So I'm glad to have backed it off to a Makefile knob.

-Peff