git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Aguilar <davvid@gmail.com>
To: Ismail Badawi <ismail@badawi.io>
Cc: git@vger.kernel.org, John Keeping <john@keeping.me.uk>,
	Tim Henigan <tim.henigan@gmail.com>
Subject: Re: git difftool --dir-diff error in the presence of symlinks to directories
Date: Mon, 22 Jun 2015 05:20:25 +0000	[thread overview]
Message-ID: <20150622052024.GA2188@gmail.com> (raw)
In-Reply-To: <CAFB4ZjUnhEMfjg9+sHrPp271e=V=yU_5QFHBbmPpWMkb72+ENw@mail.gmail.com>

On Wed, Jun 17, 2015 at 06:39:27PM -0400, Ismail Badawi wrote:
> Reproduce like this (using git 2.4.3):
> 
> git init
> mkdir foo
> touch foo/bar
> git add .
> git commit -m "Initial commit."
> ln -s foo link
> git add .
> git commit -m "Add link to foo."
> git difftool -d HEAD^ HEAD
> 
> That last command outputs:
> 
> fatal: Unable to hash /Users/isbadawi/test/link
> hash-object /Users/isbadawi/test/link: command returned error: 128
> 
> Briefly looking at the 'git difftool' source it looks like it uses the
> output of 'git diff --raw' and calls 'hash-object' on any object whose
> mode is nonzero, including symlinks.
> 
> I'm not sure what the right thing to do here is -- just thought I'd
> report this failure.
> 
> Thanks,
> Ismail

Thanks for the bug report, Ismail.

This looks like an unhandled edge case that difftool's dir-diff
mode doesn't take into account.  I've cc'd Tim and John since
they're both familiar with this part of the code.

We should take your bug recipe above and add it to difftool's
test suite when fixing the bug.  I've included a diff below that
adds the above snippet to t7800-difftool.sh.

As far as how to handle this edge case -- one option is that we
could detect that the link points at a directory and ignore it.

I think what's happening is that we're hashing the file so that
we can later detect whether it's changed.  The fix might be as
simple as skipping links to directories in that code path, but I
haven't looked too closely yet.  I can take a look in a couple
of weeks if no one beats me to it by then.

--- >8 --- >8 ---
diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index ea35a02..f67fc65 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -504,4 +504,24 @@ test_expect_success PERL 'difftool properly honors gitlink and core.worktree' '
 	)
 '
 
+test_expect_success PERL,SYMLINKS 'difftool --dir-diff symlinked directories' '
+	test_config diff.tool checktrees &&
+	test_config difftool.checktrees.cmd "echo good" &&
+	git init dirlinks &&
+	(
+		cd dirlinks &&
+		mkdir foo &&
+		: >foo/bar &&
+		git add foo/bar &&
+		test_commit one &&
+		ln -s foo link
+		git add link
+		test_commit two &&
+		echo good >expect &&
+		git difftool --dir-diff HEAD~ >actual &&
+		test_commit expect actual
+
+	)
+'
+
 test_done
-- 
David

      reply	other threads:[~2015-06-22  5:20 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-17 22:39 git difftool --dir-diff error in the presence of symlinks to directories Ismail Badawi
2015-06-22  5:20 ` David Aguilar [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150622052024.GA2188@gmail.com \
    --to=davvid@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=ismail@badawi.io \
    --cc=john@keeping.me.uk \
    --cc=tim.henigan@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).