{"thread":{"id":"30137","subject":"[PATCH 8/9 v10] difftool: teach difftool to handle directory diffs","startedAt":"2012-04-03T13:04:24Z","lastAt":"2012-04-03T13:04:24Z","messageCount":1,"participants":["Tim Henigan"],"isPatch":true,"patchVersion":10,"patchTotal":9},"messages":[{"id":"188409","messageId":"1333458265-29360-1-git-send-email-tim.henigan@gmail.com","threadId":"30137","inReplyTo":null,"subject":"[PATCH 8/9 v10] difftool: teach difftool to handle directory diffs","fromName":"Tim Henigan","fromEmail":"tim.henigan@gmail.com","sentAt":"2012-04-03T13:04:24Z","receivedAt":"2012-04-03T13:04:24Z","isPatch":true,"sender":{"key":"tim.henigan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42022?v=4"},"body":"When 'difftool' is called to compare a range of commits that modify\nmore than one file, it opens a separate instance of the diff tool for\neach file that changed.\n\nThe new '--dir-diff' option copies all the modified files to a temporary\nlocation and runs a directory diff on them in a single instance of the\ndiff tool.\n\nSigned-off-by: Tim Henigan <tim.henigan@gmail.com>\n---\n\nThis update is intended to be applied to the branch currently found at\n'th/difftool-diffall' (based on pu).\n\nChanges in v10:\n\nAs Ramsay Jones pointed out on the Git list [1], t7800.3 is failing on\nsome systems because the exit code from difftool is incorrect.\n\ndifftool wraps some commands in 'git_cmd_try' blocks. If an exception is\nraised, these blocks call Carp::croak to exit. However, there appears to\nbe a bug in Carp::croak that causes it to always exit with errno 9 \"Bad\nfile descriptor\" on some systems.\n\nThis happens even with a simple one-liner like:\n  perl -e 'use 5.008; use Carp qw(croak); print $!+0, \" $!\\n\";'\n\nThe above is expected to print \"0\" and exit without error. However, on\nthe problem systems it prints \"9 Bad file descriptor\".\n\nTo fix the problem, we need to:\n  - Drop commit 0440ed72 (patch 7/9 \"difftool: replace system call with\n    Git::command_noisy\") from the series.\n  - Replace commit 6fbdb7b (patch 8/9 \"difftool: teach difftool to handle\n    directory diffs\") with this patch. This version changes all instances\n    of 'git_cmd_try' to simple 'system' calls.\n\n[1]: http://thread.gmane.org/gmane.comp.version-control.git/194284/focus=194540\n\n Documentation/git-difftool.txt |    6 ++\n git-difftool--helper.sh        |   19 +++--\n git-difftool.perl              |  158 +++++++++++++++++++++++++++++++++++-----\n t/t7800-difftool.sh            |   39 ++++++++++\n 4 files changed, 197 insertions(+), 25 deletions(-)\n\ndiff --git a/Documentation/git-difftool.txt b/Documentation/git-difftool.txt\nindex fe38f66..aba5e76 100644\n--- a/Documentation/git-difftool.txt\n+++ b/Documentation/git-difftool.txt\n@@ -19,6 +19,12 @@ linkgit:git-diff[1].\n \n OPTIONS\n -------\n+-d::\n+--dir-diff::\n+\tCopy the modified files to a temporary location and perform\n+\ta directory diff on them. This mode never prompts before\n+\tlaunching the diff tool.\n+\n -y::\n --no-prompt::\n \tDo not prompt before launching a diff tool.\ndiff --git a/git-difftool--helper.sh b/git-difftool--helper.sh\nindex e6558d1..3d0fe0c 100755\n--- a/git-difftool--helper.sh\n+++ b/git-difftool--helper.sh\n@@ -73,9 +73,16 @@ then\n \tfi\n fi\n \n-# Launch the merge tool on each path provided by 'git diff'\n-while test $# -gt 6\n-do\n-\tlaunch_merge_tool \"$1\" \"$2\" \"$5\"\n-\tshift 7\n-done\n+if test -n \"$GIT_DIFFTOOL_DIRDIFF\"\n+then\n+\tLOCAL=\"$1\"\n+\tREMOTE=\"$2\"\n+\trun_merge_tool \"$merge_tool\" false\n+else\n+\t# Launch the merge tool on each path provided by 'git diff'\n+\twhile test $# -gt 6\n+\tdo\n+\t\tlaunch_merge_tool \"$1\" \"$2\" \"$5\"\n+\t\tshift 7\n+\tdone\n+fi\ndiff --git a/git-difftool.perl b/git-difftool.perl\nindex d4fe998..828c11e 100755\n--- a/git-difftool.perl\n+++ b/git-difftool.perl\n@@ -1,21 +1,29 @@\n #!/usr/bin/env perl\n # Copyright (c) 2009, 2010 David Aguilar\n+# Copyright (c) 2012 Tim Henigan\n #\n # This is a wrapper around the GIT_EXTERNAL_DIFF-compatible\n # git-difftool--helper script.\n #\n # This script exports GIT_EXTERNAL_DIFF and GIT_PAGER for use by git.\n-# GIT_DIFFTOOL_NO_PROMPT, GIT_DIFFTOOL_PROMPT, and GIT_DIFF_TOOL\n-# are exported for use by git-difftool--helper.\n+# GIT_DIFFTOOL_NO_PROMPT, GIT_DIFFTOOL_PROMPT, GIT_DIFFTOOL_DIRDIFF,\n+# and GIT_DIFF_TOOL are exported for use by git-difftool--helper.\n #\n # Any arguments that are unknown to this script are forwarded to 'git diff'.\n \n use 5.008;\n use strict;\n use warnings;\n+use File::Basename qw(dirname);\n+use File::Copy;\n+use File::stat;\n+use File::Path qw(mkpath);\n+use File::Temp qw(tempdir);\n use Getopt::Long qw(:config pass_through);\n use Git;\n \n+my @working_tree;\n+\n sub usage\n {\n \tmy $exitcode = shift;\n@@ -24,15 +32,109 @@ usage: git difftool [-t|--tool=<tool>]\n                     [-x|--extcmd=<cmd>]\n                     [-g|--gui] [--no-gui]\n                     [--prompt] [-y|--no-prompt]\n+                    [-d|--dir-diff]\n                     ['git diff' options]\n USAGE\n \texit($exitcode);\n }\n \n+sub setup_dir_diff\n+{\n+\t# Run the diff; exit immediately if no diff found\n+\tmy $repo = Git->repository();\n+\tmy $diffrtn = $repo->command_oneline(['diff', '--raw', '--no-abbrev', '-z', @ARGV]);\n+\texit(0) if (length($diffrtn) == 0);\n+\n+\t# Setup temp directories\n+\tmy $tmpdir = tempdir('git-diffall.XXXXX', CLEANUP => 1, TMPDIR => 1);\n+\tmy $ldir = \"$tmpdir/left\";\n+\tmy $rdir = \"$tmpdir/right\";\n+\tmkpath($ldir) or die $!;\n+\tmkpath($rdir) or die $!;\n+\n+\t# Build index info for left and right sides of the diff\n+\tmy $submodule_mode = \"160000\";\n+\tmy $null_mode = 0 x 6;\n+\tmy $null_sha1 = 0 x 40;\n+\tmy $lindex = \"\";\n+\tmy $rindex = \"\";\n+\tmy %submodule;\n+\tmy @rawdiff = split('\\0', $diffrtn);\n+\n+\tfor (my $i=0; $i<@rawdiff; $i+=2) {\n+\t\tmy ($lmode, $rmode, $lsha1, $rsha1, $status) = split(' ', substr($rawdiff[$i], 1));\n+\t\tmy $path = $rawdiff[$i + 1];\n+\n+\t\tif (($lmode eq $submodule_mode) or ($rmode eq $submodule_mode)) {\n+\t\t\t$submodule{$path}{left} = $lsha1;\n+\t\t\t$submodule{$path}{right} = $rsha1;\n+\t\t\tnext;\n+\t\t}\n+\n+\t\tif ($lmode ne $null_mode) {\n+\t\t\t$lindex .= \"$lmode $lsha1\\t$path\\0\";\n+\t\t}\n+\n+\t\tif ($rmode ne $null_mode) {\n+\t\t\tif ($rsha1 ne $null_sha1) {\n+\t\t\t\t$rindex .= \"$rmode $rsha1\\t$path\\0\";\n+\t\t\t} else {\n+\t\t\t\tpush(@working_tree, $path);\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\t# Populate the left and right directories based on each index file\n+\tmy ($inpipe, $ctx);\n+\t$ENV{GIT_DIR} = $repo->repo_path();\n+\t$ENV{GIT_INDEX_FILE} = \"$tmpdir/lindex\";\n+\t($inpipe, $ctx) = $repo->command_input_pipe(qw/update-index -z --index-info/);\n+\tprint($inpipe $lindex);\n+\t$repo->command_close_pipe($inpipe, $ctx);\n+\tsystem(('git', 'checkout-index', '--all', \"--prefix=$ldir/\"));\n+\n+\t$ENV{GIT_INDEX_FILE} = \"$tmpdir/rindex\";\n+\t($inpipe, $ctx) = $repo->command_input_pipe(qw/update-index -z --index-info/);\n+\tprint($inpipe $rindex);\n+\t$repo->command_close_pipe($inpipe, $ctx);\n+\tsystem(('git', 'checkout-index', '--all', \"--prefix=$rdir/\"));\n+\n+\t# Changes in the working tree need special treatment since they are\n+\t# not part of the index\n+\tmy $workdir = $repo->repo_path() . \"/..\";\n+\tfor (@working_tree) {\n+\t\tmy $dir = dirname($_);\n+\t\tunless (-d \"$rdir/$dir\") {\n+\t\t\tmkpath(\"$rdir/$dir\") or die $!;\n+\t\t}\n+\t\tcopy(\"$workdir/$_\", \"$rdir/$_\") or die $!;\n+\t\tchmod(stat(\"$workdir/$_\")->mode, \"$rdir/$_\") or die $!;\n+\t}\n+\n+\t# Changes to submodules require special treatment. This loop writes a\n+\t# temporary file to both the left and right directories to show the\n+\t# change in the recorded SHA1 for the submodule.\n+\tforeach my $path (keys %submodule) {\n+\t\tif (defined $submodule{$path}{left}) {\n+\t\t\topen(my $fh, \">\", \"$ldir/$path\") or die $!;\n+\t\t\tprint($fh \"Subproject commit $submodule{$path}{left}\");\n+\t\t\tclose($fh);\n+\t\t}\n+\t\tif (defined $submodule{$path}{right}) {\n+\t\t\topen(my $fh, \">\", \"$rdir/$path\") or die $!;\n+\t\t\tprint($fh \"Subproject commit $submodule{$path}{right}\");\n+\t\t\tclose($fh);\n+\t\t}\n+\t}\n+\n+\treturn ($ldir, $rdir);\n+}\n+\n # parse command-line options. all unrecognized options and arguments\n # are passed through to the 'git diff' command.\n-my ($difftool_cmd, $extcmd, $gui, $help, $prompt);\n+my ($difftool_cmd, $dirdiff, $extcmd, $gui, $help, $prompt);\n GetOptions('g|gui!' => \\$gui,\n+\t'd|dir-diff' => \\$dirdiff,\n \t'h' => \\$help,\n \t'prompt!' => \\$prompt,\n \t'y' => sub { $prompt = 0; },\n@@ -65,22 +167,40 @@ if ($gui) {\n \t\t$ENV{GIT_DIFF_TOOL} = $guitool;\n \t}\n }\n-if (defined($prompt)) {\n-\tif ($prompt) {\n-\t\t$ENV{GIT_DIFFTOOL_PROMPT} = 'true';\n+\n+# In directory diff mode, 'git-difftool--helper' is called once\n+# to compare the a/b directories.  In file diff mode, 'git diff'\n+# will invoke a separate instance of 'git-difftool--helper' for\n+# each file that changed.\n+if (defined($dirdiff)) {\n+\tmy ($a, $b) = setup_dir_diff();\n+\tif (defined($extcmd)) {\n+\t\tsystem(($extcmd, $a, $b));\n \t} else {\n-\t\t$ENV{GIT_DIFFTOOL_NO_PROMPT} = 'true';\n+\t\t$ENV{GIT_DIFFTOOL_DIRDIFF} = 'true';\n+\t\tsystem(('git', 'difftool--helper', $a, $b));\n+\t}\n+\n+\t# If the diff including working copy files and those\n+\t# files were modified during the diff, then the changes\n+\t# should be copied back to the working tree\n+\tmy $repo = Git->repository();\n+\tmy $workdir = $repo->repo_path() . \"/..\";\n+\tfor (@working_tree) {\n+\t\tcopy(\"$b/$_\", \"$workdir/$_\") or die $!;\n+\t\tchmod(stat(\"$b/$_\")->mode, \"$workdir/$_\") or die $!;\n+\t}\n+} else {\n+\tif (defined($prompt)) {\n+\t\tif ($prompt) {\n+\t\t\t$ENV{GIT_DIFFTOOL_PROMPT} = 'true';\n+\t\t} else {\n+\t\t\t$ENV{GIT_DIFFTOOL_NO_PROMPT} = 'true';\n+\t\t}\n \t}\n-}\n \n-$ENV{GIT_PAGER} = '';\n-$ENV{GIT_EXTERNAL_DIFF} = 'git-difftool--helper';\n-my @command = ('git', 'diff', @ARGV);\n-\n-# ActiveState Perl for Win32 does not implement POSIX semantics of\n-# exec* system call. It just spawns the given executable and finishes\n-# the starting program, exiting with code 0.\n-# system will at least catch the errors returned by git diff,\n-# allowing the caller of git difftool better handling of failures.\n-my $rc = system(@command);\n-exit($rc | ($rc >> 8));\n+\t$ENV{GIT_PAGER} = '';\n+\t$ENV{GIT_EXTERNAL_DIFF} = 'git-difftool--helper';\n+\tmy $rc = system(('git', 'diff', @ARGV));\n+\texit($rc | ($rc >> 8));\n+}\ndiff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh\nindex e716d06..478c1be 100755\n--- a/t/t7800-difftool.sh\n+++ b/t/t7800-difftool.sh\n@@ -319,4 +319,43 @@ test_expect_success PERL 'say no to the second file' '\n \techo \"$diff\" | stdin_doesnot_contain br2\n '\n \n+test_expect_success PERL 'setup change in subdirectory' '\n+\tgit checkout master &&\n+\tmkdir sub &&\n+\techo master >sub/sub &&\n+\tgit add sub/sub &&\n+\tgit commit -m \"added sub/sub\" &&\n+\techo test >>file &&\n+\techo test >>sub/sub &&\n+\tgit add . &&\n+\tgit commit -m \"modified both\"\n+'\n+\n+test_expect_success PERL 'difftool -d' '\n+\tdiff=$(git difftool -d --extcmd ls branch) &&\n+\techo \"$diff\" | stdin_contains sub &&\n+\techo \"$diff\" | stdin_contains file\n+'\n+\n+test_expect_success PERL 'difftool --dir-diff' '\n+\tdiff=$(git difftool --dir-diff --extcmd ls branch) &&\n+\techo \"$diff\" | stdin_contains sub &&\n+\techo \"$diff\" | stdin_contains file\n+'\n+\n+test_expect_success PERL 'difftool --dir-diff ignores --prompt' '\n+\tdiff=$(git difftool --dir-diff --prompt --extcmd ls branch) &&\n+\techo \"$diff\" | stdin_contains sub &&\n+\techo \"$diff\" | stdin_contains file\n+'\n+\n+test_expect_success PERL 'difftool --dir-diff from subdirectory' '\n+\t(\n+\t\tcd sub &&\n+\t\tdiff=$(git difftool --dir-diff --extcmd ls branch) &&\n+\t\techo \"$diff\" | stdin_contains sub &&\n+\t\techo \"$diff\" | stdin_contains file\n+\t)\n+'\n+\n test_done\n-- \n1.7.10.rc1.28.geb866\n"}]}