Re: [PATCH v14 12/13] fsmonitor: add tests for Linux

Re: [PATCH v14 12/13] fsmonitor: add tests for Linux

[Paul Tarjan]

Jeff King <peff@peff.net> writes:

> Do we need to put it in curly braces to avoid interfering with the &&
> chain? Otherwise a failure of anything before the maybe_timeout will hit
> the "||".

Good point. Fixed in v15:

  stop_daemon_delete_repo () {
      r=$1 &&
      { maybe_timeout 30 git -C $r fsmonitor--daemon stop 2>/dev/null || :; } &&
      rm -rf $1
  }

Thanks for catching this. The broken &&-chain and
2>/dev/null were hiding the test_might_fail error.

Verified that test_might_fail rejects maybe_timeout (exit code 1,
command never runs), while the { ... || :; } version actually
executes the stop. No other instances of test_might_fail maybe_timeout in the
repo.

Paul

[PATCH v13 00/13] fsmonitor: implement filesystem change listener for Linux

[Paul Tarjan via GitGitGadget]

This series implements the built-in fsmonitor daemon for Linux using the
inotify API, bringing it to feature parity with the existing Windows and
macOS implementations. It also fixes two memory leaks in the
platform-independent daemon code and deduplicates the IPC and settings logic
that is now shared between macOS and Linux.

The implementation uses inotify rather than fanotify because fanotify
requires either CAP_SYS_ADMIN or CAP_PERFMON capabilities, making it
unsuitable for an unprivileged user-space daemon. While inotify has the
limitation of requiring a separate watch on every directory (unlike macOS
FSEvents, which can monitor an entire directory tree with a single watch),
it operates without elevated privileges and provides the per-file event
granularity needed for fsmonitor.

The listener uses inotify_init1(O_NONBLOCK) with a poll loop that checks for
events with a 50-millisecond timeout, keeping the inotify queue well-drained
to minimize the risk of overflows. Bidirectional hashmaps map between watch
descriptors and directory paths for efficient event resolution. Directory
renames are tracked using inotify cookie mechanism to correlate
IN_MOVED_FROM and IN_MOVED_TO event pairs; a periodic check detects stale
renames where the matching IN_MOVED_TO never arrived, forcing a resync.

New directory creation triggers recursive watch registration to ensure all
subdirectories are monitored. The IN_MASK_CREATE flag is used where
available to prevent modifying existing watches, with a fallback for older
kernels. When IN_MASK_CREATE is available and inotify_add_watch returns
EEXIST, it means another thread or recursive scan has already registered the
watch, so it is safe to ignore.

Remote filesystem detection uses statfs() to identify network-mounted
filesystems (NFS, CIFS, SMB, FUSE, etc.) via their magic numbers. Mount
point information is read from /proc/mounts and matched against the statfs
f_fsid to get accurate, human-readable filesystem type names for logging.
When the .git directory is on a remote filesystem, the IPC socket falls back
to $HOME or a user-configured directory via the fsmonitor.socketDir setting.

This series builds on work from https://github.com/git/git/pull/1352 by Eric
DeCosta and https://github.com/git/git/pull/1667 by Marziyeh Esipreh,
updated to work with the current codebase and address all review feedback.

Changes since v12:

 * Dropped both the fsmonitor.c workaround and the read-cache.c skipHash fix
   per Dscho's review: split-index and index.skipHash are fundamentally
   incompatible
 * Added sane_unset GIT_TEST_SPLIT_INDEX to scalar clone tests in t9210
   (patch 1/13), matching the existing workaround in test 16

Changes since v11:

 * Fix t9210 BUG assertion with GIT_TEST_SPLIT_INDEX=yes: guard
   fsmonitor_dirty bitmap access against split-index having fewer entries
   than the bitmap expects

Changes since v10:

 * Reverted pre_exec_cb callback back to simple close_fd_above_stderr flag
   per Junio's clarification (same as v8)

Changes since v9:

 * Fixed Windows build: close_fd_above_stderr() compiles as a no-op on
   Windows since there is no fork/exec

Changes since v8:

 * Replaced close_fd_above_stderr flag with generic pre_exec_cb callback in
   struct child_process per Junio's review, with close_fd_above_stderr() as
   a ready-made callback

Changes since v7:

 * Added patch 12: convert khash to strset in do_handle_client (Patrick's
   #leftoverbit suggestion)
 * Fixed "Forcing shutdown" trace message to start with lowercase
 * Fixed redundant statfs() call in find_mount() (caller already had the
   result)
 * Fixed CMakeLists.txt GIT-BUILD-OPTIONS: was hardcoded to "win32" for
   FSMONITOR_DAEMON_BACKEND and FSMONITOR_OS_SETTINGS, now uses the CMake
   variables
 * Fixed uninitialized strset on trivial response path (STRSET_INIT)
 * Removed V9FS_MAGIC from get_fs_typename() to match is_remote_fs() (9p is
   local VM mounts)
 * Split 30-second stop timeout into its own commit per review request
 * Fixed misleading indentation on shutdown assignment in handle_events()
 * Updated commit messages to describe all changes (test hardening,
   fsmonitor-ipc.c spawn changes)
 * Updated Makefile comment for FSMONITOR_OS_SETTINGS to mention fsm-ipc

Changes since v6:

 * Introduced FSMONITOR_OS_SETTINGS build variable (set to "unix" for macOS
   and Linux, "win32" for Windows) to eliminate if/else conditionals in
   Makefile, meson.build, and CMakeLists.txt per Junio's review
 * Moved fsm-path-utils from FSMONITOR_OS_SETTINGS to
   FSMONITOR_DAEMON_BACKEND since path-utils files are platform-specific
 * Removed V9FS_MAGIC from remote filesystem detection (9p is used for local
   VM/container host mounts where fsmonitor works fine)
 * Removed redundant #include <libgen.h> (already provided by
   compat/posix.h)
 * Fixed cookie wait comment wording ("see" to "observe")
 * Rewrote commit messages for IPC and settings dedup patches

