Re: [PATCH 2/5] Add watchman support to reduce index refresh cost

From: David Turner <dturner@twopensource.com>
Date: Nov 2, 2015, 21:19 UTC
Message-ID: <1446499174.4131.20.camel@twopensource.com>
In-Reply-To: <1446386146-10438-3-git-send-email-pclouds@gmail.com>

On Sun, 2015-11-01 at 14:55 +0100, Nguyễn Thái Ngọc Duy wrote:

Show 8 quoted lines

> The previous patch has the logic to clear bits in 'WAMA' bitmap. This
> patch has logic to set bits as told by watchman. The missing bit,
> _using_ these bits, are not here yet.
> 
> A lot of this code is written by David Turner originally, mostly from
> [1]. I'm just copying and polishing it a bit.
>
> [1] http://article.gmane.org/gmane.comp.version-control.git/248006

Our code has evolved somewhat from there[1]. It looks like you've incorporated some of these updates. But I wanted to call out one thing in particular that it doesn't look like this patch handles: there's some real ugliness on at least OSX around case-changing renames.

You probably won't be able to use our code directly as-is, but we'll want to do something about this situation. This is thicket of TOCTOU issues. For instance, watchman learns that a file called FOO has changed, but by the time it goes to the filesystem to look up the canonical case, FOO has been renamed to foo already (or renamed and then deleted).

I won't say for sure that your code is insufficient as I haven't yet tried it out, but we should ensure this case is handled before this is merged.

[1] https://github.com/dturner-tw/git/tree/dturner/watchman

> +
> +		pos = index_name_pos(istate, wm->name, strlen(wm->name));

This is the bit where case matters.