Re: [PATCH] t0021: make sure clean filter runs

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 21, 2019, 15:40 UTC
Message-ID: <xmqq36huttku.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20190821145215.GA2679@cat>

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

> It will also check the contents if the mtime is greater than the
> timestamp of the index, so the 'touch' here would also cover that.
>
> So the changes here do solve the race completely.

OK, the explanation makes sense.

Either test.r has been correctly checked out and has an older timestamp or a more recent timestamp. In the former case, the index knows that we did not touch it, so the next "checkout" knows it does not have to ask the clean filter to work on it. In the latter case, the index is unsure if we touched it (or, suspects that it has updated contents in it), so the clean filter needs to read from the working tree to see if we did change it (and we find it is not modified). The outcome at the higher level, the answer to the question "checkout" wanted to ask, is the same: test.r has no local modificaiton and we can switch branches safely.

And that is already validated by seeing what exit status "checkout" gives us, so it sort-of feels to be testing a bit too low level implementation detail to see on which paths the filters are or are not called, but that is not a problem with this fix. If we want to check at that level, we should do so correctly, and making sure that the test.r file has recent timestamp to convince "checkout" that it needs to verify contents is the right thing to do.

Thanks.