Changes since v5:

 * Split monolithic commit into 10-patch series per Patrick's review
 * Deduplicated fsm-ipc and fsm-settings into shared Unix implementations
 * Rewrote commit message with prose paragraphs, explain inotify vs
   fanotify, removed "Issues addressed" sections, added Based-on-patch-by
   trailers
 * Removed redundant includes already provided by compat/posix.h
 * Fixed error/trace message capitalization per coding guidelines
 * Fixed stale rename check interval from 1000 seconds to 1 second
 * Changed poll timeout from 1ms to 50ms to reduce idle CPU wake-ups
 * Replaced infinite pthread_cond_wait cookie loop with one-second
   pthread_cond_timedwait (prevents daemon hangs on overlay filesystems
   where events are never delivered)
 * Added pthread_cond_timedwait to Windows pthread compatibility layer
 * Separated test into its own commit with smoke test that skips when
   inotify events are not delivered (e.g., overlayfs with older kernels)
 * Fixed test hang on Fedora CI: stop_git() looped forever when ps was
   unavailable because bash in POSIX/sh mode returns exit 0 from kill with
   an empty process group argument. Fixed by falling back to /proc/$pid/stat
   for process group ID and guarding stop_git against empty pgid.
 * Redirect spawn_daemon() stdout/stderr to /dev/null and close inherited
   file descriptors to prevent the intermediate process from holding test
   pipe file descriptors
 * Call setsid() on daemon detach to prevent shells with job control from
   waiting on the daemon process group
 * Close inherited file descriptors 3-7 in the test watchdog subprocess
 * Added 30-second timeout to "fsmonitor--daemon stop" to prevent indefinite
   blocking
 * Added helpful error message when inotify watch limit (max_user_watches)
   is reached
 * Initialize fd_inotify to -1 and use fd >= 0 check for correct fd 0
   handling
 * Use sysconf(_SC_OPEN_MAX) instead of hardcoded 1024 for fd close limit
 * Check setsid() return value

Changes since v4:

 * Added Meson build support

Changes since v3:

 * Fix crash on rapid nested directory creation (EEXIST from
   inotify_add_watch with IN_MASK_CREATE)
 * Extensive stress testing

Changes since v2:

 * Fix khash memory leak in do_handle_client

Changes since v1:

 * Fix hashmap memory leak in fsmonitor_run_daemon()

Paul Tarjan (13):
  t9210: disable GIT_TEST_SPLIT_INDEX for scalar clone tests
  fsmonitor: fix khash memory leak in do_handle_client
  fsmonitor: fix hashmap memory leak in fsmonitor_run_daemon
  compat/win32: add pthread_cond_timedwait
  fsmonitor: use pthread_cond_timedwait for cookie wait
  fsmonitor: rename fsm-ipc-darwin.c to fsm-ipc-unix.c
  fsmonitor: rename fsm-settings-darwin.c to fsm-settings-unix.c
  fsmonitor: implement filesystem change listener for Linux
  run-command: add close_fd_above_stderr option
  fsmonitor: close inherited file descriptors and detach in daemon
  fsmonitor: add timeout to daemon stop command
  fsmonitor: add tests for Linux
  fsmonitor: convert shown khash to strset in do_handle_client

 Documentation/config/fsmonitor--daemon.adoc   |   4 +-
 Documentation/git-fsmonitor--daemon.adoc      |  28 +-
 Makefile                                      |   6 +-
 builtin/fsmonitor--daemon.c                   |  92 ++-
 compat/fsmonitor/fsm-health-linux.c           |  33 +
 .../{fsm-ipc-darwin.c => fsm-ipc-unix.c}      |   0
 compat/fsmonitor/fsm-listen-linux.c           | 746 ++++++++++++++++++
 compat/fsmonitor/fsm-path-utils-linux.c       | 217 +++++
 ...-settings-darwin.c => fsm-settings-unix.c} |   0
 compat/win32/pthread.c                        |  26 +
 compat/win32/pthread.h                        |   2 +
 config.mak.uname                              |  12 +-
 contrib/buildsystems/CMakeLists.txt           |  33 +-
 fsmonitor-ipc.c                               |   3 +
 meson.build                                   |  13 +-
 run-command.c                                 |  12 +
 run-command.h                                 |   9 +
 t/t7527-builtin-fsmonitor.sh                  |  89 ++-
 t/t9210-scalar.sh                             |   6 +
 19 files changed, 1268 insertions(+), 63 deletions(-)
 create mode 100644 compat/fsmonitor/fsm-health-linux.c
 rename compat/fsmonitor/{fsm-ipc-darwin.c => fsm-ipc-unix.c} (100%)
 create mode 100644 compat/fsmonitor/fsm-listen-linux.c
 create mode 100644 compat/fsmonitor/fsm-path-utils-linux.c
 rename compat/fsmonitor/{fsm-settings-darwin.c => fsm-settings-unix.c} (100%)


base-commit: 3e0db84c88c57e70ac8be8c196dfa92c5d656fbc
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2147%2Fptarjan%2Fclaude%2Fupdate-pr-1352-current-85Gk8-v13
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2147/ptarjan/claude/update-pr-1352-current-85Gk8-v13
Pull-Request: https://github.com/git/git/pull/2147

