Re: [PATCH 1/4] fsmonitor: Watch, and ask about, the top of the repo, not the CWD

[Alex Vandiver <alexmv@dropbox.com>]

On Fri, 20 Oct 2017, Johannes Schindelin wrote:
> This is super expensive, as it means a full-blown new process instead of
> just a simple environment variable expansion.
> 
> The idea behind using `PWD` instead was that Git will already have done
> all of the work of figuring out the top-level directory and switched to
> there before calling the fsmonitor hook.

I'm not seeing that PWD has been at all altered.  The following does
seem like a better solution:
------8<-----
diff --git a/fsmonitor.c b/fsmonitor.c
index 7c1540c05..4ea44dcc6 100644
--- a/fsmonitor.c
+++ b/fsmonitor.c
@@ -121,6 +121,7 @@ static int query_fsmonitor(int version, uint64_t last_update, struct strbuf *que
        argv[3] = NULL;
        cp.argv = argv;
        cp.use_shell = 1;
+       cp.dir = get_git_work_tree();

        return capture_command(&cp, query_result, 1024);
 }
------8<-----

I'll re-roll with that.

> Did you see any case where the script was *not* called from the top-level
> directory?

Merely calling `git status` inside a subdirectory is enough to for the
stock watchman config to report that it's in a "new" directory:

    $ watchman watch-list
    {
        "roots": [],
        "version": "4.7.0"
    }
    $ git status
    Adding '/Users/alexmv/src/git' to watchman's watch list.
    On branch test
    nothing to commit, working tree clean
    $ cd builtin/
    $ git status
    Adding '/Users/alexmv/src/git/builtin' to watchman's watch list.
    On branch test
    nothing to commit, working tree clean
    $ watchman watch-list
    {
        "roots": [
            "/Users/alexmv/src/git/builtin",
            "/Users/alexmv/src/git"
        ],
        "version": "4.7.0"
    }

As I understand it, that means that it then loses all performance
gains in the new directory, as it spits out "all dirty."

 - Alex