| Message ID | 421d77775dc24e52ab26336a1a82ed0e7b15ff5a.1664048782.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | fsmonitor: option to allow fsmonitor to run against network-mounted repos | expand |
> -----Original Message----- > From: Eric DeCosta via GitGitGadget <gitgitgadget@gmail.com> > Sent: Saturday, September 24, 2022 3:46 PM > To: git@vger.kernel.org > Cc: Jeff Hostetler <git@jeffhostetler.com>; Eric Sunshine > <sunshine@sunshineco.com>; Torsten Bögershausen <tboegi@web.de>; > Ævar Arnfjörð Bjarmason <avarab@gmail.com>; Ramsay Jones > <ramsay@ramsayjones.plus.com>; Johannes Schindelin > <Johannes.Schindelin@gmx.de>; Eric DeCosta <edecosta@mathworks.com>; > Eric DeCosta <edecosta@mathworks.com> > Subject: [PATCH v12 5/6] fsmonitor: check for compatability before > communicating with fsmonitor > > From: Eric DeCosta <edecosta@mathworks.com> > > If fsmonitor is not in a compatible state, die with an appropriate error > messge. > > Signed-off-by: Eric DeCosta <edecosta@mathworks.com> > --- Grr. Should be "warn with an appropriate error message". -Eric > compat/fsmonitor/fsm-settings-darwin.c | 2 +- > fsmonitor-settings.c | 10 +++++++--- > fsmonitor-settings.h | 2 +- > fsmonitor.c | 7 +++++++ > 4 files changed, 16 insertions(+), 5 deletions(-) > > diff --git a/compat/fsmonitor/fsm-settings-darwin.c > b/compat/fsmonitor/fsm-settings-darwin.c > index 40da2d3b533..44233125df8 100644 > --- a/compat/fsmonitor/fsm-settings-darwin.c > +++ b/compat/fsmonitor/fsm-settings-darwin.c > @@ -38,7 +38,7 @@ static enum fsmonitor_reason > check_uds_volume(struct repository *r) > strbuf_release(&path); > > if (fs.is_remote) > - return FSMONITOR_REASON_REMOTE; > + return FSMONITOR_REASON_NOSOCKETS; > > if (!strcmp(fs.typename, "msdos")) /* aka FAT32 */ > return FSMONITOR_REASON_NOSOCKETS; > diff --git a/fsmonitor-settings.c b/fsmonitor-settings.c index > 531a1b6f956..8592a4d9bad 100644 > --- a/fsmonitor-settings.c > +++ b/fsmonitor-settings.c > @@ -1,6 +1,7 @@ > #include "cache.h" > #include "config.h" > #include "repository.h" > +#include "fsmonitor-ipc.h" > #include "fsmonitor-settings.h" > #include "fsmonitor-path-utils.h" > > @@ -242,10 +243,11 @@ enum fsmonitor_reason > fsm_settings__get_reason(struct repository *r) > return r->settings.fsmonitor->reason; > } > > -char *fsm_settings__get_incompatible_msg(const struct repository *r, > +char *fsm_settings__get_incompatible_msg(struct repository *r, > enum fsmonitor_reason reason) > { > struct strbuf msg = STRBUF_INIT; > + const char *socket_dir; > > switch (reason) { > case FSMONITOR_REASON_UNTESTED: > @@ -281,9 +283,11 @@ char *fsm_settings__get_incompatible_msg(const > struct repository *r, > goto done; > > case FSMONITOR_REASON_NOSOCKETS: > + socket_dir = dirname((char *)fsmonitor_ipc__get_path(r)); > strbuf_addf(&msg, > - _("repository '%s' is incompatible with fsmonitor > due to lack of Unix sockets"), > - r->worktree); > + _("socket directory '%s' is incompatible with > fsmonitor due" > + " to lack of Unix sockets support"), > + socket_dir); > goto done; > } > > diff --git a/fsmonitor-settings.h b/fsmonitor-settings.h index > 0721617b95a..ab02e3995ee 100644 > --- a/fsmonitor-settings.h > +++ b/fsmonitor-settings.h > @@ -33,7 +33,7 @@ enum fsmonitor_mode fsm_settings__get_mode(struct > repository *r); const char *fsm_settings__get_hook_path(struct repository > *r); > > enum fsmonitor_reason fsm_settings__get_reason(struct repository *r); - > char *fsm_settings__get_incompatible_msg(const struct repository *r, > +char *fsm_settings__get_incompatible_msg(struct repository *r, > enum fsmonitor_reason reason); > > struct fsmonitor_settings; > diff --git a/fsmonitor.c b/fsmonitor.c > index 57d6a483bee..540736b39fd 100644 > --- a/fsmonitor.c > +++ b/fsmonitor.c > @@ -295,6 +295,7 @@ static int fsmonitor_force_update_threshold = 100; > > void refresh_fsmonitor(struct index_state *istate) { > + static int warn_once = 0; > struct strbuf query_result = STRBUF_INIT; > int query_success = 0, hook_version = -1; > size_t bol = 0; /* beginning of line */ @@ -305,6 +306,12 @@ void > refresh_fsmonitor(struct index_state *istate) > int is_trivial = 0; > struct repository *r = istate->repo ? istate->repo : the_repository; > enum fsmonitor_mode fsm_mode = fsm_settings__get_mode(r); > + enum fsmonitor_reason reason = fsm_settings__get_reason(r); > + > + if (!warn_once && reason > FSMONITOR_REASON_OK) { > + warn_once = 1; > + warning("%s", fsm_settings__get_incompatible_msg(r, > reason)); > + } > > if (fsm_mode <= FSMONITOR_MODE_DISABLED || > istate->fsmonitor_has_run_once) > -- > gitgitgadget
On Sat, Sep 24 2022, Eric DeCosta via GitGitGadget wrote: > From: Eric DeCosta <edecosta@mathworks.com> > [...] > @@ -281,9 +283,11 @@ char *fsm_settings__get_incompatible_msg(const struct repository *r, > goto done; > > case FSMONITOR_REASON_NOSOCKETS: > + socket_dir = dirname((char *)fsmonitor_ipc__get_path(r)); > strbuf_addf(&msg, > - _("repository '%s' is incompatible with fsmonitor due to lack of Unix sockets"), > - r->worktree); > + _("socket directory '%s' is incompatible with fsmonitor due" > + " to lack of Unix sockets support"), > + socket_dir); Could do with less "while at it" here. We are: * Wrapping the string, making the functional change(s) harder to spot. * replacing r->worktree with socket_dir * Adding " support" to the end of the string, and replacing "repository" with "socket directory" AFAICT the continuation of the string isn't indented in the way we usually do, i.e. to align with the opening ".
> -----Original Message----- > From: Ævar Arnfjörð Bjarmason <avarab@gmail.com> > Sent: Monday, September 26, 2022 11:24 AM > To: Eric DeCosta via GitGitGadget <gitgitgadget@gmail.com> > Cc: git@vger.kernel.org; Jeff Hostetler <git@jeffhostetler.com>; Eric Sunshine > <sunshine@sunshineco.com>; Torsten Bögershausen <tboegi@web.de>; > Ramsay Jones <ramsay@ramsayjones.plus.com>; Johannes Schindelin > <Johannes.Schindelin@gmx.de>; Eric DeCosta <edecosta@mathworks.com> > Subject: Re: [PATCH v12 5/6] fsmonitor: check for compatability before > communicating with fsmonitor > > > On Sat, Sep 24 2022, Eric DeCosta via GitGitGadget wrote: > > > From: Eric DeCosta <edecosta@mathworks.com> [...] @@ -281,9 +283,11 > @@ > > char *fsm_settings__get_incompatible_msg(const struct repository *r, > > goto done; > > > > case FSMONITOR_REASON_NOSOCKETS: > > + socket_dir = dirname((char *)fsmonitor_ipc__get_path(r)); > > strbuf_addf(&msg, > > - _("repository '%s' is incompatible with fsmonitor > due to lack of Unix sockets"), > > - r->worktree); > > + _("socket directory '%s' is incompatible with > fsmonitor due" > > + " to lack of Unix sockets support"), > > + socket_dir); > > Could do with less "while at it" here. We are: > > * Wrapping the string, making the functional change(s) harder to spot. > * replacing r->worktree with socket_dir > * Adding " support" to the end of the string, and replacing "repository" with > "socket directory" > > AFAICT the continuation of the string isn't indented in the way we usually do, > i.e. to align with the opening ". The string, when properly indented, exceeds an 80 character line length. I'll fix the indentation, but I don't think there's a much better alternative to the wrapping. The worktree could be in a perfectly fine location whereas the socket_dir may not . Crafting the error message the way I did reflects where the problem is rather than reporting a potentially misleading error about the repository. -Eric
diff --git a/compat/fsmonitor/fsm-settings-darwin.c b/compat/fsmonitor/fsm-settings-darwin.c index 40da2d3b533..44233125df8 100644 --- a/compat/fsmonitor/fsm-settings-darwin.c +++ b/compat/fsmonitor/fsm-settings-darwin.c @@ -38,7 +38,7 @@ static enum fsmonitor_reason check_uds_volume(struct repository *r) strbuf_release(&path); if (fs.is_remote) - return FSMONITOR_REASON_REMOTE; + return FSMONITOR_REASON_NOSOCKETS; if (!strcmp(fs.typename, "msdos")) /* aka FAT32 */ return FSMONITOR_REASON_NOSOCKETS; diff --git a/fsmonitor-settings.c b/fsmonitor-settings.c index 531a1b6f956..8592a4d9bad 100644 --- a/fsmonitor-settings.c +++ b/fsmonitor-settings.c @@ -1,6 +1,7 @@ #include "cache.h" #include "config.h" #include "repository.h" +#include "fsmonitor-ipc.h" #include "fsmonitor-settings.h" #include "fsmonitor-path-utils.h" @@ -242,10 +243,11 @@ enum fsmonitor_reason fsm_settings__get_reason(struct repository *r) return r->settings.fsmonitor->reason; } -char *fsm_settings__get_incompatible_msg(const struct repository *r, +char *fsm_settings__get_incompatible_msg(struct repository *r, enum fsmonitor_reason reason) { struct strbuf msg = STRBUF_INIT; + const char *socket_dir; switch (reason) { case FSMONITOR_REASON_UNTESTED: @@ -281,9 +283,11 @@ char *fsm_settings__get_incompatible_msg(const struct repository *r, goto done; case FSMONITOR_REASON_NOSOCKETS: + socket_dir = dirname((char *)fsmonitor_ipc__get_path(r)); strbuf_addf(&msg, - _("repository '%s' is incompatible with fsmonitor due to lack of Unix sockets"), - r->worktree); + _("socket directory '%s' is incompatible with fsmonitor due" + " to lack of Unix sockets support"), + socket_dir); goto done; } diff --git a/fsmonitor-settings.h b/fsmonitor-settings.h index 0721617b95a..ab02e3995ee 100644 --- a/fsmonitor-settings.h +++ b/fsmonitor-settings.h @@ -33,7 +33,7 @@ enum fsmonitor_mode fsm_settings__get_mode(struct repository *r); const char *fsm_settings__get_hook_path(struct repository *r); enum fsmonitor_reason fsm_settings__get_reason(struct repository *r); -char *fsm_settings__get_incompatible_msg(const struct repository *r, +char *fsm_settings__get_incompatible_msg(struct repository *r, enum fsmonitor_reason reason); struct fsmonitor_settings; diff --git a/fsmonitor.c b/fsmonitor.c index 57d6a483bee..540736b39fd 100644 --- a/fsmonitor.c +++ b/fsmonitor.c @@ -295,6 +295,7 @@ static int fsmonitor_force_update_threshold = 100; void refresh_fsmonitor(struct index_state *istate) { + static int warn_once = 0; struct strbuf query_result = STRBUF_INIT; int query_success = 0, hook_version = -1; size_t bol = 0; /* beginning of line */ @@ -305,6 +306,12 @@ void refresh_fsmonitor(struct index_state *istate) int is_trivial = 0; struct repository *r = istate->repo ? istate->repo : the_repository; enum fsmonitor_mode fsm_mode = fsm_settings__get_mode(r); + enum fsmonitor_reason reason = fsm_settings__get_reason(r); + + if (!warn_once && reason > FSMONITOR_REASON_OK) { + warn_once = 1; + warning("%s", fsm_settings__get_incompatible_msg(r, reason)); + } if (fsm_mode <= FSMONITOR_MODE_DISABLED || istate->fsmonitor_has_run_once)