fsmonitor-watchman: fix variable reference and remove redundant code#2180
Open
ptarjan wants to merge 1 commit intogit:masterfrom
Open
fsmonitor-watchman: fix variable reference and remove redundant code#2180ptarjan wants to merge 1 commit intogit:masterfrom
ptarjan wants to merge 1 commit intogit:masterfrom
Conversation
|
There is an issue in commit 041f96c:
|
e960026 to
116d262
Compare
Contributor
Author
|
/submit |
|
Submitted as pull.2180.git.git.1769391202338.gitgitgadget@gmail.com To fetch this version into To fetch this version to local tag WARNING: ptarjan has no public email address set on GitHub; GitGitGadget needs an email address to Cc: you on your contribution, so that you receive any feedback on the Git mailing list. Go to https://github.com/settings/profile to make your preferred email public to let GitGitGadget know which email address to use. |
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/.
Cc: Paul Tarjan <github@paulisageek.com>
Signed-off-by: Paul Tarjan <github@paulisageek.com>
116d262 to
4c4084f
Compare
Contributor
Author
|
Is there anything more you need from me for this change? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The is_work_tree_watched() function in fsmonitor-watchman.sample has two bugs:
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.
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.