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

[Junio C Hamano <gitster@pobox.com>]

Christian Couder <christian.couder@gmail.com> writes:

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

More difficult to notice by humans, hence deserves more help from
the tool.

When these two are both mistakes, which one do you think is easier
to notice (thusly unlikely to happen)?

	git replace A A
        git replace C A

Of course, the former is immediately obvious to the human who is
typing that it is a typo.

The latter would be harder to spot as a mistake as the invoker has
to know that there is an existing "pretend as if what is in A is in
C" aka "replace A C" done earlier in the repository.

And the cost of detecting such a possible "too deep replace chain"
(do not call that a loop---the runtime barfs if you have too deep a
replace chain without any loop) wouldn't be too high.  You only need
to look at existing refs/replace/* refs, their contents, and the two
object names that form the proposed new replacement <old,new>.

Even a kludge (read: I am not suggesting that you solve it this way)
like "first install the replacement as proposed, then enumerate all
the replacement refs and their values and try to see if the runtime
check would barf, and if it would, fail the operation and revert the
change" would catch a mistake to cause the repository in trouble.