Range-diff vs v12:

  -:  ---------- >  1:  28c5aca413 t9210: disable GIT_TEST_SPLIT_INDEX for scalar clone tests
  1:  4d4dec8fa1 =  2:  9f666beea7 fsmonitor: fix khash memory leak in do_handle_client
  2:  cb270120f0 =  3:  a4a65a6dfa fsmonitor: fix hashmap memory leak in fsmonitor_run_daemon
  3:  44a063074d =  4:  427073fc38 compat/win32: add pthread_cond_timedwait
  4:  b1081d1e13 =  5:  71effc4d47 fsmonitor: use pthread_cond_timedwait for cookie wait
  5:  dec0fb144f =  6:  e897193b0e fsmonitor: rename fsm-ipc-darwin.c to fsm-ipc-unix.c
  6:  b2aaadb4ae =  7:  e13d938ddb fsmonitor: rename fsm-settings-darwin.c to fsm-settings-unix.c
  7:  03cf12d01b =  8:  3431eae60f fsmonitor: implement filesystem change listener for Linux
  8:  50f5b4676e =  9:  7ce0ab87fb run-command: add close_fd_above_stderr option
  9:  057b3098bc = 10:  2bf134a041 fsmonitor: close inherited file descriptors and detach in daemon
 10:  e6bc3bfcb2 = 11:  cb4d511f21 fsmonitor: add timeout to daemon stop command
 11:  81f8cd1599 = 12:  9a8647884e fsmonitor: add tests for Linux
 12:  8fa6a74e0d = 13:  a5fc9ad415 fsmonitor: convert shown khash to strset in do_handle_client
 13:  84ddbb30bb <  -:  ---------- fsmonitor: fix split-index bitmap bounds in tweak_fsmonitor()

-- 
gitgitgadget

[PATCH v14 00/13] fsmonitor: implement filesystem change listener for Linux

[Paul Tarjan via GitGitGadget]

This series implements the built-in fsmonitor daemon for Linux using the
inotify API, bringing it to feature parity with the existing Windows and
macOS implementations. It also fixes two memory leaks in the
platform-independent daemon code and deduplicates the IPC and settings logic
that is now shared between macOS and Linux.

The implementation uses inotify rather than fanotify because fanotify
requires either CAP_SYS_ADMIN or CAP_PERFMON capabilities, making it
unsuitable for an unprivileged user-space daemon. While inotify has the
limitation of requiring a separate watch on every directory (unlike macOS
FSEvents, which can monitor an entire directory tree with a single watch),
it operates without elevated privileges and provides the per-file event
granularity needed for fsmonitor.

The listener uses inotify_init1(O_NONBLOCK) with a poll loop that checks for
events with a 50-millisecond timeout, keeping the inotify queue well-drained
to minimize the risk of overflows. Bidirectional hashmaps map between watch
descriptors and directory paths for efficient event resolution. Directory
renames are tracked using inotify cookie mechanism to correlate
IN_MOVED_FROM and IN_MOVED_TO event pairs; a periodic check detects stale
renames where the matching IN_MOVED_TO never arrived, forcing a resync.

New directory creation triggers recursive watch registration to ensure all
subdirectories are monitored. The IN_MASK_CREATE flag is used where
available to prevent modifying existing watches, with a fallback for older
kernels. When IN_MASK_CREATE is available and inotify_add_watch returns
EEXIST, it means another thread or recursive scan has already registered the
watch, so it is safe to ignore.

Remote filesystem detection uses statfs() to identify network-mounted
filesystems (NFS, CIFS, SMB, FUSE, etc.) via their magic numbers. Mount
point information is read from /proc/mounts and matched against the statfs
f_fsid to get accurate, human-readable filesystem type names for logging.
When the .git directory is on a remote filesystem, the IPC socket falls back
to $HOME or a user-configured directory via the fsmonitor.socketDir setting.

This series builds on work from https://github.com/git/git/pull/1352 by Eric
DeCosta and https://github.com/git/git/pull/1667 by Marziyeh Esipreh,
updated to work with the current codebase and address all review feedback.

Changes since v13:

 * Also disable GIT_TEST_SPLIT_INDEX in t9211 per Junio's feedback (every
   test does scalar clone)

Changes since v12:

 * Dropped both the fsmonitor.c workaround and the read-cache.c skipHash fix
   per Dscho's review: split-index and index.skipHash are fundamentally
   incompatible
 * Added sane_unset GIT_TEST_SPLIT_INDEX to scalar clone tests in t9210
   (patch 1/13)

Changes since v11:

 * Fix t9210 BUG assertion with GIT_TEST_SPLIT_INDEX=yes: guard
   fsmonitor_dirty bitmap access against split-index having fewer entries
   than the bitmap expects

Changes since v10:

 * Reverted pre_exec_cb callback back to simple close_fd_above_stderr flag
   per Junio's clarification (same as v8)

Changes since v9:

 * Fixed Windows build: close_fd_above_stderr() compiles as a no-op on
   Windows since there is no fork/exec

Changes since v8:

 * Replaced close_fd_above_stderr flag with generic pre_exec_cb callback in
   struct child_process per Junio's review, with close_fd_above_stderr() as
   a ready-made callback

