Re: correct git merge behavior or corner case?
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Apr 21, 2009, 03:09 UTC
- Message-ID
- <7vskk2bt3x.fsf@gitster.siamese.dyndns.org>
- In-Reply-To
- <20090421024433.GC14479@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 12 quoted lines
> So basically one branch removes a file and adds an identical file under > a different name, while the other branch modifies the original file. Git > detects it as a rename, and applies the change from the second branch to > the newly added file instead of generating a conflict. > > This is _exactly_ what git's rename detection is designed to do. Yes, it > seems horribly confusing in this toy example, but that is because it is > a toy example: both 'date' and 'LICENSE' are empty files. But with real > files, if a source file has actual content but is deleted, there is a > new filename with the identical or near-identical content, and the patch > applies to the new content without conflicts, then applying it there is > probably exactly what you want.
I had to briefly wonder what the fallout would be if we begin special- casing empty blobs excluded even from exact renames. We effectively do not consider fuzzy renames for blobs smaller than certain threshold, and sane projects would not have an empty file tracked anyway, so...
A much lessor impact change would be to keep the diffcore-rename as-is, so that it does detect exact renames between a pair of empty files, but special case it in merge-recursive. I think I like the latter approach better.
In any case, here is what the damage would look like...
diffcore-rename.c | 13 +++++++++---- 1 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/diffcore-rename.c b/diffcore-rename.c index 0b0d6b8..dc1f159 100644 --- a/diffcore-rename.c +++ b/diffcore-rename.c @@ -59,11 +59,14 @@ static struct diff_rename_src { } *rename_src; static int rename_src_nr, rename_src_alloc; -static struct diff_rename_src *register_rename_src(struct diff_filespec *one, - unsigned short score) +static void register_rename_src(struct diff_filespec *one, + unsigned short score) { int first, last; + if (is_empty_blob_sha1(one->sha1)) + return; + first = 0; last = rename_src_nr; while (last > first) { @@ -71,7 +74,7 @@ static struct diff_rename_src *register_rename_src(struct diff_filespec *one, struct diff_rename_src *src = &(rename_src[next]); int cmp = strcmp(one->path, src->one->path); if (!cmp) - return src; + return; if (cmp < 0) { last = next; continue; @@ -91,7 +94,7 @@ static struct diff_rename_src *register_rename_src(struct diff_filespec *one, (rename_src_nr - first - 1) * sizeof(*rename_src)); rename_src[first].one = one; rename_src[first].score = score; - return &(rename_src[first]); + return; } static int basename_same(struct diff_filespec *src, struct diff_filespec *dst) @@ -436,6 +439,8 @@ void diffcore_rename(struct diff_options *options) else if (options->single_follow && strcmp(options->single_follow, p->two->path)) continue; /* not interested */ + else if (is_empty_blob_sha1(p->two->sha1)) + continue; /* not interested */ else locate_rename_dst(p->two, 1); }