> -----Original Message-----
> From: Ben Peart [mailto:benpe...@microsoft.com]
> Sent: Friday, September 15, 2017 3:21 PM
> To: benpe...@microsoft.com
> Cc: David Turner <david.tur...@twosigma.com>; ava...@gmail.com;
> christian.cou...@gmail.com; git@vger.kernel.org; gits...@pobox.com;
> johannes.schinde...@gmx.de; pclo...@gmail.com; p...@peff.net
> Subject: [PATCH v6 04/12] fsmonitor: teach git to optionally utilize a file
> system
> monitor to speed up detecting new or changed files.
> +int git_config_get_fsmonitor(void)
> +{
> + if (git_config_get_pathname("core.fsmonitor", &core_fsmonitor))
> + core_fsmonitor = getenv("GIT_FSMONITOR_TEST");
> +
> + if (core_fsmonitor && !*core_fsmonitor)
> + core_fsmonitor = NULL;
> +
> + if (core_fsmonitor)
> + return 1;
> +
> + return 0;
> +}
This functions return values are backwards relative to the rest of the
git_config_* functions.
[snip]
+> /*
+> * With fsmonitor, we can trust the untracked cache's valid field.
+> */
[snip]
> +int read_fsmonitor_extension(struct index_state *istate, const void *data,
> + unsigned long sz)
> +{
If git_config_get_fsmonitor returns 0, fsmonitor_dirty will leak.
[snip]
> + /* a fsmonitor process can return '*' to indicate all entries are
> invalid */
That's not documented in your documentation. Also, I'm not sure I like it:
what
if I have a file whose name starts with '*'? Yeah, that would be silly, but
this indicates the need
for the protocol to have some sort of signaling mechanism that's out-of-band
Maybe
have some key\0value\0 pairs and then \0\0 and then the list of files? Or, if
you want to keep
it really simple, allow an entry of '/' (which is an invalid filename) to mean
'all'.
> +void add_fsmonitor(struct index_state *istate) {
> + int i;
> +
> + if (!istate->fsmonitor_last_update) {
[snip]
> + /* reset the untracked cache */
Is this really necessary? Shouldn't the untracked cache be in a correct state
already?
> +/*
> + * Clear the given cache entries CE_FSMONITOR_VALID bit and invalidate
Nit: "s/entries/entry's/".
