From: David Aguilar <davvid@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Ismail Badawi <ismail@badawi.io>,
John Keeping <john@keeping.me.uk>,
Tim Henigan <tim.henigan@gmail.com>,
Git Mailing List <git@vger.kernel.org>
Subject: Re: [PATCH] difftool: gracefully handle symlinks to directories
Date: Tue, 27 Oct 2015 14:30:56 -0700 [thread overview]
Message-ID: <20151027213056.GA6527@gmail.com> (raw)
In-Reply-To: <xmqq8u6usqx1.fsf@gitster.mtv.corp.google.com>
On Thu, Oct 22, 2015 at 11:23:54AM -0700, Junio C Hamano wrote:
> 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.
Ah, right. I think the simplest thing to do is to tighten
use_wt_file() so that it always rejects symlinks. That seems
like a safe way to go for now without needing to invent a new
paradigm for how to handle symlinks in the dir-diff code.
I just sent a follow-up patch that does just that. Let me know
if you'd like a replacement patch that combines the two patches
instead.
Thanks for the review,
--
David
prev parent reply other threads:[~2015-10-27 21:55 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-10-21 8:04 [PATCH] difftool: gracefully handle symlinks to directories David Aguilar
2015-10-22 18:23 ` Junio C Hamano
2015-10-27 21:30 ` 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=20151027213056.GA6527@gmail.com \
--to=davvid@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.