git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* What to do with fsmonitor-watchman hook and config-based hooks?
@ 2021-03-11 18:42 Emily Shaffer
  2021-03-11 19:23 ` Derrick Stolee
  0 siblings, 1 reply; 6+ messages in thread
From: Emily Shaffer @ 2021-03-11 18:42 UTC (permalink / raw)
  To: Git List
  Cc: Jonathan Nieder, Jonathan Tan, Kevin Willford, Derrick Stolee,
	Jeff Hostetler, Jeff King

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.

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:

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-03-12 20:34 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-11 18:42 What to do with fsmonitor-watchman hook and config-based hooks? Emily Shaffer
2021-03-11 19:23 ` Derrick Stolee
2021-03-11 19:36   ` Emily Shaffer
2021-03-12 16:51     ` Jeff Hostetler
2021-03-12 18:33       ` Ævar Arnfjörð Bjarmason
2021-03-12 20:33       ` Emily Shaffer

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).