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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, May 13, 2015 at 11:10:31AM +0200, Christian Couder wrote:
> On Wed, May 13, 2015 at 3:13 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> > Trevor Saunders <tbsaunde@xxxxxxxxxxxx> writes:
> >
> >> On Tue, May 12, 2015 at 04:24:00PM -0700, Junio C Hamano wrote:
> >>> Trevor Saunders <tbsaunde@xxxxxxxxxxxx> 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 saw that, and thought it was pretty gross.

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

*shrug* I'd probably agree more if it came last after the more useful
information the first bad commit has been found.

> >  - 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.

sounds like we all agree about that part at least :)

> And for people who want something else we can implement config options.
> 
> For example a bisect.outputformat that could be used like in the

I'd been thinking of a config option that was a little more user
friendly than writing "The first bad commit is%nCommit %h%n..." but
doing it that way is both simple and might allow us to replace the
printing code in bisect.c with a default format string.

Trev

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]