From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: "Zbigniew Jędrzejewski-Szmek" <zbyszek@in.waw.pl>,
"Randal L. Schwartz" <merlyn@stonehenge.com>,
"Ralf Nyren" <ralf.nyren@ericsson.com>,
git@vger.kernel.org
Subject: Re: Strange effect merging empty file
Date: Thu, 22 Mar 2012 15:03:03 -0400 [thread overview]
Message-ID: <20120322190303.GA32756@sigill.intra.peff.net> (raw)
In-Reply-To: <7v62dwxybd.fsf@alter.siamese.dyndns.org>
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
next prev parent reply other threads:[~2012-03-22 19:03 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-03-21 10:28 Strange effect merging empty file Ralf Nyren
2012-03-21 10:54 ` Zbigniew Jędrzejewski-Szmek
2012-03-21 17:14 ` Junio C Hamano
2012-03-22 12:17 ` Randal L. Schwartz
2012-03-22 12:39 ` Ralf Nyren
2012-03-22 12:47 ` Zbigniew Jędrzejewski-Szmek
2012-03-22 14:01 ` Jeff King
2012-03-22 17:03 ` Junio C Hamano
2012-03-22 17:59 ` Jeff King
2012-03-22 18:25 ` Jeff King
2012-03-22 18:52 ` Jeff King
2012-03-22 18:53 ` [PATCH 1/3] drop casts from users EMPTY_TREE_SHA1_BIN Jeff King
2012-03-22 18:53 ` [PATCH 2/3] make is_empty_blob_sha1 available everywhere Jeff King
2012-03-22 18:53 ` [PATCH 3/3] merge-recursive: don't detect renames from empty files Jeff King
2012-03-22 19:18 ` Jonathan Nieder
2012-03-22 21:53 ` Jeff King
2012-03-22 18:52 ` Strange effect merging empty file Junio C Hamano
2012-03-22 19:03 ` Jeff King [this message]
2012-03-22 19:12 ` Junio C Hamano
2012-03-22 22:46 ` [PATCH 0/2] merging renames of empty files Jeff King
2012-03-22 22:52 ` [PATCH 1/2] teach diffcore-rename to optionally ignore empty content Jeff King
2012-03-22 22:52 ` [PATCH 2/2] merge-recursive: don't detect renames of empty files Jeff King
2012-03-22 23:37 ` [PATCH 0/2] merging " Junio C Hamano
2012-03-23 0:23 ` Jeff King
2012-03-23 4:56 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20120322190303.GA32756@sigill.intra.peff.net \
--to=peff@peff.net \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=merlyn@stonehenge.com \
--cc=ralf.nyren@ericsson.com \
--cc=zbyszek@in.waw.pl \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).