git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Christian Couder <chriscool@tuxfamily.org>
To: Junio C Hamano <gitster@pobox.com>
Cc: Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	Michael Haggerty <mhagger@alum.mit.edu>,
	Jeff King <peff@peff.net>,
	git@vger.kernel.org
Subject: Re: [NON-TOY PATCH] git bisect: introduce 'fixed' and 'unfixed'
Date: Thu, 26 Jun 2008 08:03:20 +0200	[thread overview]
Message-ID: <200806260803.20731.chriscool@tuxfamily.org> (raw)
In-Reply-To: <7v8wwubh3j.fsf@gitster.siamese.dyndns.org>

Le mercredi 25 juin 2008, Junio C Hamano a écrit :
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> > On Tue, 24 Jun 2008, Michael Haggerty wrote:
> > ...
> >
> >> It seems to me that your problem is that git-bisect requires the
> >> "good" revision to be older than the "bad" one.  

Yes, "git bisect" works if the good revisions are ancestors of the bad 
revision.

Currently if you mistake good and bad revs (and if one of the rev is
an ancestor of the other) you get something like:

$ git bisect start HEAD~3 HEAD
'git rev-list --bisect-vars' failed:
maybe you mistake good and bad revs?

I also noticed that if the good and bad are siblings for example like:

A-B-C-D
   \E-F

and you say:

$ git bisect start D F

(that means D is bad and F is good)

then it will kind of "work" but only C and D will be considered as possible 
first bad commits. This is arguably a bug because for example E could have 
fixed a bug that always existed, and then the first bad commit is B or A 
depending how we define it.

> >> If this requirement 
> >> were removed, would there still be a need for "fixed" vs. "unfixed"?

Well this requirement can be "removed" in different ways.

1) We could just allow anything to be called "bad" and "good" as long as 
there is either:

- only one bad revision and all good revisions are its ancestor, or
- only one good revision and all bad revisions are its ancestor

2) Another way to remove the requirement is to make it work in the siblings 
case above.

> > Nope.
> >
> > The thing that makes "fixed" and "bad" special is that _one_ commit
> > introduced that.
>
> That was my initial reaction, and I actually was about to phrase it more
> bluntly: you do not understand what "bisect" is.
>
> But that was a reaction without thinking things through.  It may not be
> what "git bisect" currently is, but the suggestion does not go against
> what the underlying "git rev-list --bisect" is at all.

If we want to make the siblings case (case 2) work, then "git 
rev-list --bisect" needs work though.

> I think what 
> Michael is speculating is different, and it makes sense in its own way.
>
> Instead of having a set of bisect/good-* refs and a single bisect-bad
> ref, your "fixed and unfixed" mode could work quite differently.  By
> noticing that the topology the user specified with initial good and bad
> have ancient bad and recent good --- that is, "it used to be bad but now
> it is good" --- you could instead use a set of bisect/bad-* refs and a
> single bisect-good ref, and feed good and bad swapped to "rev-list
> --bisect" in bisect_next().  That way, the labels given by visualize will
> match what the user is doing automatically.

Yes, that is the case 1 above.

> I said "it makes sense in its own way", because it is _quite_ different
> from how git-bisect currently assumes, and restructuring git-bisect to
> operate naturally in a way Michael describes would be a much larger
> surgery with costs (including risks of bugs) associated with it, which
> needs to be weighed in when judging that approach would actually make
> sense.

Yes it needs work in git-bisect.sh and I don't think the current situation 
with the "maybe you mistake good and bad revs?" error message is too bad.

Regards,
Christian.

  parent reply	other threads:[~2008-06-26  6:00 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-06-24 14:17 [TOY PATCH] git bisect: introduce 'fixed' and 'unfixed' Johannes Schindelin
2008-06-24 14:42 ` Stephan Beyer
2008-06-24 14:55   ` Johannes Schindelin
2008-06-24 15:16     ` Stephan Beyer
2008-06-24 15:02 ` Nicolas Pitre
2008-06-24 16:38 ` Jeff King
2008-06-24 16:50   ` Johannes Schindelin
2008-06-24 17:09     ` [NON-TOY " Johannes Schindelin
2008-06-24 17:41       ` Jeff King
2008-06-24 19:22         ` Daniel Barkalow
2008-06-24 19:26           ` Johannes Schindelin
2008-06-24 22:30         ` Junio C Hamano
2008-06-27 13:48           ` [PATCH, next version] " Johannes Schindelin
2008-06-27 23:03             ` Junio C Hamano
2008-06-28 13:48               ` Johannes Schindelin
2008-06-28 17:52                 ` Junio C Hamano
2008-06-24 19:59       ` [NON-TOY PATCH] " SZEDER Gábor
2008-06-24 20:06       ` Michael Haggerty
2008-06-24 20:38         ` Johannes Schindelin
2008-06-24 22:31           ` Junio C Hamano
2008-06-24 22:43             ` Nicolas Pitre
2008-06-26  6:03             ` Christian Couder [this message]
2008-06-24 22:48         ` Lea Wiemann
2008-06-24 23:53           ` A Large Angry SCM
2008-06-25  7:27             ` Karl Hasselström
2008-06-24 16:54   ` [TOY " Reini Urban

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

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200806260803.20731.chriscool@tuxfamily.org \
    --to=chriscool@tuxfamily.org \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=mhagger@alum.mit.edu \
    --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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).