Re: [PATCH v2] bisect: improve output when bad commit is found

From: Christian Couder <christian.couder@gmail.com>
Date: May 13, 2015, 09:10 UTC
Message-ID: <CAP8UFD0VsJG6G=sP9mtkwwG30mZeiGwQLs_9d48Ovx=ws15YuA@mail.gmail.com>
In-Reply-To: <xmqqoalpcm9f.fsf@gitster.dls.corp.google.com>

On Wed, May 13, 2015 at 3:13 AM, Junio C Hamano <gitster@pobox.com> wrote:

Show 37 quoted lines

> Trevor Saunders <tbsaunde@tbsaunde.org> writes:
>
>> On Tue, May 12, 2015 at 04:24:00PM -0700, Junio C Hamano wrote:
>>> Trevor Saunders <tbsaunde@tbsaunde.org> writes:
>>>
>>> > When the first bad commit has been found git bisect prints something
>>> > like this:
>>> >
>>> >    <40 char sha1> is the first bad commit
>>> >    Commit <40 char sha1>
>>> >    ...
>>> >
>>> >    :100644 100644 10f5e57... a46cfeb... M  bisect.c
>>> >    :100755 100755 ae3fec2... 65a19fa... M  git-bisect.sh
>>> >
>>> > The raw diff output is not really useful, and its kind of silly to print
>>> > the sha1 twice.  Instead lets print something like this:
>>> >
>>> >    The first bad commit is
>>> >    Commit <sha1>
>>> >    ...
>>>
>>> According to +CCouder, this change will break existing people's use
>>> cases.
>>>
>>> See $gmane/268881
>>
>> Well, technically he just said it might be that people are parsing the
>> output and could be broken, but if you'd rather not take that risk then
>> I guess we just have to leave things the way they are.
>
> FWIW.
>
>  - I personally do not agree that those who scripted around "git
>    bisect" (as opposed to those who wrote scripts to be driven by
>    the "bisect run" interface) are worth worrying about.  But I am
>    not the whole of the Git world ;-)

You know in git-bisect.sh:bisect_run() we do:

                if sane_grep "is the first bad commit"
"$GIT_DIR/BISECT_RUN" >/dev/null
                then
                        gettextln "bisect run success"
                        exit 0;
                fi

so we are doing it too!

>  - I personally do not find two same 40-hex on two lines is silly at
>    all.

I agree.

Show 11 quoted lines

>  - I _do_ think diff-tree --raw output without recursive is silly.
>    It is not useful for humans (it doesn't even give paths fully),
>    and it is insufficient for scripts, which can grok more through
>    information out of the 40-hex.
>
> So perhaps if we keep
>
>         <40 char sha1> is the first bad commit
>
> and then replace the diff-tree output with "show -s", then the
> result would be good enough, I would say.

Yeah I agree.

And for people who want something else we can implement config options.

For example a bisect.outputformat that could be used like in the following pseudo shell code:

format=$(git config bisect.outputformat)

test -z "$format" && format="medium"

git show -s --format="$format" "$firstbadcommit"