untracked cache: fix entry invalidation

First, the current code in untracked_cache_invalidate_path() is wrong
because it can only handle paths "a" or "a/b", not "a/b/c" because
lookup_untracked() only looks for entries directly under the given
directory. In the last case, it will look for the entry "b/c" in
directory "a" instead. This means if you delete or add an entry in a
subdirectory, untracked cache may become out of date because it does not
invalidate properly. This is noticed by David Turner.

The second problem is about invalidation inside a fully untracked/excluded
directory. In this case we may have to invalidate back to root. See the
comment block for detail.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/dir.c b/dir.c
index cd4ac779a66ad2ff901cde926ae11c86b699993e..c1edabf34d5a1e3466a71bd679d3fd6cf50fc5ee 100644 (file)
--- a/dir.c
+++ b/dir.c
@@ -2616,23 +2616,67 @@ struct untracked_cache *read_untracked_extension(const void *data, unsigned long
        return uc;
 }
 
        return uc;
 }
 

+static void invalidate_one_directory(struct untracked_cache *uc,
+                                    struct untracked_cache_dir *ucd)
+{
+       uc->dir_invalidated++;
+       ucd->valid = 0;
+       ucd->untracked_nr = 0;
+}
+
+/*
+ * Normally when an entry is added or removed from a directory,
+ * invalidating that directory is enough. No need to touch its
+ * ancestors. When a directory is shown as "foo/bar/" in git-status
+ * however, deleting or adding an entry may have cascading effect.
+ *
+ * Say the "foo/bar/file" has become untracked, we need to tell the
+ * untracked_cache_dir of "foo" that "bar/" is not an untracked
+ * directory any more (because "bar" is managed by foo as an untracked
+ * "file").
+ *
+ * Similarly, if "foo/bar/file" moves from untracked to tracked and it
+ * was the last untracked entry in the entire "foo", we should show
+ * "foo/" instead. Which means we have to invalidate past "bar" up to
+ * "foo".
+ *
+ * This function traverses all directories from root to leaf. If there
+ * is a chance of one of the above cases happening, we invalidate back
+ * to root. Otherwise we just invalidate the leaf. There may be a more
+ * sophisticated way than checking for SHOW_OTHER_DIRECTORIES to
+ * detect these cases and avoid unnecessary invalidation, for example,
+ * checking for the untracked entry named "bar/" in "foo", but for now
+ * stick to something safe and simple.
+ */
+static int invalidate_one_component(struct untracked_cache *uc,
+                                   struct untracked_cache_dir *dir,
+                                   const char *path, int len)
+{
+       const char *rest = strchr(path, '/');
+
+       if (rest) {
+               int component_len = rest - path;
+               struct untracked_cache_dir *d =
+                       lookup_untracked(uc, dir, path, component_len);
+               int ret =
+                       invalidate_one_component(uc, d, rest + 1,
+                                                len - (component_len + 1));
+               if (ret)
+                       invalidate_one_directory(uc, dir);
+               return ret;
+       }
+
+       invalidate_one_directory(uc, dir);
+       return uc->dir_flags & DIR_SHOW_OTHER_DIRECTORIES;
+}
+

 void untracked_cache_invalidate_path(struct index_state *istate,
                                     const char *path)
 {
 void untracked_cache_invalidate_path(struct index_state *istate,
                                     const char *path)
 {

-       const char *sep;
-       struct untracked_cache_dir *d;

        if (!istate->untracked || !istate->untracked->root)
                return;
        if (!istate->untracked || !istate->untracked->root)
                return;

-       sep = strrchr(path, '/');
-       if (sep)
-               d = lookup_untracked(istate->untracked,
-                                    istate->untracked->root,
-                                    path, sep - path);
-       else
-               d = istate->untracked->root;
-       istate->untracked->dir_invalidated++;
-       d->valid = 0;
-       d->untracked_nr = 0;
+       invalidate_one_component(istate->untracked, istate->untracked->root,
+                                path, strlen(path));

 }
 
 void untracked_cache_remove_from_index(struct index_state *istate,
 }
 
 void untracked_cache_remove_from_index(struct index_state *istate,

diff --git a/t/t7063-status-untracked-cache.sh b/t/t7063-status-untracked-cache.sh
index 22393b9511c65feb06798f41bae832a124e0cd57..37a24c131240c46d0694e0a148ec7c15b89e17fe 100755 (executable)
--- a/t/t7063-status-untracked-cache.sh
+++ b/t/t7063-status-untracked-cache.sh
@@ -375,7 +375,7 @@ EOF
 node creation: 0
 gitignore invalidation: 0
 directory invalidation: 0
 node creation: 0
 gitignore invalidation: 0
 directory invalidation: 0

-opendir: 1
+opendir: 2

 EOF
        test_cmp ../trace.expect ../trace
 '
 EOF
        test_cmp ../trace.expect ../trace
 '


@@ -543,4 +543,30 @@ EOF
        test_cmp ../trace.expect ../trace
 '
 
        test_cmp ../trace.expect ../trace
 '
 

+test_expect_success 'move entry in subdir from untracked to cached' '
+       git add dtwo/two &&
+       git status --porcelain >../status.actual &&
+       cat >../status.expect <<EOF &&
+ M done/two
+A  dtwo/two
+?? .gitignore
+?? done/five
+?? done/sub/
+EOF
+       test_cmp ../status.expect ../status.actual
+'
+
+test_expect_success 'move entry in subdir from cached to untracked' '
+       git rm --cached dtwo/two &&
+       git status --porcelain >../status.actual &&
+       cat >../status.expect <<EOF &&
+ M done/two
+?? .gitignore
+?? done/five
+?? done/sub/
+?? dtwo/
+EOF
+       test_cmp ../status.expect ../status.actual
+'
+

 test_done
 test_done