Merge branch 'jk/difftool-no-overwrite-on-copyback'
authorJunio C Hamano <gitster@pobox.com>
Wed, 3 Apr 2013 16:34:09 +0000 (09:34 -0700)
committerJunio C Hamano <gitster@pobox.com>
Wed, 3 Apr 2013 16:34:09 +0000 (09:34 -0700)
Try to be careful when difftool backend allows the user to write
into the temporary files being shown *and* the user makes changes
to the working tree at the same time. One of the changes has to be
lost in such a case, but at least tell the user what he did.

* jk/difftool-no-overwrite-on-copyback:
t7800: run --dir-diff tests with and without symlinks
t7800: fix tests when difftool uses --no-symlinks
t7800: don't hide grep output
difftool: don't overwrite modified files
t7800: move '--symlinks' specific test to the end

git-difftool.perl
t/t7800-difftool.sh
index 663640d33cb99a98135b38b95947416b4a8d49b9..67802922ccc41fa2993c1bce4ea1a3d2899c7a40 100755 (executable)
@@ -13,9 +13,9 @@
 use 5.008;
 use strict;
 use warnings;
+use Error qw(:try);
 use File::Basename qw(dirname);
 use File::Copy;
-use File::Compare;
 use File::Find;
 use File::stat;
 use File::Path qw(mkpath rmtree);
@@ -88,14 +88,45 @@ 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) {
+       if ($sha1 ne $null_sha1 and not $symlinks) {
                return 0;
        }
 
        my $wt_sha1 = $repo->command_oneline('hash-object', "$workdir/$file");
-       return $sha1 eq $wt_sha1;
+       my $use = ($sha1 eq $null_sha1) || ($sha1 eq $wt_sha1);
+       return ($use, $wt_sha1);
+}
+
+sub changed_files
+{
+       my ($repo_path, $index, $worktree) = @_;
+       $ENV{GIT_INDEX_FILE} = $index;
+       $ENV{GIT_WORK_TREE} = $worktree;
+       my $must_unset_git_dir = 0;
+       if (not defined($ENV{GIT_DIR})) {
+               $must_unset_git_dir = 1;
+               $ENV{GIT_DIR} = $repo_path;
+       }
+
+       my @refreshargs = qw/update-index --really-refresh -q --unmerged/;
+       my @gitargs = qw/diff-files --name-only -z/;
+       try {
+               Git::command_oneline(@refreshargs);
+       } catch Git::Error::Command with {};
+
+       my $line = Git::command_oneline(@gitargs);
+       my @files;
+       if (defined $line) {
+               @files = split('\0', $line);
+       } else {
+               @files = ();
+       }
+
+       delete($ENV{GIT_INDEX_FILE});
+       delete($ENV{GIT_WORK_TREE});
+       delete($ENV{GIT_DIR}) if ($must_unset_git_dir);
+
+       return map { $_ => 1 } @files;
 }
 
 sub setup_dir_diff
@@ -121,6 +152,7 @@ sub setup_dir_diff
        my $null_sha1 = '0' x 40;
        my $lindex = '';
        my $rindex = '';
+       my $wtindex = '';
        my %submodule;
        my %symlink;
        my @working_tree = ();