Changes since v7:

 * Added patch 12: convert khash to strset in do_handle_client (Patrick's
   #leftoverbit suggestion)
 * Fixed "Forcing shutdown" trace message to start with lowercase
 * Fixed redundant statfs() call in find_mount() (caller already had the
   result)
 * Fixed CMakeLists.txt GIT-BUILD-OPTIONS: was hardcoded to "win32" for
   FSMONITOR_DAEMON_BACKEND and FSMONITOR_OS_SETTINGS, now uses the CMake
   variables
 * Fixed uninitialized strset on trivial response path (STRSET_INIT)
 * Removed V9FS_MAGIC from get_fs_typename() to match is_remote_fs() (9p is
   local VM mounts)
 * Split 30-second stop timeout into its own commit per review request
 * Fixed misleading indentation on shutdown assignment in handle_events()
 * Updated commit messages to describe all changes (test hardening,
   fsmonitor-ipc.c spawn changes)
 * Updated Makefile comment for FSMONITOR_OS_SETTINGS to mention fsm-ipc

Changes since v6:

 * Introduced FSMONITOR_OS_SETTINGS build variable (set to "unix" for macOS
   and Linux, "win32" for Windows) to eliminate if/else conditionals in
   Makefile, meson.build, and CMakeLists.txt per Junio's review
 * Moved fsm-path-utils from FSMONITOR_OS_SETTINGS to
   FSMONITOR_DAEMON_BACKEND since path-utils files are platform-specific
 * Removed V9FS_MAGIC from remote filesystem detection (9p is used for local
   VM/container host mounts where fsmonitor works fine)
 * Removed redundant #include <libgen.h> (already provided by
   compat/posix.h)
 * Fixed cookie wait comment wording ("see" to "observe")
 * Rewrote commit messages for IPC and settings dedup patches

Changes since v5:

 * Split monolithic commit into 10-patch series per Patrick's review
 * Deduplicated fsm-ipc and fsm-settings into shared Unix implementations
 * Rewrote commit message with prose paragraphs, explain inotify vs
   fanotify, removed "Issues addressed" sections, added Based-on-patch-by
   trailers
 * Removed redundant includes already provided by compat/posix.h
 * Fixed error/trace message capitalization per coding guidelines
 * Fixed stale rename check interval from 1000 seconds to 1 second
 * Changed poll timeout from 1ms to 50ms to reduce idle CPU wake-ups
 * Replaced infinite pthread_cond_wait cookie loop with one-second
   pthread_cond_timedwait (prevents daemon hangs on overlay filesystems
   where events are never delivered)
 * Added pthread_cond_timedwait to Windows pthread compatibility layer
 * Separated test into its own commit with smoke test that skips when
   inotify events are not delivered (e.g., overlayfs with older kernels)
 * Fixed test hang on Fedora CI: stop_git() looped forever when ps was
   unavailable because bash in POSIX/sh mode returns exit 0 from kill with
   an empty process group argument. Fixed by falling back to /proc/$pid/stat
   for process group ID and guarding stop_git against empty pgid.
 * Redirect spawn_daemon() stdout/stderr to /dev/null and close inherited
   file descriptors to prevent the intermediate process from holding test
   pipe file descriptors
 * Call setsid() on daemon detach to prevent shells with job control from
   waiting on the daemon process group
 * Close inherited file descriptors 3-7 in the test watchdog subprocess
 * Added 30-second timeout to "fsmonitor--daemon stop" to prevent indefinite
   blocking
 * Added helpful error message when inotify watch limit (max_user_watches)
   is reached
 * Initialize fd_inotify to -1 and use fd >= 0 check for correct fd 0
   handling
 * Use sysconf(_SC_OPEN_MAX) instead of hardcoded 1024 for fd close limit
 * Check setsid() return value

Changes since v4:

 * Added Meson build support

Changes since v3:

 * Fix crash on rapid nested directory creation (EEXIST from
   inotify_add_watch with IN_MASK_CREATE)
 * Extensive stress testing

Changes since v2:

 * Fix khash memory leak in do_handle_client

Changes since v1:

 * Fix hashmap memory leak in fsmonitor_run_daemon()

Paul Tarjan (13):
  t9210, t9211: disable GIT_TEST_SPLIT_INDEX for scalar clone tests
  fsmonitor: fix khash memory leak in do_handle_client
  fsmonitor: fix hashmap memory leak in fsmonitor_run_daemon
  compat/win32: add pthread_cond_timedwait
  fsmonitor: use pthread_cond_timedwait for cookie wait
  fsmonitor: rename fsm-ipc-darwin.c to fsm-ipc-unix.c
  fsmonitor: rename fsm-settings-darwin.c to fsm-settings-unix.c
  fsmonitor: implement filesystem change listener for Linux
  run-command: add close_fd_above_stderr option
  fsmonitor: close inherited file descriptors and detach in daemon
  fsmonitor: add timeout to daemon stop command
  fsmonitor: add tests for Linux
  fsmonitor: convert shown khash to strset in do_handle_client

 Documentation/config/fsmonitor--daemon.adoc   |   4 +-
 Documentation/git-fsmonitor--daemon.adoc      |  28 +-
 Makefile                                      |   6 +-
 builtin/fsmonitor--daemon.c                   |  92 ++-
 compat/fsmonitor/fsm-health-linux.c           |  33 +
 .../{fsm-ipc-darwin.c => fsm-ipc-unix.c}      |   0
 compat/fsmonitor/fsm-listen-linux.c           | 746 ++++++++++++++++++
 compat/fsmonitor/fsm-path-utils-linux.c       | 217 +++++
 ...-settings-darwin.c => fsm-settings-unix.c} |   0
 compat/win32/pthread.c                        |  26 +
 compat/win32/pthread.h                        |   2 +
 config.mak.uname                              |  12 +-
 contrib/buildsystems/CMakeLists.txt           |  33 +-
 fsmonitor-ipc.c                               |   3 +
 meson.build                                   |  13 +-
 run-command.c                                 |  12 +
 run-command.h                                 |   9 +
 t/t7527-builtin-fsmonitor.sh                  |  89 ++-
 t/t9210-scalar.sh                             |   6 +
 t/t9211-scalar-clone.sh                       |   5 +
 20 files changed, 1273 insertions(+), 63 deletions(-)
 create mode 100644 compat/fsmonitor/fsm-health-linux.c
 rename compat/fsmonitor/{fsm-ipc-darwin.c => fsm-ipc-unix.c} (100%)
 create mode 100644 compat/fsmonitor/fsm-listen-linux.c
 create mode 100644 compat/fsmonitor/fsm-path-utils-linux.c
 rename compat/fsmonitor/{fsm-settings-darwin.c => fsm-settings-unix.c} (100%)


