Re: [PATCH 1/2] range-diff: fix a crash in parsing git-log output

From: Vasil Dimov <vd@freebsd.org>
Date: Apr 15, 2020, 16:16 UTC
Message-ID: <20200415161647.GA39892@smle>
In-Reply-To: <xmqqsgh47okk.fsf@gitster.c.googlers.com>

On Wed, Apr 15, 2020 at 08:31:39 -0700, Junio C Hamano wrote:

Show 17 quoted lines

> "Vasil Dimov via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > From: Vasil Dimov <vd@FreeBSD.org>
> >
> > `git range-diff` calls `git log` internally and tries to parse its
> > output. But `git log` output can be customized by the user in their
> > git config and for certain configurations either an error will be
> > returned by `git range-diff` or it will crash.
> >
> > To fix this explicitly set the output format of the internally
> > executed `git log` with `--pretty=medium`. Because that cancels
> > `--notes`, add explicitly `--notes` at the end.
>
> Good finding.
>
> Shouldn't we also disable customizations that come from the
> configuration variables like diff.external, diff.<driver>.command?

Hmm, I am not sure. I just read the doc about diff.<driver>.command. Is it possible that the user has set up some custom diff tools to compare e.g. .jpg files by only comparing their EXIF data instead of the entire (binary) files?

Surely if the customizations wrt diff.external and diff.<driver>.command produce result that is unparsable by git range-diff, then they are not good. But maybe the opposite is the case?

I don't feel confident enough to judge.

-- 
Vasil Dimov
gro.DSBeerF@dv
%
Sometimes I really think people ought to have to pass a proper exam
before they're allowed to be parents. Not just the practical, I mean.
    -- (Terry Pratchett, Thief of Time)