Re: jc/rerere-multi

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 21, 2016, 22:07 UTC
Message-ID: <xmqq7fj2y4bg.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <56A0CA5D.8080407@kdbg.org>

Johannes Sixt <j6t@kdbg.org> writes:

Show 10 quoted lines

> I finally found some time to test and review this series. I have one
> case where there are many identical conflicts (up to 15!) that rerere
> was unable to resolve. But with this series applied, all of them are
> now resolved automatically and correctly. That's a nice achievement!
>
> Tested-by: Johannes Sixt <j6t@kdbg.org>
>
> I don't have the original submission anymore. So, I'm responding here.
>
> Generally, the patches make sense.

Thanks. As the tip commit says, this is still incomplete in that "record and replay" part should work reasonably well, but things like "forget" and "gc" are areas that needs further looking into.

Show 7 quoted lines

> Except for 510936082eb4 "handle leftover rr-cache/$ID directory and
> postimage files": After the subsequent e2a6344cca47 "delay the
> recording of preimage" is in place, nothing of what the former patch
> changed (except test cases) remains, and the problem that the former
> solved is still solved, and in addition the NEEDSWORK that the former
> introduced is resolved by the latter. I think the two should be
> squashed together.

Yeah, they were originally done as separate patches as I felt 'delay the recording' step was much riskier and I was shooting for a series whose earlier parts can be moved forward independently if necessary in order to keep the size of the really hard part that has to be in flight longer time manageably small.

But re-reading these two with fresh eyes, I think it is OK to squash them into one.

> e2a6344cca47 (rerere: delay the recording of preimage) needs this
> fixup, I think:

Thanks for catching this (and this needs to be carried forward to 7/7 as well, I think). This shows how little testing the new code has gone through--a test certainly would have prepared an entry in the rerere database with a stale postimage without the matching preimage and noticed that things going wrong.