{"thread":{"id":"46042","subject":"[PATCH v3 1/6] bswap: add 64 bit endianness helper get_be64","startedAt":"2017-05-25T18:36:45Z","lastAt":"2017-05-31T14:57:21Z","messageCount":14,"participants":["Ben Peart","Christian Couder","Junio C Hamano"],"isPatch":true,"patchVersion":3,"patchTotal":6},"messages":[{"id":"320720","messageId":"20170525183612.3128-2-benpeart@microsoft.com","threadId":"46042","inReplyTo":"20170525183612.3128-1-benpeart@microsoft.com","subject":"[PATCH v3 1/6] bswap: add 64 bit endianness helper get_be64","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2017-05-25T18:36:07Z","receivedAt":"2017-05-25T18:36:45Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"Add a new get_be64 macro to enable 64 bit endian conversions on memory\nthat may or may not be aligned.\n\nSigned-off-by: Ben Peart <benpeart@microsoft.com>\n---\n compat/bswap.h | 4 ++++\n 1 file changed, 4 insertions(+)\n\ndiff --git a/compat/bswap.h b/compat/bswap.h\nindex d47c003544..f89fe7f4b5 100644\n--- a/compat/bswap.h\n+++ b/compat/bswap.h\n@@ -158,6 +158,7 @@ static inline uint64_t git_bswap64(uint64_t x)\n \n #define get_be16(p)\tntohs(*(unsigned short *)(p))\n #define get_be32(p)\tntohl(*(unsigned int *)(p))\n+#define get_be64(p)\tntohll(*(uint64_t *)(p))\n #define put_be32(p, v)\tdo { *(unsigned int *)(p) = htonl(v); } while (0)\n \n #else\n@@ -170,6 +171,9 @@ static inline uint64_t git_bswap64(uint64_t x)\n \t(*((unsigned char *)(p) + 1) << 16) | \\\n \t(*((unsigned char *)(p) + 2) <<  8) | \\\n \t(*((unsigned char *)(p) + 3) <<  0) )\n+#define get_be64(p)\t( \\\n+\t((uint64_t)get_be32((unsigned char *)(p) + 0) << 32) | \\\n+\t((uint64_t)get_be32((unsigned char *)(p) + 4) <<  0)\n #define put_be32(p, v)\tdo { \\\n \tunsigned int __v = (v); \\\n \t*((unsigned char *)(p) + 0) = __v >> 24; \\\n-- \n2.13.0.windows.1.9.gc201c67b71\n\n"},{"id":"320721","messageId":"20170525183612.3128-1-benpeart@microsoft.com","threadId":"46042","inReplyTo":null,"subject":"[PATCH v3 0/6] Fast git status via a file system watcher","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2017-05-25T18:36:06Z","receivedAt":"2017-05-25T18:36:48Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"Changes from V2 include:\n - switch to nanoseconds for last update time saved in index extension\n   and passed to hook\n - pass the hook a version to enable simpler future updates\n - fixup compiler warnings found with different compilers\n - update test to run properly on Mac\n - fix documentation formatting and spelling errors\n - update code formatting based on feedback\n - rename fsmonitor_dirty_bitmap to fsmonitor_dirty\n\nBen Peart (6):\n  bswap: add 64 bit endianness helper get_be64\n  dir: make lookup_untracked() available outside of dir.c\n  fsmonitor: teach git to optionally utilize a file system monitor to\n    speed up detecting new or changed files.\n  fsmonitor: add test cases for fsmonitor extension\n  fsmonitor: add documentation for the fsmonitor extension.\n  fsmonitor: add a sample query-fsmonitor hook script for Watchman\n\n Documentation/config.txt                 |   7 +\n Documentation/githooks.txt               |  23 +++\n Documentation/technical/index-format.txt |  19 +++\n Makefile                                 |   1 +\n builtin/update-index.c                   |   1 +\n cache.h                                  |   5 +\n compat/bswap.h                           |   4 +\n config.c                                 |   5 +\n dir.c                                    |  16 ++-\n dir.h                                    |   5 +\n entry.c                                  |   1 +\n environment.c                            |   1 +\n fsmonitor.c                              | 238 +++++++++++++++++++++++++++++++\n fsmonitor.h                              |   9 ++\n read-cache.c                             |  28 +++-\n t/t7519-status-fsmonitor.sh              | 158 ++++++++++++++++++++\n templates/hooks--query-fsmonitor.sample  |  37 +++++\n unpack-trees.c                           |   1 +\n 18 files changed, 556 insertions(+), 3 deletions(-)\n create mode 100644 fsmonitor.c\n create mode 100644 fsmonitor.h\n create mode 100755 t/t7519-status-fsmonitor.sh\n create mode 100644 templates/hooks--query-fsmonitor.sample\n\ndiff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\nindex f7b4b4a844..48127e8729 100644\n--- a/Documentation/githooks.txt\n+++ b/Documentation/githooks.txt\n@@ -450,12 +450,12 @@ processed by rebase.\n \n [[query-fsmonitor]]\n query-fsmonitor\n-~~~~~~~~~~~~\n+~~~~~~~~~~~~~~~\n \n This hook is invoked when the configuration option core.fsmonitor is\n set and git needs to identify changed or untracked files.  It takes\n-a single argument which is the time in elapsed seconds since midnight,\n-January 1, 1970.\n+two arguments, a version (currently 1) and the time in elapsed\n+nanoseconds since midnight, January 1, 1970.\n \n The hook should output to stdout the list of all files in the working\n directory that may have changed since the requested time.  The logic\ndiff --git a/Documentation/technical/index-format.txt b/Documentation/technical/index-format.txt\nindex b002d23c05..7aeeea6f94 100644\n--- a/Documentation/technical/index-format.txt\n+++ b/Documentation/technical/index-format.txt\n@@ -307,7 +307,8 @@ The remaining data of each directory block is grouped by type:\n   - 32-bit version number: the current supported version is 1.\n \n   - 64-bit time: the extension data reflects all changes through the given\n-\ttime which is stored as the seconds elapsed since midnight, January 1, 1970.\n+\ttime which is stored as the nanoseconds elapsed since midnight,\n+\tJanuary 1, 1970.\n \n   - 32-bit bitmap size: the size of the CE_FSMONITOR_DIRTY bitmap.\n \ndiff --git a/cache.h b/cache.h\nindex 36423c77cc..58e5abf69f 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -344,8 +344,8 @@ struct index_state {\n \tstruct hashmap dir_hash;\n \tunsigned char sha1[20];\n \tstruct untracked_cache *untracked;\n-\ttime_t fsmonitor_last_update;\n-\tstruct ewah_bitmap *fsmonitor_dirty_bitmap;\n+\tuint64_t fsmonitor_last_update;\n+\tstruct ewah_bitmap *fsmonitor_dirty;\n };\n \n extern struct index_state the_index;\ndiff --git a/fsmonitor.c b/fsmonitor.c\nindex 9f08e66db9..3ce262d47d 100644\n--- a/fsmonitor.c\n+++ b/fsmonitor.c\n@@ -5,6 +5,9 @@\n #include \"strbuf.h\"\n #include \"fsmonitor.h\"\n \n+#define INDEX_EXTENSION_VERSION\t1\n+#define HOOK_INTERFACE_VERSION\t\t1\n+\n static struct untracked_cache_dir *find_untracked_cache_dir(\n \tstruct untracked_cache *uc, struct untracked_cache_dir *ucd,\n \tconst char *name)\n@@ -56,20 +59,20 @@ int read_fsmonitor_extension(struct index_state *istate, const void *data,\n \n \thdr_version = get_be32(index);\n \tindex += sizeof(uint32_t);\n-\tif (hdr_version != 1)\n+\tif (hdr_version != INDEX_EXTENSION_VERSION)\n \t\treturn error(\"bad fsmonitor version %d\", hdr_version);\n \n-\tistate->fsmonitor_last_update = (time_t)get_be64(index);\n+\tistate->fsmonitor_last_update = get_be64(index);\n \tindex += sizeof(uint64_t);\n \n \tewah_size = get_be32(index);\n \tindex += sizeof(uint32_t);\n \n-\tistate->fsmonitor_dirty_bitmap = ewah_new();\n-\tret = ewah_read_mmap(istate->fsmonitor_dirty_bitmap, index, ewah_size);\n+\tistate->fsmonitor_dirty = ewah_new();\n+\tret = ewah_read_mmap(istate->fsmonitor_dirty, index, ewah_size);\n \tif (ret != ewah_size) {\n-\t\tewah_free(istate->fsmonitor_dirty_bitmap);\n-\t\tistate->fsmonitor_dirty_bitmap = NULL;\n+\t\tewah_free(istate->fsmonitor_dirty);\n+\t\tistate->fsmonitor_dirty = NULL;\n \t\treturn error(\"failed to parse ewah bitmap reading fsmonitor index extension\");\n \t}\n \n@@ -86,7 +89,7 @@ void write_fsmonitor_extension(struct strbuf *sb, struct index_state *istate)\n \tuint32_t ewah_size = 0;\n \tint fixup = 0;\n \n-\thdr_version = htonl(1);\n+\thdr_version = htonl(INDEX_EXTENSION_VERSION);\n \tstrbuf_add(sb, &hdr_version, sizeof(uint32_t));\n \n \ttm = htonll((uint64_t)istate->fsmonitor_last_update);\n@@ -132,18 +135,21 @@ static void mark_file_dirty(struct index_state *istate, const char *name)\n /*\n  * Call the query-fsmonitor hook passing the time of the last saved results.\n  */\n-static int query_fsmonitor(time_t last_update, struct strbuf *query_result)\n+static int query_fsmonitor(int version, uint64_t last_update, struct strbuf *query_result)\n {\n \tstruct child_process cp = CHILD_PROCESS_INIT;\n+\tchar ver[64];\n \tchar date[64];\n-\tconst char *argv[3];\n+\tconst char *argv[4];\n \n \tif (!(argv[0] = find_hook(\"query-fsmonitor\")))\n \t\treturn -1;\n \n+\tsnprintf(ver, sizeof(version), \"%d\", version);\n \tsnprintf(date, sizeof(date), \"%\" PRIuMAX, (uintmax_t)last_update);\n-\targv[1] = date;\n-\targv[2] = NULL;\n+\targv[1] = ver;\n+\targv[2] = date;\n+\targv[3] = NULL;\n \tcp.argv = argv;\n \tcp.out = -1;\n \n@@ -152,38 +158,41 @@ static int query_fsmonitor(time_t last_update, struct strbuf *query_result)\n \n void process_fsmonitor_extension(struct index_state *istate)\n {\n-\tif (!istate->fsmonitor_dirty_bitmap)\n+\tif (!istate->fsmonitor_dirty)\n \t\treturn;\n \n-\tewah_each_bit(istate->fsmonitor_dirty_bitmap, mark_no_fsmonitor, istate);\n-\tewah_free(istate->fsmonitor_dirty_bitmap);\n-\tistate->fsmonitor_dirty_bitmap = NULL;\n+\tewah_each_bit(istate->fsmonitor_dirty, mark_no_fsmonitor, istate);\n+\tewah_free(istate->fsmonitor_dirty);\n+\tistate->fsmonitor_dirty = NULL;\n }\n \n void refresh_by_fsmonitor(struct index_state *istate)\n {\n-\tstatic int has_run_once = FALSE;\n+\tstatic int has_run_once = 0;\n \tstruct strbuf query_result = STRBUF_INIT;\n \tint query_success = 0;\n \tsize_t bol = 0; /* beginning of line */\n-\ttime_t last_update;\n+\tuint64_t last_update;\n \tchar *buf, *entry;\n \tint i;\n \n \tif (!core_fsmonitor || has_run_once)\n \t\treturn;\n-\thas_run_once = TRUE;\n+\thas_run_once = 1;\n \n \t/*\n \t * This could be racy so save the date/time now and the hook\n \t * should be inclusive to ensure we don't miss potential changes.\n \t */\n-\tlast_update = time(NULL);\n+\tlast_update = getnanotime();\n \n-\t/* If we have a last update time, call query-monitor for the set of changes since that time */\n-\tif (istate->fsmonitor_last_update) {\n-\t\tquery_success = !query_fsmonitor(istate->fsmonitor_last_update, &query_result);\n-\t}\n+\t/*\n+\t * If we have a last update time, call query-monitor for the set of\n+\t * changes since that time.\n+\t */\n+\tif (istate->fsmonitor_last_update)\n+\t\tquery_success = !query_fsmonitor(HOOK_INTERFACE_VERSION,\n+\t\t\t\t\t\t\tistate->fsmonitor_last_update, &query_result);\n \n \tif (query_success) {\n \t\t/* Mark all entries returned by the monitor as dirty */\n@@ -210,11 +219,9 @@ void refresh_by_fsmonitor(struct index_state *istate)\n \t\t * untracked-cache itself, we can mark the untracked cache for\n \t\t * fsmonitor usage.\n \t\t */\n-\t\tif (istate->untracked) {\n+\t\tif (istate->untracked)\n \t\t\tistate->untracked->use_fsmonitor = 1;\n-\t\t}\n-\t}\n-\telse {\n+\t} else {\n \t\t/* if we can't update the cache, fall back to checking them all */\n \t\tfor (i = 0; i < istate->cache_nr; i++)\n \t\t\tistate->cache[i]->ce_flags |= CE_FSMONITOR_DIRTY;\ndiff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh\nindex d3cffc758f..395db46d55 100755\n--- a/t/t7519-status-fsmonitor.sh\n+++ b/t/t7519-status-fsmonitor.sh\n@@ -7,7 +7,7 @@ test_description='git status with file system watcher'\n clean_repo () {\n \tgit reset --hard HEAD\n \tgit clean -fd\n-\trm marker -f\n+\trm -f marker\n }\n \n dirty_repo () {\n@@ -31,6 +31,11 @@ dirty_repo () {\n test_expect_success 'setup' '\n \tmkdir -p .git/hooks &&\n \twrite_script .git/hooks/query-fsmonitor<<-\\EOF &&\n+\tif [ $1 -ne 1 ]\n+\tthen\n+\t\techo -e \"Unsupported query-fsmonitor hook version.\\n\" >&2\n+\t\texit 1;\n+\tfi\n \t: >marker\n \tprintf \"untracked\\0\"\n \tprintf \"dir1/untracked\\0\"\ndiff --git a/templates/hooks--query-fsmonitor.sample b/templates/hooks--query-fsmonitor.sample\nindex 4bd22f21d8..615f3332fa 100644\n--- a/templates/hooks--query-fsmonitor.sample\n+++ b/templates/hooks--query-fsmonitor.sample\n@@ -4,13 +4,23 @@\n # (https://facebook.github.io/watchman/) with git to provide fast\n # git status.\n #\n-# The hook is passed a time_t formatted as a string and outputs to\n-# stdout all files that have been modified since the given time.\n-# Paths must be relative to the root of the working tree and\n+# The hook is passed a time in nanoseconds formatted as a string and\n+# outputs to stdout all files that have been modified since the given\n+# time. Paths must be relative to the root of the working tree and\n # separated by a single NUL.\n #\n # To enable this hook, rename this file to \"query-fsmonitor\"\n \n+# check the hook interface version\n+if [ $1 -eq 1 ]\n+then\n+\t# convert nanoseconds to seconds\n+\ttime_t=$(($2/1000000000))\n+else\n+\techo -e \"Unsupported query-fsmonitor hook version.\\nFalling back to scanning...\\n\" >&2\n+\texit 1;\n+fi\n+\n # Convert unix style paths to escaped Windows style paths\n case \"$(uname -s)\" in\n MINGW*|MSYS_NT*)\n@@ -22,6 +32,6 @@ MINGW*|MSYS_NT*)\n esac\n \n # Query Watchman for all the changes since the requested time\n-echo \"[\\\"query\\\", \\\"$GIT_WORK_TREE\\\", {\\\"since\\\": $1, \\\"fields\\\":[\\\"name\\\"]}]\" | \\\n+echo \"[\\\"query\\\", \\\"$GIT_WORK_TREE\\\", {\\\"since\\\": $time_t, \\\"fields\\\":[\\\"name\\\"]}]\" | \\\n watchman -j | \\\n perl -e 'use JSON::PP; my $o = JSON::PP->new->utf8->decode(join(\"\", <>)); die \"Watchman: $o->{'error'}.\\nFalling back to scanning...\\n\" if defined($o->{\"error\"}); print(join(\"\\0\", @{$o->{\"files\"}}));'\n\n\n-- \n2.13.0.windows.1.9.gc201c67b71\n\n"},{"id":"320722","messageId":"20170525183612.3128-7-benpeart@microsoft.com","threadId":"46042","inReplyTo":"20170525183612.3128-1-benpeart@microsoft.com","subject":"[PATCH v3 6/6] fsmonitor: add a sample query-fsmonitor hook script for Watchman","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2017-05-25T18:36:12Z","receivedAt":"2017-05-25T18:36:54Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"This hook script integrates the new fsmonitor capabilities of git with\nthe cross platform Watchman file watching service. To use the script:\n\nDownload and install Watchman from https://facebook.github.io/watchman/\nand instruct Watchman to watch your working directory for changes\n('watchman watch-project /usr/src/git').\n\nRename the sample integration hook from query-fsmonitor.sample to\nquery-fsmonitor.\n\nConfigure git to use the extension ('git config core.fsmonitor true')\nand optionally turn on the untracked cache for optimal performance\n('git config core.untrackedcache true').\n\nSigned-off-by: Ben Peart <benpeart@microsoft.com>\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n templates/hooks--query-fsmonitor.sample | 37 +++++++++++++++++++++++++++++++++\n 1 file changed, 37 insertions(+)\n create mode 100644 templates/hooks--query-fsmonitor.sample\n\ndiff --git a/templates/hooks--query-fsmonitor.sample b/templates/hooks--query-fsmonitor.sample\nnew file mode 100644\nindex 0000000000..615f3332fa\n--- /dev/null\n+++ b/templates/hooks--query-fsmonitor.sample\n@@ -0,0 +1,37 @@\n+#!/bin/sh\n+#\n+# An example hook script to integrate Watchman\n+# (https://facebook.github.io/watchman/) with git to provide fast\n+# git status.\n+#\n+# The hook is passed a time in nanoseconds formatted as a string and\n+# outputs to stdout all files that have been modified since the given\n+# time. Paths must be relative to the root of the working tree and\n+# separated by a single NUL.\n+#\n+# To enable this hook, rename this file to \"query-fsmonitor\"\n+\n+# check the hook interface version\n+if [ $1 -eq 1 ]\n+then\n+\t# convert nanoseconds to seconds\n+\ttime_t=$(($2/1000000000))\n+else\n+\techo -e \"Unsupported query-fsmonitor hook version.\\nFalling back to scanning...\\n\" >&2\n+\texit 1;\n+fi\n+\n+# Convert unix style paths to escaped Windows style paths\n+case \"$(uname -s)\" in\n+MINGW*|MSYS_NT*)\n+  GIT_WORK_TREE=\"$(cygpath -aw \"$PWD\" | sed 's,\\\\,\\\\\\\\,g')\"\n+  ;;\n+*)\n+  GIT_WORK_TREE=\"$PWD\"\n+  ;;\n+esac\n+\n+# Query Watchman for all the changes since the requested time\n+echo \"[\\\"query\\\", \\\"$GIT_WORK_TREE\\\", {\\\"since\\\": $time_t, \\\"fields\\\":[\\\"name\\\"]}]\" | \\\n+watchman -j | \\\n+perl -e 'use JSON::PP; my $o = JSON::PP->new->utf8->decode(join(\"\", <>)); die \"Watchman: $o->{'error'}.\\nFalling back to scanning...\\n\" if defined($o->{\"error\"}); print(join(\"\\0\", @{$o->{\"files\"}}));'\n-- \n2.13.0.windows.1.9.gc201c67b71\n\n"},{"id":"320723","messageId":"20170525183612.3128-4-benpeart@microsoft.com","threadId":"46042","inReplyTo":"20170525183612.3128-1-benpeart@microsoft.com","subject":"[PATCH v3 3/6] fsmonitor: teach git to optionally utilize a file system monitor to speed up detecting new or changed files.","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2017-05-25T18:36:09Z","receivedAt":"2017-05-25T18:36:58Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"When the index is read from disk, the query-fsmonitor index extension is\nused to flag the last known potentially dirty index and untracked cache\nentries.\n\nIf git finds out some entries are 'fsmonitor-dirty', but are really\nunchanged (e.g. the file was changed, then reverted back), then Git will\nclear the marking in the extension. If git adds or updates an index\nentry, it is marked 'fsmonitor-dirty' to ensure it is checked for\nchanges in the working directory.\n\nBefore the 'fsmonitor-dirty' flags are used to limit the scope of the\nfiles to be checked, the query-fsmonitor hook proc is called with the\ntime the index was last updated.  The hook proc returns the list of\nfiles changed since that last updated time and the list of\npotentially dirty entries is updated to reflect the current state.\n\nrefresh_index() and valid_cached_dir() are updated so that any entry not\nflagged as potentially dirty is not checked as it cannot have any\nchanges.\n\nSigned-off-by: Ben Peart <benpeart@microsoft.com>\n---\n Makefile               |   1 +\n builtin/update-index.c |   1 +\n cache.h                |   5 ++\n config.c               |   5 ++\n dir.c                  |  14 +++\n dir.h                  |   2 +\n entry.c                |   1 +\n environment.c          |   1 +\n fsmonitor.c            | 238 +++++++++++++++++++++++++++++++++++++++++++++++++\n fsmonitor.h            |   9 ++\n read-cache.c           |  28 +++++-\n unpack-trees.c         |   1 +\n 12 files changed, 304 insertions(+), 2 deletions(-)\n create mode 100644 fsmonitor.c\n create mode 100644 fsmonitor.h\n\ndiff --git a/Makefile b/Makefile\nindex e35542e631..488a4466cc 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -760,6 +760,7 @@ LIB_OBJS += ewah/ewah_rlw.o\n LIB_OBJS += exec_cmd.o\n LIB_OBJS += fetch-pack.o\n LIB_OBJS += fsck.o\n+LIB_OBJS += fsmonitor.o\n LIB_OBJS += gettext.o\n LIB_OBJS += gpg-interface.o\n LIB_OBJS += graph.o\ndiff --git a/builtin/update-index.c b/builtin/update-index.c\nindex ebfc09faa0..32fd977b43 100644\n--- a/builtin/update-index.c\n+++ b/builtin/update-index.c\n@@ -232,6 +232,7 @@ static int mark_ce_flags(const char *path, int flag, int mark)\n \t\telse\n \t\t\tactive_cache[pos]->ce_flags &= ~flag;\n \t\tactive_cache[pos]->ce_flags |= CE_UPDATE_IN_BASE;\n+\t\tactive_cache[pos]->ce_flags |= CE_FSMONITOR_DIRTY;\n \t\tcache_tree_invalidate_path(&the_index, path);\n \t\tactive_cache_changed |= CE_ENTRY_CHANGED;\n \t\treturn 0;\ndiff --git a/cache.h b/cache.h\nindex 188811920c..58e5abf69f 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -201,6 +201,7 @@ struct cache_entry {\n #define CE_ADDED             (1 << 19)\n \n #define CE_HASHED            (1 << 20)\n+#define CE_FSMONITOR_DIRTY   (1 << 21)\n #define CE_WT_REMOVE         (1 << 22) /* remove in work directory */\n #define CE_CONFLICTED        (1 << 23)\n \n@@ -324,6 +325,7 @@ static inline unsigned int canon_mode(unsigned int mode)\n #define CACHE_TREE_CHANGED\t(1 << 5)\n #define SPLIT_INDEX_ORDERED\t(1 << 6)\n #define UNTRACKED_CHANGED\t(1 << 7)\n+#define FSMONITOR_CHANGED\t(1 << 8)\n \n struct split_index;\n struct untracked_cache;\n@@ -342,6 +344,8 @@ struct index_state {\n \tstruct hashmap dir_hash;\n \tunsigned char sha1[20];\n \tstruct untracked_cache *untracked;\n+\tuint64_t fsmonitor_last_update;\n+\tstruct ewah_bitmap *fsmonitor_dirty;\n };\n \n extern struct index_state the_index;\n@@ -766,6 +770,7 @@ extern int precomposed_unicode;\n extern int protect_hfs;\n extern int protect_ntfs;\n extern int git_db_env, git_index_env, git_graft_env, git_common_dir_env;\n+extern int core_fsmonitor;\n \n /*\n  * Include broken refs in all ref iterations, which will\ndiff --git a/config.c b/config.c\nindex bb4d735701..1a8108636d 100644\n--- a/config.c\n+++ b/config.c\n@@ -1232,6 +1232,11 @@ static int git_default_core_config(const char *var, const char *value)\n \t\treturn 0;\n \t}\n \n+\tif (!strcmp(var, \"core.fsmonitor\")) {\n+\t\tcore_fsmonitor = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n+\n \t/* Add other config variables here and to Documentation/config.txt. */\n \treturn 0;\n }\ndiff --git a/dir.c b/dir.c\nindex 1b5558fdf9..37f1c580a5 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -16,6 +16,7 @@\n #include \"utf8.h\"\n #include \"varint.h\"\n #include \"ewah/ewok.h\"\n+#include \"fsmonitor.h\"\n \n /*\n  * Tells read_directory_recursive how a file or directory should be treated.\n@@ -1652,6 +1653,18 @@ static int valid_cached_dir(struct dir_struct *dir,\n \tif (!untracked)\n \t\treturn 0;\n \n+\trefresh_by_fsmonitor(&the_index);\n+\tif (dir->untracked->use_fsmonitor) {\n+\t\t/*\n+\t\t * With fsmonitor, we can trust the untracked cache's\n+\t\t * valid field.\n+\t\t */\n+\t\tif (untracked->valid)\n+\t\t\tgoto skip_stat;\n+\t\telse\n+\t\t\tinvalidate_directory(dir->untracked, untracked);\n+\t}\n+\n \tif (stat(path->len ? path->buf : \".\", &st)) {\n \t\tinvalidate_directory(dir->untracked, untracked);\n \t\tmemset(&untracked->stat_data, 0, sizeof(untracked->stat_data));\n@@ -1665,6 +1678,7 @@ static int valid_cached_dir(struct dir_struct *dir,\n \t\treturn 0;\n \t}\n \n+skip_stat:\n \tif (untracked->check_only != !!check_only) {\n \t\tinvalidate_directory(dir->untracked, untracked);\n \t\treturn 0;\ndiff --git a/dir.h b/dir.h\nindex 9e387551bd..ff6a00abcc 100644\n--- a/dir.h\n+++ b/dir.h\n@@ -139,6 +139,8 @@ struct untracked_cache {\n \tint gitignore_invalidated;\n \tint dir_invalidated;\n \tint dir_opened;\n+\t/* fsmonitor invalidation data */\n+\tunsigned int use_fsmonitor : 1;\n };\n \n struct dir_struct {\ndiff --git a/entry.c b/entry.c\nindex d2b512da90..c2d3c1079c 100644\n--- a/entry.c\n+++ b/entry.c\n@@ -221,6 +221,7 @@ static int write_entry(struct cache_entry *ce,\n \t\t\tlstat(ce->name, &st);\n \t\tfill_stat_cache_info(ce, &st);\n \t\tce->ce_flags |= CE_UPDATE_IN_BASE;\n+\t\tce->ce_flags |= CE_FSMONITOR_DIRTY;\n \t\tstate->istate->cache_changed |= CE_ENTRY_CHANGED;\n \t}\n \treturn 0;\ndiff --git a/environment.c b/environment.c\nindex 560408953c..1afabbae8c 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -64,6 +64,7 @@ int precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */\n unsigned long pack_size_limit_cfg;\n enum hide_dotfiles_type hide_dotfiles = HIDE_DOTFILES_DOTGITONLY;\n enum log_refs_config log_all_ref_updates = LOG_REFS_UNSET;\n+int core_fsmonitor;\n \n #ifndef PROTECT_HFS_DEFAULT\n #define PROTECT_HFS_DEFAULT 0\ndiff --git a/fsmonitor.c b/fsmonitor.c\nnew file mode 100644\nindex 0000000000..3ce262d47d\n--- /dev/null\n+++ b/fsmonitor.c\n@@ -0,0 +1,238 @@\n+#include \"cache.h\"\n+#include \"dir.h\"\n+#include \"ewah/ewok.h\"\n+#include \"run-command.h\"\n+#include \"strbuf.h\"\n+#include \"fsmonitor.h\"\n+\n+#define INDEX_EXTENSION_VERSION\t1\n+#define HOOK_INTERFACE_VERSION\t\t1\n+\n+static struct untracked_cache_dir *find_untracked_cache_dir(\n+\tstruct untracked_cache *uc, struct untracked_cache_dir *ucd,\n+\tconst char *name)\n+{\n+\tconst char *end;\n+\tstruct untracked_cache_dir *dir = ucd;\n+\n+\tif (!*name)\n+\t\treturn dir;\n+\n+\tend = strchr(name, '/');\n+\tif (end) {\n+\t\tdir = lookup_untracked(uc, ucd, name, end - name);\n+\t\tif (dir)\n+\t\t\treturn find_untracked_cache_dir(uc, dir, end + 1);\n+\t}\n+\n+\treturn dir;\n+}\n+\n+/* This function will be passed to ewah_each_bit() */\n+static void mark_no_fsmonitor(size_t pos, void *is)\n+{\n+\tstruct index_state *istate = is;\n+\tstruct untracked_cache_dir *dir;\n+\tstruct cache_entry *ce = istate->cache[pos];\n+\n+\tassert(pos < istate->cache_nr);\n+\tce->ce_flags |= CE_FSMONITOR_DIRTY;\n+\n+\tif (!istate->untracked || !istate->untracked->root)\n+\t\treturn;\n+\n+\tdir = find_untracked_cache_dir(istate->untracked, istate->untracked->root, ce->name);\n+\tif (dir)\n+\t\tdir->valid = 0;\n+}\n+\n+int read_fsmonitor_extension(struct index_state *istate, const void *data,\n+\tunsigned long sz)\n+{\n+\tconst char *index = data;\n+\tuint32_t hdr_version;\n+\tuint32_t ewah_size;\n+\tint ret;\n+\n+\tif (sz < sizeof(uint32_t) + sizeof(uint64_t) + sizeof(uint32_t))\n+\t\treturn error(\"corrupt fsmonitor extension (too short)\");\n+\n+\thdr_version = get_be32(index);\n+\tindex += sizeof(uint32_t);\n+\tif (hdr_version != INDEX_EXTENSION_VERSION)\n+\t\treturn error(\"bad fsmonitor version %d\", hdr_version);\n+\n+\tistate->fsmonitor_last_update = get_be64(index);\n+\tindex += sizeof(uint64_t);\n+\n+\tewah_size = get_be32(index);\n+\tindex += sizeof(uint32_t);\n+\n+\tistate->fsmonitor_dirty = ewah_new();\n+\tret = ewah_read_mmap(istate->fsmonitor_dirty, index, ewah_size);\n+\tif (ret != ewah_size) {\n+\t\tewah_free(istate->fsmonitor_dirty);\n+\t\tistate->fsmonitor_dirty = NULL;\n+\t\treturn error(\"failed to parse ewah bitmap reading fsmonitor index extension\");\n+\t}\n+\n+\treturn 0;\n+}\n+\n+void write_fsmonitor_extension(struct strbuf *sb, struct index_state *istate)\n+{\n+\tuint32_t hdr_version;\n+\tuint64_t tm;\n+\tstruct ewah_bitmap *bitmap;\n+\tint i;\n+\tuint32_t ewah_start;\n+\tuint32_t ewah_size = 0;\n+\tint fixup = 0;\n+\n+\thdr_version = htonl(INDEX_EXTENSION_VERSION);\n+\tstrbuf_add(sb, &hdr_version, sizeof(uint32_t));\n+\n+\ttm = htonll((uint64_t)istate->fsmonitor_last_update);\n+\tstrbuf_add(sb, &tm, sizeof(uint64_t));\n+\tfixup = sb->len;\n+\tstrbuf_add(sb, &ewah_size, sizeof(uint32_t)); /* we'll fix this up later */\n+\n+\tewah_start = sb->len;\n+\tbitmap = ewah_new();\n+\tfor (i = 0; i < istate->cache_nr; i++)\n+\t\tif (istate->cache[i]->ce_flags & CE_FSMONITOR_DIRTY)\n+\t\t\tewah_set(bitmap, i);\n+\tewah_serialize_strbuf(bitmap, sb);\n+\tewah_free(bitmap);\n+\n+\t/* fix up size field */\n+\tewah_size = htonl(sb->len - ewah_start);\n+\tmemcpy(sb->buf + fixup, &ewah_size, sizeof(uint32_t));\n+}\n+\n+static void mark_file_dirty(struct index_state *istate, const char *name)\n+{\n+\tstruct untracked_cache_dir *dir;\n+\tint pos;\n+\n+\t/* find it in the index and mark that entry as dirty */\n+\tpos = index_name_pos(istate, name, strlen(name));\n+\tif (pos >= 0)\n+\t\tistate->cache[pos]->ce_flags |= CE_FSMONITOR_DIRTY;\n+\n+\t/*\n+\t * Find the corresponding directory in the untracked cache\n+\t * and mark it as invalid\n+\t */\n+\tif (!istate->untracked || !istate->untracked->root)\n+\t\treturn;\n+\n+\tdir = find_untracked_cache_dir(istate->untracked, istate->untracked->root, name);\n+\tif (dir)\n+\t\tdir->valid = 0;\n+}\n+\n+/*\n+ * Call the query-fsmonitor hook passing the time of the last saved results.\n+ */\n+static int query_fsmonitor(int version, uint64_t last_update, struct strbuf *query_result)\n+{\n+\tstruct child_process cp = CHILD_PROCESS_INIT;\n+\tchar ver[64];\n+\tchar date[64];\n+\tconst char *argv[4];\n+\n+\tif (!(argv[0] = find_hook(\"query-fsmonitor\")))\n+\t\treturn -1;\n+\n+\tsnprintf(ver, sizeof(version), \"%d\", version);\n+\tsnprintf(date, sizeof(date), \"%\" PRIuMAX, (uintmax_t)last_update);\n+\targv[1] = ver;\n+\targv[2] = date;\n+\targv[3] = NULL;\n+\tcp.argv = argv;\n+\tcp.out = -1;\n+\n+\treturn capture_command(&cp, query_result, 1024);\n+}\n+\n+void process_fsmonitor_extension(struct index_state *istate)\n+{\n+\tif (!istate->fsmonitor_dirty)\n+\t\treturn;\n+\n+\tewah_each_bit(istate->fsmonitor_dirty, mark_no_fsmonitor, istate);\n+\tewah_free(istate->fsmonitor_dirty);\n+\tistate->fsmonitor_dirty = NULL;\n+}\n+\n+void refresh_by_fsmonitor(struct index_state *istate)\n+{\n+\tstatic int has_run_once = 0;\n+\tstruct strbuf query_result = STRBUF_INIT;\n+\tint query_success = 0;\n+\tsize_t bol = 0; /* beginning of line */\n+\tuint64_t last_update;\n+\tchar *buf, *entry;\n+\tint i;\n+\n+\tif (!core_fsmonitor || has_run_once)\n+\t\treturn;\n+\thas_run_once = 1;\n+\n+\t/*\n+\t * This could be racy so save the date/time now and the hook\n+\t * should be inclusive to ensure we don't miss potential changes.\n+\t */\n+\tlast_update = getnanotime();\n+\n+\t/*\n+\t * If we have a last update time, call query-monitor for the set of\n+\t * changes since that time.\n+\t */\n+\tif (istate->fsmonitor_last_update)\n+\t\tquery_success = !query_fsmonitor(HOOK_INTERFACE_VERSION,\n+\t\t\t\t\t\t\tistate->fsmonitor_last_update, &query_result);\n+\n+\tif (query_success) {\n+\t\t/* Mark all entries returned by the monitor as dirty */\n+\t\tbuf = entry = query_result.buf;\n+\t\tfor (i = 0; i < query_result.len; i++) {\n+\t\t\tif (buf[i] != '\\0')\n+\t\t\t\tcontinue;\n+\t\t\tmark_file_dirty(istate, buf + bol);\n+\t\t\tbol = i + 1;\n+\t\t}\n+\t\tif (bol < query_result.len)\n+\t\t\tmark_file_dirty(istate, buf + bol);\n+\n+\t\t/* Mark all clean entries up-to-date */\n+\t\tfor (i = 0; i < istate->cache_nr; i++) {\n+\t\t\tstruct cache_entry *ce = istate->cache[i];\n+\t\t\tif (ce_stage(ce) || (ce->ce_flags & CE_FSMONITOR_DIRTY))\n+\t\t\t\tcontinue;\n+\t\t\tce_mark_uptodate(ce);\n+\t\t}\n+\n+\t\t/*\n+\t\t * Now that we've marked the invalid entries in the\n+\t\t * untracked-cache itself, we can mark the untracked cache for\n+\t\t * fsmonitor usage.\n+\t\t */\n+\t\tif (istate->untracked)\n+\t\t\tistate->untracked->use_fsmonitor = 1;\n+\t} else {\n+\t\t/* if we can't update the cache, fall back to checking them all */\n+\t\tfor (i = 0; i < istate->cache_nr; i++)\n+\t\t\tistate->cache[i]->ce_flags |= CE_FSMONITOR_DIRTY;\n+\n+\t\t/* mark the untracked cache as unusable for fsmonitor */\n+\t\tif (istate->untracked)\n+\t\t\tistate->untracked->use_fsmonitor = 0;\n+\t}\n+\tstrbuf_release(&query_result);\n+\n+\t/* Now that we've updated istate, save the last_update time */\n+\tistate->fsmonitor_last_update = last_update;\n+\tistate->cache_changed |= FSMONITOR_CHANGED;\n+}\ndiff --git a/fsmonitor.h b/fsmonitor.h\nnew file mode 100644\nindex 0000000000..57b061688f\n--- /dev/null\n+++ b/fsmonitor.h\n@@ -0,0 +1,9 @@\n+#ifndef FSMONITOR_H\n+#define FSMONITOR_H\n+\n+int read_fsmonitor_extension(struct index_state *istate, const void *data, unsigned long sz);\n+void write_fsmonitor_extension(struct strbuf *sb, struct index_state *istate);\n+void process_fsmonitor_extension(struct index_state *istate);\n+void refresh_by_fsmonitor(struct index_state *istate);\n+\n+#endif\ndiff --git a/read-cache.c b/read-cache.c\nindex 3339de8124..5b52f08db2 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -18,6 +18,7 @@\n #include \"varint.h\"\n #include \"split-index.h\"\n #include \"utf8.h\"\n+#include \"fsmonitor.h\"\n \n /* Mask for the name length in ce_flags in the on-disk index */\n \n@@ -37,11 +38,12 @@\n #define CACHE_EXT_RESOLVE_UNDO 0x52455543 /* \"REUC\" */\n #define CACHE_EXT_LINK 0x6c696e6b\t  /* \"link\" */\n #define CACHE_EXT_UNTRACKED 0x554E5452\t  /* \"UNTR\" */\n+#define CACHE_EXT_FSMONITOR 0x46534D4E\t  /* \"FSMN\" */\n \n /* changes that can be kept in $GIT_DIR/index (basically all extensions) */\n #define EXTMASK (RESOLVE_UNDO_CHANGED | CACHE_TREE_CHANGED | \\\n \t\t CE_ENTRY_ADDED | CE_ENTRY_REMOVED | CE_ENTRY_CHANGED | \\\n-\t\t SPLIT_INDEX_ORDERED | UNTRACKED_CHANGED)\n+\t\t SPLIT_INDEX_ORDERED | UNTRACKED_CHANGED | FSMONITOR_CHANGED)\n \n struct index_state the_index;\n static const char *alternate_index_output;\n@@ -61,6 +63,7 @@ static void replace_index_entry(struct index_state *istate, int nr, struct cache\n \tfree(old);\n \tset_index_entry(istate, nr, ce);\n \tce->ce_flags |= CE_UPDATE_IN_BASE;\n+\tce->ce_flags |= CE_FSMONITOR_DIRTY;\n \tistate->cache_changed |= CE_ENTRY_CHANGED;\n }\n \n@@ -777,6 +780,7 @@ int chmod_index_entry(struct index_state *istate, struct cache_entry *ce,\n \t}\n \tcache_tree_invalidate_path(istate, ce->name);\n \tce->ce_flags |= CE_UPDATE_IN_BASE;\n+\tce->ce_flags |= CE_FSMONITOR_DIRTY;\n \tistate->cache_changed |= CE_ENTRY_CHANGED;\n \n \treturn 0;\n@@ -1344,6 +1348,8 @@ int refresh_index(struct index_state *istate, unsigned int flags,\n \tconst char *added_fmt;\n \tconst char *unmerged_fmt;\n \n+\trefresh_by_fsmonitor(istate);\n+\n \tmodified_fmt = (in_porcelain ? \"M\\t%s\\n\" : \"%s: needs update\\n\");\n \tdeleted_fmt = (in_porcelain ? \"D\\t%s\\n\" : \"%s: needs update\\n\");\n \ttypechange_fmt = (in_porcelain ? \"T\\t%s\\n\" : \"%s needs update\\n\");\n@@ -1380,8 +1386,11 @@ int refresh_index(struct index_state *istate, unsigned int flags,\n \t\t\tcontinue;\n \n \t\tnew = refresh_cache_ent(istate, ce, options, &cache_errno, &changed);\n-\t\tif (new == ce)\n+\t\tif (new == ce) {\n+\t\t\tce->ce_flags &= ~CE_FSMONITOR_DIRTY;\n \t\t\tcontinue;\n+\t\t}\n+\n \t\tif (!new) {\n \t\t\tconst char *fmt;\n \n@@ -1391,6 +1400,7 @@ int refresh_index(struct index_state *istate, unsigned int flags,\n \t\t\t\t */\n \t\t\t\tce->ce_flags &= ~CE_VALID;\n \t\t\t\tce->ce_flags |= CE_UPDATE_IN_BASE;\n+\t\t\t\tce->ce_flags |= CE_FSMONITOR_DIRTY;\n \t\t\t\tistate->cache_changed |= CE_ENTRY_CHANGED;\n \t\t\t}\n \t\t\tif (quiet)\n@@ -1549,6 +1559,9 @@ static int read_index_extension(struct index_state *istate,\n \tcase CACHE_EXT_UNTRACKED:\n \t\tistate->untracked = read_untracked_extension(data, sz);\n \t\tbreak;\n+\tcase CACHE_EXT_FSMONITOR:\n+\t\tread_fsmonitor_extension(istate, data, sz);\n+\t\tbreak;\n \tdefault:\n \t\tif (*ext < 'A' || 'Z' < *ext)\n \t\t\treturn error(\"index uses %.4s extension, which we do not understand\",\n@@ -1721,6 +1734,7 @@ static void post_read_index_from(struct index_state *istate)\n \tcheck_ce_order(istate);\n \ttweak_untracked_cache(istate);\n \ttweak_split_index(istate);\n+\tprocess_fsmonitor_extension(istate);\n }\n \n /* remember to discard_cache() before reading a different cache! */\n@@ -2300,6 +2314,16 @@ static int do_write_index(struct index_state *istate, int newfd,\n \t\tif (err)\n \t\t\treturn -1;\n \t}\n+\tif (!strip_extensions && istate->fsmonitor_last_update) {\n+\t\tstruct strbuf sb = STRBUF_INIT;\n+\n+\t\twrite_fsmonitor_extension(&sb, istate);\n+\t\terr = write_index_ext_header(&c, newfd, CACHE_EXT_FSMONITOR, sb.len) < 0\n+\t\t\t|| ce_write(&c, newfd, sb.buf, sb.len) < 0;\n+\t\tstrbuf_release(&sb);\n+\t\tif (err)\n+\t\t\treturn -1;\n+\t}\n \n \tif (ce_flush(&c, newfd, istate->sha1) || fstat(newfd, &st))\n \t\treturn -1;\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex aa15111fef..259e6960b9 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -412,6 +412,7 @@ static int apply_sparse_checkout(struct index_state *istate,\n \t\tce->ce_flags &= ~CE_SKIP_WORKTREE;\n \tif (was_skip_worktree != ce_skip_worktree(ce)) {\n \t\tce->ce_flags |= CE_UPDATE_IN_BASE;\n+\t\tce->ce_flags |= CE_FSMONITOR_DIRTY;\n \t\tistate->cache_changed |= CE_ENTRY_CHANGED;\n \t}\n \n-- \n2.13.0.windows.1.9.gc201c67b71\n\n"},{"id":"320724","messageId":"20170525183612.3128-6-benpeart@microsoft.com","threadId":"46042","inReplyTo":"20170525183612.3128-1-benpeart@microsoft.com","subject":"[PATCH v3 5/6] fsmonitor: add documentation for the fsmonitor extension.","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2017-05-25T18:36:11Z","receivedAt":"2017-05-25T18:37:03Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"This includes the core.fsmonitor setting, the query-fsmonitor hook,\nand the fsmonitor index extension.\n\nSigned-off-by: Ben Peart <benpeart@microsoft.com>\n---\n Documentation/config.txt                 |  7 +++++++\n Documentation/githooks.txt               | 23 +++++++++++++++++++++++\n Documentation/technical/index-format.txt | 19 +++++++++++++++++++\n 3 files changed, 49 insertions(+)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex e0b9fd0bc3..5211388167 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -389,6 +389,13 @@ core.protectNTFS::\n \t8.3 \"short\" names.\n \tDefaults to `true` on Windows, and `false` elsewhere.\n \n+core.fsmonitor::\n+\tIf set to true, call the query-fsmonitor hook proc which will\n+\tidentify all files that may have had changes since the last\n+\trequest. This information is used to speed up operations like\n+\t'git commit' and 'git status' by limiting what git must scan to\n+\tdetect changes.\n+\n core.trustctime::\n \tIf false, the ctime differences between the index and the\n \tworking tree are ignored; useful when the inode change time\ndiff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\nindex 706091a569..48127e8729 100644\n--- a/Documentation/githooks.txt\n+++ b/Documentation/githooks.txt\n@@ -448,6 +448,29 @@ The commits are guaranteed to be listed in the order that they were\n processed by rebase.\n \n \n+[[query-fsmonitor]]\n+query-fsmonitor\n+~~~~~~~~~~~~~~~\n+\n+This hook is invoked when the configuration option core.fsmonitor is\n+set and git needs to identify changed or untracked files.  It takes\n+two arguments, a version (currently 1) and the time in elapsed\n+nanoseconds since midnight, January 1, 1970.\n+\n+The hook should output to stdout the list of all files in the working\n+directory that may have changed since the requested time.  The logic\n+should be inclusive so that it does not miss any potential changes.\n+The paths should be relative to the root of the working directory\n+and be separated by a single NUL.\n+\n+Git will limit what files it checks for changes as well as which\n+directories are checked for untracked files based on the path names\n+given.\n+\n+The exit status determines whether git will use the data from the\n+hook to limit its search.  On error, it will fall back to verifying\n+all files and folders.\n+\n GIT\n ---\n Part of the linkgit:git[1] suite\ndiff --git a/Documentation/technical/index-format.txt b/Documentation/technical/index-format.txt\nindex ade0b0c445..7aeeea6f94 100644\n--- a/Documentation/technical/index-format.txt\n+++ b/Documentation/technical/index-format.txt\n@@ -295,3 +295,22 @@ The remaining data of each directory block is grouped by type:\n     in the previous ewah bitmap.\n \n   - One NUL.\n+\n+== File System Monitor cache\n+\n+  The file system monitor cache tracks files for which the query-fsmonitor\n+  hook has told us about changes.  The signature for this extension is\n+  { 'F', 'S', 'M', 'N' }.\n+\n+  The extension starts with\n+\n+  - 32-bit version number: the current supported version is 1.\n+\n+  - 64-bit time: the extension data reflects all changes through the given\n+\ttime which is stored as the nanoseconds elapsed since midnight,\n+\tJanuary 1, 1970.\n+\n+  - 32-bit bitmap size: the size of the CE_FSMONITOR_DIRTY bitmap.\n+\n+  - An ewah bitmap, the n-th bit indicates whether the n-th index entry\n+    is CE_FSMONITOR_DIRTY.\n-- \n2.13.0.windows.1.9.gc201c67b71\n\n"},{"id":"320725","messageId":"20170525183612.3128-3-benpeart@microsoft.com","threadId":"46042","inReplyTo":"20170525183612.3128-1-benpeart@microsoft.com","subject":"[PATCH v3 2/6] dir: make lookup_untracked() available outside of dir.c","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2017-05-25T18:36:08Z","receivedAt":"2017-05-25T18:37:07Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"Remove the static qualifier from lookup_untracked() and make it\navailable to other modules by exporting it from dir.h.  This will be\nused later when we need to find entries to mark 'fsmonitor dirty.'\n\nSigned-off-by: Ben Peart <benpeart@microsoft.com>\n---\n dir.c | 2 +-\n dir.h | 3 +++\n 2 files changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/dir.c b/dir.c\nindex f451bfa48c..1b5558fdf9 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -660,7 +660,7 @@ static void trim_trailing_spaces(char *buf)\n  *\n  * If \"name\" has the trailing slash, it'll be excluded in the search.\n  */\n-static struct untracked_cache_dir *lookup_untracked(struct untracked_cache *uc,\n+struct untracked_cache_dir *lookup_untracked(struct untracked_cache *uc,\n \t\t\t\t\t\t    struct untracked_cache_dir *dir,\n \t\t\t\t\t\t    const char *name, int len)\n {\ndiff --git a/dir.h b/dir.h\nindex bf23a470af..9e387551bd 100644\n--- a/dir.h\n+++ b/dir.h\n@@ -339,4 +339,7 @@ extern void connect_work_tree_and_git_dir(const char *work_tree, const char *git\n extern void relocate_gitdir(const char *path,\n \t\t\t    const char *old_git_dir,\n \t\t\t    const char *new_git_dir);\n+struct untracked_cache_dir *lookup_untracked(struct untracked_cache *uc,\n+\t\t\t\t\t     struct untracked_cache_dir *dir,\n+\t\t\t\t\t     const char *name, int len);\n #endif\n-- \n2.13.0.windows.1.9.gc201c67b71\n\n"},{"id":"320726","messageId":"20170525183612.3128-5-benpeart@microsoft.com","threadId":"46042","inReplyTo":"20170525183612.3128-1-benpeart@microsoft.com","subject":"[PATCH v3 4/6] fsmonitor: add test cases for fsmonitor extension","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2017-05-25T18:36:10Z","receivedAt":"2017-05-25T18:37:13Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"Add test cases that ensure status results are correct when using the new\nfsmonitor extension.  Test untracked, modified, and new files by\nensuring the results are identical to when not using the extension.\n\nAdd a test to ensure updates to the index properly mark corresponding\nentries in the index extension as dirty so that the status is correct\nafter commands that modify the index but don't trigger changes in the\nworking directory.\n\nAdd a test that verifies that if the fsmonitor extension doesn't tell\ngit about a change, it doesn't discover it on its own.  This ensures\ngit is honoring the extension and that we get the performance benefits\ndesired.\n\nAll test hooks output a marker file that is used to ensure the hook\nwas actually used to generate the test results.\n\nSigned-off-by: Ben Peart <benpeart@microsoft.com>\n---\n t/t7519-status-fsmonitor.sh | 158 ++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 158 insertions(+)\n create mode 100755 t/t7519-status-fsmonitor.sh\n\ndiff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh\nnew file mode 100755\nindex 0000000000..395db46d55\n--- /dev/null\n+++ b/t/t7519-status-fsmonitor.sh\n@@ -0,0 +1,158 @@\n+#!/bin/sh\n+\n+test_description='git status with file system watcher'\n+\n+. ./test-lib.sh\n+\n+clean_repo () {\n+\tgit reset --hard HEAD\n+\tgit clean -fd\n+\trm -f marker\n+}\n+\n+dirty_repo () {\n+\t: >untracked\n+\t: >dir1/untracked\n+\t: >dir2/untracked\n+\techo 1 >modified\n+\techo 2 >dir1/modified\n+\techo 3 >dir2/modified\n+\techo 4 >new\n+\techo 5 >dir1/new\n+\techo 6 >dir2/new\n+\tgit add new\n+\tgit add dir1/new\n+\tgit add dir2/new\n+}\n+\n+# The test query-fsmonitor hook proc will output a marker file we can use to\n+# ensure the hook was actually used to generate the correct results.\n+\n+test_expect_success 'setup' '\n+\tmkdir -p .git/hooks &&\n+\twrite_script .git/hooks/query-fsmonitor<<-\\EOF &&\n+\tif [ $1 -ne 1 ]\n+\tthen\n+\t\techo -e \"Unsupported query-fsmonitor hook version.\\n\" >&2\n+\t\texit 1;\n+\tfi\n+\t: >marker\n+\tprintf \"untracked\\0\"\n+\tprintf \"dir1/untracked\\0\"\n+\tprintf \"dir2/untracked\\0\"\n+\tprintf \"modified\\0\"\n+\tprintf \"dir1/modified\\0\"\n+\tprintf \"dir2/modified\\0\"\n+\tprintf \"new\\0\"\"\n+\tprintf \"dir1/new\\0\"\n+\tprintf \"dir2/new\\0\"\n+\tEOF\n+\t: >tracked &&\n+\t: >modified &&\n+\tmkdir dir1 &&\n+\t: >dir1/tracked &&\n+\t: >dir1/modified &&\n+\tmkdir dir2 &&\n+\t: >dir2/tracked &&\n+\t: >dir2/modified &&\n+\tgit add . &&\n+\ttest_tick &&\n+\tgit commit -m initial &&\n+\tdirty_repo\n+'\n+\n+cat >.gitignore <<\\EOF\n+.gitignore\n+expect*\n+output*\n+marker*\n+EOF\n+\n+# Status is well tested elsewhere so we'll just ensure that the results are\n+# the same when using core.fsmonitor. First call after turning on the option\n+# does a complete scan so need to do two calls to ensure we test the new\n+# codepath.\n+\n+test_expect_success 'status with core.untrackedcache true' '\n+\tgit config core.fsmonitor true  &&\n+\tgit config core.untrackedcache true &&\n+\tgit -c core.fsmonitor=false -c core.untrackedcache=true status >expect &&\n+\tclean_repo &&\n+\tgit status &&\n+\ttest_path_is_missing marker &&\n+\tdirty_repo &&\n+\tgit status >output &&\n+\ttest_path_is_file marker &&\n+\ttest_i18ncmp expect output\n+'\n+\n+\n+test_expect_success 'status with core.untrackedcache false' '\n+\tgit config core.fsmonitor true &&\n+\tgit config core.untrackedcache false &&\n+\tgit -c core.fsmonitor=false -c core.untrackedcache=false status >expect &&\n+\tclean_repo &&\n+\tgit status &&\n+\ttest_path_is_missing marker &&\n+\tdirty_repo &&\n+\tgit status >output &&\n+\ttest_path_is_file marker &&\n+\ttest_i18ncmp expect output\n+'\n+\n+# Ensure commands that call refresh_index() to move the index back in time\n+# properly invalidate the fsmonitor cache\n+\n+test_expect_success 'refresh_index() invalidates fsmonitor cache' '\n+\tgit config core.fsmonitor true &&\n+\tgit config core.untrackedcache true &&\n+\tclean_repo &&\n+\tgit status &&\n+\ttest_path_is_missing marker &&\n+\tdirty_repo &&\n+\twrite_script .git/hooks/query-fsmonitor<<-\\EOF &&\n+\t:>marker\n+\tEOF\n+\tgit add . &&\n+\tgit commit -m \"to reset\" &&\n+\tgit status &&\n+\ttest_path_is_file marker &&\n+\tgit reset HEAD~1 &&\n+\tgit status >output &&\n+\ttest_path_is_file marker &&\n+\tgit -c core.fsmonitor=false status >expect &&\n+\ttest_i18ncmp expect output\n+'\n+\n+# Now make sure it's actually skipping the check for modified and untracked\n+# files unless it is told about them.  Note, after \"git reset --hard HEAD\" no\n+# extensions exist other than 'TREE' so do a \"git status\" to get the extension\n+# written before testing the results.\n+\n+test_expect_success 'status doesnt detect unreported modifications' '\n+\tgit config core.fsmonitor true &&\n+\tgit config core.untrackedcache true &&\n+\twrite_script .git/hooks/query-fsmonitor<<-\\EOF &&\n+\t:>marker\n+\tEOF\n+\tclean_repo &&\n+\tgit status &&\n+\ttest_path_is_missing marker &&\n+\t: >untracked &&\n+\techo 2 >dir1/modified &&\n+\tgit status >output &&\n+\ttest_path_is_file marker &&\n+\ttest_i18ngrep ! \"Changes not staged for commit:\" output &&\n+\ttest_i18ngrep ! \"Untracked files:\" output &&\n+\twrite_script .git/hooks/query-fsmonitor<<-\\EOF &&\n+\t:>marker\n+\tprintf \"untracked%s\\0\"\n+\tprintf \"dir1/modified\\0\"\n+\tEOF\n+\tgit status >output &&\n+\ttest_path_is_file marker &&\n+\ttest_i18ngrep \"Changes not staged for commit:\" output &&\n+\ttest_i18ngrep \"Untracked files:\" output\n+'\n+\n+test_done\n-- \n2.13.0.windows.1.9.gc201c67b71\n\n"},{"id":"321018","messageId":"CAP8UFD0OEUogQYL5Eo1kBy_1B=dBcdEJyc4B_qAUOU=2XnrxxA@mail.gmail.com","threadId":"46042","inReplyTo":"20170525183612.3128-5-benpeart@microsoft.com","subject":"Re: [PATCH v3 4/6] fsmonitor: add test cases for fsmonitor extension","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2017-05-30T13:18:49Z","receivedAt":"2017-05-30T13:19:02Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Thu, May 25, 2017 at 8:36 PM, Ben Peart <peartben@gmail.com> wrote:\n\n[...]\n\n> diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh\n> new file mode 100755\n> index 0000000000..395db46d55\n> --- /dev/null\n> +++ b/t/t7519-status-fsmonitor.sh\n> @@ -0,0 +1,158 @@\n> +#!/bin/sh\n> +\n> +test_description='git status with file system watcher'\n> +\n> +. ./test-lib.sh\n> +\n> +clean_repo () {\n> +       git reset --hard HEAD\n> +       git clean -fd\n> +       rm -f marker\n> +}\n\nMaybe link all the commands in the function with \"&&\".\n\n> +dirty_repo () {\n> +       : >untracked\n> +       : >dir1/untracked\n> +       : >dir2/untracked\n> +       echo 1 >modified\n> +       echo 2 >dir1/modified\n> +       echo 3 >dir2/modified\n> +       echo 4 >new\n> +       echo 5 >dir1/new\n> +       echo 6 >dir2/new\n> +       git add new\n> +       git add dir1/new\n> +       git add dir2/new\n> +}\n\nIdem.\n\n> +# The test query-fsmonitor hook proc will output a marker file we can use to\n> +# ensure the hook was actually used to generate the correct results.\n> +\n> +test_expect_success 'setup' '\n> +       mkdir -p .git/hooks &&\n> +       write_script .git/hooks/query-fsmonitor<<-\\EOF &&\n> +       if [ $1 -ne 1 ]\n> +       then\n> +               echo -e \"Unsupported query-fsmonitor hook version.\\n\" >&2\n> +               exit 1;\n> +       fi\n> +       : >marker\n> +       printf \"untracked\\0\"\n> +       printf \"dir1/untracked\\0\"\n> +       printf \"dir2/untracked\\0\"\n> +       printf \"modified\\0\"\n> +       printf \"dir1/modified\\0\"\n> +       printf \"dir2/modified\\0\"\n> +       printf \"new\\0\"\"\n> +       printf \"dir1/new\\0\"\n> +       printf \"dir2/new\\0\"\n\nMaybe something like the following to save a few lines and remove some\nredundancies:\n\n       printf \"%s\\0\" untracked dir1/untracked dir2/untracked \\\n                             modified dir1/modified dir2/modified \\\n                             new dir1/new dir2/new\n\nor perhaps:\n\n       for f in untracked modified new\n       do\n              printf \"%s\\0\" \"$f\" \"dir1/$f\" \"dir2/$f\"\n       done\n\n> +       EOF\n> +       : >tracked &&\n> +       : >modified &&\n> +       mkdir dir1 &&\n> +       : >dir1/tracked &&\n> +       : >dir1/modified &&\n> +       mkdir dir2 &&\n> +       : >dir2/tracked &&\n> +       : >dir2/modified &&\n> +       git add . &&\n> +       test_tick &&\n> +       git commit -m initial &&\n> +       dirty_repo\n> +'\n> +\n> +cat >.gitignore <<\\EOF\n> +.gitignore\n> +expect*\n> +output*\n> +marker*\n> +EOF\n\nThis could be part of the previous setup test.\n\n> +# Status is well tested elsewhere so we'll just ensure that the results are\n> +# the same when using core.fsmonitor. First call after turning on the option\n> +# does a complete scan so need to do two calls to ensure we test the new\n> +# codepath.\n> +\n> +test_expect_success 'status with core.untrackedcache true' '\n\nIf this test is using untracked cache, it should perhaps first check\nthat untracked cache can be used on the current file system.\nt7063-status-untracked-cache.sh does that with the following:\n\ntest_lazy_prereq UNTRACKED_CACHE '\n    { git update-index --test-untracked-cache; ret=$?; } &&\n    test $ret -ne 1\n'\n\nif ! test_have_prereq UNTRACKED_CACHE; then\n    skip_all='This system does not support untracked cache'\n    test_done\nfi\n\n> +       git config core.fsmonitor true  &&\n> +       git config core.untrackedcache true &&\n> +       git -c core.fsmonitor=false -c core.untrackedcache=true status >expect &&\n\nI don't understand why there is \" -c core.untrackedcache=true\" in the\nabove command as you already set core.untrackedcache to true on the\nprevious line.\n\n> +       clean_repo &&\n> +       git status &&\n> +       test_path_is_missing marker &&\n> +       dirty_repo &&\n> +       git status >output &&\n> +       test_path_is_file marker &&\n> +       test_i18ncmp expect output\n> +'\n> +\n> +\n\nSpurious new line.\n\n> +test_expect_success 'status with core.untrackedcache false' '\n> +       git config core.fsmonitor true &&\n> +       git config core.untrackedcache false &&\n> +       git -c core.fsmonitor=false -c core.untrackedcache=false status >expect &&\n\nAgain core.untrackedcache is already set to false on the previous line.\n\n> +       clean_repo &&\n> +       git status &&\n> +       test_path_is_missing marker &&\n> +       dirty_repo &&\n> +       git status >output &&\n> +       test_path_is_file marker &&\n> +       test_i18ncmp expect output\n> +'\n> +\n> +# Ensure commands that call refresh_index() to move the index back in time\n> +# properly invalidate the fsmonitor cache\n> +\n> +test_expect_success 'refresh_index() invalidates fsmonitor cache' '\n> +       git config core.fsmonitor true &&\n> +       git config core.untrackedcache true &&\n> +       clean_repo &&\n> +       git status &&\n> +       test_path_is_missing marker &&\n> +       dirty_repo &&\n> +       write_script .git/hooks/query-fsmonitor<<-\\EOF &&\n> +       :>marker\n> +       EOF\n> +       git add . &&\n> +       git commit -m \"to reset\" &&\n> +       git status &&\n> +       test_path_is_file marker &&\n> +       git reset HEAD~1 &&\n> +       git status >output &&\n> +       test_path_is_file marker &&\n\nYou already checked that \"marker\" exists 3 lines above, and as far as\nI can see nothing could remove this file since the previous test, as\nthe hook can only create it.\nSo I wonder if something is missing or if this test is redundant.\n\n> +       git -c core.fsmonitor=false status >expect &&\n> +       test_i18ncmp expect output\n> +'\n> +\n> +# Now make sure it's actually skipping the check for modified and untracked\n> +# files unless it is told about them.  Note, after \"git reset --hard HEAD\" no\n> +# extensions exist other than 'TREE' so do a \"git status\" to get the extension\n> +# written before testing the results.\n> +\n> +test_expect_success 'status doesnt detect unreported modifications' '\n\nMaybe:\n\ntest_expect_success \"status doesn't detect unreported modifications\" '\n\n> +       git config core.fsmonitor true &&\n> +       git config core.untrackedcache true &&\n> +       write_script .git/hooks/query-fsmonitor<<-\\EOF &&\n> +       :>marker\n> +       EOF\n> +       clean_repo &&\n> +       git status &&\n> +       test_path_is_missing marker &&\n> +       : >untracked &&\n> +       echo 2 >dir1/modified &&\n> +       git status >output &&\n> +       test_path_is_file marker &&\n> +       test_i18ngrep ! \"Changes not staged for commit:\" output &&\n> +       test_i18ngrep ! \"Untracked files:\" output &&\n> +       write_script .git/hooks/query-fsmonitor<<-\\EOF &&\n> +       :>marker\n> +       printf \"untracked%s\\0\"\n\nWhy is there a %s in the above?\n\n> +       printf \"dir1/modified\\0\"\n> +       EOF\n> +       git status >output &&\n> +       test_path_is_file marker &&\n> +       test_i18ngrep \"Changes not staged for commit:\" output &&\n> +       test_i18ngrep \"Untracked files:\" output\n> +'\n> +\n> +test_done\n> --\n> 2.13.0.windows.1.9.gc201c67b71\n>\n"},{"id":"321082","messageId":"bfab707a-7f3a-ca7d-1f67-66dcb6ab2ea6@gmail.com","threadId":"46042","inReplyTo":"CAP8UFD0OEUogQYL5Eo1kBy_1B=dBcdEJyc4B_qAUOU=2XnrxxA@mail.gmail.com","subject":"Re: [PATCH v3 4/6] fsmonitor: add test cases for fsmonitor extension","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2017-05-30T21:21:51Z","receivedAt":"2017-05-30T21:22:05Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"\n\nOn 5/30/2017 9:18 AM, Christian Couder wrote:\n> On Thu, May 25, 2017 at 8:36 PM, Ben Peart <peartben@gmail.com> wrote:\n> \n> [...]\n> \n>> diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh\n>> new file mode 100755\n>> index 0000000000..395db46d55\n>> --- /dev/null\n>> +++ b/t/t7519-status-fsmonitor.sh\n>> @@ -0,0 +1,158 @@\n>> +#!/bin/sh\n>> +\n>> +test_description='git status with file system watcher'\n>> +\n>> +. ./test-lib.sh\n>> +\n>> +clean_repo () {\n>> +       git reset --hard HEAD\n>> +       git clean -fd\n>> +       rm -f marker\n>> +}\n> \n> Maybe link all the commands in the function with \"&&\".\n> \n>> +dirty_repo () {\n>> +       : >untracked\n>> +       : >dir1/untracked\n>> +       : >dir2/untracked\n>> +       echo 1 >modified\n>> +       echo 2 >dir1/modified\n>> +       echo 3 >dir2/modified\n>> +       echo 4 >new\n>> +       echo 5 >dir1/new\n>> +       echo 6 >dir2/new\n>> +       git add new\n>> +       git add dir1/new\n>> +       git add dir2/new\n>> +}\n> \n> Idem.\n> \n\nI did a quick search through the existing test scripts and the majority \ndo not link commands together with && when they are in a sub function \nlike this.  I find not having them linked together is easier to write, \nmaintain and is more readable.\n\n>> +# The test query-fsmonitor hook proc will output a marker file we can use to\n>> +# ensure the hook was actually used to generate the correct results.\n>> +\n>> +test_expect_success 'setup' '\n>> +       mkdir -p .git/hooks &&\n>> +       write_script .git/hooks/query-fsmonitor<<-\\EOF &&\n>> +       if [ $1 -ne 1 ]\n>> +       then\n>> +               echo -e \"Unsupported query-fsmonitor hook version.\\n\" >&2\n>> +               exit 1;\n>> +       fi\n>> +       : >marker\n>> +       printf \"untracked\\0\"\n>> +       printf \"dir1/untracked\\0\"\n>> +       printf \"dir2/untracked\\0\"\n>> +       printf \"modified\\0\"\n>> +       printf \"dir1/modified\\0\"\n>> +       printf \"dir2/modified\\0\"\n>> +       printf \"new\\0\"\"\n>> +       printf \"dir1/new\\0\"\n>> +       printf \"dir2/new\\0\"\n> \n> Maybe something like the following to save a few lines and remove some\n> redundancies:\n> \n>         printf \"%s\\0\" untracked dir1/untracked dir2/untracked \\\n>                               modified dir1/modified dir2/modified \\\n>                               new dir1/new dir2/new\n> \n> or perhaps:\n> \n>         for f in untracked modified new\n>         do\n>                printf \"%s\\0\" \"$f\" \"dir1/$f\" \"dir2/$f\"\n>         done\n> \n\nThat is a clever solution that certainly is fewer lines of code. \nHowever, I have to read the loop and think through the logic to figure \nout what it is doing vs the existing implementation where what it is \ndoing is apparent from just glancing at the code.  I was also trying to \nmaintain consistency with the other status test code in t7508-status.sh\n\n>> +       EOF\n>> +       : >tracked &&\n>> +       : >modified &&\n>> +       mkdir dir1 &&\n>> +       : >dir1/tracked &&\n>> +       : >dir1/modified &&\n>> +       mkdir dir2 &&\n>> +       : >dir2/tracked &&\n>> +       : >dir2/modified &&\n>> +       git add . &&\n>> +       test_tick &&\n>> +       git commit -m initial &&\n>> +       dirty_repo\n>> +'\n>> +\n>> +cat >.gitignore <<\\EOF\n>> +.gitignore\n>> +expect*\n>> +output*\n>> +marker*\n>> +EOF\n> \n> This could be part of the previous setup test.\n> \n\nI had followed the pattern in t7508-status.sh with this but I can move \nit in if that is the preferred model.\n\n>> +# Status is well tested elsewhere so we'll just ensure that the results are\n>> +# the same when using core.fsmonitor. First call after turning on the option\n>> +# does a complete scan so need to do two calls to ensure we test the new\n>> +# codepath.\n>> +\n>> +test_expect_success 'status with core.untrackedcache true' '\n> \n> If this test is using untracked cache, it should perhaps first check\n> that untracked cache can be used on the current file system.\n> t7063-status-untracked-cache.sh does that with the following:\n> \n> test_lazy_prereq UNTRACKED_CACHE '\n>      { git update-index --test-untracked-cache; ret=$?; } &&\n>      test $ret -ne 1\n> '\n> \n> if ! test_have_prereq UNTRACKED_CACHE; then\n>      skip_all='This system does not support untracked cache'\n>      test_done\n> fi\n> \n\nGood point. I'll change it so that untracked cache is only turned on if \nit is available and that the one test that requires it is skipped if it \nisn't available.\n\n>> +       git config core.fsmonitor true  &&\n>> +       git config core.untrackedcache true &&\n>> +       git -c core.fsmonitor=false -c core.untrackedcache=true status >expect &&\n> \n> I don't understand why there is \" -c core.untrackedcache=true\" in the\n> above command as you already set core.untrackedcache to true on the\n> previous line.\n> \n\nDefensive programming. :) The global setting was to ensure it was set \nwhen the test sub-functions clean and dirty were called and the command \nline settings were used to make it explicit what was being tested.  I \ncan remove them if it is causing confusion.\n\n>> +       clean_repo &&\n>> +       git status &&\n>> +       test_path_is_missing marker &&\n>> +       dirty_repo &&\n>> +       git status >output &&\n>> +       test_path_is_file marker &&\n>> +       test_i18ncmp expect output\n>> +'\n>> +\n>> +\n> \n> Spurious new line.\n> \n\nFixed\n\n>> +test_expect_success 'status with core.untrackedcache false' '\n>> +       git config core.fsmonitor true &&\n>> +       git config core.untrackedcache false &&\n>> +       git -c core.fsmonitor=false -c core.untrackedcache=false status >expect &&\n> \n> Again core.untrackedcache is already set to false on the previous line.\n> \n>> +       clean_repo &&\n>> +       git status &&\n>> +       test_path_is_missing marker &&\n>> +       dirty_repo &&\n>> +       git status >output &&\n>> +       test_path_is_file marker &&\n>> +       test_i18ncmp expect output\n>> +'\n>> +\n>> +# Ensure commands that call refresh_index() to move the index back in time\n>> +# properly invalidate the fsmonitor cache\n>> +\n>> +test_expect_success 'refresh_index() invalidates fsmonitor cache' '\n>> +       git config core.fsmonitor true &&\n>> +       git config core.untrackedcache true &&\n>> +       clean_repo &&\n>> +       git status &&\n>> +       test_path_is_missing marker &&\n>> +       dirty_repo &&\n>> +       write_script .git/hooks/query-fsmonitor<<-\\EOF &&\n>> +       :>marker\n>> +       EOF\n>> +       git add . &&\n>> +       git commit -m \"to reset\" &&\n>> +       git status &&\n>> +       test_path_is_file marker &&\n>> +       git reset HEAD~1 &&\n>> +       git status >output &&\n>> +       test_path_is_file marker &&\n> \n> You already checked that \"marker\" exists 3 lines above, and as far as\n> I can see nothing could remove this file since the previous test, as\n> the hook can only create it.\n> So I wonder if something is missing or if this test is redundant.\n> \n\nTesting it each time ensures it is being created when it is supposed to \nbe (ie when the test believes it is using the query-fsmonitor hook) and \nthat it isn't when it isn't supposed to be (ie when the hook should not \nbe called).\n\n>> +       git -c core.fsmonitor=false status >expect &&\n>> +       test_i18ncmp expect output\n>> +'\n>> +\n>> +# Now make sure it's actually skipping the check for modified and untracked\n>> +# files unless it is told about them.  Note, after \"git reset --hard HEAD\" no\n>> +# extensions exist other than 'TREE' so do a \"git status\" to get the extension\n>> +# written before testing the results.\n>> +\n>> +test_expect_success 'status doesnt detect unreported modifications' '\n> \n> Maybe:\n> \n> test_expect_success \"status doesn't detect unreported modifications\" '\n\nFixed\n\n> \n>> +       git config core.fsmonitor true &&\n>> +       git config core.untrackedcache true &&\n>> +       write_script .git/hooks/query-fsmonitor<<-\\EOF &&\n>> +       :>marker\n>> +       EOF\n>> +       clean_repo &&\n>> +       git status &&\n>> +       test_path_is_missing marker &&\n>> +       : >untracked &&\n>> +       echo 2 >dir1/modified &&\n>> +       git status >output &&\n>> +       test_path_is_file marker &&\n>> +       test_i18ngrep ! \"Changes not staged for commit:\" output &&\n>> +       test_i18ngrep ! \"Untracked files:\" output &&\n>> +       write_script .git/hooks/query-fsmonitor<<-\\EOF &&\n>> +       :>marker\n>> +       printf \"untracked%s\\0\"\n> \n> Why is there a %s in the above?\n\nFixed\n\n> \n>> +       printf \"dir1/modified\\0\"\n>> +       EOF\n>> +       git status >output &&\n>> +       test_path_is_file marker &&\n>> +       test_i18ngrep \"Changes not staged for commit:\" output &&\n>> +       test_i18ngrep \"Untracked files:\" output\n>> +'\n>> +\n>> +test_done\n>> --\n>> 2.13.0.windows.1.9.gc201c67b71\n>>\n"},{"id":"321086","messageId":"xmqqa85uas42.fsf@gitster.mtv.corp.google.com","threadId":"46042","inReplyTo":"bfab707a-7f3a-ca7d-1f67-66dcb6ab2ea6@gmail.com","subject":"Re: [PATCH v3 4/6] fsmonitor: add test cases for fsmonitor extension","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-30T22:37:33Z","receivedAt":"2017-05-30T22:37:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ben Peart <peartben@gmail.com> writes:\n\n> I did a quick search through the existing test scripts and the\n> majority do not link commands together with && when they are in a sub\n> function like this.  I find not having them linked together is easier\n> to write, maintain and is more readable.\n\nI had an impression that it is true only when the series of commands\nare not about Git.  When testing git commands, we should expect any\nof them to be broken (by somebody else ;-) and prepare to notice it.\n"},{"id":"321101","messageId":"35520cf4-6a73-29cb-e058-a0e7e29bfdce@gmail.com","threadId":"46042","inReplyTo":"xmqqa85uas42.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v3 4/6] fsmonitor: add test cases for fsmonitor extension","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2017-05-31T00:10:09Z","receivedAt":"2017-05-31T00:10:28Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"\n\nOn 5/30/2017 6:37 PM, Junio C Hamano wrote:\n> Ben Peart <peartben@gmail.com> writes:\n> \n>> I did a quick search through the existing test scripts and the\n>> majority do not link commands together with && when they are in a sub\n>> function like this.  I find not having them linked together is easier\n>> to write, maintain and is more readable.\n> \n> I had an impression that it is true only when the series of commands\n> are not about Git.  When testing git commands, we should expect any\n> of them to be broken (by somebody else ;-) and prepare to notice it.\n> \n\nFixed\n"},{"id":"321109","messageId":"CAP8UFD3jkAOOpFqJg2LurBfXRzeOLNO6+Wso3OPt_40TUVZy9g@mail.gmail.com","threadId":"46042","inReplyTo":"bfab707a-7f3a-ca7d-1f67-66dcb6ab2ea6@gmail.com","subject":"Re: [PATCH v3 4/6] fsmonitor: add test cases for fsmonitor extension","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2017-05-31T04:33:07Z","receivedAt":"2017-05-31T04:33:14Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Tue, May 30, 2017 at 11:21 PM, Ben Peart <peartben@gmail.com> wrote:\n>\n> On 5/30/2017 9:18 AM, Christian Couder wrote:\n>>\n>> On Thu, May 25, 2017 at 8:36 PM, Ben Peart <peartben@gmail.com> wrote:\n>>\n>>> +       printf \"untracked\\0\"\n>>> +       printf \"dir1/untracked\\0\"\n>>> +       printf \"dir2/untracked\\0\"\n>>> +       printf \"modified\\0\"\n>>> +       printf \"dir1/modified\\0\"\n>>> +       printf \"dir2/modified\\0\"\n>>> +       printf \"new\\0\"\"\n>>> +       printf \"dir1/new\\0\"\n>>> +       printf \"dir2/new\\0\"\n>>\n>> Maybe something like the following to save a few lines and remove some\n>> redundancies:\n>>\n>>         printf \"%s\\0\" untracked dir1/untracked dir2/untracked \\\n>>                               modified dir1/modified dir2/modified \\\n>>                               new dir1/new dir2/new\n>>\n>> or perhaps:\n>>\n>>         for f in untracked modified new\n>>         do\n>>                printf \"%s\\0\" \"$f\" \"dir1/$f\" \"dir2/$f\"\n>>         done\n>\n> That is a clever solution that certainly is fewer lines of code. However, I\n> have to read the loop and think through the logic to figure out what it is\n> doing vs the existing implementation where what it is doing is apparent from\n> just glancing at the code.  I was also trying to maintain consistency with\n> the other status test code in t7508-status.sh\n\nOk fair enough.\n\n>>> +       EOF\n>>> +       : >tracked &&\n>>> +       : >modified &&\n>>> +       mkdir dir1 &&\n>>> +       : >dir1/tracked &&\n>>> +       : >dir1/modified &&\n>>> +       mkdir dir2 &&\n>>> +       : >dir2/tracked &&\n>>> +       : >dir2/modified &&\n>>> +       git add . &&\n>>> +       test_tick &&\n>>> +       git commit -m initial &&\n>>> +       dirty_repo\n>>> +'\n>>> +\n>>> +cat >.gitignore <<\\EOF\n>>> +.gitignore\n>>> +expect*\n>>> +output*\n>>> +marker*\n>>> +EOF\n>>\n>> This could be part of the previous setup test.\n>\n> I had followed the pattern in t7508-status.sh with this but I can move it in\n> if that is the preferred model.\n\nYeah, I think it is preferred these days to have all the setup code\ninside tests.\n\n>>> +       git config core.fsmonitor true  &&\n>>> +       git config core.untrackedcache true &&\n>>> +       git -c core.fsmonitor=false -c core.untrackedcache=true status\n>>> >expect &&\n>>\n>> I don't understand why there is \" -c core.untrackedcache=true\" in the\n>> above command as you already set core.untrackedcache to true on the\n>> previous line.\n>\n> Defensive programming. :) The global setting was to ensure it was set when\n> the test sub-functions clean and dirty were called and the command line\n> settings were used to make it explicit what was being tested.  I can remove\n> them if it is causing confusion.\n\nI think it is a bit confusing indeed.\n\n>>> +test_expect_success 'refresh_index() invalidates fsmonitor cache' '\n>>> +       git config core.fsmonitor true &&\n>>> +       git config core.untrackedcache true &&\n>>> +       clean_repo &&\n>>> +       git status &&\n>>> +       test_path_is_missing marker &&\n>>> +       dirty_repo &&\n>>> +       write_script .git/hooks/query-fsmonitor<<-\\EOF &&\n>>> +       :>marker\n>>> +       EOF\n>>> +       git add . &&\n>>> +       git commit -m \"to reset\" &&\n>>> +       git status &&\n>>> +       test_path_is_file marker &&\n\nOk so \"marker\" is there now.\n\n>>> +       git reset HEAD~1 &&\n>>> +       git status >output &&\n>>> +       test_path_is_file marker &&\n>>\n>> You already checked that \"marker\" exists 3 lines above, and as far as\n>> I can see nothing could remove this file since the previous test, as\n>> the hook can only create it.\n>> So I wonder if something is missing or if this test is redundant.\n>\n> Testing it each time ensures it is being created when it is supposed to be\n> (ie when the test believes it is using the query-fsmonitor hook) and that it\n> isn't when it isn't supposed to be (ie when the hook should not be called).\n\nI would agree with that if the \"marker\" file was removed after the\nprevious \"test_path_is_file marker\", but I don't see any \"clean_repo\"\nor \"rm marker\" call that removes it.\n"},{"id":"321126","messageId":"CAP8UFD3H6PXm+0DLOa+25++xC018ntFMKd7XwqbYELzHwHWa1w@mail.gmail.com","threadId":"46042","inReplyTo":"20170525183612.3128-7-benpeart@microsoft.com","subject":"Re: [PATCH v3 6/6] fsmonitor: add a sample query-fsmonitor hook script for Watchman","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2017-05-31T13:21:43Z","receivedAt":"2017-05-31T13:21:57Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Thu, May 25, 2017 at 8:36 PM, Ben Peart <peartben@gmail.com> wrote:\n\n>  templates/hooks--query-fsmonitor.sample | 37 +++++++++++++++++++++++++++++++++\n>  1 file changed, 37 insertions(+)\n>  create mode 100644 templates/hooks--query-fsmonitor.sample\n\nPlease make this file executable like the other sample hook scripts.\n"},{"id":"321133","messageId":"47d76489-3e2f-ca25-8ff3-4350ca47aa76@gmail.com","threadId":"46042","inReplyTo":"CAP8UFD3jkAOOpFqJg2LurBfXRzeOLNO6+Wso3OPt_40TUVZy9g@mail.gmail.com","subject":"Re: [PATCH v3 4/6] fsmonitor: add test cases for fsmonitor extension","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2017-05-31T14:57:14Z","receivedAt":"2017-05-31T14:57:21Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"\n\nOn 5/31/2017 12:33 AM, Christian Couder wrote:\n>>>> +test_expect_success 'refresh_index() invalidates fsmonitor cache' '\n>>>> +       git config core.fsmonitor true &&\n>>>> +       git config core.untrackedcache true &&\n>>>> +       clean_repo &&\n>>>> +       git status &&\n>>>> +       test_path_is_missing marker &&\n>>>> +       dirty_repo &&\n>>>> +       write_script .git/hooks/query-fsmonitor<<-\\EOF &&\n>>>> +       :>marker\n>>>> +       EOF\n>>>> +       git add . &&\n>>>> +       git commit -m \"to reset\" &&\n>>>> +       git status &&\n>>>> +       test_path_is_file marker &&\n> \n> Ok so \"marker\" is there now.\n> \n>>>> +       git reset HEAD~1 &&\n>>>> +       git status >output &&\n>>>> +       test_path_is_file marker &&\n>>>\n>>> You already checked that \"marker\" exists 3 lines above, and as far as\n>>> I can see nothing could remove this file since the previous test, as\n>>> the hook can only create it.\n>>> So I wonder if something is missing or if this test is redundant.\n>>\n>> Testing it each time ensures it is being created when it is supposed to be\n>> (ie when the test believes it is using the query-fsmonitor hook) and that it\n>> isn't when it isn't supposed to be (ie when the hook should not be called).\n> \n> I would agree with that if the \"marker\" file was removed after the\n> previous \"test_path_is_file marker\", but I don't see any \"clean_repo\"\n> or \"rm marker\" call that removes it.\n> \n\nGot it.  I've added a call to \"rm -f marker\" to ensure the marker isn't \nleft over from the previous status command.  I found another instance of \nwhere this could happen in \"status doesn't detect unreported \nmodifications\" and fixed it as well.  Thanks!\n"}]}