Re: [PATCH v10.1 7/7] bisect: allow any terms set by user

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH v10.1 7/7] bisect: allow any terms set by user

From: Matthieu Moy <hidden>
Date: 2016-06-15 23:05:34

Christian Couder [off-list ref] writes:
On Mon, Jun 29, 2015 at 9:34 AM, Matthieu Moy
quoted
As a user, when I
discovered "git bisect", I was actually surprised that it expected one
particular order between good and bad. I would have expected to be able
to say "this is good, this is bad, tell me where it changed" without
having an idea of who's good and who's bad.
Maybe, but it's not how it has been developed.
... but it's how it would behave if we had this guessing feature. The
user does not have to care whether we internally need "good is an
ancestor of bad" if we can provide a user-interface which does not need
that.

I find it particularly frustrating that we issue the message:

  "The merge base %s is bad.\n"
  "This means the bug has been fixed "
  "between %s and [%s].\n"

bisect is all about finding the commit where a property has changed, and
here it stops, saying "I know it's between A and B, but I won't go
further".
quoted
In particular when bisecting from two branches, the user knows that
branch A is good, and branch B is bad, but does not necessarily know
whether it's a regression in B or a
fix in A. The fact that bisect can find out should be just "normal" from
the user point of view. There's no mistake involved, nothing to fix, and
nothing that went wrong.
Well in this case, it's possible that the merge base is bad and what
the user is interested in is the first bad commit that was commited
before the merge base. We just don't know, in the case the merge base
is bad, what is more interesting for the user.
The question asked by the user is "X is good, Y is bad, tell me where
exactly it changed". We can't know for sure what is "interesting" for
the user, but we can answer his question anyway.

Similarly, there can be several commits that introduce the same
regression (this may happen if you cherry picked the guilty commit from
branch A to branch B, and then merged A and B, or if you broke, fixed,
and then broke again). bisect finds one transition from good to bad, but
not all. It may or may not be the one the user wanted, but we can't
know.
quoted
I think I prefer "term" to "name".
Ok with that. I agree that it would be more consistent to have a "git
bisect terms" and "--term-{old,new,bad,good}".
OK, I've applied it.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH v10.1 7/7] bisect: allow any terms set by user

From: Christian Couder <hidden>
Date: 2016-06-15 23:05:34

On Mon, Jun 29, 2015 at 11:32 AM, Matthieu Moy
[off-list ref] wrote:
Christian Couder [off-list ref] writes:
[...]
I find it particularly frustrating that we issue the message:

  "The merge base %s is bad.\n"
  "This means the bug has been fixed "
  "between %s and [%s].\n"
I find it a good safety feature.
bisect is all about finding the commit where a property has changed,
That is your interpretation of this command. On the man page there is:

    git-bisect - Find by binary search the change that introduced a bug

So its stated purpose is to find the first "bad" commit. Not to find a fix.

This doesn't mean that we cannot or should not make it possible to
find a fix, but we shoudn't try to find a fix when there is doubt
about whether the user wants to find a fix or the first bad commit.
and
here it stops, saying "I know it's between A and B, but I won't go
further".
You can interpret it that way, but my interpretation of this is that
the real reason why we stop is because we don't know what the users
really wants to do, find the fix, or really find the first bad commit.
quoted
quoted
In particular when bisecting from two branches, the user knows that
branch A is good, and branch B is bad, but does not necessarily know
whether it's a regression in B or a
fix in A. The fact that bisect can find out should be just "normal" from
the user point of view. There's no mistake involved, nothing to fix, and
nothing that went wrong.
Well in this case, it's possible that the merge base is bad and what
the user is interested in is the first bad commit that was commited
before the merge base. We just don't know, in the case the merge base
is bad, what is more interesting for the user.
The question asked by the user is "X is good, Y is bad, tell me where
exactly it changed".
That's your interpretation of the question asked by the user.
It can be interpreted otherwise.
We can't know for sure what is "interesting" for
the user, but we can answer his question anyway.
You can answer your interpretation of the question, yes, but this
means taking risks that we don't need to take in my opinion.

If people really want it, we can still have an option called for
example --find-fix that could automatically try to find the fix when
the merge base is "bad" without displaying the message that annoys
you. It would make it explicit that it is ok to find a fix.
Similarly, there can be several commits that introduce the same
regression (this may happen if you cherry picked the guilty commit from
branch A to branch B, and then merged A and B, or if you broke, fixed,
and then broke again). bisect finds one transition from good to bad, but
not all. It may or may not be the one the user wanted, but we can't
know.
Yeah, but this is different as we would still find a first "bad"
commit, not a fix.
quoted
quoted
I think I prefer "term" to "name".
Ok with that. I agree that it would be more consistent to have a "git
bisect terms" and "--term-{old,new,bad,good}".
OK, I've applied it.
Great!
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help