base-commit: 3e0db84c88c57e70ac8be8c196dfa92c5d656fbc
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2147%2Fptarjan%2Fclaude%2Fupdate-pr-1352-current-85Gk8-v14
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2147/ptarjan/claude/update-pr-1352-current-85Gk8-v14
Pull-Request: https://github.com/git/git/pull/2147

Range-diff vs v13:

  1:  28c5aca413 !  1:  721a951423 t9210: disable GIT_TEST_SPLIT_INDEX for scalar clone tests
     @@ Metadata
      Author: Paul Tarjan <github@paulisageek.com>
      
       ## Commit message ##
     -    t9210: disable GIT_TEST_SPLIT_INDEX for scalar clone tests
     +    t9210, t9211: disable GIT_TEST_SPLIT_INDEX for scalar clone tests
      
          index.skipHash (Scalar default) and split-index are incompatible:
          the shared index gets a null OID when skipHash skips computing the
     @@ Commit message
          index has.
      
          Disable GIT_TEST_SPLIT_INDEX in the scalar clone tests that hit
     -    this, matching the existing workaround in test 16.
     +    this: tests 12, 13, and 22 in t9210 (matching the existing
     +    workaround in test 16), and all of t9211 (every test does scalar
     +    clone).
      
          Signed-off-by: Paul Tarjan <github@paulisageek.com>
      
     @@ t/t9210-scalar.sh: test_expect_success '`scalar [...] <dir>` errors out when dir
       	scalar clone "file://$(pwd)" cloned --single-branch &&
       	git repack &&
       	echo "$(pwd)/.git/objects/" >>cloned/src/.git/objects/info/alternates &&
     +
     + ## t/t9211-scalar-clone.sh ##
     +@@ t/t9211-scalar-clone.sh: test_description='test the `scalar clone` subcommand'
     + GIT_TEST_MAINT_SCHEDULER="crontab:test-tool crontab cron.txt,launchctl:true,schtasks:true"
     + export GIT_TEST_MAINT_SCHEDULER
     + 
     ++# index.skipHash (Scalar default) and GIT_TEST_SPLIT_INDEX are
     ++# incompatible: the shared index gets a null OID and fails to
     ++# load on re-read.  Every test here uses scalar clone.
     ++sane_unset GIT_TEST_SPLIT_INDEX
     ++
     + test_expect_success 'set up repository to clone' '
     + 	rm -rf .git &&
     + 	git init to-clone &&
  2:  9f666beea7 =  2:  1283d25968 fsmonitor: fix khash memory leak in do_handle_client
  3:  a4a65a6dfa =  3:  11ba6ca9ac fsmonitor: fix hashmap memory leak in fsmonitor_run_daemon
  4:  427073fc38 =  4:  a0d430c2f4 compat/win32: add pthread_cond_timedwait
  5:  71effc4d47 =  5:  7b62c95c44 fsmonitor: use pthread_cond_timedwait for cookie wait
  6:  e897193b0e =  6:  7086cd4530 fsmonitor: rename fsm-ipc-darwin.c to fsm-ipc-unix.c
  7:  e13d938ddb =  7:  46e8c2b74f fsmonitor: rename fsm-settings-darwin.c to fsm-settings-unix.c
  8:  3431eae60f =  8:  b3f40a497b fsmonitor: implement filesystem change listener for Linux
  9:  7ce0ab87fb =  9:  5791edbef2 run-command: add close_fd_above_stderr option
 10:  2bf134a041 = 10:  22d425ebeb fsmonitor: close inherited file descriptors and detach in daemon
 11:  cb4d511f21 = 11:  fd6bdc8c55 fsmonitor: add timeout to daemon stop command
 12:  9a8647884e = 12:  f85983ca93 fsmonitor: add tests for Linux
 13:  a5fc9ad415 = 13:  2085b21e23 fsmonitor: convert shown khash to strset in do_handle_client

-- 
gitgitgadget

[PATCH v14 12/13] fsmonitor: add tests for Linux

[Paul Tarjan via GitGitGadget]

From: Paul Tarjan <github@paulisageek.com>

Add a smoke test that verifies the filesystem actually delivers
inotify events to the daemon.  On some configurations (e.g.,
overlayfs with older kernels), inotify watches succeed but events
are never delivered.  The daemon cookie wait will time out, but
every subsequent test would fail.  Skip the entire test file early
when this is detected.

Add a test that exercises rapid nested directory creation to verify
the daemon correctly handles the EEXIST race between recursive scan
and queued inotify events.  When IN_MASK_CREATE is available and a
directory watch is added during recursive registration, the kernel
may also deliver a queued IN_CREATE event for the same directory.
The second inotify_add_watch() returns EEXIST, which must be treated
as harmless.  An earlier version of the listener crashed in this
scenario.

Reduce --start-timeout from the default 60 seconds to 10 seconds so
that tests fail promptly when the daemon cannot start.

Harden the test helpers to work in environments without procps
(e.g., Fedora CI): fall back to reading /proc/$pid/stat for the
process group ID when ps is unavailable, guard stop_git() against
an empty pgid, and redirect stderr from kill to /dev/null to avoid
noise when processes have already exited.

Use set -m to enable job control in the submodule-pull test so that
the background git pull gets its own process group, preventing the
shell wait from blocking on the daemon.  setsid() in the previous
commit detaches the daemon itself, but the intermediate git pull
process still needs its own process group for the test shell to
manage it correctly.

