Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

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 <davvid@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information...
commit ceb1497a74e75a0c9bd53716b787d33ad6e2397f 1 parent a4cd5be
David Aguilar davvid authored gitster committed

Showing 1 changed file with 68 additions and 31 deletions. Show diff stats Hide diff stats

  1. +68 31 git-difftool.perl
99 git-difftool.perl
@@ -18,7 +18,7 @@
18 18 use File::Compare;
19 19 use File::Find;
20 20 use File::stat;
21   -use File::Path qw(mkpath);
  21 +use File::Path qw(mkpath rmtree);
22 22 use File::Temp qw(tempdir);
23 23 use Getopt::Long qw(:config pass_through);
24 24 use Git;
@@ -112,6 +112,18 @@ sub print_tool_help
112 112 exit(0);
113 113 }
114 114
  115 +sub exit_cleanup
  116 +{
  117 + my ($tmpdir, $status) = @_;
  118 + my $errno = $!;
  119 + rmtree($tmpdir);
  120 + if ($status and $errno) {
  121 + my ($package, $file, $line) = caller();
  122 + warn "$file line $line: $errno\n";
  123 + }
  124 + exit($status | ($status >> 8));
  125 +}
  126 +
115 127 sub setup_dir_diff
116 128 {
117 129 my ($repo, $workdir, $symlinks) = @_;
@@ -128,13 +140,6 @@ sub setup_dir_diff
128 140 my $diffrtn = $diffrepo->command_oneline(@gitargs);
129 141 exit(0) if (length($diffrtn) == 0);
130 142
131   - # Setup temp directories
132   - my $tmpdir = tempdir('git-difftool.XXXXX', CLEANUP => 1, TMPDIR => 1);
133   - my $ldir = "$tmpdir/left";
134   - my $rdir = "$tmpdir/right";
135   - mkpath($ldir) or die $!;
136   - mkpath($rdir) or die $!;
137   -
138 143 # Build index info for left and right sides of the diff
139 144 my $submodule_mode = '160000';
140 145 my $symlink_mode = '120000';
@@ -203,6 +208,13 @@ sub setup_dir_diff
203 208 }
204 209 }
205 210
  211 + # Setup temp directories
  212 + my $tmpdir = tempdir('git-difftool.XXXXX', CLEANUP => 0, TMPDIR => 1);
  213 + my $ldir = "$tmpdir/left";
  214 + my $rdir = "$tmpdir/right";
  215 + mkpath($ldir) or exit_cleanup($tmpdir, 1);
  216 + mkpath($rdir) or exit_cleanup($tmpdir, 1);
  217 +
206 218 # If $GIT_DIR is not set prior to calling 'git update-index' and
207 219 # 'git checkout-index', then those commands will fail if difftool
208 220 # is called from a directory other than the repo root.
@@ -219,16 +231,18 @@ sub setup_dir_diff
219 231 $repo->command_input_pipe(qw(update-index -z --index-info));
220 232 print($inpipe $lindex);
221 233 $repo->command_close_pipe($inpipe, $ctx);
  234 +
222 235 my $rc = system('git', 'checkout-index', '--all', "--prefix=$ldir/");
223   - exit($rc | ($rc >> 8)) if ($rc != 0);
  236 + exit_cleanup($tmpdir, $rc) if $rc != 0;
224 237
225 238 $ENV{GIT_INDEX_FILE} = "$tmpdir/rindex";
226 239 ($inpipe, $ctx) =
227 240 $repo->command_input_pipe(qw(update-index -z --index-info));
228 241 print($inpipe $rindex);
229 242 $repo->command_close_pipe($inpipe, $ctx);
  243 +
230 244 $rc = system('git', 'checkout-index', '--all', "--prefix=$rdir/");
231   - exit($rc | ($rc >> 8)) if ($rc != 0);
  245 + exit_cleanup($tmpdir, $rc) if $rc != 0;
