Commit 02c5631 (difftool --dir-diff: symlink all files matching the working tree, 2013-03-14) does not handle the case where a file that is being compared does not exist in the working tree. Fix this by checking for existence explicitly before running git-hash-object.
Reported-by: Kevin Bracey <ke...@bracey.fi> Signed-off-by: John Keeping <j...@keeping.me.uk> --- On Fri, May 17, 2013 at 11:10:40AM -0700, Junio C Hamano wrote: > John Keeping <j...@keeping.me.uk> writes: > > > Commit 02c5631 (difftool --dir-diff: symlink all files matching the > > working tree, 2013-03-14) does not handle the case where a file that is > > being compared does not exist in the working tree. Fix this by checking > > for existence explicitly before running git-hash-object. > > > > Reported-by: Kevin Bracey <ke...@bracey.fi> > > Signed-off-by: John Keeping <j...@keeping.me.uk> > > --- > > This fixes a regression in 1.8.3-rc0. > > > > git-difftool.perl | 9 ++++++++- > > t/t7800-difftool.sh | 7 +++++++ > > 2 files changed, 15 insertions(+), 1 deletion(-) > > > > diff --git a/git-difftool.perl b/git-difftool.perl > > index 6780292..0a1cb0a 100755 > > --- a/git-difftool.perl > > +++ b/git-difftool.perl > > @@ -92,7 +92,14 @@ sub use_wt_file > > return 0; > > } > > > > - my $wt_sha1 = $repo->command_oneline('hash-object', "$workdir/$file"); > > + my $wt_sha1; > > + if (-e "$workdir/$file") { > > + $wt_sha1 = $repo->command_oneline('hash-object', > > "$workdir/$file"); > > + } else { > > + # If the file doesn't exist in the working tree, use something > > + # that cannot match a SHA-1. > > + $wt_sha1 = ''; > > Yuck. > > "that cannot match" might be a good justification to say "this does > not break the next line to set $use and forces it to false", but > "when we return false in $use, the value of $wt_sha1 is not used" > needs to be said to convince why this is a safe change. > > But if $sha1 is $null_sha1, we do end up setting $use to true and > the caller would stuff the empty $wt_sha1 to form: > > $wtindex .= "$rmode \$dst_path\0"; > > Is that what we want to do here, or is it a "will never happen" > condition? If the latter, the reason need to be described in this > comment (and in the log). It can't ever happen because we only call this if the mode is non-zero in which case the SHA-1 is only null if there are unstaged changes. However, I think this revised version is much clearer. git-difftool.perl | 6 ++++++ t/t7800-difftool.sh | 7 +++++++ 2 files changed, 13 insertions(+) diff --git a/git-difftool.perl b/git-difftool.perl index 6780292..8a75205 100755 --- a/git-difftool.perl +++ b/git-difftool.perl @@ -92,6 +92,12 @@ sub use_wt_file return 0; } + if (! -e "$workdir/$file") { + # If the file doesn't exist in the working tree, we cannot + # use it. + return (0, $null_sha1); + } + my $wt_sha1 = $repo->command_oneline('hash-object', "$workdir/$file"); my $use = ($sha1 eq $null_sha1) || ($sha1 eq $wt_sha1); return ($use, $wt_sha1); diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh index a6bd99e..d46f041 100755 --- a/t/t7800-difftool.sh +++ b/t/t7800-difftool.sh @@ -356,6 +356,13 @@ run_dir_diff_test 'difftool --dir-diff from subdirectory' ' ) ' +run_dir_diff_test 'difftool --dir-diff when worktree file is missing' ' + test_when_finished git reset --hard && + rm file2 && + git difftool --dir-diff $symlinks --extcmd ls branch master >output && + grep file2 output +' + write_script .git/CHECK_SYMLINKS <<\EOF for f in file file2 sub/sub do -- 1.8.3.rc2.285.gfc18c2c -- 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