Signed-off-by: Paul Tarjan <github@paulisageek.com>
---
 t/t7527-builtin-fsmonitor.sh | 89 +++++++++++++++++++++++++++++++++---
 1 file changed, 82 insertions(+), 7 deletions(-)

diff --git a/t/t7527-builtin-fsmonitor.sh b/t/t7527-builtin-fsmonitor.sh
index 409cd0cd12..774da5ac60 100755
--- a/t/t7527-builtin-fsmonitor.sh
+++ b/t/t7527-builtin-fsmonitor.sh
@@ -10,9 +10,58 @@ then
 	test_done
 fi
 
+# Verify that the filesystem delivers events to the daemon.
+# On some configurations (e.g., overlayfs with older kernels),
+# inotify watches succeed but events are never delivered.  The
+# cookie wait will time out and the daemon logs a trace message.
+#
+# Use "timeout" (if available) to guard each step against hangs.
+maybe_timeout () {
+	if type timeout >/dev/null 2>&1
+	then
+		timeout "$@"
+	else
+		shift
+		"$@"
+	fi
+}
+verify_fsmonitor_works () {
+	git init test_fsmonitor_smoke || return 1
+
+	GIT_TRACE_FSMONITOR="$PWD/smoke.trace" &&
+	export GIT_TRACE_FSMONITOR &&
+	maybe_timeout 30 \
+		git -C test_fsmonitor_smoke fsmonitor--daemon start \
+			--start-timeout=10
+	ret=$?
+	unset GIT_TRACE_FSMONITOR
+	if test $ret -ne 0
+	then
+		rm -rf test_fsmonitor_smoke smoke.trace
+		return 1
+	fi
+
+	maybe_timeout 10 \
+		test-tool -C test_fsmonitor_smoke fsmonitor-client query \
+			--token 0 >/dev/null 2>&1
+	maybe_timeout 5 \
+		git -C test_fsmonitor_smoke fsmonitor--daemon stop 2>/dev/null
+	! grep -q "cookie_wait timed out" "$PWD/smoke.trace" 2>/dev/null
+	ret=$?
+	rm -rf test_fsmonitor_smoke smoke.trace
+	return $ret
+}
+
+if ! verify_fsmonitor_works
+then
+	skip_all="filesystem does not deliver fsmonitor events (container/overlayfs?)"
+	test_done
+fi
+
 stop_daemon_delete_repo () {
 	r=$1 &&
-	test_might_fail git -C $r fsmonitor--daemon stop &&
+	test_might_fail maybe_timeout 30 \
+		git -C $r fsmonitor--daemon stop 2>/dev/null
 	rm -rf $1
 }
 
@@ -67,7 +116,7 @@ start_daemon () {
 			export GIT_TEST_FSMONITOR_TOKEN
 		fi &&
 
-		git $r fsmonitor--daemon start &&
+		git $r fsmonitor--daemon start --start-timeout=10 &&
 		git $r fsmonitor--daemon status
 	)
 }
@@ -520,6 +569,28 @@ test_expect_success 'directory changes to a file' '
 	grep "^event: dir1$" .git/trace
 '
 
+test_expect_success 'rapid nested directory creation' '
+	test_when_finished "git fsmonitor--daemon stop; rm -rf rapid" &&
+
+	start_daemon --tf "$PWD/.git/trace" &&
+
+	# Rapidly create nested directories to exercise race conditions
+	# where directory watches may be added concurrently during
+	# event processing and recursive scanning.
+	for i in $(test_seq 1 20)
+	do
+		mkdir -p "rapid/nested/dir$i/subdir/deep" || return 1
+	done &&
+
+	# Give the daemon time to process all events
+	sleep 1 &&
+
+	test-tool fsmonitor-client query --token 0 &&
+
+	# Verify daemon is still running (did not crash)
+	git fsmonitor--daemon status
+'
+
 # The next few test cases exercise the token-resync code.  When filesystem
 # drops events (because of filesystem velocity or because the daemon isn't
 # polling fast enough), we need to discard the cached data (relative to the
