Re: [PATCH] fsmonitor: eliminate call to deprecated FSEventStream function

From: Stefan Sundin <git@stefansundin.com>
Date: Dec 2, 2022, 21:44 UTC
Message-ID: <CAMDaVWGKx3YMuzRvqGTvvcwh1y9CotFtOCpyAzm=QW3YLy5ROg@mail.gmail.com>
In-Reply-To: <1b090929-f2da-f075-01d4-458804fc0717@github.com>

On Fri, Dec 2, 2022 at 6:24 AM Jeff Hostetler via GitGitGadget <gitgitgadget@gmail.com> wrote:

Show 6 quoted lines

> The MacOS version of the builtin FSMonitor feature uses the
> `FSEventStreamScheduleWithRunLoop()` function to drive the event loop
> and process FSEvents from the system.  This routine has now been
> deprecated by Apple.  The MacOS 13 (Ventana) compiler tool chain now
> generates a warning when compiling calls to this function.  In
> DEVELOPER=1 mode, this now causes a compile error.

Typo here, MacOS 13 is Ventura not Ventana.

On Fri, Dec 2, 2022 at 1:17 PM Victoria Dye <vdye@github.com> wrote:

Show 9 quoted lines

> My point is that such a user for this scenario is so unlikely to exist that
> holding up this patch - which provides a real, tangible benefit to
> developers *right now* - to implement your suggestion or modify the commit
> message is, at best, an unnecessary distraction.
>
> If, somewhere, there is a user that 1) keeps up-to-date with the latest
> version of Git, 2) uses FSMonitor, and 3) is working on the sole version of
> MacOS that was theoretically compatible with FSMonitor before this change
> but now is not, we can accommodate that once such a need is shown to exist.

Looking at config.mak.uname it seems quite easy to keep git working on old MacOS versions by adding a check like Ævar suggested.

Something like this:

--- a/config.mak.uname
+++ b/config.mak.uname
@@ -161,12 +161,15 @@ ifeq ($(uname_S),Darwin)

        # The builtin FSMonitor on MacOS builds upon Simple-IPC.  Both require
        # Unix domain sockets and PThreads.
+       # FSMonitor on Darwin requires MacOS 10.6 or later.
        ifndef NO_PTHREADS
        ifndef NO_UNIX_SOCKETS
+       ifeq ($(shell test "`expr "$(uname_R)" : '\([0-9][0-9]*\)\.'`"
-ge 10 && echo 1),1)
        FSMONITOR_DAEMON_BACKEND = darwin
        FSMONITOR_OS_SETTINGS = darwin
        endif
        endif
+       endif

        BASIC_LDFLAGS += -framework CoreServices
 endif


Looking at that file it seems like a lot of care has gone into keeping
compatibility with older MacOS versions so in my mind it seems
appropriate to continue that legacy, especially since it is so easy.

Great work Jeff.