Re: What to do with fsmonitor-watchman hook and config-based hooks?

On 3/11/2021 1:42 PM, Emily Shaffer wrote:

Hi folks, I grabbed a bunch of CC from 'git blame fsmonitor.c' so
sorry if you don't care about fsmonitor-watchman anymore... :) Note
that this whole conversation has to do with the series proposed at
https://lore.kernel.org/git/20210311021037.3001235-1-emilyshaffer@google.com (local).

When I was looking through the remaining hooks in
Documentation/githooks.txt I noticed that the fsmonitor-watchman hook
is implemented somewhat differently than most other hooks. As I
understand it, to use that hook someone needs to:

1. Configure core.fsmonitor with a path to some fsmonitor-watchman
hook. The documentation in 'Documentation/githooks.txt' claims that it
needs to point to '.git/hooks/fsmonitor-watchman' or
'.git/hooks/fsmonitor-watchmanv2', but I don't see that constraint
enforced when the config is read (config.c:git_config_get_fsmonitor()
and fsmonitor.c:query_fsmonitor()), so it seems that core.fsmonitor
can point to wherever it wants. (Plus
'templates/blt/hooks/fsmonitor-watchman.sample' suggests setting
'core.fsmonitor' = '.git/hooks/query-watchman'...)
2. Configure core.fsmonitorhookversion to 1 or 2, to indicate the arg
structure for the executable specified in core.fsmonitor.

This is correct.

Because the executable doesn't necessarily live in .git/hooks/,
fsmonitor.c:query_fsmonitor() completely skips the "API" for running
hooks (run-command.h:run_hook_le()) and just uses
run-command.h:capture_command() directly.

Interestingly, this is really similar to the way config-based hooks
(https://lore.kernel.org/git/20210311021037.3001235-1-emilyshaffer@google.com (local))
work - but different enough that I think it may be awkward to
transition fsmonitor-watchman to work like everything else. So, some
questions, plus a proposal:

You'll want to get Jeff Hostetler's perspective first, but I'm of
the opinion that we'll want to stop recommending the Watchman hook
when the Git-native FS Monitor feature lands, with some time to
let things release and simmer before we remove the core.fsmonitor
config option. We would also need a Linux implementation, but that
is planned.

If we think that the plan of "eventually, FS Monitor won't use hooks"
is reasonable, then how much do you want to spend time unifying it
with your config-based hooks? Can they live together temporarily?

(Naturally, deprecating FS Monitor through the hook might not be
a reasonable plan.)

Thanks,
-Stolee