All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Paul Tarjan via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Claude <noreply@anthropic.com>
Subject: [PATCH] fsmonitor: fix two bugs in watchman hook retry path
Date: Mon, 26 Jan 2026 01:33:22 +0000	[thread overview]
Message-ID: <pull.2180.git.git.1769391202338.gitgitgadget@gmail.com> (raw)

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

             reply	other threads:[~2026-01-26  1:33 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-26  1:33 Paul Tarjan via GitGitGadget [this message]
2026-02-08  6:03 ` [PATCH] fsmonitor: fix two bugs in watchman hook retry path Paul Tarjan
2026-02-27  6:25   ` [PATCH] fsmonitor-watchman: fix variable reference and remove redundant code Paul Tarjan
2026-02-28 17:02 ` [PATCH] fsmonitor: fix two bugs in watchman hook retry path Koji Nakamaru
2026-02-28 17:33   ` [PATCH] fsmonitor-watchman: fix variable reference and remove redundant code Paul Tarjan
2026-02-28 17:15 ` [PATCH] fsmonitor: fix two bugs in watchman hook retry path brian m. carlson
2026-02-28 17:33   ` [PATCH] fsmonitor-watchman: fix variable reference and remove redundant code Paul Tarjan
2026-02-28 17:37 ` [PATCH v2] " Paul Tarjan via GitGitGadget

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=pull.2180.git.git.1769391202338.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=noreply@anthropic.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 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.