@@ -910,7 +981,10 @@ test_expect_success "submodule absorbgitdirs implicitly starts daemon" '
 start_git_in_background () {
 	git "$@" &
 	git_pid=$!
-	git_pgid=$(ps -o pgid= -p $git_pid)
+	git_pgid=$(ps -o pgid= -p $git_pid 2>/dev/null ||
+		awk '{print $5}' /proc/$git_pid/stat 2>/dev/null) &&
+	git_pgid="${git_pgid## }" &&
+	git_pgid="${git_pgid%% }"
 	nr_tries_left=10
 	while true
 	do
@@ -921,15 +995,16 @@ start_git_in_background () {
 		fi
 		sleep 1
 		nr_tries_left=$(($nr_tries_left - 1))
-	done >/dev/null 2>&1 &
+	done >/dev/null 2>&1 3>&- 4>&- 5>&- 6>&- 7>&- &
 	watchdog_pid=$!
 	wait $git_pid
 }
 
 stop_git () {
-	while kill -0 -- -$git_pgid
+	test -n "$git_pgid" || return 0
+	while kill -0 -- -$git_pgid 2>/dev/null
 	do
-		kill -- -$git_pgid
+		kill -- -$git_pgid 2>/dev/null
 		sleep 1
 	done
 }
@@ -944,7 +1019,7 @@ stop_watchdog () {
 
 test_expect_success !MINGW "submodule implicitly starts daemon by pull" '
 	test_atexit "stop_watchdog" &&
-	test_when_finished "stop_git; rm -rf cloned super sub" &&
+	test_when_finished "set +m; stop_git; rm -rf cloned super sub" &&
 
 	create_super super &&
 	create_sub sub &&
-- 
gitgitgadget

Re: [PATCH v14 12/13] fsmonitor: add tests for Linux

[SZEDER Gábor]

On Thu, Apr 09, 2026 at 04:59:34AM +0000, Paul Tarjan via GitGitGadget wrote:
> From: Paul Tarjan <github@paulisageek.com>
> 
> Add a smoke test that verifies the filesystem actually delivers
> inotify events to the daemon.  On some configurations (e.g.,
> overlayfs with older kernels), inotify watches succeed but events
> are never delivered.  The daemon cookie wait will time out, but
> every subsequent test would fail.  Skip the entire test file early
> when this is detected.
> 
> Add a test that exercises rapid nested directory creation to verify
> the daemon correctly handles the EEXIST race between recursive scan
> and queued inotify events.  When IN_MASK_CREATE is available and a
> directory watch is added during recursive registration, the kernel
> may also deliver a queued IN_CREATE event for the same directory.
> The second inotify_add_watch() returns EEXIST, which must be treated
> as harmless.  An earlier version of the listener crashed in this
> scenario.
> 
> Reduce --start-timeout from the default 60 seconds to 10 seconds so
> that tests fail promptly when the daemon cannot start.
> 
> Harden the test helpers to work in environments without procps
> (e.g., Fedora CI): fall back to reading /proc/$pid/stat for the
> process group ID when ps is unavailable, guard stop_git() against
> an empty pgid, and redirect stderr from kill to /dev/null to avoid
> noise when processes have already exited.
> 
> Use set -m to enable job control in the submodule-pull test so that
> the background git pull gets its own process group, preventing the
> shell wait from blocking on the daemon.  setsid() in the previous
> commit detaches the daemon itself, but the intermediate git pull
> process still needs its own process group for the test shell to
> manage it correctly.
> 
> Signed-off-by: Paul Tarjan <github@paulisageek.com>
> ---
>  t/t7527-builtin-fsmonitor.sh | 89 +++++++++++++++++++++++++++++++++---
>  1 file changed, 82 insertions(+), 7 deletions(-)
> 
> diff --git a/t/t7527-builtin-fsmonitor.sh b/t/t7527-builtin-fsmonitor.sh
> index 409cd0cd12..774da5ac60 100755
> --- a/t/t7527-builtin-fsmonitor.sh
> +++ b/t/t7527-builtin-fsmonitor.sh
> @@ -10,9 +10,58 @@ then
>  	test_done
>  fi
>  
> +# Verify that the filesystem delivers events to the daemon.
> +# On some configurations (e.g., overlayfs with older kernels),
> +# inotify watches succeed but events are never delivered.  The
> +# cookie wait will time out and the daemon logs a trace message.
> +#
> +# Use "timeout" (if available) to guard each step against hangs.
> +maybe_timeout () {
> +	if type timeout >/dev/null 2>&1
> +	then
> +		timeout "$@"
> +	else
> +		shift
> +		"$@"
> +	fi
> +}
> +verify_fsmonitor_works () {
> +	git init test_fsmonitor_smoke || return 1
> +
> +	GIT_TRACE_FSMONITOR="$PWD/smoke.trace" &&
> +	export GIT_TRACE_FSMONITOR &&
> +	maybe_timeout 30 \
> +		git -C test_fsmonitor_smoke fsmonitor--daemon start \
> +			--start-timeout=10
> +	ret=$?
> +	unset GIT_TRACE_FSMONITOR
> +	if test $ret -ne 0
> +	then
> +		rm -rf test_fsmonitor_smoke smoke.trace
> +		return 1
> +	fi
> +
> +	maybe_timeout 10 \
> +		test-tool -C test_fsmonitor_smoke fsmonitor-client query \
> +			--token 0 >/dev/null 2>&1
> +	maybe_timeout 5 \
> +		git -C test_fsmonitor_smoke fsmonitor--daemon stop 2>/dev/null
> +	! grep -q "cookie_wait timed out" "$PWD/smoke.trace" 2>/dev/null
> +	ret=$?
> +	rm -rf test_fsmonitor_smoke smoke.trace
> +	return $ret
> +}
> +
> +if ! verify_fsmonitor_works
> +then
> +	skip_all="filesystem does not deliver fsmonitor events (container/overlayfs?)"
> +	test_done
> +fi
> +
>  stop_daemon_delete_repo () {
>  	r=$1 &&
> -	test_might_fail git -C $r fsmonitor--daemon stop &&
> +	test_might_fail maybe_timeout 30 \
> +		git -C $r fsmonitor--daemon stop 2>/dev/null

"test_might_fail" only allows a few select commands and functions, and
the "maybe_timeout" helper function introduced in this patch is, of
course, not one of them, so it returns with error and without running
the given command.  Consequently, after this test script is finished I
have still two fsmonitor daemon processes running in the background.

Alas, this went unnoticed, because this patch broke the &&-chain and
redirected "test_might_fail"'s

  test_must_fail: only 'git' is allowed: maybe_timeout 30 git -C test_explicit fsmonitor--daemon stop

error messages to /dev/null.  With the &&-chain restored over 40 test
cases fail because of this.

>  	rm -rf $1
>  }
>  
> @@ -67,7 +116,7 @@ start_daemon () {
>  			export GIT_TEST_FSMONITOR_TOKEN
>  		fi &&
>  
> -		git $r fsmonitor--daemon start &&
> +		git $r fsmonitor--daemon start --start-timeout=10 &&
>  		git $r fsmonitor--daemon status
>  	)
>  }
> @@ -520,6 +569,28 @@ test_expect_success 'directory changes to a file' '
>  	grep "^event: dir1$" .git/trace
>  '
>  
> +test_expect_success 'rapid nested directory creation' '
> +	test_when_finished "git fsmonitor--daemon stop; rm -rf rapid" &&
> +
> +	start_daemon --tf "$PWD/.git/trace" &&
> +
> +	# Rapidly create nested directories to exercise race conditions
> +	# where directory watches may be added concurrently during
> +	# event processing and recursive scanning.
> +	for i in $(test_seq 1 20)
> +	do
> +		mkdir -p "rapid/nested/dir$i/subdir/deep" || return 1
> +	done &&
> +
> +	# Give the daemon time to process all events
> +	sleep 1 &&
> +
> +	test-tool fsmonitor-client query --token 0 &&
> +
> +	# Verify daemon is still running (did not crash)
> +	git fsmonitor--daemon status
> +'
> +
>  # The next few test cases exercise the token-resync code.  When filesystem
>  # drops events (because of filesystem velocity or because the daemon isn't
>  # polling fast enough), we need to discard the cached data (relative to the
> @@ -910,7 +981,10 @@ test_expect_success "submodule absorbgitdirs implicitly starts daemon" '
>  start_git_in_background () {
>  	git "$@" &
>  	git_pid=$!
> -	git_pgid=$(ps -o pgid= -p $git_pid)
> +	git_pgid=$(ps -o pgid= -p $git_pid 2>/dev/null ||
> +		awk '{print $5}' /proc/$git_pid/stat 2>/dev/null) &&
> +	git_pgid="${git_pgid## }" &&
> +	git_pgid="${git_pgid%% }"
>  	nr_tries_left=10
>  	while true
>  	do
> @@ -921,15 +995,16 @@ start_git_in_background () {
>  		fi
>  		sleep 1
>  		nr_tries_left=$(($nr_tries_left - 1))
> -	done >/dev/null 2>&1 &
> +	done >/dev/null 2>&1 3>&- 4>&- 5>&- 6>&- 7>&- &
>  	watchdog_pid=$!
>  	wait $git_pid
>  }
>  
>  stop_git () {
> -	while kill -0 -- -$git_pgid
> +	test -n "$git_pgid" || return 0
> +	while kill -0 -- -$git_pgid 2>/dev/null
>  	do
> -		kill -- -$git_pgid
> +		kill -- -$git_pgid 2>/dev/null
>  		sleep 1
>  	done
>  }
> @@ -944,7 +1019,7 @@ stop_watchdog () {
>  
>  test_expect_success !MINGW "submodule implicitly starts daemon by pull" '
>  	test_atexit "stop_watchdog" &&
> -	test_when_finished "stop_git; rm -rf cloned super sub" &&
> +	test_when_finished "set +m; stop_git; rm -rf cloned super sub" &&
>  
>  	create_super super &&
>  	create_sub sub &&
> -- 
> gitgitgadget
> 

Re: [PATCH v14 12/13] fsmonitor: add tests for Linux

[Junio C Hamano]

SZEDER Gábor <szeder.dev@gmail.com> writes:

>>  stop_daemon_delete_repo () {
>>  	r=$1 &&
>> -	test_might_fail git -C $r fsmonitor--daemon stop &&
>> +	test_might_fail maybe_timeout 30 \
>> +		git -C $r fsmonitor--daemon stop 2>/dev/null
>
> "test_might_fail" only allows a few select commands and functions, and
> the "maybe_timeout" helper function introduced in this patch is, of
> course, not one of them, so it returns with error and without running
> the given command.  Consequently, after this test script is finished I
> have still two fsmonitor daemon processes running in the background.
>
> Alas, this went unnoticed, because this patch broke the &&-chain and
> redirected "test_might_fail"'s
>
>   test_must_fail: only 'git' is allowed: maybe_timeout 30 git -C test_explicit fsmonitor--daemon stop
>
> error messages to /dev/null.  With the &&-chain restored over 40 test
> cases fail because of this.
>
>>  	rm -rf $1
>>  }

Yikes.  Would it help to apply a patch like this, then?

 t/t7527-builtin-fsmonitor.sh | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git c/t/t7527-builtin-fsmonitor.sh w/t/t7527-builtin-fsmonitor.sh
index 774da5ac60..dfa06395f6 100755
--- c/t/t7527-builtin-fsmonitor.sh
+++ w/t/t7527-builtin-fsmonitor.sh
@@ -60,8 +60,7 @@ fi
 
 stop_daemon_delete_repo () {
 	r=$1 &&
-	test_might_fail maybe_timeout 30 \
-		git -C $r fsmonitor--daemon stop 2>/dev/null
+	maybe_timeout 30 git -C $r fsmonitor--daemon stop || : &&
 	rm -rf $1
 }
 

Re: [PATCH v14 12/13] fsmonitor: add tests for Linux

[Jeff King]

On Tue, Apr 14, 2026 at 01:40:38PM -0700, Junio C Hamano wrote:

> diff --git c/t/t7527-builtin-fsmonitor.sh w/t/t7527-builtin-fsmonitor.sh
> index 774da5ac60..dfa06395f6 100755
> --- c/t/t7527-builtin-fsmonitor.sh
> +++ w/t/t7527-builtin-fsmonitor.sh
> @@ -60,8 +60,7 @@ fi
>  
>  stop_daemon_delete_repo () {
>  	r=$1 &&
> -	test_might_fail maybe_timeout 30 \
> -		git -C $r fsmonitor--daemon stop 2>/dev/null
> +	maybe_timeout 30 git -C $r fsmonitor--daemon stop || : &&
>  	rm -rf $1
>  }

Do we need to put it in curly braces to avoid interfering with the &&
chain? Otherwise a failure of anything before the maybe_timeout will hit
the "||".

I guess in this case it is just "r=$1", which will never fail, but it
feels like we should model best practice.

-Peff