git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
David Aguilar <davvid@gmail.com>
Date
Oct 30, 2015, 07:28 UTC
Message-ID
<20151030072857.GA15031@gmail.com>
In-Reply-To
<xmqq4mh9sfl6.fsf@gitster.mtv.corp.google.com>
On Thu, Oct 29, 2015 at 11:19:01AM -0700, Junio C Hamano wrote:
Show 40 quoted lines
> Junio C Hamano <gitster@pobox.com> 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@gmail.com>
> 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@gmail.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---

This is a very nicely worded commit message. Thanks for the thorough explanation.

Show 18 quoted lines
>  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
Previous: Junio C HamanoNext: Junio C Hamano
Message 6 of 8 in “difftool: avoid symlinks when reusing worktree files”
  1. difftool: avoid symlinks when reusing worktree filesDavid Aguilar, Oct 27, 2015
  2. Junio C HamanoOct 27, 2015
  3. David AguilarOct 29, 2015
  4. Junio C HamanoOct 29, 2015
  5. Junio C HamanoOct 29, 2015
  6. David AguilarOct 30, 2015
  7. Junio C HamanoOct 30, 2015
  8. Junio C HamanoOct 27, 2015

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.