Re: [PATCH v7 1/2] rerere: wait for MERGE_RR.lock before giving up

From

Thomas Bachem <mail@thomasbachem.com>

Date

Oct 11, 2026, 14:18 UTC

Message-ID

<CAA0xjtpuQE1UiQEPjfmsp9BZPROucfXCOpok5ADjMRB-rTZRiw@mail.gmail.com>

In-Reply-To

<xmqqqzhxf4uo.fsf@gitster.g>

Hi Junio,

Thanks for the careful review.

On 11/10/2026 02:39, Junio C Hamano wrote:

>> +     When the time is up, the command fails as it does
>> +     for any other lock it cannot take.
>
> Is it necessary to say this?

No, I'll drop it.

> The backgrounded unlocker sleeps for a second.  Is the idea that,
> even in a heavily loaded CI environment, the backgrounded unlocker
> will have a sufficient chance to sleep for a second and unlock while
> the 5000-millisecond timeout waits for it?

> Is the idea behind this 'wait' that the 'rerere.locktimeout'
> implementation might break in the future and we could reach this
> point before the background unlocker has finished sleeping for a
> full second?  And we want to ensure it has exited before proceeding
> by waiting for it ourselves.  If that is the case, perhaps a comment
> is warranted after '&&', such as:
>
>         wait && # just in case the background unlocker is still active

> Or make the string a bit more specific, such as:
>
>         test_grep ! "Unable to create.*MERGE_RR\.lock" err &&

I'll use your pattern there and in the other tests.

> Should we ensure that there is no leftover pid file by removing it
> before creating .git/MERGE_RR.lock, by the way?

> I'll stop here, but you can grasp the principles used in reviewing
> this test and apply them to the remaining tests to ensure they are
> clearly written.

Will do.