On Thu, Mar 22, 2012 at 11:52:54AM -0700, Junio C Hamano wrote:
> I still wonder why checking only the preimage side is sufficient, though.
> Shouldn't we check both sides?
> [...]
> > + if (pair->status != 'R' || is_empty_blob_sha1(pair->one->sha1)) {
If the source is an empty file, then the other side must be an empty
file, too, no? Otherwise it would not be a rename. It could not be
exact, for obvious reasons. And it could not be inexact, because there
is no content to analyze on the source side.
Ditto for the destination side. How could something be renamed to an
empty file, but not be empty on the source side?
That is a slight layering violation, in that we are making assumptions
about how the diffcore-rename subsystem works. In theory diffcore-rename
could learn to read rename annotations from the commit message or
something (bleh!). But then, we'd probably want to update
merge-recursive in that instance to accept those "yes, it's definitely a
rename" markers, even for an empty file.
I think it would be slightly cleaner to tell diffcore-rename "be
conservative, and don't use empty files as sources" via an option. It's
not as trivial as this one-liner, but it shouldn't be much more than the
small patch you posted in the earlier thread.
-Peff
--
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