fsmonitor-settings: bare repos are incompatible with FSMonitor

Bare repos do not have a worktree, so there is nothing for the
daemon watch.

Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/fsmonitor--daemon.c b/builtin/fsmonitor--daemon.c
index 46be55a4618740c5c0746c8c1bed31b20094d73c..66b78a0353f6af50056d8d2a8a37f7020fd076c5 100644 (file)
--- a/builtin/fsmonitor--daemon.c
+++ b/builtin/fsmonitor--daemon.c
@@ -1423,6 +1423,7 @@ static int try_to_start_background_daemon(void)
 int cmd_fsmonitor__daemon(int argc, const char **argv, const char *prefix)
 {
        const char *subcmd;
+       enum fsmonitor_reason reason;
        int detach_console = 0;
 
        struct option options[] = {
@@ -1449,6 +1450,23 @@ int cmd_fsmonitor__daemon(int argc, const char **argv, const char *prefix)
                die(_("invalid 'ipc-threads' value (%d)"),
                    fsmonitor__ipc_threads);
 
+       prepare_repo_settings(the_repository);
+       /*
+        * If the repo is fsmonitor-compatible, explicitly set IPC-mode
+        * (without bothering to load the `core.fsmonitor` config settings).
+        *
+        * If the repo is not compatible, the repo-settings will be set to
+        * incompatible rather than IPC, so we can use one of the __get
+        * routines to detect the discrepancy.
+        */
+       fsm_settings__set_ipc(the_repository);
+
+       reason = fsm_settings__get_reason(the_repository);
+       if (reason > FSMONITOR_REASON_OK)
+               die("%s",
+                   fsm_settings__get_incompatible_msg(the_repository,
+                                                      reason));
+
        if (!strcmp(subcmd, "start"))
                return !!try_to_start_background_daemon();
 

diff --git a/builtin/update-index.c b/builtin/update-index.c
index 876112abb21a4dbc6f222dc8da8838be4819de3d..01ed4c4976b2e794842cd387f85d20c510095e29 100644 (file)
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -1237,6 +1237,22 @@ 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);
+               enum fsmonitor_reason reason = fsm_settings__get_reason(r);
+
+               /*
+                * The user wants to turn on FSMonitor using the command
+                * line argument.  (We don't know (or care) whether that
+                * is the IPC or HOOK version.)
+                *
+                * Use one of the __get routines to force load the FSMonitor
+                * config settings into the repo-settings.  That will detect
+                * whether the file system is compatible so that we can stop
+                * here with a nice error message.
+                */
+               if (reason > FSMONITOR_REASON_OK)
+                       die("%s",
+                           fsm_settings__get_incompatible_msg(r, reason));
+
                if (fsm_mode == FSMONITOR_MODE_DISABLED) {
                        warning(_("core.fsmonitor is unset; "
                                "set it if you really want to "

diff --git a/fsmonitor-settings.c b/fsmonitor-settings.c
index 757d230d538056fb88e7ed8d290df3f68c06a78d..7d3177d441aa08d2dd77b3d4859c0d70f754d194 100644 (file)
--- a/fsmonitor-settings.c
+++ b/fsmonitor-settings.c
@@ -9,23 +9,42 @@
  */
 struct fsmonitor_settings {
        enum fsmonitor_mode mode;
+       enum fsmonitor_reason reason;
        char *hook_path;
 };
 
-static void lookup_fsmonitor_settings(struct repository *r)
+static enum fsmonitor_reason check_for_incompatible(struct repository *r)
+{
+       if (!r->worktree) {
+               /*
+                * Bare repositories don't have a working directory and
+                * therefore have nothing to watch.
+                */
+               return FSMONITOR_REASON_BARE;
+       }
+
+       return FSMONITOR_REASON_OK;
+}
+
+static struct fsmonitor_settings *alloc_settings(void)
 {
        struct fsmonitor_settings *s;
+
+       CALLOC_ARRAY(s, 1);
+       s->mode = FSMONITOR_MODE_DISABLED;
+       s->reason = FSMONITOR_REASON_UNTESTED;
+
+       return s;
+}
+
+static void lookup_fsmonitor_settings(struct repository *r)
+{
        const char *const_str;
        int bool_value;
 
        if (r->settings.fsmonitor)
                return;
 
-       CALLOC_ARRAY(s, 1);
-       s->mode = FSMONITOR_MODE_DISABLED;
-
-       r->settings.fsmonitor = s;
-
        /*
         * Overload the existing "core.fsmonitor" config setting (which
         * has historically been either unset or a hook pathname) to
@@ -38,6 +57,8 @@ static void lookup_fsmonitor_settings(struct repository *r)
        case 0: /* config value was set to <bool> */
                if (bool_value)
                        fsm_settings__set_ipc(r);
+               else
+                       fsm_settings__set_disabled(r);
                return;
 
        case 1: /* config value was unset */
@@ -53,18 +74,18 @@ static void lookup_fsmonitor_settings(struct repository *r)
                return;
        }
 
-       if (!const_str || !*const_str)
-               return;
-
-       fsm_settings__set_hook(r, const_str);
+       if (const_str && *const_str)
+               fsm_settings__set_hook(r, const_str);
+       else
+               fsm_settings__set_disabled(r);
 }
 
 enum fsmonitor_mode fsm_settings__get_mode(struct repository *r)
 {
        if (!r)
                r = the_repository;
-
-       lookup_fsmonitor_settings(r);
+       if (!r->settings.fsmonitor)
+               lookup_fsmonitor_settings(r);
 
        return r->settings.fsmonitor->mode;
 }
@@ -73,31 +94,55 @@ const char *fsm_settings__get_hook_path(struct repository *r)
 {
        if (!r)
                r = the_repository;
-
-       lookup_fsmonitor_settings(r);
+       if (!r->settings.fsmonitor)
+               lookup_fsmonitor_settings(r);
 
        return r->settings.fsmonitor->hook_path;
 }
 
 void fsm_settings__set_ipc(struct repository *r)
 {
+       enum fsmonitor_reason reason = check_for_incompatible(r);
+
+       if (reason != FSMONITOR_REASON_OK) {
+               fsm_settings__set_incompatible(r, reason);
+               return;
+       }
+
+       /*
+        * Caller requested IPC explicitly, so avoid (possibly
+        * recursive) config lookup.
+        */
        if (!r)
                r = the_repository;
-
-       lookup_fsmonitor_settings(r);
+       if (!r->settings.fsmonitor)
+               r->settings.fsmonitor = alloc_settings();
 
        r->settings.fsmonitor->mode = FSMONITOR_MODE_IPC;
+       r->settings.fsmonitor->reason = reason;
        FREE_AND_NULL(r->settings.fsmonitor->hook_path);
 }
 
 void fsm_settings__set_hook(struct repository *r, const char *path)
 {
+       enum fsmonitor_reason reason = check_for_incompatible(r);
+
+       if (reason != FSMONITOR_REASON_OK) {
+               fsm_settings__set_incompatible(r, reason);
+               return;
+       }
+
+       /*
+        * Caller requested hook explicitly, so avoid (possibly
+        * recursive) config lookup.
+        */
        if (!r)
                r = the_repository;
-
-       lookup_fsmonitor_settings(r);
+       if (!r->settings.fsmonitor)
+               r->settings.fsmonitor = alloc_settings();
 
        r->settings.fsmonitor->mode = FSMONITOR_MODE_HOOK;
+       r->settings.fsmonitor->reason = reason;
        FREE_AND_NULL(r->settings.fsmonitor->hook_path);
        r->settings.fsmonitor->hook_path = strdup(path);
 }
@@ -106,9 +151,57 @@ void fsm_settings__set_disabled(struct repository *r)
 {
        if (!r)
                r = the_repository;
-
-       lookup_fsmonitor_settings(r);
+       if (!r->settings.fsmonitor)
+               r->settings.fsmonitor = alloc_settings();
 
        r->settings.fsmonitor->mode = FSMONITOR_MODE_DISABLED;
+       r->settings.fsmonitor->reason = FSMONITOR_REASON_OK;
+       FREE_AND_NULL(r->settings.fsmonitor->hook_path);
+}
+
+void fsm_settings__set_incompatible(struct repository *r,
+                                   enum fsmonitor_reason reason)
+{
+       if (!r)
+               r = the_repository;
+       if (!r->settings.fsmonitor)
+               r->settings.fsmonitor = alloc_settings();
+
+       r->settings.fsmonitor->mode = FSMONITOR_MODE_INCOMPATIBLE;
+       r->settings.fsmonitor->reason = reason;
        FREE_AND_NULL(r->settings.fsmonitor->hook_path);
 }
+
+enum fsmonitor_reason fsm_settings__get_reason(struct repository *r)
+{
+       if (!r)
+               r = the_repository;
+       if (!r->settings.fsmonitor)
+               lookup_fsmonitor_settings(r);
+
+       return r->settings.fsmonitor->reason;
+}
+
+char *fsm_settings__get_incompatible_msg(const struct repository *r,
+                                        enum fsmonitor_reason reason)
+{
+       struct strbuf msg = STRBUF_INIT;
+
+       switch (reason) {
+       case FSMONITOR_REASON_UNTESTED:
+       case FSMONITOR_REASON_OK:
+               goto done;
+
+       case FSMONITOR_REASON_BARE:
+               strbuf_addf(&msg,
+                           _("bare repository '%s' is incompatible with fsmonitor"),
+                           xgetcwd());
+               goto done;
+       }
+
+       BUG("Unhandled case in fsm_settings__get_incompatible_msg: '%d'",
+           reason);
+
+done:
+       return strbuf_detach(&msg, NULL);
+}

diff --git a/fsmonitor-settings.h b/fsmonitor-settings.h
index a4c5d7b48891fe6d6bb3705fe1107fcae8e9dc29..8d9331c0c0a4fd78bfec515397cb0184af11e822 100644 (file)
--- a/fsmonitor-settings.h
+++ b/fsmonitor-settings.h
@@ -4,18 +4,34 @@
 struct repository;
 
 enum fsmonitor_mode {
+       FSMONITOR_MODE_INCOMPATIBLE = -1, /* see _reason */
        FSMONITOR_MODE_DISABLED = 0,
        FSMONITOR_MODE_HOOK = 1, /* core.fsmonitor=<hook_path> */
        FSMONITOR_MODE_IPC = 2,  /* core.fsmonitor=<true> */
 };
 
+/*
+ * Incompatibility reasons.
+ */
+enum fsmonitor_reason {
+       FSMONITOR_REASON_UNTESTED = 0,
+       FSMONITOR_REASON_OK, /* no incompatibility or when disabled */
+       FSMONITOR_REASON_BARE,
+};
+
 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);
 
 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,
+                                        enum fsmonitor_reason reason);
+
 struct fsmonitor_settings;
 
 #endif /* FSMONITOR_SETTINGS_H */

diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh
index a6308acf006c9e4d35e47578465e348bb213356f..9a8e21c56084508c8cfb9d033adfbf7e85f3777e 100755 (executable)
--- a/t/t7519-status-fsmonitor.sh
+++ b/t/t7519-status-fsmonitor.sh
@@ -55,6 +55,29 @@ test_lazy_prereq UNTRACKED_CACHE '
        test $ret -ne 1
 '
 
+# Test that we detect and disallow repos that are incompatible with FSMonitor.
+test_expect_success 'incompatible bare repo' '
+       test_when_finished "rm -rf ./bare-clone actual expect" &&
+       git init --bare bare-clone &&
+
+       test_must_fail \
+               git -C ./bare-clone -c core.fsmonitor=foo \
+                       update-index --fsmonitor 2>actual &&
+       grep "bare repository .* is incompatible with fsmonitor" actual &&
+
+       test_must_fail \
+               git -C ./bare-clone -c core.fsmonitor=true \
+                       update-index --fsmonitor 2>actual &&
+       grep "bare repository .* is incompatible with fsmonitor" actual
+'
+
+test_expect_success FSMONITOR_DAEMON 'run fsmonitor-daemon in bare repo' '
+       test_when_finished "rm -rf ./bare-clone actual" &&
+       git init --bare bare-clone &&
+       test_must_fail git -C ./bare-clone fsmonitor--daemon run 2>actual &&
+       grep "bare repository .* is incompatible with fsmonitor" actual
+'
+
 test_expect_success 'setup' '
        mkdir -p .git/hooks &&
        : >tracked &&