Re: [RFC] use typechange as rename source

[Jeff King <peff@peff.net>]

On Fri, Nov 30, 2007 at 06:36:56PM -0800, Junio C Hamano wrote:

> > I have always been a bit confused about diffcore-break, so I am probably
> > misunderstanding what you mean. But are you saying that
> > diffcore-break.c:should_break should return 1 for typechanges?
> 
> What I had in mind was to do something like that in spirit, but instead
> break such a filepair inside diffcore-rename (iow, even when the user
> did not say -B) early on.

Ah, I see. BTW, I totally screwed up the tests I did earlier. Returning
1 from should_break _does_ produce the same results for my simple case
(copy + typechange).

> But after re-reading your patch and the surrounding code, that is
> more or less what you are doing (without actually recording the extra
> broken pair to be merged back later).

I don't think we need to, because they are never actually "broken"; we
simply consider the source a candidate for renaming, but keep the pair
together to note the typechange.

> which is essentially doing the same thing but only for the "remove the
> regular file" half.  One has to wonder how the lack of handling the
> other half affects the outcome and still produce a result more intuitive
> than the current code.

AIUI, because we never broke the pair in the first place, we don't need
to look for a source for that dest (the "add a new symlink" half). It's
already part of the same filepair.

Whether this is by design or simply a happy accident that we record both
renames and typechanges in diff_filepairs, I'm not sure. Or perhaps I'm
totally misunderstanding how the breaking works.

> In your test case, the "new" symlink won't have any similar symlink that
> is removed from the preimage, so registering it as a rename destination
> would not make a difference (it will say "no match found, so create this
> as usual"), but I am not convinced if that would work well in general.

I don't know that it makes a difference. We are impacting only a
'typechange', which implies that we have a filepair in which both p->one
and p->two are valid; thus, the current code doesn't use it as a rename
dst at all.

-Peff