On Sun, Mar 24, 2013 at 09:31:45AM -0400, Matt McClure wrote:
> On Sun, Mar 24, 2013 at 8:36 AM, John Keeping <j...@keeping.me.uk> wrote:
> > In the
> > non-symlink case I think a user might find it surprising if the
> > (unmodified) file used by their diff tool were suddenly copied over the
> > working tree wiping out the changes they have just made.
>
> That's exactly what I was describing here:
> http://thread.gmane.org/gmane.comp.version-control.git/217979/focus=218006
Ahh, I guess I didn't fully register the impact of that at the time and
had to rediscover the problem for myself ;-)
How about doing this (on top of jk/difftool-dir-diff-edit-fix)?
-- >8 --
Subject: [PATCH] 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, store the
initial hash of the working tree file 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.
Signed-off-by: John Keeping <j...@keeping.me.uk>
---
git-difftool.perl | 35 +++++++++++++++++++++--------------
t/t7800-difftool.sh | 26 ++++++++++++++++++++++++++
2 files changed, 47 insertions(+), 14 deletions(-)
diff --git a/git-difftool.perl b/git-difftool.perl
index c433e86..be82b5a 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -15,7 +15,6 @@ use strict;
use warnings;
use File::Basename qw(dirname);
use File::Copy;
-use File::Compare;
use File::Find;
use File::stat;
use File::Path qw(mkpath rmtree);
@@ -123,7 +122,7 @@ sub setup_dir_diff
my $rindex = '';
my %submodule;
my %symlink;
- my @working_tree = ();
+ my %working_tree;
my @rawdiff = split('\0', $diffrtn);
my $i = 0;
@@ -175,7 +174,9 @@ EOF
if ($rmode ne $null_mode) {
if (use_wt_file($repo, $workdir, $dst_path, $rsha1,
$symlinks)) {
- push(@working_tree, $dst_path);
+ $working_tree{$dst_path} =
+ $repo->command_oneline('hash-object',
+ "$workdir/$dst_path");
} else {
$rindex .= "$rmode $rsha1\t$dst_path\0";
}
@@ -227,7 +228,7 @@ EOF
# 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) {
+ for my $file (keys %working_tree) {
my $dir = dirname($file);
unless (-d "$rdir/$dir") {
mkpath("$rdir/$dir") or
@@ -278,7 +279,7 @@ EOF
exit_cleanup($tmpdir, 1) if not $ok;
}
- return ($ldir, $rdir, $tmpdir, @working_tree);
+ return ($ldir, $rdir, $tmpdir, %working_tree);
}
sub write_to_file
@@ -376,7 +377,7 @@ sub dir_diff
my $error = 0;
my $repo = Git->repository();
my $workdir = find_worktree($repo);
- my ($a, $b, $tmpdir, @worktree) =
+ my ($a, $b, $tmpdir, %worktree) =
setup_dir_diff($repo, $workdir, $symlinks);
if (defined($extcmd)) {
@@ -390,19 +391,25 @@ 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.
- for my $file (@worktree) {
+ for my $file (keys %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";
+ my $wt_hash = $repo->command_oneline('hash-object',
+ "$workdir/$file");
+ my $tmp_hash = $repo->command_oneline('hash-object',
+ "$b/$file");
+ my $wt_modified = $wt_hash ne $worktree{$file};
+ my $tmp_modified = $tmp_hash ne $worktree{$file};
+
+ if ($wt_modified and $tmp_modified) {
+ 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 ($tmp_modified) {
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 db3d3d6..be2042d 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -407,4 +407,30 @@ test_expect_success PERL 'difftool --dir-diff from
subdirectory' '
)
'
+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 ' '
+ 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
--
1.8.2.411.g65a544e
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
