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

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

From
Emily Shaffer <emilyshaffer@google.com>
Date
Mar 11, 2021, 19:36 UTC
Message-ID
<YEpxVELGCxtnNxQK@google.com>
In-Reply-To
<33a12a7a-d19c-63b8-f21e-db7e517b0f53@gmail.com>
On Thu, Mar 11, 2021 at 02:23:03PM -0500, Derrick Stolee wrote:
Show 47 quoted lines
> 
> 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.
> > 
> > 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)
> > 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?

Oh, that's useful context. If fsmonitor-watchman hook is going away, I don't think it's necessary to convert it at all, unless someone starts asking for multihooks or something. There's no practical conflict between config-based hooks and the current implementation - it's just a surprising inconsistency. I'll be curious to hear Jeff's opinion, of course, but given what you're describing, I'm not convinced it's worth spending any time on - and when we're ready to stop checking core.fsmonitor then the inconsistency will just go away.

The documentation in githooks.txt could use an update, though. :)
 - Emily
Previous: Derrick StoleeNext: Jeff Hostetler
Message 3 of 6 in “What to do with fsmonitor-watchman hook and config-based hooks?”
  1. Emily ShafferMar 11, 2021
  2. Derrick StoleeMar 11, 2021
  3. Emily ShafferMar 11, 2021
  4. Jeff HostetlerMar 12, 2021
  5. Ævar Arnfjörð BjarmasonMar 12, 2021
  6. Emily ShafferMar 12, 2021

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.