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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



[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@xxxxxxxxx> wrote:
> Manzur Mukhitdinov <manzurmm@xxxxxxxxx> 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@xxxxxxxxx>
>> ---
>
> 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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]