git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Stefan Sundin <git@stefansundin.com>
To: Victoria Dye <vdye@github.com>
Cc: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Jeff Hostetler" <git@jeffhostetler.com>,
	"Jeff Hostetler via GitGitGadget" <gitgitgadget@gmail.com>,
	git@vger.kernel.org, "Bagas Sanjaya" <bagasdotme@gmail.com>,
	"René Scharfe" <l.s.r@web.de>,
	"Jeff Hostetler" <jeffhostetler@github.com>,
	"Eric DeCosta" <edecosta@mathworks.com>
Subject: Re: [PATCH] fsmonitor: eliminate call to deprecated FSEventStream function
Date: Fri, 2 Dec 2022 13:44:55 -0800	[thread overview]
Message-ID: <CAMDaVWGKx3YMuzRvqGTvvcwh1y9CotFtOCpyAzm=QW3YLy5ROg@mail.gmail.com> (raw)
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:
> 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:
> 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.

  reply	other threads:[~2022-12-02 21:45 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-02 14:24 [PATCH] fsmonitor: eliminate call to deprecated FSEventStream function Jeff Hostetler via GitGitGadget
2022-12-02 17:42 ` Victoria Dye
2022-12-02 18:02 ` Ævar Arnfjörð Bjarmason
2022-12-02 18:37   ` Jeff Hostetler
2022-12-02 19:08     ` Ævar Arnfjörð Bjarmason
2022-12-02 19:51       ` Victoria Dye
2022-12-02 20:34         ` Ævar Arnfjörð Bjarmason
2022-12-02 21:17           ` Victoria Dye
2022-12-02 21:44             ` Stefan Sundin [this message]
2022-12-02 23:02               ` Ævar Arnfjörð Bjarmason
2022-12-03  1:05             ` Junio C Hamano
2022-12-05  0:58               ` Junio C Hamano
2022-12-05 14:34                 ` Jeff Hostetler
2022-12-05 23:13                   ` Junio C Hamano
2022-12-06 17:25                     ` Jeff Hostetler
2022-12-14 19:12 ` [PATCH v2] " Jeff Hostetler via GitGitGadget
2022-12-15  0:09   ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAMDaVWGKx3YMuzRvqGTvvcwh1y9CotFtOCpyAzm=QW3YLy5ROg@mail.gmail.com' \
    --to=git@stefansundin.com \
    --cc=avarab@gmail.com \
    --cc=bagasdotme@gmail.com \
    --cc=edecosta@mathworks.com \
    --cc=git@jeffhostetler.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=jeffhostetler@github.com \
    --cc=l.s.r@web.de \
    --cc=vdye@github.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).