Re: [PATCH v6 05/10] fsmonitor: deduplicate IPC path logic for Unix platforms
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Feb 25, 2026, 21:30 UTC
- Message-ID
- <xmqqqzq88q9u.fsf@gitster.g>
- In-Reply-To
- <ff31e359a7070c8d410518da0230ad0ecae7d771.1772050636.git.gitgitgadget@gmail.com>
"Paul Tarjan via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 14 quoted lines
> From: Paul Tarjan <github@paulisageek.com> > > The IPC path logic for determining the Unix domain socket location is > nearly identical between macOS and Linux. Both need to check whether > the .git directory is on a remote filesystem and, if so, fall back to > a socket path under $HOME or a user-configured directory. > > Merge the two implementations into a single fsm-ipc-unix.c that is > shared by both platforms. The unified version includes the worktree > NULL check (BUG guard) from the Linux implementation, which was missing > in the macOS version. > > Update Makefile, meson.build, and CMakeLists.txt to use the new shared > file for non-Windows platforms.
This sounds as if the patch started with two IPC path logic for macOS and Linux, and the patch removes one of them and updates the other one so that both platforms can use the surviving one.
But the patch seems to indicate somewhat different story. The code before this patch started with a single macOS (darwin) one, but because it is mostly applicable to other UNIX variants as well, the patch renames the existing macOS one for unix and makes a small adjustment (namely, asserts that r->worktree is not NULL).
Perhaps you started with two separate implementations (possibly with a new one called UNIX that was added by largely copying and pasting from the macOS one) and unified them, but that history does not exist in this 10-patch series, so the above story would need to be rewritten to say what actually is happening in this series, e.g., we realized macOS one is applicable generally for UNIX variants so we are renaming it from darwin to unix, or something.
Show 10 quoted lines
> @@ -2365,7 +2365,11 @@ ifdef FSMONITOR_DAEMON_BACKEND > COMPAT_CFLAGS += -DHAVE_FSMONITOR_DAEMON_BACKEND > COMPAT_OBJS += compat/fsmonitor/fsm-listen-$(FSMONITOR_DAEMON_BACKEND).o > COMPAT_OBJS += compat/fsmonitor/fsm-health-$(FSMONITOR_DAEMON_BACKEND).o > - COMPAT_OBJS += compat/fsmonitor/fsm-ipc-$(FSMONITOR_DAEMON_BACKEND).o > +ifeq ($(FSMONITOR_DAEMON_BACKEND),win32) > + COMPAT_OBJS += compat/fsmonitor/fsm-ipc-win32.o > +else > + COMPAT_OBJS += compat/fsmonitor/fsm-ipc-unix.o > +endif
Makes me wonder if doing
#define FSMONITOR_DAEMON_BACKEND unix
for macOS and then keeping
COMPAT_OBJS += compat/fsmonitor/fsm-ipc-$(FSMONITOR_DAEMON_BACKEND).o
as-is would be cleaner.
Later, we can add the same "#define FSMONITOR_DAEMON_BACKEND unix" for Linux, perhaps?
This is doubly true when we look at the build recipe changes to the meson one below ...
Show 20 quoted lines
> diff --git a/meson.build b/meson.build > index dd52efd1c8..8de795f9d4 100644 > --- a/meson.build > +++ b/meson.build > @@ -1332,11 +1332,16 @@ if fsmonitor_backend != '' > > libgit_sources += [ > 'compat/fsmonitor/fsm-health-' + fsmonitor_backend + '.c', > - 'compat/fsmonitor/fsm-ipc-' + fsmonitor_backend + '.c', > 'compat/fsmonitor/fsm-listen-' + fsmonitor_backend + '.c', > 'compat/fsmonitor/fsm-path-utils-' + fsmonitor_backend + '.c', > 'compat/fsmonitor/fsm-settings-' + fsmonitor_backend + '.c', > ] > + > + if fsmonitor_backend == 'win32' > + libgit_sources += 'compat/fsmonitor/fsm-ipc-win32.c' > + else > + libgit_sources += 'compat/fsmonitor/fsm-ipc-unix.c' > + endif > endif
... which would not be needed if fsmonitor_backend is set to 'unix' instead of 'darwin'.