From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Wed, 31 Aug 2022 19:37:53 GMT
Subject: Re: [PATCH v4 4/4] fsmonitor: normalize FSEvents event paths to the real path
Message-ID: <220831.86ler48apq.gmgdl@evledraar.gmail.com>
In-Reply-To: <56cabf3be3b9e228bc948da372db4c9d11fd3926.1661962145.git.gitgitgadget@gmail.com>


On Wed, Aug 31 2022, Eric DeCosta via GitGitGadget wrote:

> From: Eric DeCosta <edecosta@mathworks.com>
>
> Consider the following network working directory that is mounted under
> /System/Volumes/Data:
>
> /network/working/directory
>
> The git working directory path is:
>
> /System/Volumes/Data/network/working/directory
>
> The paths reported by FSEvents always start with /network. fsmonitor
> expects paths to be under the working directory; therefore it
> fails to match /network/... and ignores the change.
>
> Change things such that if fsmonitor.allowRemote is true that the
> paths reported via FSEevents are normalized to the real path.
>
> Signed-off-by: Eric DeCosta <edecosta@mathworks.com>
> ---
>  compat/fsmonitor/fsm-listen-darwin.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/compat/fsmonitor/fsm-listen-darwin.c b/compat/fsmonitor/fsm-listen-darwin.c
> index 8e208e8289e..2ed828649ff 100644
> --- a/compat/fsmonitor/fsm-listen-darwin.c
> +++ b/compat/fsmonitor/fsm-listen-darwin.c
> @@ -26,6 +26,7 @@
>  #include "fsmonitor.h"
>  #include "fsm-listen.h"
>  #include "fsmonitor--daemon.h"
> +#include "fsmonitor-settings.h"
>  
>  struct fsm_listen_data
>  {
> @@ -183,7 +184,6 @@ static void my_add_path(struct fsmonitor_batch *batch, const char *path)
>  	free(composed);
>  }
>  
> -

Stray whitespace change, any one isn't much, but they add up (I saw
another one earlier in this topic).

>  static void fsevent_callback(ConstFSEventStreamRef streamRef,
>  			     void *ctx,
>  			     size_t num_of_events,
> @@ -209,7 +209,12 @@ static void fsevent_callback(ConstFSEventStreamRef streamRef,
>  		/*
>  		 * On Mac, we receive an array of absolute paths.
>  		 */
> -		path_k = paths[k];
> +		if (fsm_settings__get_allow_remote(the_repository) > 0) {
> +			strbuf_reset(&tmp);
> +			strbuf_realpath_forgiving(&tmp, paths[k], 0);
> +			path_k = tmp.buf;
> +		} else
> +			path_k = paths[k];

Style: This else should have braces if any if/else arm does, see
CodingGuidelines.

> [...]
> @@ -313,7 +319,6 @@ static void fsevent_callback(ConstFSEventStreamRef streamRef,
>  
>  		case IS_WORKDIR_PATH:
>  			/* try to queue normal pathnames */
> -

More stray whitespace