Merge branch 'jk/difftool-dir-diff-edit-fix'

"git difftool --dir-diff" made symlinks to working tree files when
preparing a temporary directory structure, so that accidental edits
of these files in the difftool are reflected back to the working
tree, but the logic to decide when to do so was not quite right.

* jk/difftool-dir-diff-edit-fix:
  difftool --dir-diff: symlink all files matching the working tree
  difftool: avoid double slashes in symlink targets
  git-difftool(1): fix formatting of --symlink description

3 files changed, 48 insertions, 7 deletions

diff --git a/Documentation/git-difftool.txt b/Documentation/git-difftool.txt
index e0e12e9470..8361e6e4e3 100644
--- a/Documentation/git-difftool.txt
+++ b/Documentation/git-difftool.txt
@@ -72,10 +72,12 @@ with custom merge tool commands and has the same value as `$MERGED`.
 --symlinks::
 --no-symlinks::
 	'git difftool''s default behavior is create symlinks to the
-	working tree when run in `--dir-diff` mode.
+	working tree when run in `--dir-diff` mode and the right-hand
+	side of the comparison yields the same content as the file in
+	the working tree.
 +
-	Specifying `--no-symlinks` instructs 'git difftool' to create
-	copies instead.  `--no-symlinks` is the default on Windows.
+Specifying `--no-symlinks` instructs 'git difftool' to create copies
+instead.  `--no-symlinks` is the default on Windows.
 
 -x <command>::
 --extcmd=<command>::
diff --git a/git-difftool.perl b/git-difftool.perl
index 12231fbc67..663640d33c 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -83,6 +83,21 @@ sub exit_cleanup
 	exit($status | ($status >> 8));
 }
 
+sub use_wt_file
+{
+	my ($repo, $workdir, $file, $sha1, $symlinks) = @_;
+	my $null_sha1 = '0' x 40;
+
+	if ($sha1 eq $null_sha1) {
+		return 1;
+	} elsif (not $symlinks) {
+		return 0;
+	}
+
+	my $wt_sha1 = $repo->command_oneline('hash-object', "$workdir/$file");
+	return $sha1 eq $wt_sha1;
+}
+
 sub setup_dir_diff
 {
 	my ($repo, $workdir, $symlinks) = @_;
@@ -159,10 +174,10 @@ EOF
 		}
 
 		if ($rmode ne $null_mode) {
-			if ($rsha1 ne $null_sha1) {
-				$rindex .= "$rmode $rsha1\t$dst_path\0";
-			} else {
+			if (use_wt_file($repo, $workdir, $dst_path, $rsha1, $symlinks)) {
 				push(@working_tree, $dst_path);
+			} else {
+				$rindex .= "$rmode $rsha1\t$dst_path\0";
 			}
 		}
 	}
@@ -209,7 +224,9 @@ EOF
 	delete($ENV{GIT_INDEX_FILE});
 
 	# Changes in the working tree need special treatment since they are
-	# not part of the index
+	# not part of the index. Remove any trailing slash from $workdir
+	# before starting to avoid double slashes in symlink targets.
+	$workdir =~ s|/$||;
 	for my $file (@working_tree) {
 		my $dir = dirname($file);
 		unless (-d "$rdir/$dir") {
diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index 3aab6e1500..c6d6b1c99f 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -340,6 +340,28 @@ test_expect_success PERL 'difftool --dir-diff' '
 	stdin_contains file <output
 '
 
+write_script .git/CHECK_SYMLINKS <<\EOF
+for f in file file2 sub/sub
+do
+	echo "$f"
+	readlink "$2/$f"
+done >actual
+EOF
+
+test_expect_success PERL,SYMLINKS 'difftool --dir-diff --symlink without unstaged changes' '
+	cat >expect <<-EOF &&
+	file
+	$(pwd)/file
+	file2
+	$(pwd)/file2
+	sub/sub
+	$(pwd)/sub/sub
+	EOF
+	git difftool --dir-diff --symlink \
+		--extcmd "./.git/CHECK_SYMLINKS" branch HEAD &&
+	test_cmp actual expect
+'
+
 test_expect_success PERL 'difftool --dir-diff ignores --prompt' '
 	git difftool --dir-diff --prompt --extcmd ls branch >output &&
 	stdin_contains sub <output &&