Christian Couder <christian.couder@xxxxxxxxx> writes:
> On Tue, Mar 17, 2015 at 7:33 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>> Christian Couder <christian.couder@xxxxxxxxx> writes:
>>
>>> Yes, but the user is supposed to not change the "bad" pointer for no
>>> good reason.
>>
>> That is irrelevant, no? Nobody is questioning that the user is
>> supposed to judge if a commit is "good" or "bad" correctly.
>
> So if there is already a bad commit and the user gives another
> bad commit, that means that the user knows that it will replace the
> existing bad commit with the new one and that it's done for this
> purpose.
ECANNOTQUITEPARSE. The user may say "git bisect bad $that" and we
do not question $that is bad. Git does not know better than the
user.
But that does not mean Git does not know better than the user how
the current bad commit and $that commit are related. The user is
not interested in "replacing" at all. The user is telling just one
single fact, that is, "$that is bad".
>> I am not quite sure if I am correctly getting what you meant to say,
>> but if you meant "only when --alternate is given, we should do the
>> merge-base thing; we should keep losing the current 'bad' and
>> replace it with the new one without the --alternate option", I would
>> see that as an exercise of a bad taste.
>
> What I wanted to say is that if we change "git bisect bad <commitish>",
> so that now it means "add a new bad commit" instead of the previous
> "replace the current bad commit, if any, with this one", then experienced
> users might see that change as a regression in the user interface and
> it might even break scripts.
Huh?
Step back a bit. The place you need to start from is to admit the
fact that what "git bisect bad <committish>" currently does is
broken.
Try creating this history yourself
a---b---c---d---e---f
and start bisection this way:
$ git bisect start f c
$ git bisect bad a
Immediately after the second command, "git bisect" moans
Some good revs are not ancestor of the bad rev.
git bisect cannot work properly in this case.
Maybe you mistake good and bad revs?
when it notices that the good rev (i.e. 'c') is no longer an
ancestor of the 'bad', which now points at 'a'.
But that is because "git bisect bad" _blindly_ moved 'bad' that used
to point at 'f' to 'a', making a good rev (i.e. 'c') an ancestor of
the bad rev, without even bothering to check.
Now, if we fixed this bug and made the bisect_state function more
careful (namely, when accepting "bad", make sure it is not beyond
any existing "good", or barf like the above, _without_ moving the
bad pointer), the user interface and behaviour would be changed. Is
that a regression? No, it is a usability fix and a progress.
Simply put, bisect_state function can become more careful and
intelligent to help users.
I view this "user goes out of way to tell us a commit that is known
to be bad as bad, even though it is not what we offered to test and
is not an ancestor of the commit that currently marked as bad" case
the same way. We by now hopefully understand that blindly replacing
the current 'bad' is suboptimal. By teaching bisect_state to do the
"merge-base thing", we would be fixing that.
Why is it a regression?
--
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