git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH] fsmonitor: fix two bugs in watchman hook retry path

From
PGPaul Tarjan via GitGitGadget <gitgitgadget@gmail.com>
Date
Jan 26, 2026, 01:33 UTC
Message-ID
<pull.2180.git.git.1769391202338.gitgitgadget@gmail.com>
From: Claude <noreply@anthropic.com>

The is_work_tree_watched() function in fsmonitor-watchman.sample has two bugs:

1. Wrong variable in error check: After calling watchman_clock(), the
   result is stored in $o, but the code checks $output->{error} instead
   of $o->{error}. This means errors from the clock command are silently
   ignored.
2. Double output violates protocol: When the retry path triggers (the
   directory wasn't initially watched), output_result() is called with
   the "/" flag, then launch_watchman() is called recursively which
   calls output_result() again. This outputs two clock tokens to stdout,
   but git's fsmonitor v2 protocol expects exactly one response.
Fix #1 by checking $o->{error} after watchman_clock().

Fix #2 by removing the recursive launch_watchman() call. The "/" "everything is dirty" flag already tells git to do a full scan, and git will call the hook again on the next invocation with a valid clock token.

Apply the same fixes to the test helper scripts in t/t7519/.
Signed-off-by: Paul Tarjan <github@paulisageek.com>
---
    fsmonitor-watchman: fix variable reference and remove redundant code
    
    The is_work_tree_watched() function in fsmonitor-watchman.sample has two
    bugs:
    
     1. Wrong variable in error check: After calling watchman_clock(), the
        result is stored in $o, but the code checks $output->{error} instead
        of $o->{error}. This means errors from the clock command are
        silently ignored.
    
     2. Double output violates protocol: When the retry path triggers (the
        directory wasn't initially watched), output_result() is called with
        the "/" flag, then launch_watchman() is called recursively which
        calls output_result() again. This outputs two clock tokens to
        stdout, but git's fsmonitor v2 protocol expects exactly one
        response.
    
    Fix #1 by checking $o->{error} after watchman_clock().
    
    Fix #2 by removing the recursive launch_watchman() call. The "/"
    "everything is dirty" flag already tells git to do a full scan, and git
    will call the hook again on the next invocation with a valid clock
    token.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2180%2Fptarjan%2Fclaude%2Ffix-watchman-query-bug-sfbIw-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2180/ptarjan/claude/fix-watchman-query-bug-sfbIw-v1
Pull-Request: https://github.com/git/git/pull/2180
 t/t7519/fsmonitor-watchman                | 1 -
 t/t7519/fsmonitor-watchman-v2             | 5 +----
 templates/hooks/fsmonitor-watchman.sample | 5 +----
 3 files changed, 2 insertions(+), 9 deletions(-)
diff --git a/t/t7519/fsmonitor-watchman b/t/t7519/fsmonitor-watchman
index 264b9daf83..907bd1533c 100755
--- a/t/t7519/fsmonitor-watchman
+++ b/t/t7519/fsmonitor-watchman
@@ -109,7 +109,6 @@ sub launch_watchman {
 		close $fh;
 
 		print "/\0";
-		eval { launch_watchman() };
 		exit 0;
 	}
 
diff --git a/t/t7519/fsmonitor-watchman-v2 b/t/t7519/fsmonitor-watchman-v2
index 14ed0aa42d..2c59120c3d 100755
--- a/t/t7519/fsmonitor-watchman-v2
+++ b/t/t7519/fsmonitor-watchman-v2
@@ -141,15 +141,12 @@ sub is_work_tree_watched {
 		# Watchman query just to get it over with now so we won't pay
 		# the cost in git to look up each individual file.
 		my $o = watchman_clock();
-		$error = $output->{error};
+		$error = $o->{error};
 
 		die "Watchman: $error.\n" .
 		"Falling back to scanning...\n" if $error;
 
 		output_result($o->{clock}, ("/"));
-		$last_update_token = $o->{clock};
-
-		eval { launch_watchman() };
 		return 0;
 	}
 
diff --git a/templates/hooks/fsmonitor-watchman.sample b/templates/hooks/fsmonitor-watchman.sample
index 23e856f5de..21c81b6804 100755
--- a/templates/hooks/fsmonitor-watchman.sample
+++ b/templates/hooks/fsmonitor-watchman.sample
@@ -142,15 +142,12 @@ sub is_work_tree_watched {
 		# Watchman query just to get it over with now so we won't pay
 		# the cost in git to look up each individual file.
 		my $o = watchman_clock();
-		$error = $output->{error};
+		$error = $o->{error};
 
 		die "Watchman: $error.\n" .
 		"Falling back to scanning...\n" if $error;
 
 		output_result($o->{clock}, ("/"));
-		$last_update_token = $o->{clock};
-
-		eval { launch_watchman() };
 		return 0;
 	}
 

base-commit: 68cb7f9e92a5d8e9824f5b52ac3d0a9d8f653dbe
-- 
gitgitgadget
Next: Paul Tarjan
Message 1 of 8 in “fsmonitor: fix two bugs in watchman hook retry path”
  1. fsmonitor: fix two bugs in watchman hook retry pathPaul Tarjan via GitGitGadget, Jan 26, 2026
  2. Paul TarjanFeb 8, 2026
  3. Paul TarjanFeb 27, 2026
  4. Koji NakamaruFeb 28, 2026
  5. Paul TarjanFeb 28, 2026
  6. brian m. carlsonFeb 28, 2026
  7. Paul TarjanFeb 28, 2026
  8. fsmonitor-watchman: fix variable reference and remove redundant codePaul Tarjan via GitGitGadget, Feb 28, 2026

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.