On Sun, Jan 28, 2018 at 9:44 PM, Johannes Schindelin
<Johannes.Schindelin@xxxxxx> wrote:
> Hi,
>
> On Sat, 27 Jan 2018, Ævar Arnfjörð Bjarmason wrote:
>
>> I just got around to testing this since it landed, for context some
>> previous poking of mine in [1].
>>
>> Issues / stuff I've noticed:
>>
>> 1) We end up invalidating the untracked cache because stuff in .git/
>> changed. For example:
>>
>> 01:09:24.975524 fsmonitor.c:173 fsmonitor process '.git/hooks/fsmonitor-watchman' returned success
>> 01:09:24.975548 fsmonitor.c:138 fsmonitor_refresh_callback '.git'
>> 01:09:24.975556 fsmonitor.c:138 fsmonitor_refresh_callback '.git/config'
>> 01:09:24.975568 fsmonitor.c:138 fsmonitor_refresh_callback '.git/index'
>> 01:09:25.122726 fsmonitor.c:91 write fsmonitor extension successful
>>
>> Am I missing something or should we do something like:
>>
>> diff --git a/fsmonitor.c b/fsmonitor.c
>> index 0af7c4edba..5067b89bda 100644
>> --- a/fsmonitor.c
>> +++ b/fsmonitor.c
>> @@ -118,7 +118,12 @@ static int query_fsmonitor(int version, uint64_t last_update, struct strbuf *que
>>
>> static void fsmonitor_refresh_callback(struct index_state *istate, const char *name)
>> {
>> - int pos = index_name_pos(istate, name, strlen(name));
>> + int pos;
>> +
>> + if (!strcmp(name, ".git") || starts_with(name, ".git/"))
>> + return;
>> +
>> + pos = index_name_pos(istate, name, strlen(name));
>
> I would much rather have the fsmonitor hook already exclude those.
As documented the fsmonitor-watchman hook we ship doesn't work as
described in githooks(5), unless "files in the working directory" is
taken to include .git/, but I haven't seen that ever used.
On the other hand relying on arbitrary user-provided hooks to do the
right thing at the cost of silent performance degradation is bad. If
we're going to expect the hook to remove these we should probably
warn/die here if it does send us .git/* files.
> If you *must* add these comparisons, you have to use fspathcmp() and
> fspathncmp() instead (because case-insensitivity).
Thanks.
