Merge branch 'jk/diff-do-not-reuse-wtf-needs-cleaning'

gitster · gitster · commit 767da54bf8bd · 2016-08-03T15:10:29.000-07:00
There is an optimization used in "git diff $treeA $treeB" to borrow
an already checked-out copy in the working tree when it is known to
be the same as the blob being compared, expecting that open/mmap of
such a file is faster than reading it from the object store, which
involves inflating and applying delta.  This however kicked in even
when the checked-out copy needs to go through the convert-to-git
conversion (including the clean filter), which defeats the whole
point of the optimization.  The optimization has been disabled when
the conversion is necessary.

* jk/diff-do-not-reuse-wtf-needs-cleaning:
  diff: do not reuse worktree files that need "clean" conversion
diff --git a/diff.c b/diff.c
@@ -2683,6 +2683,13 @@ static int reuse_worktree_file(const char *name, const unsigned char *sha1, int
 	if (!FAST_WORKING_DIRECTORY && !want_file && has_sha1_pack(sha1))
 		return 0;
 
+	/*
+	 * Similarly, if we'd have to convert the file contents anyway, that
+	 * makes the optimization not worthwhile.
+	 */
+	if (!want_file && would_convert_to_git(name))
+		return 0;
+
 	len = strlen(name);
 	pos = cache_name_pos(name, len);
 	if (pos < 0)
diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
@@ -268,4 +268,15 @@ test_expect_success 'disable filter with empty override' '
 	test_must_be_empty err
 '
 
+test_expect_success 'diff does not reuse worktree files that need cleaning' '
+	test_config filter.counter.clean "echo . >>count; sed s/^/clean:/" &&
+	echo "file filter=counter" >.gitattributes &&
+	test_commit one file &&
+	test_commit two file &&
+
+	>count &&
+	git diff-tree -p HEAD &&
+	test_line_count = 0 count
+'
+
 test_done
