Am 3/24/2013 16:15, schrieb John Keeping:
> 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");
This is gross. Can't we do much better here? Difftool already keeps a
GIT_INDEX of the files in the temporary tree ($tmpdir/rindex). Running
git-diff-files should be sufficient to tell which ones where edited via
the users's diff-tool. Then you can restrict calling hash-object to only
those worktree files where an "edit collision" needs to be checked for.
You could also keep a parallel index that keeps the state of the same set
of files in the worktree. Then another git-diff-files call could replace
the other half of hash-object calls.
> + 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);
-- Hannes
--
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
