difftool: don't overwrite modified files
After running the user's diff tool, git-difftool will copy any files that differ between the working tree and the temporary tree. This is useful when the user edits the file in their diff tool but is wrong if they edit the working tree file while examining the diff. Instead of copying unconditionally when the files differ, create and index from the working tree files and only copy the temporary file back if it was modified and the working tree file was not. If both files have been modified, print a warning and exit with an error. Note that we cannot use an existing index in git-difftool since those contain the modified files that need to be checked out but here we are looking at those files which are copied from the working tree and not checked out. These are precisely the files which are not in the existing indices. Signed-off-by: John Keeping <john@keeping.me.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
John Keeping committed
Mar 29, 2013 at 22:07 UTC
67aa147af7c699348ba8c3afe9f7ee88aa043cd0
2 files changed
+101
-14
git-difftool.perl
+71
-14
@@ -13,9 +13,9 @@
13
use 5.008;
14
use strict;
15
use warnings;
16
+use Error qw(:try);
17
use File::Basename qw(dirname);
18
use File::Copy;
18
-use File::Compare;
19
use File::Find;
20
use File::stat;
21
use File::Path qw(mkpath rmtree);
@@ -88,14 +88,45 @@ sub use_wt_file
88
my ($repo, $workdir, $file, $sha1, $symlinks) = @_;
89
my $null_sha1 = '0' x 40;
90
91
- if ($sha1 eq $null_sha1) {
92
- return 1;
93
- } elsif (not $symlinks) {
91
+ if ($sha1 ne $null_sha1 and not $symlinks) {
92
return 0;
93
}
94
95
my $wt_sha1 = $repo->command_oneline('hash-object', "$workdir/$file");
98
- return $sha1 eq $wt_sha1;
96
+ my $use = ($sha1 eq $null_sha1) || ($sha1 eq $wt_sha1);
97
+ return ($use, $wt_sha1);
98
+}
99
+
100
+sub changed_files
101
+{
102
+ my ($repo_path, $index, $worktree) = @_;
103
+ $ENV{GIT_INDEX_FILE} = $index;
104
+ $ENV{GIT_WORK_TREE} = $worktree;
105
+ my $must_unset_git_dir = 0;
106
+ if (not defined($ENV{GIT_DIR})) {
107
+ $must_unset_git_dir = 1;
108
+ $ENV{GIT_DIR} = $repo_path;
109
+ }
110
+
111
+ my @refreshargs = qw/update-index --really-refresh -q --unmerged/;
112
+ my @gitargs = qw/diff-files --name-only -z/;
113
+ try {
114
+ Git::command_oneline(@refreshargs);
115
+ } catch Git::Error::Command with {};
116
+
117
+ my $line = Git::command_oneline(@gitargs);
118
+ my @files;
119
+ if (defined $line) {
120
+ @files = split('\0', $line);
121
+ } else {
122
+ @files = ();
123
+ }
124
+
125
+ delete($ENV{GIT_INDEX_FILE});
126
+ delete($ENV{GIT_WORK_TREE});
127
+ delete($ENV{GIT_DIR}) if ($must_unset_git_dir);
128
+
129
+ return map { $_ => 1 } @files;
130
}
131
132
sub setup_dir_diff
@@ -121,6 +152,7 @@ sub setup_dir_diff
152
my $null_sha1 = '0' x 40;
153
my $lindex = '';
154
my $rindex = '';
155
+ my $wtindex = '';
156
my %submodule;
157
my %symlink;
158
my @working_tree = ();
@@ -174,8 +206,12 @@ EOF
206
}
207
208
if ($rmode ne $null_mode) {
177
- if (use_wt_file($repo, $workdir, $dst_path, $rsha1, $symlinks)) {
178
- push(@working_tree, $dst_path);
209
+ my ($use, $wt_sha1) = use_wt_file($repo, $workdir,
210
+ $dst_path, $rsha1,
211
+ $symlinks);
212
+ if ($use) {
213
+ push @working_tree, $dst_path;
214
+ $wtindex .= "$rmode $wt_sha1\t$dst_path\0";
215
} else {
216
$rindex .= "$rmode $rsha1\t$dst_path\0";
217
}
@@ -218,6 +254,12 @@ EOF
254
$rc = system('git', 'checkout-index', '--all', "--prefix=$rdir/");
255
exit_cleanup($tmpdir, $rc) if $rc != 0;
256
257
+ $ENV{GIT_INDEX_FILE} = "$tmpdir/wtindex";
258
+ ($inpipe, $ctx) =
259
+ $repo->command_input_pipe(qw(update-index --info-only -z --index-info));
260
+ print($inpipe $wtindex);
261
+ $repo->command_close_pipe($inpipe, $ctx);
262
+
263
# If $GIT_DIR was explicitly set just for the update/checkout
264
# commands, then it should be unset before continuing.
265
delete($ENV{GIT_DIR}) if ($must_unset_git_dir);
@@ -390,19 +432,34 @@ sub dir_diff
432
# should be copied back to the working tree.
433
# Do not copy back files when symlinks are used and the
434
# external tool did not replace the original link with a file.
435
+ #
436
+ # These hashes are loaded lazily since they aren't needed
437
+ # in the common case of --symlinks and the difftool updating
438
+ # files through the symlink.
439
+ my %wt_modified;
440
+ my %tmp_modified;
441
+ my $indices_loaded = 0;
442
+
443
for my $file (@worktree) {
444
next if $symlinks && -l "$b/$file";
445
next if ! -f "$b/$file";
446
397
- my $diff = compare("$b/$file", "$workdir/$file");
398
- if ($diff == 0) {
399
- next;
400
- } elsif ($diff == -1) {
401
- my $errmsg = "warning: Could not compare ";
402
- $errmsg += "'$b/$file' with '$workdir/$file'\n";
447
+ if (!$indices_loaded) {
448
+ %wt_modified = changed_files($repo->repo_path(),
449
+ "$tmpdir/wtindex", "$workdir");
450
+ %tmp_modified = changed_files($repo->repo_path(),
451
+ "$tmpdir/wtindex", "$b");
452
+ $indices_loaded = 1;
453
+ }
454
+
455
+ if (exists $wt_modified{$file} and exists $tmp_modified{$file}) {
456
+ my $errmsg = "warning: Both files modified: ";
457
+ $errmsg .= "'$workdir/$file' and '$b/$file'.\n";
458
+ $errmsg .= "warning: Working tree file has been left.\n";
459
+ $errmsg .= "warning:\n";
460
warn $errmsg;
461
$error = 1;
405
- } elsif ($diff == 1) {
462
+ } elsif (exists $tmp_modified{$file}) {
463
my $mode = stat("$b/$file")->mode;
464
copy("$b/$file", "$workdir/$file") or
465
exit_cleanup($tmpdir, 1);
t/t7800-difftool.sh
+30
@@ -377,4 +377,34 @@ test_expect_success PERL,SYMLINKS 'difftool --dir-diff --symlink without unstage
377
test_cmp actual expect
378
'
379
380
+write_script modify-file <<\EOF
381
+echo "new content" >file
382
+EOF
383
+
384
+test_expect_success PERL 'difftool --no-symlinks does not overwrite working tree file ' '
385
+ echo "orig content" >file &&
386
+ git difftool --dir-diff --no-symlinks --extcmd "$(pwd)/modify-file" branch &&
387
+ echo "new content" >expect &&
388
+ test_cmp expect file
389
+'
390
+
391
+write_script modify-both-files <<\EOF
392
+echo "wt content" >file &&
393
+echo "tmp content" >"$2/file" &&
394
+echo "$2" >tmpdir
395
+EOF
396
+
397
+test_expect_success PERL 'difftool --no-symlinks detects conflict ' '
398
+ (
399
+ TMPDIR=$TRASH_DIRECTORY &&
400
+ export TMPDIR &&
401
+ echo "orig content" >file &&
402
+ test_must_fail git difftool --dir-diff --no-symlinks --extcmd "$(pwd)/modify-both-files" branch &&
403
+ echo "wt content" >expect &&
404
+ test_cmp expect file &&
405
+ echo "tmp content" >expect &&
406
+ test_cmp expect "$(cat tmpdir)/file"
407
+ )
408
+'
409
+
410
test_done