From: Jeff Hostetler Date: Thu, 01 Sep 2022 17:53:08 GMT Subject: Re: [PATCH v4 1/4] fsmonitor: add two new config options, allowRemote and socketDir Message-ID: <8acf1e78-bac4-7f59-11a4-996aa160bbdc@jeffhostetler.com> In-Reply-To: <836a791e6b7fd4490674254ce03105a8ca2175cb.1661962145.git.gitgitgadget@gmail.com> On 8/31/22 12:09 PM, Eric DeCosta via GitGitGadget wrote: > From: Eric DeCosta > > Introduce two new configuration options > > fsmonitor.allowRemote - setting this to true overrides fsmonitor's > default behavior of erroring out when enountering network file > systems. Additionly, when true, the Unix domain socket (UDS) file > used for IPC is located in $HOME rather than in the .git directory. > > fsmonitor.socketDir - allows for the UDS file to be located > anywhere the user chooses rather $HOME. > > Signed-off-by: Eric DeCosta > --- > fsmonitor-settings.c | 67 ++++++++++++++++++++++++++++++++++++++++++-- > fsmonitor-settings.h | 4 +++ > 2 files changed, 69 insertions(+), 2 deletions(-) > > diff --git a/fsmonitor-settings.c b/fsmonitor-settings.c > index 464424a1e92..a15eeecebf4 100644 > --- a/fsmonitor-settings.c > +++ b/fsmonitor-settings.c > @@ -10,7 +10,9 @@ > struct fsmonitor_settings { > enum fsmonitor_mode mode; > enum fsmonitor_reason reason; > + int allow_remote; > char *hook_path; > + char *sock_dir; > }; > > static enum fsmonitor_reason check_for_incompatible(struct repository *r) > @@ -43,6 +45,7 @@ static struct fsmonitor_settings *alloc_settings(void) > CALLOC_ARRAY(s, 1); > s->mode = FSMONITOR_MODE_DISABLED; > s->reason = FSMONITOR_REASON_UNTESTED; > + s->allow_remote = -1; > > return s; > } > @@ -90,6 +93,26 @@ static void lookup_fsmonitor_settings(struct repository *r) > fsm_settings__set_disabled(r); > } > > +int fsm_settings__get_allow_remote(struct repository *r) > +{ > + if (!r) > + r = the_repository; > + if (!r->settings.fsmonitor) > + lookup_fsmonitor_settings(r); > + > + return r->settings.fsmonitor->allow_remote; > +} > + > +const char *fsm_settings__get_socket_dir(struct repository *r) > +{ > + if (!r) > + r = the_repository; > + if (!r->settings.fsmonitor) > + lookup_fsmonitor_settings(r); > + > + return r->settings.fsmonitor->sock_dir; > +} > + > enum fsmonitor_mode fsm_settings__get_mode(struct repository *r) > { > if (!r) > @@ -100,6 +123,7 @@ enum fsmonitor_mode fsm_settings__get_mode(struct repository *r) > return r->settings.fsmonitor->mode; > } > > + > const char *fsm_settings__get_hook_path(struct repository *r) > { > if (!r) > @@ -110,9 +134,44 @@ const char *fsm_settings__get_hook_path(struct repository *r) > return r->settings.fsmonitor->hook_path; > } > > +void fsm_settings__set_allow_remote(struct repository *r) > +{ > + int allow; > + > + if (!r) > + r = the_repository; > + if (!r->settings.fsmonitor) > + r->settings.fsmonitor = alloc_settings(); > + if (!repo_config_get_bool(r, "fsmonitor.allowremote", &allow)) > + r->settings.fsmonitor->allow_remote = allow; > + > + return; > +} > + > +void fsm_settings__set_socket_dir(struct repository *r) > +{ > + const char *path; > + > + if (!r) > + r = the_repository; > + if (!r->settings.fsmonitor) > + r->settings.fsmonitor = alloc_settings(); > + > + if (!repo_config_get_pathname(r, "fsmonitor.socketdir", &path)) { > + FREE_AND_NULL(r->settings.fsmonitor->sock_dir); > + r->settings.fsmonitor->sock_dir = strdup(path); > + } > + > + return; > +} > + > void fsm_settings__set_ipc(struct repository *r) > { > - enum fsmonitor_reason reason = check_for_incompatible(r); > + enum fsmonitor_reason reason; > + > + fsm_settings__set_allow_remote(r); > + fsm_settings__set_socket_dir(r); > + reason = check_for_incompatible(r); > > if (reason != FSMONITOR_REASON_OK) { > fsm_settings__set_incompatible(r, reason); > @@ -135,7 +194,11 @@ void fsm_settings__set_ipc(struct repository *r) > > void fsm_settings__set_hook(struct repository *r, const char *path) > { > - enum fsmonitor_reason reason = check_for_incompatible(r); > + enum fsmonitor_reason reason; > + > + fsm_settings__set_allow_remote(r); > + fsm_settings__set_socket_dir(r); > + reason = check_for_incompatible(r); When we use the hook interface (to talk to a third-party app like Watchman) we do not use the unix domain socket. (We talk to the hook child using a normal pipe.) FWIW, I added code in my series to limit the use of hook-based fsmonitors to local filesystems for the paranoia-based reasons I've described before. It wasn't because of the UDS. So we don't need to set the socket directory in this case. > > if (reason != FSMONITOR_REASON_OK) { > fsm_settings__set_incompatible(r, reason); > diff --git a/fsmonitor-settings.h b/fsmonitor-settings.h > index d9c2605197f..2de54c85e94 100644 > --- a/fsmonitor-settings.h > +++ b/fsmonitor-settings.h > @@ -23,12 +23,16 @@ enum fsmonitor_reason { > FSMONITOR_REASON_NOSOCKETS, /* NTFS,FAT32 do not support Unix sockets */ > }; > > +void fsm_settings__set_allow_remote(struct repository *r); > +void fsm_settings__set_socket_dir(struct repository *r); > void fsm_settings__set_ipc(struct repository *r); > void fsm_settings__set_hook(struct repository *r, const char *path); > void fsm_settings__set_disabled(struct repository *r); > void fsm_settings__set_incompatible(struct repository *r, > enum fsmonitor_reason reason); > > +int fsm_settings__get_allow_remote(struct repository *r); > +const char *fsm_settings__get_socket_dir(struct repository *r); > enum fsmonitor_mode fsm_settings__get_mode(struct repository *r); > const char *fsm_settings__get_hook_path(struct repository *r); > >