Re: [PATCH] t3429: try to protect against a potential racy todo file problem

[Junio C Hamano <gitster@pobox.com>]

SZEDER Gábor <szeder.dev@gmail.com> writes:

> On Sun, Nov 24, 2019 at 10:44:10AM +0000, Phillip Wood wrote:
>> On 24/11/2019 04:49, Junio C Hamano wrote:
>>  ...
>> >Hmph, that makes it sound as if the right fix is to re-read after
>> >writing the first version of the todo file out, so that the stat
>> >data matches reality and tells us that it has never been modified?
>> 
>> I think we should update the stat data after we write the todo list.
>
> Well, yes and no.
>
> No, because we are dealing with regression in v2.24.0 here, so the
> simpler the fix the better it is for maint.  I don't think a fix can
> get any simpler than my patch, with or without the suggestions from
> Phillip.

Of course, the simplest "fix" for regression is to revert the
offending one, and anything else is a band-aid ;-).  The question is
which band-aid is the least risky and which one takes us the closest
to the real solution.  I tend to agree that forcing to skip checking
no matter what the variable "check_todo" says unless is_rebase_i()
qualifies as the band-aid that is the least risky.

> Yes, we should definitely consider updating the stat data after the
> sequencer writes the todo list, or any other options with which the
> sequencer could notice a modified todo list file with less subtlety.
> Alas, there is a big can of worms in that direction, see the patch
> below, and we have to be very careful going that way, so I think it's
> only viable in the long term, but less suitable as a regression fix
> for maint.

Yes, I agree that it is much less suitable than even reverting the
offending one outright.

> (Hrm, perhaps I spent too many words on the all zeroed out stat data,
> and managed to sidetrack you a bit...)

No, I do not think so.  Thinking about what we need to do in the
longer term, while coming up with a shorter term fix, is a necessary
step of gaining confidence in the latter.

Again, thanks both for thinking about this issue.