Re: [PATCH] difftool: avoid symlinks when reusing worktree files

> David Aguilar <davvid@gmail.com> writes:
> 
> > difftool's dir-diff should never reuse a symlink, regardless of
> > what it points to.  Tighten use_wt_file() so that it rejects all
> > symlinks.
> >
> > Helped-by: Junio C Hamano <gitster@pobox.com>
> > Signed-off-by: David Aguilar <davvid@gmail.com>
> > ---
> 
> Sorry.  I do recall saying "it is wrong to feed the contents of a
> file that a symlink points at to hash-object" but other than that,
> I completely lost track.
> 
> What purpose does this function play in its callchain?  What does
> its caller wants it to compute?  Is use of the entity in the working
> tree completely optional?  Would the caller happily produce correct
> result even if we changed this function to unconditionally return
> ($use=0, $wt_sha1='0'x40) regardless of the result of lstat(2) on
> "$workdir/$file"?
> 
> The conclusion of the thought process that starts from "it is wrong
> to feed the contents of a file that a symlink points at to
> hash-object" may not be "so let's return $use=0 for all symlinks",
> which is this patch. Depending on what its caller wants it to
> compute, the right conclusion may be "we need to call hash-object
> correctly by first running readlink and then feeding the result to
> it".
> 
> And if the answer is "the caller wants us to compute the hash for a
> symbolic link and say $use=1", then we would instead need to do
> an equivalent of
> 
> 	wt_sha1=$(readlink "$workdir/$file" | hash-object --stdin)
> 
> I cannot quite tell which from the patch and explanation.
> 
> Perhaps an additional test or two would help illustrate what issues
> are being addressed better?
> 
> Thanks.