From 35989c68ee5f577ae959f69281e456fd14460469 Mon Sep 17 00:00:00 2001 From: David Aguilar Date: Sun, 22 Jul 2012 20:57:07 -0700 Subject: difftool: Simplify print_tool_help() Eliminate a global variable and File::Find usage by building upon basename() and glob() instead. Signed-off-by: David Aguilar Signed-off-by: Junio C Hamano --- git-difftool.perl | 25 +++++-------------------- 1 file changed, 5 insertions(+), 20 deletions(-) (limited to 'git-difftool.perl') diff --git a/git-difftool.perl b/git-difftool.perl index c0798540ad..ac0ed633bd 100755 --- a/git-difftool.perl +++ b/git-difftool.perl @@ -13,17 +13,15 @@ use 5.008; use strict; use warnings; -use File::Basename qw(dirname); +use File::Basename qw(basename dirname); use File::Copy; use File::Compare; -use File::Find; use File::stat; use File::Path qw(mkpath); use File::Temp qw(tempdir); use Getopt::Long qw(:config pass_through); use Git; -my @tools; my @working_tree; my $rc; my $repo = Git->repository(); @@ -65,26 +63,13 @@ sub find_worktree my $workdir = find_worktree(); -sub filter_tool_scripts -{ - if (-d $_) { - if ($_ ne ".") { - # Ignore files in subdirectories - $File::Find::prune = 1; - } - } else { - if ((-f $_) && ($_ ne "defaults")) { - push(@tools, $_); - } - } -} - sub print_tool_help { my ($cmd, @found, @notfound); my $gitpath = Git::exec_path(); - find(\&filter_tool_scripts, "$gitpath/mergetools"); + my @files = map { basename($_) } glob("$gitpath/mergetools/*"); + my @tools = sort(grep { !m{^defaults$} } @files); foreach my $tool (@tools) { $cmd = "TOOL_MODE=diff"; @@ -99,10 +84,10 @@ sub print_tool_help } print "'git difftool --tool=' may be set to one of the following:\n"; - print "\t$_\n" for (sort(@found)); + print "\t$_\n" for (@found); print "\nThe following tools are valid, but not currently available:\n"; - print "\t$_\n" for (sort(@notfound)); + print "\t$_\n" for (@notfound); print "\nNOTE: Some of the tools listed above only work in a windowed\n"; print "environment. If run in a terminal-only session, they will fail.\n"; -- cgit v1.2.3 From 75cd75830993e3b545f2b151d8eea2f3178f9ba4 Mon Sep 17 00:00:00 2001 From: David Aguilar Date: Sun, 22 Jul 2012 20:57:08 -0700 Subject: difftool: Eliminate global variables Organize the script so that it has a single main() function which calls out to dir_diff() and file_diff() functions. This eliminates "dir-diff"-specific variables that do not need to be calculated when performing a regular file-diff. Signed-off-by: David Aguilar Signed-off-by: Junio C Hamano --- git-difftool.perl | 128 ++++++++++++++++++++++++++++++++---------------------- 1 file changed, 75 insertions(+), 53 deletions(-) (limited to 'git-difftool.perl') diff --git a/git-difftool.perl b/git-difftool.perl index ac0ed633bd..41ba9323f8 100755 --- a/git-difftool.perl +++ b/git-difftool.perl @@ -22,11 +22,6 @@ use File::Temp qw(tempdir); use Getopt::Long qw(:config pass_through); use Git; -my @working_tree; -my $rc; -my $repo = Git->repository(); -my $repo_path = $repo->repo_path(); - sub usage { my $exitcode = shift; @@ -43,6 +38,8 @@ USAGE sub find_worktree { + my ($repo) = @_; + # Git->repository->wc_path() does not honor changes to the working # tree location made by $ENV{GIT_WORK_TREE} or the 'core.worktree' # config variable. @@ -61,8 +58,6 @@ sub find_worktree return $worktree; } -my $workdir = find_worktree(); - sub print_tool_help { my ($cmd, @found, @notfound); @@ -97,10 +92,13 @@ sub print_tool_help sub setup_dir_diff { + my ($repo, $workdir) = @_; + # Run the diff; exit immediately if no diff found # 'Repository' and 'WorkingCopy' must be explicitly set to insure that # if $GIT_DIR and $GIT_WORK_TREE are set in ENV, they are actually used # by Git->repository->command*. + my $repo_path = $repo->repo_path(); my $diffrepo = Git->repository(Repository => $repo_path, WorkingCopy => $workdir); my $diffrtn = $diffrepo->command_oneline('diff', '--raw', '--no-abbrev', '-z', @ARGV); exit(0) if (length($diffrtn) == 0); @@ -121,6 +119,7 @@ sub setup_dir_diff my $rindex = ''; my %submodule; my %symlink; + my @working_tree = (); my @rawdiff = split('\0', $diffrtn); my $i = 0; @@ -188,7 +187,7 @@ sub setup_dir_diff ($inpipe, $ctx) = $repo->command_input_pipe(qw/update-index -z --index-info/); print($inpipe $lindex); $repo->command_close_pipe($inpipe, $ctx); - $rc = system('git', 'checkout-index', '--all', "--prefix=$ldir/"); + my $rc = system('git', 'checkout-index', '--all', "--prefix=$ldir/"); exit($rc | ($rc >> 8)) if ($rc != 0); $ENV{GIT_INDEX_FILE} = "$tmpdir/rindex"; @@ -238,7 +237,7 @@ sub setup_dir_diff } } - return ($ldir, $rdir); + return ($ldir, $rdir, @working_tree); } sub write_to_file @@ -261,54 +260,70 @@ sub write_to_file close($fh); } -# parse command-line options. all unrecognized options and arguments -# are passed through to the 'git diff' command. -my ($difftool_cmd, $dirdiff, $extcmd, $gui, $help, $prompt, $tool_help); -GetOptions('g|gui!' => \$gui, - 'd|dir-diff' => \$dirdiff, - 'h' => \$help, - 'prompt!' => \$prompt, - 'y' => sub { $prompt = 0; }, - 't|tool:s' => \$difftool_cmd, - 'tool-help' => \$tool_help, - 'x|extcmd:s' => \$extcmd); - -if (defined($help)) { - usage(0); -} -if (defined($tool_help)) { - print_tool_help(); -} -if (defined($difftool_cmd)) { - if (length($difftool_cmd) > 0) { - $ENV{GIT_DIFF_TOOL} = $difftool_cmd; - } else { - print "No given for --tool=\n"; - usage(1); +sub main +{ + # parse command-line options. all unrecognized options and arguments + # are passed through to the 'git diff' command. + my ($difftool_cmd, $dirdiff, $extcmd, $gui, $help, $prompt, $tool_help); + GetOptions('g|gui!' => \$gui, + 'd|dir-diff' => \$dirdiff, + 'h' => \$help, + 'prompt!' => \$prompt, + 'y' => sub { $prompt = 0; }, + 't|tool:s' => \$difftool_cmd, + 'tool-help' => \$tool_help, + 'x|extcmd:s' => \$extcmd); + + if (defined($help)) { + usage(0); } -} -if (defined($extcmd)) { - if (length($extcmd) > 0) { - $ENV{GIT_DIFFTOOL_EXTCMD} = $extcmd; - } else { - print "No given for --extcmd=\n"; - usage(1); + if (defined($tool_help)) { + print_tool_help(); } -} -if ($gui) { - my $guitool = ''; - $guitool = Git::config('diff.guitool'); - if (length($guitool) > 0) { - $ENV{GIT_DIFF_TOOL} = $guitool; + if (defined($difftool_cmd)) { + if (length($difftool_cmd) > 0) { + $ENV{GIT_DIFF_TOOL} = $difftool_cmd; + } else { + print "No given for --tool=\n"; + usage(1); + } + } + if (defined($extcmd)) { + if (length($extcmd) > 0) { + $ENV{GIT_DIFFTOOL_EXTCMD} = $extcmd; + } else { + print "No given for --extcmd=\n"; + usage(1); + } + } + if ($gui) { + my $guitool = ''; + $guitool = Git::config('diff.guitool'); + if (length($guitool) > 0) { + $ENV{GIT_DIFF_TOOL} = $guitool; + } + } + + # In directory diff mode, 'git-difftool--helper' is called once + # to compare the a/b directories. In file diff mode, 'git diff' + # will invoke a separate instance of 'git-difftool--helper' for + # each file that changed. + if (defined($dirdiff)) { + dir_diff($extcmd); + } else { + file_diff($prompt); } } -# In directory diff mode, 'git-difftool--helper' is called once -# to compare the a/b directories. In file diff mode, 'git diff' -# will invoke a separate instance of 'git-difftool--helper' for -# each file that changed. -if (defined($dirdiff)) { - my ($a, $b) = setup_dir_diff(); +sub dir_diff +{ + my ($extcmd) = @_; + + my $rc; + my $repo = Git->repository(); + + my $workdir = find_worktree($repo); + my ($a, $b, @working_tree) = setup_dir_diff($repo, $workdir); if (defined($extcmd)) { $rc = system($extcmd, $a, $b); } else { @@ -327,7 +342,12 @@ if (defined($dirdiff)) { chmod(stat("$b/$file")->mode, "$workdir/$file") or die $!; } } -} else { +} + +sub file_diff +{ + my ($prompt) = @_; + if (defined($prompt)) { if ($prompt) { $ENV{GIT_DIFFTOOL_PROMPT} = 'true'; @@ -347,3 +367,5 @@ if (defined($dirdiff)) { my $rc = system('git', 'diff', @ARGV); exit($rc | ($rc >> 8)); } + +main(); -- cgit v1.2.3 From c9bdd505200120a8dab0f7417ad3fd7553e39e3f Mon Sep 17 00:00:00 2001 From: David Aguilar Date: Sun, 22 Jul 2012 20:57:09 -0700 Subject: difftool: Move option values into a hash Shorten the "my" declaration for all of the option-specific variables by wrapping all of them in a hash. This also gives us a place to specify default values, should we need them. Signed-off-by: David Aguilar Signed-off-by: Junio C Hamano --- git-difftool.perl | 55 +++++++++++++++++++++++++++++++------------------------ 1 file changed, 31 insertions(+), 24 deletions(-) (limited to 'git-difftool.perl') diff --git a/git-difftool.perl b/git-difftool.perl index 41ba9323f8..0ce6168407 100755 --- a/git-difftool.perl +++ b/git-difftool.perl @@ -264,41 +264,48 @@ sub main { # parse command-line options. all unrecognized options and arguments # are passed through to the 'git diff' command. - my ($difftool_cmd, $dirdiff, $extcmd, $gui, $help, $prompt, $tool_help); - GetOptions('g|gui!' => \$gui, - 'd|dir-diff' => \$dirdiff, - 'h' => \$help, - 'prompt!' => \$prompt, - 'y' => sub { $prompt = 0; }, - 't|tool:s' => \$difftool_cmd, - 'tool-help' => \$tool_help, - 'x|extcmd:s' => \$extcmd); - - if (defined($help)) { + my %opts = ( + difftool_cmd => undef, + dirdiff => undef, + extcmd => undef, + gui => undef, + help => undef, + prompt => undef, + tool_help => undef, + ); + GetOptions('g|gui!' => \$opts{gui}, + 'd|dir-diff' => \$opts{dirdiff}, + 'h' => \$opts{help}, + 'prompt!' => \$opts{prompt}, + 'y' => sub { $opts{prompt} = 0; }, + 't|tool:s' => \$opts{difftool_cmd}, + 'tool-help' => \$opts{tool_help}, + 'x|extcmd:s' => \$opts{extcmd}); + + if (defined($opts{help})) { usage(0); } - if (defined($tool_help)) { + if (defined($opts{tool_help})) { print_tool_help(); } - if (defined($difftool_cmd)) { - if (length($difftool_cmd) > 0) { - $ENV{GIT_DIFF_TOOL} = $difftool_cmd; + if (defined($opts{difftool_cmd})) { + if (length($opts{difftool_cmd}) > 0) { + $ENV{GIT_DIFF_TOOL} = $opts{difftool_cmd}; } else { print "No given for --tool=\n"; usage(1); } } - if (defined($extcmd)) { - if (length($extcmd) > 0) { - $ENV{GIT_DIFFTOOL_EXTCMD} = $extcmd; + if (defined($opts{extcmd})) { + if (length($opts{extcmd}) > 0) { + $ENV{GIT_DIFFTOOL_EXTCMD} = $opts{extcmd}; } else { print "No given for --extcmd=\n"; usage(1); } } - if ($gui) { - my $guitool = ''; - $guitool = Git::config('diff.guitool'); + if ($opts{gui}) { + my $guitool = Git::config('diff.guitool'); if (length($guitool) > 0) { $ENV{GIT_DIFF_TOOL} = $guitool; } @@ -308,10 +315,10 @@ sub main # to compare the a/b directories. In file diff mode, 'git diff' # will invoke a separate instance of 'git-difftool--helper' for # each file that changed. - if (defined($dirdiff)) { - dir_diff($extcmd); + if (defined($opts{dirdiff})) { + dir_diff($opts{extcmd}); } else { - file_diff($prompt); + file_diff($opts{prompt}); } } -- cgit v1.2.3 From b965e8f44ad76a810c60406d01b45b87d6ac2701 Mon Sep 17 00:00:00 2001 From: David Aguilar Date: Sun, 22 Jul 2012 20:57:10 -0700 Subject: difftool: Call the temp directory "git-difftool" The "diffall" name was left over from when this functionality was part of the "git-diffall" script in contrib/. Make the naming consistent. Signed-off-by: David Aguilar Signed-off-by: Junio C Hamano --- git-difftool.perl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'git-difftool.perl') diff --git a/git-difftool.perl b/git-difftool.perl index 0ce6168407..2ae344c7f4 100755 --- a/git-difftool.perl +++ b/git-difftool.perl @@ -104,7 +104,7 @@ sub setup_dir_diff exit(0) if (length($diffrtn) == 0); # Setup temp directories - my $tmpdir = tempdir('git-diffall.XXXXX', CLEANUP => 1, TMPDIR => 1); + my $tmpdir = tempdir('git-difftool.XXXXX', CLEANUP => 1, TMPDIR => 1); my $ldir = "$tmpdir/left"; my $rdir = "$tmpdir/right"; mkpath($ldir) or die $!; -- cgit v1.2.3 From 1f2293457579a5bf22bb9c8324ded22b10705cc2 Mon Sep 17 00:00:00 2001 From: David Aguilar Date: Sun, 22 Jul 2012 23:05:30 -0700 Subject: difftool: Use symlinks when diffing against the worktree Teach difftool's --dir-diff mode to use symlinks to represent files from the working copy, and make it the default behavior for the non-Windows platforms. Using symlinks is simpler and safer since we do not need to worry about copying files back into the worktree. The old behavior is still available as --no-symlinks. Signed-off-by: David Aguilar Signed-off-by: Junio C Hamano --- git-difftool.perl | 33 +++++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-) (limited to 'git-difftool.perl') diff --git a/git-difftool.perl b/git-difftool.perl index 2ae344c7f4..a5b371f729 100755 --- a/git-difftool.perl +++ b/git-difftool.perl @@ -92,7 +92,7 @@ sub print_tool_help sub setup_dir_diff { - my ($repo, $workdir) = @_; + my ($repo, $workdir, $symlinks) = @_; # Run the diff; exit immediately if no diff found # 'Repository' and 'WorkingCopy' must be explicitly set to insure that @@ -209,8 +209,13 @@ sub setup_dir_diff unless (-d "$rdir/$dir") { mkpath("$rdir/$dir") or die $!; } - copy("$workdir/$file", "$rdir/$file") or die $!; - chmod(stat("$workdir/$file")->mode, "$rdir/$file") or die $!; + if ($symlinks) { + symlink("$workdir/$file", "$rdir/$file") or die $!; + } else { + copy("$workdir/$file", "$rdir/$file") or die $!; + my $mode = stat("$workdir/$file")->mode; + chmod($mode, "$rdir/$file") or die $!; + } } # Changes to submodules require special treatment. This loop writes a @@ -271,6 +276,7 @@ sub main gui => undef, help => undef, prompt => undef, + symlinks => $^O ne 'MSWin32' && $^O ne 'msys', tool_help => undef, ); GetOptions('g|gui!' => \$opts{gui}, @@ -278,6 +284,8 @@ sub main 'h' => \$opts{help}, 'prompt!' => \$opts{prompt}, 'y' => sub { $opts{prompt} = 0; }, + 'symlinks' => \$opts{symlinks}, + 'no-symlinks' => sub { $opts{symlinks} = 0; }, 't|tool:s' => \$opts{difftool_cmd}, 'tool-help' => \$opts{tool_help}, 'x|extcmd:s' => \$opts{extcmd}); @@ -316,7 +324,7 @@ sub main # will invoke a separate instance of 'git-difftool--helper' for # each file that changed. if (defined($opts{dirdiff})) { - dir_diff($opts{extcmd}); + dir_diff($opts{extcmd}, $opts{symlinks}); } else { file_diff($opts{prompt}); } @@ -324,13 +332,13 @@ sub main sub dir_diff { - my ($extcmd) = @_; + my ($extcmd, $symlinks) = @_; my $rc; my $repo = Git->repository(); my $workdir = find_worktree($repo); - my ($a, $b, @working_tree) = setup_dir_diff($repo, $workdir); + my ($a, $b, @worktree) = setup_dir_diff($repo, $workdir, $symlinks); if (defined($extcmd)) { $rc = system($extcmd, $a, $b); } else { @@ -342,13 +350,18 @@ sub dir_diff # If the diff including working copy files and those # files were modified during the diff, then the changes - # should be copied back to the working tree - for my $file (@working_tree) { - if (-e "$b/$file" && compare("$b/$file", "$workdir/$file")) { + # 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) { + next if $symlinks && -l "$b/$file"; + if (-f "$b/$file" && compare("$b/$file", "$workdir/$file")) { copy("$b/$file", "$workdir/$file") or die $!; - chmod(stat("$b/$file")->mode, "$workdir/$file") or die $!; + my $mode = stat("$b/$file")->mode; + chmod($mode, "$workdir/$file") or die $!; } } + exit(0); } sub file_diff -- cgit v1.2.3 From 7c7584b97096a168fe1236c84e5e12d7bee24476 Mon Sep 17 00:00:00 2001 From: David Aguilar Date: Tue, 24 Jul 2012 20:14:22 -0700 Subject: difftool: Handle finding mergetools/ in a path with spaces Use the original File::Find implementation from bf73fc2 (difftool: print list of valid tools with '--tool-help', 2012-03-29) so that we properly handle mergetools/ being located in a path containing spaces. One small difference is that we avoid using a global variable by passing a reference to the list of tools. Signed-off-by: David Aguilar Signed-off-by: Junio C Hamano --- git-difftool.perl | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) (limited to 'git-difftool.perl') diff --git a/git-difftool.perl b/git-difftool.perl index a5b371f729..30574801be 100755 --- a/git-difftool.perl +++ b/git-difftool.perl @@ -13,9 +13,10 @@ use 5.008; use strict; use warnings; -use File::Basename qw(basename dirname); +use File::Basename qw(dirname); use File::Copy; use File::Compare; +use File::Find; use File::stat; use File::Path qw(mkpath); use File::Temp qw(tempdir); @@ -58,13 +59,27 @@ sub find_worktree return $worktree; } +sub filter_tool_scripts +{ + my ($tools) = @_; + if (-d $_) { + if ($_ ne ".") { + # Ignore files in subdirectories + $File::Find::prune = 1; + } + } else { + if ((-f $_) && ($_ ne "defaults")) { + push(@$tools, $_); + } + } +} + sub print_tool_help { - my ($cmd, @found, @notfound); + my ($cmd, @found, @notfound, @tools); my $gitpath = Git::exec_path(); - my @files = map { basename($_) } glob("$gitpath/mergetools/*"); - my @tools = sort(grep { !m{^defaults$} } @files); + find(sub { filter_tool_scripts(\@tools) }, "$gitpath/mergetools"); foreach my $tool (@tools) { $cmd = "TOOL_MODE=diff"; @@ -79,10 +94,10 @@ sub print_tool_help } print "'git difftool --tool=' may be set to one of the following:\n"; - print "\t$_\n" for (@found); + print "\t$_\n" for (sort(@found)); print "\nThe following tools are valid, but not currently available:\n"; - print "\t$_\n" for (@notfound); + print "\t$_\n" for (sort(@notfound)); print "\nNOTE: Some of the tools listed above only work in a windowed\n"; print "environment. If run in a terminal-only session, they will fail.\n"; -- cgit v1.2.3 From 283abb2c8abb3f40d8d0170ba45c2e45d40f8cdb Mon Sep 17 00:00:00 2001 From: David Aguilar Date: Tue, 24 Jul 2012 20:14:23 -0700 Subject: difftool: Check all return codes from compare() Handle the case where compare() is unable to read its inputs. Emit a warning so that the user knows that something went wrong. We may later want to restructure the code so that we can inhibit tempdir cleanup when this condition is reached. Signed-off-by: David Aguilar Signed-off-by: Junio C Hamano --- git-difftool.perl | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) (limited to 'git-difftool.perl') diff --git a/git-difftool.perl b/git-difftool.perl index 30574801be..92f4907bbc 100755 --- a/git-difftool.perl +++ b/git-difftool.perl @@ -370,7 +370,16 @@ sub dir_diff # external tool did not replace the original link with a file. for my $file (@worktree) { next if $symlinks && -l "$b/$file"; - if (-f "$b/$file" && compare("$b/$file", "$workdir/$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"; + warn $errmsg; + } elsif ($diff == 1) { copy("$b/$file", "$workdir/$file") or die $!; my $mode = stat("$b/$file")->mode; chmod($mode, "$workdir/$file") or die $!; -- cgit v1.2.3 From a4cd5be30cdd19c43908dc55d8dd39a0fe79075b Mon Sep 17 00:00:00 2001 From: David Aguilar Date: Wed, 25 Jul 2012 23:07:57 -0700 Subject: difftool: Wrap long lines for readability Keep everything within 80 columns. Wrap the user-facing messages too. Signed-off-by: David Aguilar Signed-off-by: Junio C Hamano --- git-difftool.perl | 46 ++++++++++++++++++++++++++++++++-------------- 1 file changed, 32 insertions(+), 14 deletions(-) (limited to 'git-difftool.perl') diff --git a/git-difftool.perl b/git-difftool.perl index 92f4907bbc..1ec676aacb 100755 --- a/git-difftool.perl +++ b/git-difftool.perl @@ -93,15 +93,22 @@ sub print_tool_help } } - print "'git difftool --tool=' may be set to one of the following:\n"; + print << 'EOF'; +'git difftool --tool=' may be set to one of the following: +EOF print "\t$_\n" for (sort(@found)); - print "\nThe following tools are valid, but not currently available:\n"; + print << 'EOF'; + +The following tools are valid, but not currently available: +EOF print "\t$_\n" for (sort(@notfound)); - print "\nNOTE: Some of the tools listed above only work in a windowed\n"; - print "environment. If run in a terminal-only session, they will fail.\n"; + print << 'EOF'; +NOTE: Some of the tools listed above only work in a windowed +environment. If run in a terminal-only session, they will fail. +EOF exit(0); } @@ -114,8 +121,11 @@ sub setup_dir_diff # if $GIT_DIR and $GIT_WORK_TREE are set in ENV, they are actually used # by Git->repository->command*. my $repo_path = $repo->repo_path(); - my $diffrepo = Git->repository(Repository => $repo_path, WorkingCopy => $workdir); - my $diffrtn = $diffrepo->command_oneline('diff', '--raw', '--no-abbrev', '-z', @ARGV); + my %repo_args = (Repository => $repo_path, WorkingCopy => $workdir); + my $diffrepo = Git->repository(%repo_args); + + my @gitargs = ('diff', '--raw', '--no-abbrev', '-z', @ARGV); + my $diffrtn = $diffrepo->command_oneline(@gitargs); exit(0) if (length($diffrtn) == 0); # Setup temp directories @@ -140,11 +150,15 @@ sub setup_dir_diff my $i = 0; while ($i < $#rawdiff) { if ($rawdiff[$i] =~ /^::/) { - print "Combined diff formats ('-c' and '--cc') are not supported in directory diff mode.\n"; + warn << 'EOF'; +Combined diff formats ('-c' and '--cc') are not supported in +directory diff mode ('-d' and '--dir-diff'). +EOF exit(1); } - my ($lmode, $rmode, $lsha1, $rsha1, $status) = split(' ', substr($rawdiff[$i], 1)); + my ($lmode, $rmode, $lsha1, $rsha1, $status) = + split(' ', substr($rawdiff[$i], 1)); my $src_path = $rawdiff[$i + 1]; my $dst_path; @@ -156,7 +170,7 @@ sub setup_dir_diff $i += 2; } - if (($lmode eq $submodule_mode) or ($rmode eq $submodule_mode)) { + if ($lmode eq $submodule_mode or $rmode eq $submodule_mode) { $submodule{$src_path}{left} = $lsha1; if ($lsha1 ne $rsha1) { $submodule{$dst_path}{right} = $rsha1; @@ -167,14 +181,16 @@ sub setup_dir_diff } if ($lmode eq $symlink_mode) { - $symlink{$src_path}{left} = $diffrepo->command_oneline('show', "$lsha1"); + $symlink{$src_path}{left} = + $diffrepo->command_oneline('show', "$lsha1"); } if ($rmode eq $symlink_mode) { - $symlink{$dst_path}{right} = $diffrepo->command_oneline('show', "$rsha1"); + $symlink{$dst_path}{right} = + $diffrepo->command_oneline('show', "$rsha1"); } - if (($lmode ne $null_mode) and ($status !~ /^C/)) { + if ($lmode ne $null_mode and $status !~ /^C/) { $lindex .= "$lmode $lsha1\t$src_path\0"; } @@ -199,14 +215,16 @@ sub setup_dir_diff # Populate the left and right directories based on each index file my ($inpipe, $ctx); $ENV{GIT_INDEX_FILE} = "$tmpdir/lindex"; - ($inpipe, $ctx) = $repo->command_input_pipe(qw/update-index -z --index-info/); + ($inpipe, $ctx) = + $repo->command_input_pipe(qw(update-index -z --index-info)); print($inpipe $lindex); $repo->command_close_pipe($inpipe, $ctx); my $rc = system('git', 'checkout-index', '--all', "--prefix=$ldir/"); exit($rc | ($rc >> 8)) if ($rc != 0); $ENV{GIT_INDEX_FILE} = "$tmpdir/rindex"; - ($inpipe, $ctx) = $repo->command_input_pipe(qw/update-index -z --index-info/); + ($inpipe, $ctx) = + $repo->command_input_pipe(qw(update-index -z --index-info)); print($inpipe $rindex); $repo->command_close_pipe($inpipe, $ctx); $rc = system('git', 'checkout-index', '--all', "--prefix=$rdir/"); -- cgit v1.2.3 From ceb1497a74e75a0c9bd53716b787d33ad6e2397f Mon Sep 17 00:00:00 2001 From: David Aguilar Date: Thu, 26 Jul 2012 12:36:19 -0700 Subject: difftool: Handle compare() returning -1 Keep the temporary directory around when compare() cannot read its input files, which is indicated by -1. Defer tempdir creation to allow an early exit in setup_dir_diff(). Wrap the rest of the entry points in an exit_cleanup() function to handle removing temporary files and error reporting. Print the temporary files' location so that the user can recover them. Signed-off-by: David Aguilar Signed-off-by: Junio C Hamano --- git-difftool.perl | 99 ++++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 68 insertions(+), 31 deletions(-) (limited to 'git-difftool.perl') diff --git a/git-difftool.perl b/git-difftool.perl index 1ec676aacb..6f8bb73cd6 100755 --- a/git-difftool.perl +++ b/git-difftool.perl @@ -18,7 +18,7 @@ use File::Copy; use File::Compare; use File::Find; use File::stat; -use File::Path qw(mkpath); +use File::Path qw(mkpath rmtree); use File::Temp qw(tempdir); use Getopt::Long qw(:config pass_through); use Git; @@ -112,6 +112,18 @@ EOF exit(0); } +sub exit_cleanup +{ + my ($tmpdir, $status) = @_; + my $errno = $!; + rmtree($tmpdir); + if ($status and $errno) { + my ($package, $file, $line) = caller(); + warn "$file line $line: $errno\n"; + } + exit($status | ($status >> 8)); +} + sub setup_dir_diff { my ($repo, $workdir, $symlinks) = @_; @@ -128,13 +140,6 @@ sub setup_dir_diff my $diffrtn = $diffrepo->command_oneline(@gitargs); exit(0) if (length($diffrtn) == 0); - # Setup temp directories - my $tmpdir = tempdir('git-difftool.XXXXX', CLEANUP => 1, TMPDIR => 1); - my $ldir = "$tmpdir/left"; - my $rdir = "$tmpdir/right"; - mkpath($ldir) or die $!; - mkpath($rdir) or die $!; - # Build index info for left and right sides of the diff my $submodule_mode = '160000'; my $symlink_mode = '120000'; @@ -203,6 +208,13 @@ EOF } } + # Setup temp directories + my $tmpdir = tempdir('git-difftool.XXXXX', CLEANUP => 0, TMPDIR => 1); + my $ldir = "$tmpdir/left"; + my $rdir = "$tmpdir/right"; + mkpath($ldir) or exit_cleanup($tmpdir, 1); + mkpath($rdir) or exit_cleanup($tmpdir, 1); + # If $GIT_DIR is not set prior to calling 'git update-index' and # 'git checkout-index', then those commands will fail if difftool # is called from a directory other than the repo root. @@ -219,16 +231,18 @@ EOF $repo->command_input_pipe(qw(update-index -z --index-info)); print($inpipe $lindex); $repo->command_close_pipe($inpipe, $ctx); + my $rc = system('git', 'checkout-index', '--all', "--prefix=$ldir/"); - exit($rc | ($rc >> 8)) if ($rc != 0); + exit_cleanup($tmpdir, $rc) if $rc != 0; $ENV{GIT_INDEX_FILE} = "$tmpdir/rindex"; ($inpipe, $ctx) = $repo->command_input_pipe(qw(update-index -z --index-info)); print($inpipe $rindex); $repo->command_close_pipe($inpipe, $ctx); + $rc = system('git', 'checkout-index', '--all', "--prefix=$rdir/"); - exit($rc | ($rc >> 8)) if ($rc != 0); + exit_cleanup($tmpdir, $rc) if $rc != 0; # If $GIT_DIR was explicitly set just for the update/checkout # commands, then it should be unset before continuing. @@ -240,14 +254,19 @@ EOF for my $file (@working_tree) { my $dir = dirname($file); unless (-d "$rdir/$dir") { - mkpath("$rdir/$dir") or die $!; + mkpath("$rdir/$dir") or + exit_cleanup($tmpdir, 1); } if ($symlinks) { - symlink("$workdir/$file", "$rdir/$file") or die $!; + symlink("$workdir/$file", "$rdir/$file") or + exit_cleanup($tmpdir, 1); } else { - copy("$workdir/$file", "$rdir/$file") or die $!; + copy("$workdir/$file", "$rdir/$file") or + exit_cleanup($tmpdir, 1); + my $mode = stat("$workdir/$file")->mode; - chmod($mode, "$rdir/$file") or die $!; + chmod($mode, "$rdir/$file") or + exit_cleanup($tmpdir, 1); } } @@ -255,27 +274,35 @@ EOF # temporary file to both the left and right directories to show the # change in the recorded SHA1 for the submodule. for my $path (keys %submodule) { + my $ok; if (defined($submodule{$path}{left})) { - write_to_file("$ldir/$path", "Subproject commit $submodule{$path}{left}"); + $ok = write_to_file("$ldir/$path", + "Subproject commit $submodule{$path}{left}"); } if (defined($submodule{$path}{right})) { - write_to_file("$rdir/$path", "Subproject commit $submodule{$path}{right}"); + $ok = write_to_file("$rdir/$path", + "Subproject commit $submodule{$path}{right}"); } + exit_cleanup($tmpdir, 1) if not $ok; } # Symbolic links require special treatment. The standard "git diff" # shows only the link itself, not the contents of the link target. # This loop replicates that behavior. for my $path (keys %symlink) { + my $ok; if (defined($symlink{$path}{left})) { - write_to_file("$ldir/$path", $symlink{$path}{left}); + $ok = write_to_file("$ldir/$path", + $symlink{$path}{left}); } if (defined($symlink{$path}{right})) { - write_to_file("$rdir/$path", $symlink{$path}{right}); + $ok = write_to_file("$rdir/$path", + $symlink{$path}{right}); } + exit_cleanup($tmpdir, 1) if not $ok; } - return ($ldir, $rdir, @working_tree); + return ($ldir, $rdir, $tmpdir, @working_tree); } sub write_to_file @@ -286,16 +313,18 @@ sub write_to_file # Make sure the path to the file exists my $dir = dirname($path); unless (-d "$dir") { - mkpath("$dir") or die $!; + mkpath("$dir") or return 0; } # If the file already exists in that location, delete it. This # is required in the case of symbolic links. - unlink("$path"); + unlink($path); - open(my $fh, '>', "$path") or die $!; + open(my $fh, '>', $path) or return 0; print($fh $value); close($fh); + + return 1; } sub main @@ -366,21 +395,19 @@ sub main sub dir_diff { my ($extcmd, $symlinks) = @_; - my $rc; + my $error = 0; my $repo = Git->repository(); - my $workdir = find_worktree($repo); - my ($a, $b, @worktree) = setup_dir_diff($repo, $workdir, $symlinks); + my ($a, $b, $tmpdir, @worktree) = + setup_dir_diff($repo, $workdir, $symlinks); + if (defined($extcmd)) { $rc = system($extcmd, $a, $b); } else { $ENV{GIT_DIFFTOOL_DIRDIFF} = 'true'; $rc = system('git', 'difftool--helper', $a, $b); } - - exit($rc | ($rc >> 8)) if ($rc != 0); - # If the diff including working copy files and those # files were modified during the diff, then the changes # should be copied back to the working tree. @@ -397,13 +424,23 @@ sub dir_diff my $errmsg = "warning: Could not compare "; $errmsg += "'$b/$file' with '$workdir/$file'\n"; warn $errmsg; + $error = 1; } elsif ($diff == 1) { - copy("$b/$file", "$workdir/$file") or die $!; my $mode = stat("$b/$file")->mode; - chmod($mode, "$workdir/$file") or die $!; + copy("$b/$file", "$workdir/$file") or + exit_cleanup($tmpdir, 1); + + chmod($mode, "$workdir/$file") or + exit_cleanup($tmpdir, 1); } } - exit(0); + if ($error) { + warn "warning: Temporary files exist in '$tmpdir'.\n"; + warn "warning: You may want to cleanup or recover these.\n"; + exit(1); + } else { + exit_cleanup($tmpdir, $rc); + } } sub file_diff -- cgit v1.2.3 From 190f5475f2a7edb777b65367d8796177aa13c830 Mon Sep 17 00:00:00 2001 From: David Aguilar Date: Tue, 24 Jul 2012 20:14:24 -0700 Subject: difftool: Disable --symlinks on cygwin Symlinks are not ubiquitous on Windows so make --no-symlinks the default. Signed-off-by: David Aguilar Signed-off-by: Junio C Hamano --- git-difftool.perl | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'git-difftool.perl') diff --git a/git-difftool.perl b/git-difftool.perl index 6f8bb73cd6..17d4de6618 100755 --- a/git-difftool.perl +++ b/git-difftool.perl @@ -338,7 +338,8 @@ sub main gui => undef, help => undef, prompt => undef, - symlinks => $^O ne 'MSWin32' && $^O ne 'msys', + symlinks => $^O ne 'cygwin' && + $^O ne 'MSWin32' && $^O ne 'msys', tool_help => undef, ); GetOptions('g|gui!' => \$opts{gui}, -- cgit v1.2.3 From ed36e5bd41f7192e42e9b4c573875a343a9daf48 Mon Sep 17 00:00:00 2001 From: Ross Lagerwall Date: Tue, 21 Aug 2012 12:21:40 +0200 Subject: difftool: silence warning Silence a warning given when running git difftool --dir-diff and there are no changes. This is because command_oneline returns undef when the command has no output, not ''. Signed-off-by: Ross Lagerwall Acked-by: David Aguilar Signed-off-by: Junio C Hamano --- git-difftool.perl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'git-difftool.perl') diff --git a/git-difftool.perl b/git-difftool.perl index 17d4de6618..edd0493a08 100755 --- a/git-difftool.perl +++ b/git-difftool.perl @@ -138,7 +138,7 @@ sub setup_dir_diff my @gitargs = ('diff', '--raw', '--no-abbrev', '-z', @ARGV); my $diffrtn = $diffrepo->command_oneline(@gitargs); - exit(0) if (length($diffrtn) == 0); + exit(0) unless defined($diffrtn); # Build index info for left and right sides of the diff my $submodule_mode = '160000'; -- cgit v1.2.3