From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
To: Junio C Hamano <gitster@pobox.com>
Cc: Jeff King <peff@peff.net>,
git@vger.kernel.org,
Christophe Macabiau <christophemacabiau@gmail.com>
Subject: Re: fatal error when diffing changed symlinks
Date: Tue, 7 Mar 2017 23:34:08 +0100 (CET) [thread overview]
Message-ID: <alpine.DEB.2.20.1703072332370.3767@virtualbox> (raw)
In-Reply-To: <xmqqfuip7y07.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Tue, 7 Mar 2017, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> >> > When viewing a working tree file, oid.hash could be 0{40} and
> >> > read_sha1_file() is not the right function to use to obtain the
> >> > contents.
> >> >
> >> > Both of these two need to pay attention to 0{40}, I think, as the
> >> > user may be running "difftool -R --dir-diff" in which case the
> >> > working tree would appear in the left hand side instead.
> >>
> >> As a side note, I think even outside of 0{40}, this should be checking
> >> the return value of read_sha1_file(). A corrupted repo should die(), not
> >> segfault.
> >
> > I agree. I am on it.
>
> Friendly ping, if only to make sure that we can keep a piece of this
> thread in the more "recent" pile.
>
> If you have other topics you need to perfect, I think it is OK to
> postpone the fix on this topic a bit longer, but I'd hate to ship
> two releases with a known breakage without an attempt to fix it, so
> if you are otherwise occupied, I may encourage others (including me)
> to take a look at this. The new "difftool" also has a reported
> regression somebody else expressed willingness to work on, which is
> sort of blocked by everybody else not knowing the timeline on this
> one. cf. <20170303212836.GB13790@arch-attack.localdomain>
>
> A patch series would be very welcome, but "Please go ahead if
> somebody else has time, and I'll help reviewing" would be also
> good.
Unfortunately, the obvious fix was not enough. My current work in progress
is in the `difftool-null-sha1` branch on https://gihub.com/dscho/git. I
still have a few other things to tend to, but hope to come back to the
difftool before the end of the week.
Ciao,
Johannes
next prev parent reply other threads:[~2017-03-07 23:07 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-02-24 11:47 fatal error when diffing changed symlinks Christophe Macabiau
2017-02-24 17:53 ` Junio C Hamano
2017-02-24 19:51 ` Junio C Hamano
2017-02-24 20:35 ` Jeff King
2017-02-25 12:36 ` Johannes Schindelin
2017-03-07 18:16 ` Junio C Hamano
2017-03-07 22:34 ` Johannes Schindelin [this message]
2017-03-13 17:56 ` [PATCH] difftool: handle changing symlinks in dir-diff mode David Aguilar
2017-03-13 18:32 ` Junio C Hamano
2017-03-13 21:04 ` Johannes Schindelin
2017-03-13 21:27 ` Johannes Schindelin
2017-03-13 21:33 ` Junio C Hamano
2017-03-14 2:20 ` David Aguilar
2017-03-14 5:52 ` Junio C Hamano
2017-03-14 4:13 ` Junio C Hamano
2017-03-13 19:02 ` Junio C Hamano
2017-03-13 21:36 ` Johannes Schindelin
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=alpine.DEB.2.20.1703072332370.3767@virtualbox \
--to=johannes.schindelin@gmx.de \
--cc=christophemacabiau@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=peff@peff.net \
/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.