From bf341b902ea1346373886f4eb4352a06b7b4cdfd Mon Sep 17 00:00:00 2001 From: John Keeping Date: Fri, 29 Mar 2013 11:28:32 +0000 Subject: [PATCH 1/5] t7800: move '--symlinks' specific test to the end This will group the tests more logically when we introduce a helper to run most --dir-diff tests with both --symlinks and --no-symlinks. Signed-off-by: John Keeping Signed-off-by: Junio C Hamano --- t/t7800-difftool.sh | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh index c6d6b1c99f..e6a16cda79 100755 --- a/t/t7800-difftool.sh +++ b/t/t7800-difftool.sh @@ -340,6 +340,21 @@ test_expect_success PERL 'difftool --dir-diff' ' stdin_contains file output && + stdin_contains sub output && + stdin_contains sub output && - stdin_contains sub output && - stdin_contains sub Date: Fri, 29 Mar 2013 22:07:39 +0000 Subject: [PATCH 2/5] 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 Signed-off-by: Junio C Hamano --- git-difftool.perl | 85 +++++++++++++++++++++++++++++++++++++-------- t/t7800-difftool.sh | 30 ++++++++++++++++ 2 files changed, 101 insertions(+), 14 deletions(-) diff --git a/git-difftool.perl b/git-difftool.perl index 663640d33c..67802922cc 100755 --- a/git-difftool.perl +++ b/git-difftool.perl @@ -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 @@ EOF } 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 @@ EOF $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); diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh index e6a16cda79..017f55adf4 100755 --- a/t/t7800-difftool.sh +++ b/t/t7800-difftool.sh @@ -377,4 +377,34 @@ test_expect_success PERL,SYMLINKS 'difftool --dir-diff --symlink without unstage test_cmp actual expect ' +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 +' + +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 ' ' + ( + 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" + ) +' + test_done From 472353a579b3c3fd645ea4f0aac582317e488775 Mon Sep 17 00:00:00 2001 From: John Keeping Date: Fri, 29 Mar 2013 11:28:34 +0000 Subject: [PATCH 3/5] t7800: don't hide grep output Remove the stdin_contains and stdin_doesnt_contain helper functions which add nothing but hide the output of grep, hurting debugging. Suggested-by: Johannes Sixt Signed-off-by: John Keeping Signed-off-by: Junio C Hamano --- t/t7800-difftool.sh | 44 +++++++++++++++++--------------------------- 1 file changed, 17 insertions(+), 27 deletions(-) diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh index 017f55adf4..9fd09dbe93 100755 --- a/t/t7800-difftool.sh +++ b/t/t7800-difftool.sh @@ -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 output && - stdin_contains m2 input && git difftool -x cat branch output && - stdin_contains master output && - stdin_contains tool output && - stdin_contains sub output && - stdin_contains sub output && - stdin_contains sub output && - stdin_contains sub Date: Fri, 29 Mar 2013 11:28:35 +0000 Subject: [PATCH 4/5] t7800: fix tests when difftool uses --no-symlinks When 'git difftool --dir-diff' is using --no-symlinks (either explicitly or implicitly because it's running on Windows), any working tree files that have been copied to the temporary directory are copied back after the difftool completes. Because an earlier test uses "git add .", the "output" file used by tests is tracked by Git and the following sequence occurs during some tests: 1) the shell opens "output" to redirect the difftool output 2) difftool copies the empty "output" to the temporary directory 3) difftool runs "ls" which writes to "output" 4) difftool copies the empty "output" file back over the output of the command 5) the output file doesn't contain the expected output, causing the test to fail Instead of adding all changes, explicitly add only the files that the test is using, allowing later tests to write their result files into the working tree. Signed-off-by: John Keeping Signed-off-by: Junio C Hamano --- t/t7800-difftool.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh index 9fd09dbe93..df443a981d 100755 --- a/t/t7800-difftool.sh +++ b/t/t7800-difftool.sh @@ -314,7 +314,7 @@ 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" ' From e01afdb74b395e63a6ee8feb8cb3a6ee470e2085 Mon Sep 17 00:00:00 2001 From: John Keeping Date: Fri, 29 Mar 2013 11:28:36 +0000 Subject: [PATCH 5/5] t7800: run --dir-diff tests with and without symlinks Currently the difftool --dir-diff tests may or may not use symlinks depending on the operating system on which they are run. In one case this has caused a test failure to be noticed only on Windows when the test also fails on Linux when difftool is invoked with --no-symlinks. Rewrite these tests so that they do not depend on the environment but run explicitly with both --symlinks and --no-symlinks, protecting the --symlinks version with a SYMLINKS prerequisite. Signed-off-by: John Keeping Signed-off-by: Junio C Hamano --- t/t7800-difftool.sh | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh index df443a981d..a6bd99eaf5 100755 --- a/t/t7800-difftool.sh +++ b/t/t7800-difftool.sh @@ -318,28 +318,39 @@ test_expect_success PERL 'setup change in subdirectory' ' git commit -m "modified both" ' -test_expect_success PERL 'difftool -d' ' - git difftool -d --extcmd ls branch >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 ' -test_expect_success PERL 'difftool --dir-diff' ' - git difftool --dir-diff --extcmd ls branch >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 ignores --prompt' ' - git difftool --dir-diff --prompt --extcmd ls branch >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 ' -test_expect_success PERL 'difftool --dir-diff from subdirectory' ' +run_dir_diff_test 'difftool --dir-diff from subdirectory' ' ( cd sub && - git difftool --dir-diff --extcmd ls branch >output && + git difftool --dir-diff $symlinks --extcmd ls branch >output && grep sub output && grep file output )