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@xxxxxxxxxx. 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. 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@xxxxxxxxxx) 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: Q1: Does fsmonitor-watchman make sense to provide more than once (since config-based hooks support multiple execs per hook)? (I get the feeling the answer is no here - but what if, for example, I have an NTFS partition with some repos and an ext4 partition with some other repos, and only want to configure fsmonitor magic once at global scope? I don't know anything about fsmonitor so maybe that is a very stupid question ;) ) Q2: Does it make sense to specify anywhere other than per-repo scope? (system/global scope, e.g., applying to all repos? One could imagine providing it in a config which is inherited by submodules of a superproject?) It looks like it operates on CWD + timestamp-ish, so it seems to me like some user might want to only set it up once (e.g. globally) and have it Just Work in all their repos afterwards. In fact, I think you could configure core.fsmonitor in ~/.gitconfig today and it would work for everything. If we want to support checking vs multiple fsmonitor-watchman execs (that is, if Q1's answer is "yes"), then I propose: 1. Check for values on 'hook.fsmonitor-watchman.command' and add them to the execution list in config order. 1a. Here we could either check for 'hookcmd.<command-value>.fsmonitor-watchman-version' or we could just check separately for 'hook.fsmonitor-watchmanv2.command' as implied by 'githooks.txt' to determine the arg syntax for each hook. 2. Check for values on 'core.fsmonitor' and 'core.fsmonitorhookversion' to add a "legacy" hook at the end of the execution list. (This is the same thing we do for hooks that live in .git/hooks/<hookname>.) If we don't want to support adding multiple fsmonitor-watchman execs, then I propose: 1. Check hook.runHookDir's value to decide whether we believe in "legacy" hooks. 2. If we do, then check for core.fsmonitor and core.fsmonitorhookversion, and run that one. 3. If we don't, or if core.fsmonitor was unset, then check for (hook.fsmonitor-watchman + hookcmd.<command-value>.fsmonitor-watchman-version) or (hook.fsmonitor-watchman.command or hook.fsmonitor-watchmanv2.command), error if we find more than one candidate, and run the one candidate we do find. (Or I could see us running only the most-local one, but that might be confusing for the user.) Thanks, all - I'm curious to learn more about this hook and the right way to move forward :) - Emily
