diff: do not reuse_worktree_file for submodules

The GIT_EXTERNAL_DIFF calling code attempts to reuse existing worktree
files for the worktree side of diffs, for performance reasons.
However, that code also tries to do the same with submodules.  This
results in calls to $GIT_EXTERNAL_DIFF where the old-file is a file of
the form "Submodule commit $sha1", but the new-file is a directory in
the worktree.

Fix it by never reusing a worktree "file" in the submodule case.

Reported-by: Grégory Pakosz <gregory.pakosz@gmail.com>
Signed-off-by: Thomas Rast <tr@thomasrast.ch>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/diff.c b/diff.c
index 266112ca6104450497fb9be504c2404cced595f0..a96992a1bdc2a722dd6b03e9a4525dc90eebf3f3 100644 (file)
--- a/diff.c
+++ b/diff.c
@@ -2842,8 +2842,9 @@ static struct diff_tempfile *prepare_temp_file(const char *name,
                remove_tempfile_installed = 1;
        }
 
                remove_tempfile_installed = 1;
        }
 

-       if (!one->sha1_valid ||
-           reuse_worktree_file(name, one->sha1, 1)) {
+       if (!S_ISGITLINK(one->mode) &&
+           (!one->sha1_valid ||
+            reuse_worktree_file(name, one->sha1, 1))) {

                struct stat st;
                if (lstat(name, &st) < 0) {
                        if (errno == ENOENT)
                struct stat st;
                if (lstat(name, &st) < 0) {
                        if (errno == ENOENT)

diff --git a/t/t4020-diff-external.sh b/t/t4020-diff-external.sh
index 2e7d73f0906e6a2808706236328470b2d855e9f9..295a563496dd4c49b8b7a850a7eb2b3d00c32f9b 100755 (executable)
--- a/t/t4020-diff-external.sh
+++ b/t/t4020-diff-external.sh
@@ -213,12 +213,13 @@ keep_only_cr () {
 }
 
 test_expect_success 'external diff with autocrlf = true' '
 }
 
 test_expect_success 'external diff with autocrlf = true' '

-       git config core.autocrlf true &&
+       test_config core.autocrlf true &&

        GIT_EXTERNAL_DIFF=./fake-diff.sh git diff &&
        test $(wc -l < crlfed.txt) = $(cat crlfed.txt | keep_only_cr | wc -c)
 '
 
 test_expect_success 'diff --cached' '
        GIT_EXTERNAL_DIFF=./fake-diff.sh git diff &&
        test $(wc -l < crlfed.txt) = $(cat crlfed.txt | keep_only_cr | wc -c)
 '
 
 test_expect_success 'diff --cached' '

+       test_config core.autocrlf true &&

        git add file &&
        git update-index --assume-unchanged file &&
        echo second >file &&
        git add file &&
        git update-index --assume-unchanged file &&
        echo second >file &&


@@ -226,4 +227,31 @@ test_expect_success 'diff --cached' '
        test_cmp "$TEST_DIRECTORY"/t4020/diff.NUL actual
 '
 
        test_cmp "$TEST_DIRECTORY"/t4020/diff.NUL actual
 '
 

+test_expect_success 'clean up crlf leftovers' '
+       git update-index --no-assume-unchanged file &&
+       rm -f file* &&
+       git reset --hard
+'
+
+test_expect_success 'submodule diff' '
+       git init sub &&
+       ( cd sub && test_commit sub1 ) &&
+       git add sub &&
+       test_tick &&
+       git commit -m "add submodule" &&
+       ( cd sub && test_commit sub2 ) &&
+       write_script gather_pre_post.sh <<-\EOF &&
+       echo "$1 $4" # path, mode
+       cat "$2" # old file
+       cat "$5" # new file
+       EOF
+       GIT_EXTERNAL_DIFF=./gather_pre_post.sh git diff >actual &&
+       cat >expected <<-EOF &&
+       sub 160000
+       Subproject commit $(git rev-parse HEAD:sub)
+       Subproject commit $(cd sub && git rev-parse HEAD)
+       EOF
+       test_cmp expected actual
+'
+

 test_done
 test_done