Re: [PATCH v5 12/30] fsmonitor--daemon: add pathname classification

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Jeff,

On Fri, 11 Feb 2022, Jeff Hostetler via GitGitGadget wrote:

> +enum fsmonitor_path_type fsmonitor_classify_path_workdir_relative(
> +	const char *rel)
> +{
> +	if (fspathncmp(rel, ".git", 4))
> +		return IS_WORKDIR_PATH;
> +	rel += 4;
> +
> +	if (!*rel)
> +		return IS_DOT_GIT;
> +	if (*rel != '/')
> +		return IS_WORKDIR_PATH; /* e.g. .gitignore */
> +	rel++;
> +
> +	if (!fspathncmp(rel, FSMONITOR_COOKIE_PREFIX,
> +			strlen(FSMONITOR_COOKIE_PREFIX)))
> +		return IS_INSIDE_DOT_GIT_WITH_COOKIE_PREFIX;
> +
> +	return IS_INSIDE_DOT_GIT;
> +}
> +
> +enum fsmonitor_path_type fsmonitor_classify_path_gitdir_relative(
> +	const char *rel)
> +{
> +	if (!fspathncmp(rel, FSMONITOR_COOKIE_PREFIX,
> +			strlen(FSMONITOR_COOKIE_PREFIX)))
> +		return IS_INSIDE_GITDIR_WITH_COOKIE_PREFIX;
> +
> +	return IS_INSIDE_GITDIR;

At first, I was puzzled why this is not `IS_INSIDE_DOT_GIT` as above, but
then...

> [...]
> diff --git a/fsmonitor--daemon.h b/fsmonitor--daemon.h
> index 3009c1a83de..7bbb3a27a1c 100644
> --- a/fsmonitor--daemon.h
> +++ b/fsmonitor--daemon.h
> @@ -30,5 +30,66 @@ struct fsmonitor_daemon_state {
>  	struct ipc_server_data *ipc_server_data;
>  };
>
> +/*
> + * Pathname classifications.
> + *
> + * The daemon classifies the pathnames that it receives from file
> + * system notification events into the following categories and uses
> + * that to decide whether clients are told about them.  (And to watch
> + * for file system synchronization events.)
> + *
> + * The client should only care about paths within the working
> + * directory proper (inside the working directory and not ".git" nor
> + * inside of ".git/").  That is, the client has read the index and is
> + * asking for a list of any paths in the working directory that have
> + * been modified since the last token.  The client does not care about
> + * file system changes within the .git directory (such as new loose
> + * objects or packfiles).  So the client will only receive paths that
> + * are classified as IS_WORKDIR_PATH.
> + *
> + * The daemon uses the IS_DOT_GIT and IS_GITDIR internally to mean the
> + * exact ".git" directory or GITDIR.  If the daemon receives a delete

... I read this, and it started to click.

However, may I suggest to point out that ".git" can also be a _file_, not
only a directory? Otherwise it would not make sense to distinguish between
the `.git` directory/file and `GITDIR`, I'd say.

The rest of the patch looks good to me.

Thanks!
Dscho