@@ -174,8 +206,12 @@ sub setup_dir_diff
                }
 
                if ($rmode ne $null_mode) {
-                       if (use_wt_file($repo, $workdir, $dst_path, $rsha1, $symlinks)) {
-                               push(@working_tree, $dst_path);
+                       my ($use, $wt_sha1) = use_wt_file($repo, $workdir,
+                                                         $dst_path, $rsha1,
+                                                         $symlinks);
+                       if ($use) {
+                               push @working_tree, $dst_path;
+                               $wtindex .= "$rmode $wt_sha1\t$dst_path\0";
                        } else {
                                $rindex .= "$rmode $rsha1\t$dst_path\0";
                        }
@@ -218,6 +254,12 @@ sub setup_dir_diff
        $rc = system('git', 'checkout-index', '--all', "--prefix=$rdir/");
        exit_cleanup($tmpdir, $rc) if $rc != 0;
 
+       $ENV{GIT_INDEX_FILE} = "$tmpdir/wtindex";
+       ($inpipe, $ctx) =
+               $repo->command_input_pipe(qw(update-index --info-only -z --index-info));
+       print($inpipe $wtindex);
+       $repo->command_close_pipe($inpipe, $ctx);
+
        # If $GIT_DIR was explicitly set just for the update/checkout
        # commands, then it should be unset before continuing.
        delete($ENV{GIT_DIR}) if ($must_unset_git_dir);
@@ -390,19 +432,34 @@ sub dir_diff
        # should be copied back to the working tree.
        # Do not copy back files when symlinks are used and the
        # external tool did not replace the original link with a file.
+       #
+       # These hashes are loaded lazily since they aren't needed
+       # in the common case of --symlinks and the difftool updating
+       # files through the symlink.
+       my %wt_modified;
+       my %tmp_modified;
+       my $indices_loaded = 0;
+
        for my $file (@worktree) {
                next if $symlinks && -l "$b/$file";
                next if ! -f "$b/$file";
 
-               my $diff = compare("$b/$file", "$workdir/$file");
-               if ($diff == 0) {
-                       next;
-               } elsif ($diff == -1) {
-                       my $errmsg = "warning: Could not compare ";
-                       $errmsg += "'$b/$file' with '$workdir/$file'\n";
+               if (!$indices_loaded) {
+                       %wt_modified = changed_files($repo->repo_path(),
+                               "$tmpdir/wtindex", "$workdir");
+                       %tmp_modified = changed_files($repo->repo_path(),
+                               "$tmpdir/wtindex", "$b");
+                       $indices_loaded = 1;
+               }
+
+               if (exists $wt_modified{$file} and exists $tmp_modified{$file}) {
+                       my $errmsg = "warning: Both files modified: ";
+                       $errmsg .= "'$workdir/$file' and '$b/$file'.\n";
+                       $errmsg .= "warning: Working tree file has been left.\n";
+                       $errmsg .= "warning:\n";
                        warn $errmsg;
                        $error = 1;
-               } elsif ($diff == 1) {
+               } elsif (exists $tmp_modified{$file}) {
                        my $mode = stat("$b/$file")->mode;
                        copy("$b/$file", "$workdir/$file") or
                        exit_cleanup($tmpdir, 1);
index c6d6b1c99fe98b7dc692eaf3828a08696bfb5e10..a6bd99eaf51943a899aa5e5801479b0a84b6b692 100755 (executable)
@@ -23,16 +23,6 @@ prompt_given ()
        test "$prompt" = "Launch 'test-tool' [Y/n]: branch"
 }
 
-stdin_contains ()
-{
-       grep >/dev/null "$1"
-}
-
-stdin_doesnot_contain ()
-{
-       ! stdin_contains "$1"
-}
-
 # Create a file on master and change it on branch
 test_expect_success PERL 'setup' '
        echo master >file &&
@@ -296,24 +286,24 @@ test_expect_success PERL 'setup with 2 files different' '
 test_expect_success PERL 'say no to the first file' '
        (echo n && echo) >input &&
        git difftool -x cat branch <input >output &&
-       stdin_contains m2 <output &&
-       stdin_contains br2 <output &&
-       stdin_doesnot_contain master <output &&
-       stdin_doesnot_contain branch <output
+       grep m2 output &&
+       grep br2 output &&
+       ! grep master output &&
+       ! grep branch output
 '
 
 test_expect_success PERL 'say no to the second file' '
        (echo && echo n) >input &&
        git difftool -x cat branch <input >output &&
-       stdin_contains master <output &&
-       stdin_contains branch  <output &&
-       stdin_doesnot_contain m2 <output &&
-       stdin_doesnot_contain br2 <output
+       grep master output &&
+       grep branch output &&
+       ! grep m2 output &&
+       ! grep br2 output
 '
 
 test_expect_success PERL 'difftool --tool-help' '
        git difftool --tool-help >output &&
-       stdin_contains tool <output
+       grep tool output
 '
 
 test_expect_success PERL 'setup change in subdirectory' '
@@ -324,20 +314,46 @@ test_expect_success PERL 'setup change in subdirectory' '
        git commit -m "added sub/sub" &&
        echo test >>file &&
        echo test >>sub/sub &&
-       git add . &&
+       git add file sub/sub &&
        git commit -m "modified both"
 '
 
-test_expect_success PERL 'difftool -d' '
-       git difftool -d --extcmd ls branch >output &&
-       stdin_contains sub <output &&
-       stdin_contains file <output
+run_dir_diff_test () {
+       test_expect_success PERL "$1 --no-symlinks" "
+               symlinks=--no-symlinks &&
+               $2
+       "
+       test_expect_success PERL,SYMLINKS "$1 --symlinks" "
+               symlinks=--symlinks &&
+               $2
+       "
+}
+
+run_dir_diff_test 'difftool -d' '
+       git difftool -d $symlinks --extcmd ls branch >output &&
+       grep sub output &&
+       grep file output
+'
+
+run_dir_diff_test 'difftool --dir-diff' '
+       git difftool --dir-diff $symlinks --extcmd ls branch >output &&
+       grep sub output &&
+       grep file output
 '
 
-test_expect_success PERL 'difftool --dir-diff' '
-       git difftool --dir-diff --extcmd ls branch >output &&
-       stdin_contains sub <output &&
-       stdin_contains file <output
+run_dir_diff_test 'difftool --dir-diff ignores --prompt' '
+       git difftool --dir-diff $symlinks --prompt --extcmd ls branch >output &&
+       grep sub output &&
+       grep file output
+'
+
+run_dir_diff_test 'difftool --dir-diff from subdirectory' '
+       (
+               cd sub &&
+               git difftool --dir-diff $symlinks --extcmd ls branch >output &&
+               grep sub output &&
+               grep file output
+       )
 '
 
 write_script .git/CHECK_SYMLINKS <<\EOF
@@ -362,18 +378,33 @@ test_expect_success PERL,SYMLINKS 'difftool --dir-diff --symlink without unstage
        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 &&
-       stdin_contains file <output
+write_script modify-file <<\EOF
+echo "new content" >file
+EOF
+
+test_expect_success PERL 'difftool --no-symlinks does not overwrite working tree file ' '
+       echo "orig content" >file &&
+       git difftool --dir-diff --no-symlinks --extcmd "$(pwd)/modify-file" branch &&
+       echo "new content" >expect &&
+       test_cmp expect file
 '
 
-test_expect_success PERL 'difftool --dir-diff from subdirectory' '
+write_script modify-both-files <<\EOF
+echo "wt content" >file &&
+echo "tmp content" >"$2/file" &&
+echo "$2" >tmpdir
+EOF
+
+test_expect_success PERL 'difftool --no-symlinks detects conflict ' '
        (
-               cd sub &&
-               git difftool --dir-diff --extcmd ls branch >output &&
-               stdin_contains sub <output &&
-               stdin_contains file <output
+               TMPDIR=$TRASH_DIRECTORY &&
+               export TMPDIR &&
+               echo "orig content" >file &&
+               test_must_fail git difftool --dir-diff --no-symlinks --extcmd "$(pwd)/modify-both-files" branch &&
+               echo "wt content" >expect &&
+               test_cmp expect file &&
+               echo "tmp content" >expect &&
+               test_cmp expect "$(cat tmpdir)/file"
        )
 '