Re: [PATCH] replace: fix replacing object with itself
- From
Christian Couder <christian.couder@gmail.com>
- Date
- Nov 15, 2014, 11:55 UTC
- Message-ID
- <CAP8UFD3dZMmGJp1p=ttVUoUkiwh7oYL3gqgsGZ+D8Bd16FiKjQ@mail.gmail.com>
- In-Reply-To
- <xmqqppcp1jvg.fsf@gitster.dls.corp.google.com>
[Sorry to resend this. I am really bad at making gmail on my Android smartphone send plain text emails.]
On Fri, Nov 14, 2014 at 11:45 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 21 quoted lines
> Manzur Mukhitdinov <manzurmm@gmail.com> writes: > >> When object is replaced with itself git shows unhelpful messages like(git log): >> "fatal: replace depth too high for object <SHA1>" >> >> Prevents user from replacing object with itself(with test for checking >> this case). >> >> Signed-off-by: Manzur Mukhitdinov <manzurmm@gmail.com> >> --- > > The patch is not wrong per-se, but I wonder how useful this "do not > replace itself but all other forms of loops are not checked at all" > would be in practice. If your user did this: > > git replace A B ;# pretend as if what is in B is in A > git replace B C ;# pretend as if what is in C is in B > git replace C A ;# pretend as if we have loop > git log C > > she would not be helped with this patch at all, no?
Yeah, but such loops are much less likely mistakes and are more difficult to detect, so I think this patch is at least a good first step.
Show 5 quoted lines
> We have the "replace depth" thing, which is a poor-man's substitute > for loop detection, primarily because we do not want to incur high > cost of loop detection at runtime. Shouldn't we be doing at least > the same amount of loop-avoidance check, if we really want to avoid > triggering the "replace depth" check at runtime?
We could check at replace ref creation time, but what if the user already has a ref that replaces A using B, and then a fetch adds a ref that replaces B using A?
Maybe we should accept that we have to rely on the runtime replace depth anyway, and improve its output first.
Best, Christian.