rename: Break filepairs with different types.

When we consider if a path has been totally rewritten, we did not
touch changes from symlinks to files or vice versa.  But a change
that modifies even the type of a blob surely should count as a
complete rewrite.

While we are at it, modernise diffcore-break to be aware of gitlinks (we
do not want to touch them).

Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/cache.h b/cache.h
index aaa135bfde23cd2529e3707e4499be7f75917336..d0e7a71c6e61e56c841f9670266a2e5d61211158 100644 (file)
--- a/cache.h
+++ b/cache.h
@@ -192,6 +192,13 @@ enum object_type {
        OBJ_MAX,
 };
 
+static inline enum object_type object_type(unsigned int mode)
+{
+       return S_ISDIR(mode) ? OBJ_TREE :
+               S_ISGITLINK(mode) ? OBJ_COMMIT :
+               OBJ_BLOB;
+}
+
 #define GIT_DIR_ENVIRONMENT "GIT_DIR"
 #define GIT_WORK_TREE_ENVIRONMENT "GIT_WORK_TREE"
 #define DEFAULT_GIT_DIR_ENVIRONMENT ".git"

diff --git a/diffcore-break.c b/diffcore-break.c
index c71a22621a4f979f62cd21e9de83ab7129a129e9..31cdcfe8bcdae7df65b0387071846299a14bb7be 100644 (file)
--- a/diffcore-break.c
+++ b/diffcore-break.c
@@ -52,8 +52,10 @@ static int should_break(struct diff_filespec *src,
                             * is the default.
                             */
 
-       if (!S_ISREG(src->mode) || !S_ISREG(dst->mode))
-               return 0; /* leave symlink rename alone */
+       if (S_ISREG(src->mode) != S_ISREG(dst->mode)) {
+               *merge_score_p = (int)MAX_SCORE;
+               return 1; /* even their types are different */
+       }
 
        if (src->sha1_valid && dst->sha1_valid &&
            !hashcmp(src->sha1, dst->sha1))
@@ -168,11 +170,13 @@ void diffcore_break(int break_score)
                struct diff_filepair *p = q->queue[i];
                int score;
 
-               /* We deal only with in-place edit of non directory.
+               /*
+                * We deal only with in-place edit of blobs.
                 * We do not break anything else.
                 */
                if (DIFF_FILE_VALID(p->one) && DIFF_FILE_VALID(p->two) &&
-                   !S_ISDIR(p->one->mode) && !S_ISDIR(p->two->mode) &&
+                   object_type(p->one->mode) == OBJ_BLOB &&
+                   object_type(p->two->mode) == OBJ_BLOB &&
                    !strcmp(p->one->path, p->two->path)) {
                        if (should_break(p->one, p->two,
                                         break_score, &score)) {

diff --git a/t/t4008-diff-break-rewrite.sh b/t/t4008-diff-break-rewrite.sh
index 1287d2ad175fffe342386eaf1c04bc8d26fd72f4..26c2e4aa65c539c527ae8e2d71b5abb40c21fbbd 100755 (executable)
--- a/t/t4008-diff-break-rewrite.sh
+++ b/t/t4008-diff-break-rewrite.sh
@@ -122,11 +122,11 @@ test_expect_success \
     'run diff with -B -M' \
     'git diff-index -B -M "$tree" >current'
 
-# This should not mistake file0 as the copy source of new file1
-# due to type differences.
+# file0 changed from regular to symlink.  file1 is very close to the preimage of file0.
+# because we break file0, file1 can become a rename of it.
 cat >expected <<\EOF
 :100644 120000 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 67be421f88824578857624f7b3dc75e99a8a1481 T     file0
-:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 M100  file1
+:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 R     file0   file1
 EOF
 
 test_expect_success \

diff --git a/t/t4023-diff-rename-typechange.sh b/t/t4023-diff-rename-typechange.sh
new file mode 100755 (executable)
index 0000000..255604e
--- /dev/null
+++ b/t/t4023-diff-rename-typechange.sh
@@ -0,0 +1,86 @@
+#!/bin/sh
+
+test_description='typechange rename detection'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+
+       rm -f foo bar &&
+       cat ../../COPYING >foo &&
+       ln -s linklink bar &&
+       git add foo bar &&
+       git commit -a -m Initial &&
+       git tag one &&
+
+       rm -f foo bar &&
+       cat ../../COPYING >bar &&
+       ln -s linklink foo &&
+       git add foo bar &&
+       git commit -a -m Second &&
+       git tag two &&
+
+       rm -f foo bar &&
+       cat ../../COPYING >foo &&
+       git add foo &&
+       git commit -a -m Third &&
+       git tag three &&
+
+       mv foo bar &&
+       ln -s linklink foo &&
+       git add foo bar &&
+       git commit -a -m Fourth &&
+       git tag four &&
+
+       # This is purely for sanity check
+
+       rm -f foo bar &&
+       cat ../../COPYING >foo &&
+       cat ../../Makefile >bar &&
+       git add foo bar &&
+       git commit -a -m Fifth &&
+       git tag five &&
+
+       rm -f foo bar &&
+       cat ../../Makefile >foo &&
+       cat ../../COPYING >bar &&
+       git add foo bar &&
+       git commit -a -m Sixth &&
+       git tag six
+
+'
+
+test_expect_success 'cross renames to be detected for regular files' '
+
+       git diff-tree five six -r --name-status -B -M | sort >actual &&
+       {
+               echo "R100      foo     bar"
+               echo "R100      bar     foo"
+       } | sort >expect &&
+       diff -u expect actual
+
+'
+
+test_expect_success 'cross renames to be detected for typechange' '
+
+       git diff-tree one two -r --name-status -B -M | sort >actual &&
+       {
+               echo "R100      foo     bar"
+               echo "R100      bar     foo"
+       } | sort >expect &&
+       diff -u expect actual
+
+'
+
+test_expect_success 'moves and renames' '
+
+       git diff-tree three four -r --name-status -B -M | sort >actual &&
+       {
+               echo "R100      foo     bar"
+               echo "T100      foo"
+       } | sort >expect &&
+       diff -u expect actual
+
+'
+
+test_done

diff --git a/tree-walk.h b/tree-walk.h
index 903a7b0f483fec5cbb6c6b372ab49cc28b655e75..db0fbdc701f1ef63cdc1a8b7d5c5e72322f91426 100644 (file)
--- a/tree-walk.h
+++ b/tree-walk.h
@@ -7,13 +7,6 @@ struct name_entry {
        unsigned int mode;
 };
 
-static inline enum object_type object_type(unsigned int mode)
-{
-       return S_ISDIR(mode) ? OBJ_TREE :
-               S_ISGITLINK(mode) ? OBJ_COMMIT :
-               OBJ_BLOB;
-}
-
 struct tree_desc {
        const void *buffer;
        struct name_entry entry;