Re: [PATCH] difftool: gracefully handle symlinks to directories
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Oct 22, 2015, 18:23 UTC
- Message-ID
- <xmqq8u6usqx1.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <1445414673-15307-1-git-send-email-davvid@gmail.com>
David Aguilar <davvid@gmail.com> writes:
> difftool's dir-diff feature was blindly feeding worktree paths > to hash-object without checking whether the path was indeed a > file, causing the feature to fail when repositories contain > symlinks to directories.
Wait. Anything that considers symlinks "to directories" any special smells like a misdesign here. Why is it safe to substitute a symbolic link that happens to point at a file with the file it points at?
Because the way you would hash a symblic link is not by hashing the file it points at, but by hashing the result of readlink(2) of it, we must not reuse the working tree files for any symbolic link, regardless of its target, I would think.
After all, a symbolic link may even be dangling and not pointing at anything.
Show 54 quoted lines
>
> Ensure that only files are ever given to hash-object.
> Add a test to demonstrate the breakage.
>
> Reported-by: Ismail Badawi <ismail@badawi.io>
> Signed-off-by: David Aguilar <davvid@gmail.com>
> ---
> git-difftool.perl | 4 +---
> t/t7800-difftool.sh | 19 +++++++++++++++++++
> 2 files changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/git-difftool.perl b/git-difftool.perl
> index 7df7c8a..1abe647 100755
> --- a/git-difftool.perl
> +++ b/git-difftool.perl
> @@ -70,9 +70,7 @@ sub use_wt_file
> my ($repo, $workdir, $file, $sha1) = @_;
> my $null_sha1 = '0' x 40;
>
> - if (! -e "$workdir/$file") {
> - # If the file doesn't exist in the working tree, we cannot
> - # use it.
> + if (! -f "$workdir/$file") {
> return (0, $null_sha1);
> }
>
> diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
> index 48c6e2b..ec8bc8c 100755
> --- a/t/t7800-difftool.sh
> +++ b/t/t7800-difftool.sh
> @@ -504,4 +504,23 @@ test_expect_success PERL 'difftool properly honors gitlink and core.worktree' '
> )
> '
>
> +test_expect_success PERL,SYMLINKS 'difftool --dir-diff symlinked directories' '
> + git init dirlinks &&
> + (
> + cd dirlinks &&
> + git config diff.tool checktrees &&
> + git config difftool.checktrees.cmd "echo good" &&
> + mkdir foo &&
> + : >foo/bar &&
> + git add foo/bar &&
> + test_commit symlink-one &&
> + ln -s foo link &&
> + git add link &&
> + test_commit symlink-two &&
> + echo good >expect &&
> + git difftool --tool=checktrees --dir-diff HEAD~ >actual &&
> + test_cmp expect actual
> + )
> +'
> +
> test_done