Re: [PATCH/RFC v2 06/16] t3700: sleep for 1 second, to avoid interfering with the racy code

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 8, 2012, 20:16 UTC
Message-ID: <7vr4rhrvfm.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20120807165947.GD913@tgummerer>

Thomas Gummerer <t.gummerer@gmail.com> writes:

Show 11 quoted lines

> On 08/05, Junio C Hamano wrote:
>> Thomas Gummerer <t.gummerer@gmail.com> writes:
>> 
>> > The new git racy code uses the mtime of cache-entries to smudge
>> > a racy clean entry, and loads the work, of checking the file-system
>> 
>> -ECANTPARSE.
>
> The git racy code for index-v5 uses the mtime of the cache-entries as
> smudge markers. The work of checking the file-system is loaded of to
> the reader.

OK, now I can parse, perhaps with either s/is loaded of/&f/ or s/is loaded of/is offloaded/.

Thanks for clarifying the grammar.

But doesn't the current code make it the responsibilty of the reader to check the contents with ce_modified_check_fs() already? You may have switched st_size to st_mtime as the field to mark a racily clean entry, but it is unclear how that change affects anything.

Show 14 quoted lines

>> > if the entry has really changed, off to the reader. This interferes
>> > with this test, because the entry is racily smudged and thus has
>> > mtime 0. We wait 1 second to avoid smudging the entry and getting
>> > correct test results.
>> 
>> Mild NAK, especially it is totally unclear why you even need to muck
>> with racy-git check in the current format of the index in the first
>> place, and even if it were necessary, it is unclear why this cannot
>> be done with test-chmtime.
>
> The racy-git code needs to be changed, to avoid problems when implementing
> the partial writing for index-v5. Otherwise it could cause problems, when
> we have entries that should be smudged, but are not due to the different
> racy algorithms.

Hrmph. But if racy detection and checking is now a responsibility of the later reader, the overall end result should be the same, no? Perhaps the existing test was checking a wrong thing?

We should not care if the index still has a racily clean entries, or how that fact is marked in the index entry. The primary thing we care about is that we do not mistake an actual change as no change due to raciness.

So whether done with "sleep" or "test-chmtime", avoiding a racily clean situation sounds like sweeping a bug in the v5 code in racy situation under the rug to me (unless I am misunderstanding what you are doing with this change and in your explanation, or the test was checking a wrong thing, that is).

Even more confused....