232 246
233 247 # If $GIT_DIR was explicitly set just for the update/checkout
234 248 # commands, then it should be unset before continuing.
@@ -240,14 +254,19 @@ sub setup_dir_diff
240 254 for my $file (@working_tree) {
241 255 my $dir = dirname($file);
242 256 unless (-d "$rdir/$dir") {
243   - mkpath("$rdir/$dir") or die $!;
  257 + mkpath("$rdir/$dir") or
  258 + exit_cleanup($tmpdir, 1);
244 259 }
245 260 if ($symlinks) {
246   - symlink("$workdir/$file", "$rdir/$file") or die $!;
  261 + symlink("$workdir/$file", "$rdir/$file") or
  262 + exit_cleanup($tmpdir, 1);
247 263 } else {
248   - copy("$workdir/$file", "$rdir/$file") or die $!;
  264 + copy("$workdir/$file", "$rdir/$file") or
  265 + exit_cleanup($tmpdir, 1);
  266 +
249 267 my $mode = stat("$workdir/$file")->mode;
250   - chmod($mode, "$rdir/$file") or die $!;
  268 + chmod($mode, "$rdir/$file") or
  269 + exit_cleanup($tmpdir, 1);
251 270 }
252 271 }
253 272
@@ -255,27 +274,35 @@ sub setup_dir_diff
255 274 # temporary file to both the left and right directories to show the
256 275 # change in the recorded SHA1 for the submodule.
257 276 for my $path (keys %submodule) {
  277 + my $ok;
258 278 if (defined($submodule{$path}{left})) {
259   - write_to_file("$ldir/$path", "Subproject commit $submodule{$path}{left}");
  279 + $ok = write_to_file("$ldir/$path",
  280 + "Subproject commit $submodule{$path}{left}");
260 281 }
261 282 if (defined($submodule{$path}{right})) {
262   - write_to_file("$rdir/$path", "Subproject commit $submodule{$path}{right}");
  283 + $ok = write_to_file("$rdir/$path",
  284 + "Subproject commit $submodule{$path}{right}");
263 285 }
  286 + exit_cleanup($tmpdir, 1) if not $ok;
264 287 }
265 288
266 289 # Symbolic links require special treatment. The standard "git diff"
267 290 # shows only the link itself, not the contents of the link target.
268 291 # This loop replicates that behavior.
269 292 for my $path (keys %symlink) {
  293 + my $ok;
270 294 if (defined($symlink{$path}{left})) {
271   - write_to_file("$ldir/$path", $symlink{$path}{left});
  295 + $ok = write_to_file("$ldir/$path",
  296 + $symlink{$path}{left});
272 297 }
273 298 if (defined($symlink{$path}{right})) {
274   - write_to_file("$rdir/$path", $symlink{$path}{right});
  299 + $ok = write_to_file("$rdir/$path",
  300 + $symlink{$path}{right});
275 301 }
  302 + exit_cleanup($tmpdir, 1) if not $ok;
276 303 }
277 304
278   - return ($ldir, $rdir, @working_tree);
  305 + return ($ldir, $rdir, $tmpdir, @working_tree);
279 306 }
280 307
281 308 sub write_to_file
@@ -286,16 +313,18 @@ sub write_to_file
286 313 # Make sure the path to the file exists
287 314 my $dir = dirname($path);
288 315 unless (-d "$dir") {
289   - mkpath("$dir") or die $!;
  316 + mkpath("$dir") or return 0;
290 317 }
291 318
292 319 # If the file already exists in that location, delete it. This
293 320 # is required in the case of symbolic links.
294   - unlink("$path");
  321 + unlink($path);
295 322
296   - open(my $fh, '>', "$path") or die $!;
  323 + open(my $fh, '>', $path) or return 0;
297 324 print($fh $value);
298 325 close($fh);
  326 +
  327 + return 1;
299 328 }
300 329
301 330 sub main
@@ -366,21 +395,19 @@ sub main
366 395 sub dir_diff
367 396 {
368 397 my ($extcmd, $symlinks) = @_;
369   -
370 398 my $rc;
  399 + my $error = 0;
371 400 my $repo = Git->repository();
372   -
373 401 my $workdir = find_worktree($repo);
374   - my ($a, $b, @worktree) = setup_dir_diff($repo, $workdir, $symlinks);
  402 + my ($a, $b, $tmpdir, @worktree) =
  403 + setup_dir_diff($repo, $workdir, $symlinks);
  404 +
375 405 if (defined($extcmd)) {
376 406 $rc = system($extcmd, $a, $b);
377 407 } else {
378 408 $ENV{GIT_DIFFTOOL_DIRDIFF} = 'true';
379 409 $rc = system('git', 'difftool--helper', $a, $b);
380 410 }
381   -
382   - exit($rc | ($rc >> 8)) if ($rc != 0);
383   -
384 411 # If the diff including working copy files and those
385 412 # files were modified during the diff, then the changes
386 413 # should be copied back to the working tree.
@@ -397,13 +424,23 @@ sub dir_diff
397 424 my $errmsg = "warning: Could not compare ";
398 425 $errmsg += "'$b/$file' with '$workdir/$file'\n";
399 426 warn $errmsg;
  427 + $error = 1;
400 428 } elsif ($diff == 1) {
401   - copy("$b/$file", "$workdir/$file") or die $!;
402 429 my $mode = stat("$b/$file")->mode;
403   - chmod($mode, "$workdir/$file") or die $!;
  430 + copy("$b/$file", "$workdir/$file") or
  431 + exit_cleanup($tmpdir, 1);
  432 +
  433 + chmod($mode, "$workdir/$file") or
  434 + exit_cleanup($tmpdir, 1);
404 435 }
405 436 }
406   - exit(0);
  437 + if ($error) {
  438 + warn "warning: Temporary files exist in '$tmpdir'.\n";
  439 + warn "warning: You may want to cleanup or recover these.\n";
  440 + exit(1);
  441 + } else {
  442 + exit_cleanup($tmpdir, $rc);
  443 + }
407 444 }
408 445
409 446 sub file_diff

0 comments on commit ceb1497

Please sign in to comment.
Something went wrong with that request. Please try again.