Re: Bug in index-helper/watchman?
flat view
From: Duy Nguyen <hidden>
Date: 2016-07-16 06:23:30
Subsystem:
the rest · Maintainer:
Linus Torvalds
On Sat, Jul 16, 2016 at 08:10:15AM +0200, Duy Nguyen wrote:
But you did spot a problem. What if UC extension is loaded _after_ watchman one? Then index->untracked_cache would have nothing in there and invalidation is no-op when we do it (at watchmain ext loading time). We can't control extension loading order (technically we can, but other git implementations may do it differently). So, maybe we should do the invalidation after _all_ index extensions have been loaded. Maybe we can do it in validate_untracked_cache? We already store the path list in the_index.untracked->invalid_untracked. validate_untracked_cache has all info it needs.
The squash for "index-helper: use watchman to avoid refreshing index with lstat()" looks something like this. I did not test it at all though. -- 8< --
diff --git a/dir.c b/dir.c
index 024b13c..04bd87c 100644
--- a/dir.c
+++ b/dir.c@@ -1902,6 +1902,43 @@ void remove_untracked_cache(struct index_state *istate) } } +static struct untracked_cache_dir *find_untracked_cache_dir( + struct untracked_cache *uc, struct untracked_cache_dir *ucd, + const char *name) +{ + int component_len; + const char *end; + struct untracked_cache_dir *dir; + + if (!*name) + return ucd; + + end = strchr(name, '/'); + if (end) + component_len = end - name; + else + component_len = strlen(name); + + dir = lookup_untracked(uc, ucd, name, component_len); + if (dir) + return find_untracked_cache_dir(uc, dir, name + component_len + 1); + + return NULL; +} + +static int mark_untracked_invalid(struct string_list_item *item, void *uc) +{ + struct untracked_cache *untracked = uc; + struct untracked_cache_dir *dir; + + dir = find_untracked_cache_dir(untracked, untracked->root, + item->string); + if (dir) + dir->valid = 0; + + return 0; +} + static struct untracked_cache_dir *validate_untracked_cache(struct dir_struct *dir, int base_len, const struct pathspec *pathspec)
@@ -1984,6 +2021,11 @@ static struct untracked_cache_dir *validate_untracked_cache(struct dir_struct *d /* Make sure this directory is not dropped out at saving phase */ root->recurse = 1; + + if (dir->untracked->use_watchman) { + for_each_string_list(&dir->untracked->invalid_untracked, + mark_untracked_invalid, dir->untracked); + } return root; }
diff --git a/dir.h b/dir.h
index ed16746..896b64a 100644
--- a/dir.h
+++ b/dir.h@@ -312,7 +312,4 @@ struct untracked_cache *read_untracked_extension(const void *data, unsigned long void write_untracked_extension(struct strbuf *out, struct untracked_cache *untracked); void add_untracked_cache(struct index_state *istate); void remove_untracked_cache(struct index_state *istate); -struct untracked_cache_dir *lookup_untracked(struct untracked_cache *uc, - struct untracked_cache_dir *dir, - const char *name, int len); #endif
diff --git a/read-cache.c b/read-cache.c
index ee777c1..b90187c 100644
--- a/read-cache.c
+++ b/read-cache.c@@ -1388,37 +1388,12 @@ static int verify_hdr(const struct cache_header *hdr, unsigned long size) return 0; } -static struct untracked_cache_dir *find_untracked_cache_dir( - struct untracked_cache *uc, struct untracked_cache_dir *ucd, - const char *name) -{ - int component_len; - const char *end; - struct untracked_cache_dir *dir; - - if (!*name) - return ucd; - - end = strchr(name, '/'); - if (end) - component_len = end - name; - else - component_len = strlen(name); - - dir = lookup_untracked(uc, ucd, name, component_len); - if (dir) - return find_untracked_cache_dir(uc, dir, name + component_len + 1); - - return NULL; -} - static void mark_no_watchman(size_t pos, void *data) { struct index_state *istate = data; struct cache_entry *ce = istate->cache[pos]; struct strbuf sb = STRBUF_INIT; char *c; - struct untracked_cache_dir *dir; assert(pos < istate->cache_nr); ce->ce_flags |= CE_WATCHMAN_DIRTY;
@@ -1438,27 +1413,11 @@ static void mark_no_watchman(size_t pos, void *data) if (c == sb.buf) strbuf_setlen(&sb, 0); - dir = find_untracked_cache_dir(istate->untracked, - istate->untracked->root, sb.buf); - if (dir) - dir->valid = 0; + string_list_append(&istate->untracked->invalid_untracked, ce->name); strbuf_release(&sb); } -static int mark_untracked_invalid(struct string_list_item *item, void *uc) -{ - struct untracked_cache *untracked = uc; - struct untracked_cache_dir *dir; - - dir = find_untracked_cache_dir(untracked, untracked->root, - item->string); - if (dir) - dir->valid = 0; - - return 0; -} - static int read_watchman_ext(struct index_state *istate, const void *data, unsigned long sz) {
@@ -1498,9 +1457,6 @@ static int read_watchman_ext(struct index_state *istate, const void *data, untracked += len + 1; } - for_each_string_list(&istate->untracked->invalid_untracked, - mark_untracked_invalid, istate->untracked); - if (untracked_nr) istate->cache_changed |= WATCHMAN_CHANGED; } -- 8< -- --
Duy