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

On Fri, Mar 12, 2021 at 11:51:38AM -0500, Jeff Hostetler wrote:

I don't think it makes sense to have multiple fsmonitors for a given
working directory.  They are fairly expensive to operate (listening
to the kernel for events and building an in-memory database of changed
files) and I'm not sure how two running concurrently (and listening to
the same kernel event stream) would come up with different answers.

The thing about the fsmonitor-watchman or query-watchman hook is that
it is a bash/perl script that talks to a long-running service daemon.
The hook script itself is just a proxy to decode the JSON response from
Watchman and emit it on stdout in a way that the foreground Git command
expects.  Git cannot directly talk to Watchman because it doesn't
currently have the plumbing to talk to it on anything other than a fd
pair that it sets up to give to the hook child.

So your example of a watcher for NTFS and a separate watcher for ext4
means you could maybe have two services running, but the foreground
Git command would only need to spawn a single hook and maybe it would
decide which service to talk to based upon the filesystem type of the
working directory.  Or you set the repo-local config for each repo to
point to a hook script that knew which service to talk to.  Either way
you only need to invoke one hook.

Thanks, that makes a lot of sense!

Setting it globally is an option, but fsmonitor is beneficial for large
repos and working directories.  There is an overhead to having it
running and talking to it.  (Spawning a hook written in PERL, having
the hook talk to Watchman via some form of IPC, the hook parsing the mess of
JSON returned, pumping that data back over stdout to Git, and
having Git apply the results to the ce_flags.)  All of that has to
happen *before* Git actually starts to do anything useful.  For small
repos, all of that overhead costs more than the cost of letting the
foreground `git status` just lstat() everything.  Of course all of this
depends on the speed of your filesystem and the size of your working
directory (and whether you have a sparse-checkout and etc), but there
are lots of repos that just don't need fsmonitor.

So yes, I would leave the existing fsmonitor code as is and not try
to combine it with your current efforts (even if I wasn't working on
a replacement for the fsmonitor-watchman setup).

As Stolee mentioned I'm currently working on a builtin fsmonitor
feature.  It will have a native coded-in-Git-C-code daemon to watch
the filesystem.  Clients, such as `git status`, will directly talk
to it via my "Simple IPC" patch series and completely bypass all of
the hook stuff.

Long term both fsmonitor solutions can co-exist.  Or we can eventually
deprecate the hook version.  Given that, I don't see a need to change
the existing fsmonitor hook code.

Yeah, that seems like the direction everyone agrees with. Thanks very
much for the detailed explanation, that helped me feel sure that doing
nothing is the right approach (how convenient...) :)

I think, then, all that's needed is a patch to the githooks.txt doc 1)
removing the incorrect info about core.fsmonitor's contents needing to
point to a specific name in .git/hooks/ and 2) explaining that because
it uses this special config, it doesn't use the usual hook.*.command
approach.

 - Emily