Re: [PATCH 19/19] hack: watchman/untracked cache mashup

On Tue, 2016-03-15 at 19:31 +0700, Duy Nguyen wrote:

On Thu, Mar 10, 2016 at 1:36 AM, David Turner <
dturner@twopensource.com> wrote:

quoted

 static struct watchman_query *make_query(const char *last_update)

@@ -60,8 +61,24 @@ static void update_index(struct index_state

*istate,
                        continue;

                pos = index_name_pos(istate, wm->name, strlen(wm
->name));
-               if (pos < 0)
+               if (pos < 0) {
+                       if (istate->untracked) {
+                               char *name = xstrdup(wm->name);
+                               char *dname = dirname(name);
+
+                               /*
+                                * dirname() returns '.' for the
root,
+                                * but we call it ''.
+                                */
+                               if (dname[0] == '.' && dname[1] ==
0)
+                                       string_list_append(&istate
->untracked->invalid_untracked, "");
+                               else
+                                       string_list_append(&istate
->untracked->invalid_untracked,
+                                                          dname);
+                               free(name);
+                       }
                        continue;
+               }

So if we detect an updated file that's not in the index, we are
prepared to invalidate that path, correct? We may invalidate more
than
necessary if that's true. Imagine a.o is already ignored. If it's
rebuilt, we should not need to update untracked cache.

Yes, that's true.  But it would be true with the mtime system too. This
is no worse, even if it's no better.  In-tree builds are a hard case to
support, and I'm totally OK with a system that encourages out-of-tree
builds.

We could check if it's ignored, but then if someone changes their
gitignore, we could be wrong.

Or we could suggest that people make their watchmanignore match their
gitignore.

What I had in mind (and argued with watchman devs a bit [1]) was
maintain the file list of each clock and compare the file list of
different clocks to figure out what files are added or deleted. Then
we can generate invalidation list more accurately. We need to keep at
least one file list corresponds to  the clock saved in the index.
When
we get the refresh request, we get a new file list (with new clock),
do a diff then save the invalidation list as an extension. Once we
notice that new clock is written back in index, we can discard older
file lists. In theory we should not need to keep too many file lists,
so even if one list is big, it should not be a big problem.

I have a note with me about race conditions with this approach, but I
haven't remembered exactly why or how yet.. My recent thoughts about
it though, are race conditions when you do "git status" is probably
tolerable. After all if you're doing some I/O when you do git-status,
you're guaranteed to miss some updates.

[1] https://github.com/facebook/watchman/issues/65

I think it would be possible to just check the UNTR extension and only
add a dir to it if that dir doesn't already contain (untracked) the
file that's being modified.  Or is that also racy?