Re: [PATCH] replace: fix replacing object with itself

[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 [off-list ref] wrote:

Manzur Mukhitdinov [off-list ref] writes:

quoted

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 <redacted>
---

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.

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.