Re: [PATCH v10 5/6] fsmonitor: check for compatability before communicating with fsmonitor
- From
- Jeff Hostetler <git@jeffhostetler.com>
- Date
- Sep 21, 2022, 11:22 UTC
- Message-ID
- <28d36af9-f493-f4a5-4aaf-18758e1c46db@jeffhostetler.com>
- In-Reply-To
- <8f6c1fbacbfd3df67d1a7591788d98f1af0e26f4.1663705986.git.gitgitgadget@gmail.com>
On 9/20/22 4:33 PM, Eric DeCosta via GitGitGadget wrote:
> From: Eric DeCosta <edecosta@mathworks.com> > > If fsmonitor is not in a compatible state, die with an appropriate error > messge.
[...]
> diff --git a/fsmonitor-settings.c b/fsmonitor-settings.c > index 531a1b6f956..24480b9806d 100644 > --- a/fsmonitor-settings.c > +++ b/fsmonitor-settings.c
[...]
Show 20 quoted lines
> +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"),
> + socket_dir);
> + strbuf_add(&msg, _(" to lack of Unix sockets support"), 32);
> goto done;I don't think we should split the error message between two calls to strbuf_add(). I realize that this was probably done because of line length concerns. But this makes assumptions on language word order during translations.
Instead, we can use C string literal joining before passing it to the translation macro. Something like:
strbuf_addf(&msg,
_("socket directory '%s' is incompatible with "
"fsmonitor due to lack of Unix sockets support"),
socket_dir);[...]
Show 12 quoted lines
> diff --git a/fsmonitor.c b/fsmonitor.c
> index 57d6a483bee..43d580132fb 100644
> --- a/fsmonitor.c
> +++ b/fsmonitor.c
> @@ -305,6 +305,10 @@ 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 (reason > FSMONITOR_REASON_OK)
> + die("%s", fsm_settings__get_incompatible_msg(r, reason));We don't want to call die() here. Maybe just silently return without doing anything or issue a warning() and return. (But I'm favoring a silent return here.)
From the clients' (`git status`, `git diff`, etc.) point of view, they just want a speed-up, if possible, but we shouldn't kill them; we should just let them do the normal scan that would have done if the feature were turned off.
Jeff