On Thu, Oct 29, 2015 at 11:19:01AM -0700, Junio C Hamano wrote:
> Junio C Hamano <gitster@xxxxxxxxx> writes:
>
> > So I think it is fine to return $use=0 for any symbolic link from
> > use_wt_file. Anything you do there will be replaced by the loop
> > over %symlink that appears later in the caller. The caller discards
> > $wt_sha1 when $use=0 is returned, so the second return value does
> > not matter.
>
> So let me try to update your patch with the result of the study of
> the codeflow.
>
> -- >8 --
> From: David Aguilar <davvid@xxxxxxxxx>
> Subject: difftool: ignore symbolic links in use_wt_file
>
> The caller is preparing a narrowed-down copy of the working tree and
> this function is asked if the path should be included in that copy.
> If we say yes, the path from the working tree will be either symlinked
> or copied into the narrowed-down copy.
>
> For any path that is a symbolic link, the caller later fixes up the
> narrowed-down copy by unlinking the path and replacing it with a
> regular file it writes out that mimics the way how "git diff"
> compares symbolic links.
>
> Let's answer "no, you do not want to copy/symlink the working tree
> file" for all symbolic links from this function, as we know the
> result will not be used because it will be overwritten anyway.
>
> Incidentally, this also stops the function from feeding a symbolic
> link in the working tree to hash-object, which is a wrong thing to
> do to begin with. The link may be pointing at a directory, or worse
> may be dangling (both would be noticed as an error). Even if the
> link points at a regular file, hashing the contents of a file that
> is pointed at by the link is not correct (Git hashes the contents of
> the link itself, not the pointee).
>
> Signed-off-by: David Aguilar <davvid@xxxxxxxxx>
> Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
> ---
This is a very nicely worded commit message. Thanks for the
thorough explanation.
> 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..488d14b 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 (-l "$workdir/$file" || ! -e _) {
> return (0, $null_sha1);
> }
The "-e _" shorthand caught my eye ~ I didn't know perl could do that!
Nice.
Underline is barely mentioned in perlvar, but it's obvious what
(I think) it means, and since Perl is DWIM, it must be right. ;-)
--
David
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html