Re: [PATCH 0/1] fsmonitor: fix watchman integration

[Derrick Stolee <stolee@gmail.com>]

On 11/6/2019 10:32 AM, Kevin Willford wrote:
>> From: Junio C Hamano <gitster@pobox.com>
>> Sent: Tuesday, November 5, 2019 8:30 PM
>>
>> "Kevin Willford via GitGitGadget" <gitgitgadget@gmail.com> writes:
>>
>>> When running Git commands quickly -- such as in a shell script or the
>>> test suite -- the Git commands frequently complete and start again
>>> during the same second. The example fsmonitor hooks to integrate with
>>> Watchman truncate the nanosecond times to seconds. In principle, this
>>> is fine, as Watchman claims to use inclusive comparisons [1]. The
>>> result should only be an over-representation of the changed paths since
>> the last Git command.
>>> ...
>>
>> So, it doesn't seem to use "inclusive" and we need a workaround?
> 
> That is what is seems like.  I would like to dig into the watchman code
> to understand what is really going on.  They also document that "Using
> a timestamp is prone to race conditions in understanding the complete
> state of the file tree." Which could be the cause since the tests are
> running things in quick succession, i.e. change a file, run a git command.
> 
> Long term we should switch to using watchman's clock id which the
> documentation says does not have the race conditions. But the clock
> id is a string and would take more invasive changes to integrate that
> into the index where we are now simply using a uint64_t.

I should mention that I'm working on a patch series that will allow us
to use GIT_TEST_FSMONITOR pointing at Watchman in our CI builds. There
are several places where our fsmonitor integration does not work well
(such as when we delete submodules), but most of the remaining fixes
are small.

This timing issue fixes MOST of the problems we see when running the
test suite.

Thanks,
-Stolee