On 3/10/22 8:31 PM, Ævar Arnfjörð Bjarmason wrote:
On Tue, Mar 08 2022, Jeff Hostetler via GitGitGadget wrote:
From: Jeff Hostetler <jeffhost@xxxxxxxxxxxxx>
[...]
+ prepare_repo_settings(the_repository);
+ fsm_settings__set_ipc(the_repository);
+
+ if (fsm_settings__get_mode(the_repository) == FSMONITOR_MODE_INCOMPATIBLE) {
+ const char *msg = fsm_settings__get_reason_msg(the_repository);
+
+ return error("%s '%s'", msg ? msg : "???", xgetcwd());
+ }
+
if (!strcmp(subcmd, "start"))
return !!try_to_start_background_daemon();
diff --git a/builtin/update-index.c b/builtin/update-index.c
index d335f1ac72a..8f460e7195f 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -1237,6 +1237,13 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
if (fsmonitor > 0) {
enum fsmonitor_mode fsm_mode = fsm_settings__get_mode(r);
+
+ if (fsm_mode == FSMONITOR_MODE_INCOMPATIBLE) {
+ const char *msg = fsm_settings__get_reason_msg(r);
+
+ return error("%s '%s'", msg ? msg : "???", xgetcwd());
+ }
+
if (fsm_mode == FSMONITOR_MODE_DISABLED) {
advise(_("core.fsmonitor is unset; "
"set it if you really want to "
Can w assert somewhere earlier that ->mode can't be
FSMONITOR_MODE_INCOMPATIBLE at the same time that ->reason ==
FSMONITOR_REASON_OK, should that ever happen?
Then we can get rid of the "???" case here.
Yeah, it would be nice to assert() the pair and simplify things. I'll
make a note to look at that.
The "%s '%s'" here should really be marked for translation, but just
"some reason '$path'" is a pretty confusing message. This will emit
e.g.:
I already have translations in the code that looks up the message,
so doing it here for a pair of %s's felt wrong.
"bare repos are incompatible with fsmonitor '/some/path/to/repo'"
Since we always hand these to error maybe have the helper do e.g.:
error(_("bare repository '%s' is incompatible with fsmonitor"), path);
I'm wondering now if we should just drop the path from the message
and use the error message from __get_reason_msg() as is. (It was
useful during debugging, but I could see it going away.)
I find the second-guessing in fsmonitor-settings.c really hard to
follow, i.e. how seemingly every function has some "not loaded yet? load
it" instead of a more typical "init it", "use it", "free it"
pattern. Including stuff like this:
enum fsmonitor_mode fsm_settings__get_mode(struct repository *r)
{
if (!r)
r = the_repository;
But anyway, seeing as we do try really hard to load the_repository (or a
repository) can't we use the_repository->gitdir etc. here instead of
xgetcwd(), or the_repository->worktree when non-bare?
I'll take a look and see if I can simplify it.
Thanks
Jeff
