All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Jeff Hostetler via GitGitGadget <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, "Stefan Sundin" <git@stefansundin.com>,
	"Bagas Sanjaya" <bagasdotme@gmail.com>,
	"René Scharfe" <l.s.r@web.de>,
	"Jeff Hostetler" <jeffhostetler@github.com>
Subject: Re: [PATCH] fsmonitor: eliminate call to deprecated FSEventStream function
Date: Fri, 02 Dec 2022 19:02:13 +0100	[thread overview]
Message-ID: <221202.86o7slfzot.gmgdl@evledraar.gmail.com> (raw)
In-Reply-To: <pull.1436.git.1669991072393.gitgitgadget@gmail.com>


On Fri, Dec 02 2022, Jeff Hostetler via GitGitGadget wrote:

> From: Jeff Hostetler <jeffhostetler@github.com>
>
> Replace the call to `FSEventStreamScheduleWithRunLoop()` function with
> the suggested `FSEventStreamSetDispatchQueue()` function.
>
> 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.
>
> The `FSEventStreamSetDispatchQueue()` function is conceptually similar
> and is the suggested replacement.  However, there are some subtle
> thread-related differences.
>
> Previously, the event stream would be processed by the
> `fsm_listen__loop()` thread while it was in the `CFRunLoopRun()`
> method.  (Conceptually, this was a blocking call on the lifetime of
> the event stream where our thread drove the event loop and individual
> events were handled by the `fsevent_callback()`.)
>
> With the change, a "dispatch queue" is created and FSEvents will be
> processed by a hidden queue-related thread (that calls the
> `fsevent_callback()` on our behalf).  Our `fsm_listen__loop()` thread
> maintains the original blocking model by waiting on a mutex/condition
> variable pair while the hidden thread does all of the work.

I just skimmed the code change and didn't see anything out of place, but
one thing that's missing about this explanation is:

Ok, it's deprecated, but when was it introduced? I.e. we now presumably
have a hard dependency on a newer API released with a newer version of
OSX?

Is it OK that we're going to throw compilation errors on older versions
that don't have it? What version is that? Is that older or newer than
our oldest supported OSX version in general, or is the plan to support
older OSX, but those users would need to compile without fsmonitor?

Depending on the answers to the above (hopefully in a re-rolled commit
message): Should we patch the bit in config.mak.uname where we do the
OSX version detection? I.e. if we're deprecating an older version anyone
still on it would be much better off with a straight-up "$(error)" from
the Makefile, rather than running into a compilation error, only to find
that we've stopped supporting that older version.

  parent reply	other threads:[~2022-12-02 18:06 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 [this message]
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
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=221202.86o7slfzot.gmgdl@evledraar.gmail.com \
    --to=avarab@gmail.com \
    --cc=bagasdotme@gmail.com \
    --cc=git@stefansundin.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=jeffhostetler@github.com \
    --cc=l.s.r@web.de \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.