fsmonitor: change last update timestamp on the index_state to opaque token

Some file system monitors might not use or take a timestamp for processing
and in the case of watchman could have race conditions with using a
timestamp. Watchman uses something called a clockid that is used for race
free queries to it. The clockid for watchman is simply a string.

Change the fsmonitor_last_update from being a uint64_t to a char pointer
so that any arbitrary data can be stored in it and passed back to the
fsmonitor.

Signed-off-by: Kevin Willford <Kevin.Willford@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/cache.h b/cache.h
index cbfaead23a384e780453c948863ef2cf3bef0437..1a2e1cce3d78c632babb253ae64734fbd67b73bd 100644 (file)
--- a/cache.h
+++ b/cache.h
@@ -324,7 +324,7 @@ struct index_state {
        struct hashmap dir_hash;
        struct object_id oid;
        struct untracked_cache *untracked;
-       uint64_t fsmonitor_last_update;
+       char *fsmonitor_last_update;
        struct ewah_bitmap *fsmonitor_dirty;
        struct mem_pool *ce_mem_pool;
        struct progress *progress;

diff --git a/fsmonitor.c b/fsmonitor.c
index 868cca01e250896054b1f7025c7180c87ab1b88d..98605872259d0dfb2b2f9a67be6abe2f3b17cc26 100644 (file)
--- a/fsmonitor.c
+++ b/fsmonitor.c
@@ -6,8 +6,9 @@
 #include "run-command.h"
 #include "strbuf.h"
 
-#define INDEX_EXTENSION_VERSION        (1)
-#define HOOK_INTERFACE_VERSION (1)
+#define INDEX_EXTENSION_VERSION1       (1)
+#define INDEX_EXTENSION_VERSION2       (2)
+#define HOOK_INTERFACE_VERSION         (1)
 
 struct trace_key trace_fsmonitor = TRACE_KEY_INIT(FSMONITOR);
 
@@ -32,17 +33,26 @@ int read_fsmonitor_extension(struct index_state *istate, const void *data,
        uint32_t ewah_size;
        struct ewah_bitmap *fsmonitor_dirty;
        int ret;
+       uint64_t timestamp;
+       struct strbuf last_update = STRBUF_INIT;
 
-       if (sz < sizeof(uint32_t) + sizeof(uint64_t) + sizeof(uint32_t))
+       if (sz < sizeof(uint32_t) + 1 + sizeof(uint32_t))
                return error("corrupt fsmonitor extension (too short)");
 
        hdr_version = get_be32(index);
        index += sizeof(uint32_t);
-       if (hdr_version != INDEX_EXTENSION_VERSION)
+       if (hdr_version == INDEX_EXTENSION_VERSION1) {
+               timestamp = get_be64(index);
+               strbuf_addf(&last_update, "%"PRIu64"", timestamp);
+               index += sizeof(uint64_t);
+       } else if (hdr_version == INDEX_EXTENSION_VERSION2) {
+               strbuf_addstr(&last_update, index);
+               index += last_update.len + 1;
+       } else {
                return error("bad fsmonitor version %d", hdr_version);
+       }
 
-       istate->fsmonitor_last_update = get_be64(index);
-       index += sizeof(uint64_t);
+       istate->fsmonitor_last_update = strbuf_detach(&last_update, NULL);
 
        ewah_size = get_be32(index);
        index += sizeof(uint32_t);
@@ -79,7 +89,6 @@ void fill_fsmonitor_bitmap(struct index_state *istate)
 void write_fsmonitor_extension(struct strbuf *sb, struct index_state *istate)
 {
        uint32_t hdr_version;
-       uint64_t tm;
        uint32_t ewah_start;
        uint32_t ewah_size = 0;
        int fixup = 0;
@@ -89,11 +98,12 @@ void write_fsmonitor_extension(struct strbuf *sb, struct index_state *istate)
                BUG("fsmonitor_dirty has more entries than the index (%"PRIuMAX" > %u)",
                    (uintmax_t)istate->fsmonitor_dirty->bit_size, istate->cache_nr);
 
-       put_be32(&hdr_version, INDEX_EXTENSION_VERSION);
+       put_be32(&hdr_version, INDEX_EXTENSION_VERSION2);
        strbuf_add(sb, &hdr_version, sizeof(uint32_t));
 
-       put_be64(&tm, istate->fsmonitor_last_update);
-       strbuf_add(sb, &tm, sizeof(uint64_t));
+       strbuf_addstr(sb, istate->fsmonitor_last_update);
+       strbuf_addch(sb, 0); /* Want to keep a NUL */
+
        fixup = sb->len;
        strbuf_add(sb, &ewah_size, sizeof(uint32_t)); /* we'll fix this up later */
 
@@ -110,9 +120,9 @@ void write_fsmonitor_extension(struct strbuf *sb, struct index_state *istate)
 }
 
 /*
- * Call the query-fsmonitor hook passing the time of the last saved results.
+ * Call the query-fsmonitor hook passing the last update token of the saved results.
  */
-static int query_fsmonitor(int version, uint64_t last_update, struct strbuf *query_result)
+static int query_fsmonitor(int version, const char *last_update, struct strbuf *query_result)
 {
        struct child_process cp = CHILD_PROCESS_INIT;
 
@@ -121,7 +131,7 @@ static int query_fsmonitor(int version, uint64_t last_update, struct strbuf *que
 
        argv_array_push(&cp.args, core_fsmonitor);
        argv_array_pushf(&cp.args, "%d", version);
-       argv_array_pushf(&cp.args, "%" PRIuMAX, (uintmax_t)last_update);
+       argv_array_pushf(&cp.args, "%s", last_update);
        cp.use_shell = 1;
        cp.dir = get_git_work_tree();
 
@@ -151,6 +161,7 @@ void refresh_fsmonitor(struct index_state *istate)
        int query_success = 0;
        size_t bol; /* beginning of line */
        uint64_t last_update;
+       struct strbuf last_update_token = STRBUF_INIT;
        char *buf;
        unsigned int i;
 
@@ -164,6 +175,7 @@ void refresh_fsmonitor(struct index_state *istate)
         * should be inclusive to ensure we don't miss potential changes.
         */
        last_update = getnanotime();
+       strbuf_addf(&last_update_token, "%"PRIu64"", last_update);
 
        /*
         * If we have a last update time, call query_fsmonitor for the set of
@@ -217,18 +229,21 @@ void refresh_fsmonitor(struct index_state *istate)
        }
        strbuf_release(&query_result);
 
-       /* Now that we've updated istate, save the last_update time */
-       istate->fsmonitor_last_update = last_update;
+       /* Now that we've updated istate, save the last_update_token */
+       FREE_AND_NULL(istate->fsmonitor_last_update);
+       istate->fsmonitor_last_update = strbuf_detach(&last_update_token, NULL);
 }
 
 void add_fsmonitor(struct index_state *istate)
 {
        unsigned int i;
+       struct strbuf last_update = STRBUF_INIT;
 
        if (!istate->fsmonitor_last_update) {
                trace_printf_key(&trace_fsmonitor, "add fsmonitor");
                istate->cache_changed |= FSMONITOR_CHANGED;
-               istate->fsmonitor_last_update = getnanotime();
+               strbuf_addf(&last_update, "%"PRIu64"", getnanotime());
+               istate->fsmonitor_last_update = strbuf_detach(&last_update, NULL);
 
                /* reset the fsmonitor state */
                for (i = 0; i < istate->cache_nr; i++)
@@ -250,7 +265,7 @@ void remove_fsmonitor(struct index_state *istate)
        if (istate->fsmonitor_last_update) {
                trace_printf_key(&trace_fsmonitor, "remove fsmonitor");
                istate->cache_changed |= FSMONITOR_CHANGED;
-               istate->fsmonitor_last_update = 0;
+               FREE_AND_NULL(istate->fsmonitor_last_update);
        }
 }
 

diff --git a/t/helper/test-dump-fsmonitor.c b/t/helper/test-dump-fsmonitor.c
index 2786f47088e975608c839f193527e1e70e84cee9..975f0ac8905125f01ce37be5b7bc862cf058fbcf 100644 (file)
--- a/t/helper/test-dump-fsmonitor.c
+++ b/t/helper/test-dump-fsmonitor.c
@@ -13,7 +13,7 @@ int cmd__dump_fsmonitor(int ac, const char **av)
                printf("no fsmonitor\n");
                return 0;
        }
-       printf("fsmonitor last update %"PRIuMAX"\n", (uintmax_t)istate->fsmonitor_last_update);
+       printf("fsmonitor last update %s\n", istate->fsmonitor_last_update);
 
        for (i = 0; i < istate->cache_nr; i++)
                printf((istate->cache[i]->ce_flags & CE_FSMONITOR_VALID) ? "+" : "-");