Re: [PATCH 10/10] push: teach push to be quiet if local ref is strict subset of remote ref

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

 



Andreas Ericsson <ae@xxxxxx> writes:

> Tom Prince wrote:
>>
>> I haven't had occasion to use git-bisect much, but I was under the
>> impression that bisect could already handle merges, or any other shaped
>> history just fine.
>
> It appears the code supports your statement. I started writing on my
> hack-around about a year ago, and the merge-handling code got in with
> 1c4fea3a40e836dcee2f16091bf7bfba96c924d0 at Wed Mar 21 22:16:24 2007.
> Perhaps I shouldn't be so paranoid about useless merges anymore then.
> Hmm. I shall have to look into it. Perhaps Junio can clarify how it
> works? The man-page was terribly silent about how git-bisect handles
> merges.

Bisecting through merge is not a problem.  Not at all, from the
very beginning of the bisect command.

	Side note.  The commit you quote does not change (let
	alone fix) the semantics at all.  It is a pure
	optimization.  The theory behind how bisect works, see
	my OLS presentation (reachable from the gitwiki).

The real problem is what to do when the culprit turns out to be
a merge commit.  How to spot what really is wrong, and figure
out how to fix.  The problem is not for the tool but for the
human, and it is real.

Imagine this history.

      ---Z---o---X---...---o---A---C---D
          \                       /
           o---o---Y---...---o---B

Suppose that on the upper development line, the meaning of one
of the functions existed at Z was changed at commit X.  The
commits from Z leading to A change both the function's
implementation and all calling sites that existed at Z, as well
as new calling sites they add, to be consistent.  There is no
bug at A.

Suppose in the meantime the lower development line somebody
added a new calling site for that function at commit Y.  The
commits from Z leading to B all assume the old semantics of that
function and the callers and the callee are consistent with each
other.  There is no bug at B, either.

You merge to create C.  There is no textual conflict with this
three way merge, and the result merges cleanly.  You bisect
this, because you found D is bad and you know Z was good.  Your
bisect will find that C (merge) is broken.  Understandably so,
as at C, the new calling site of the function added by the lower
branch is not converted to the new semantics, while all the
other calling sites that already existed at Z would have been
converted by the merge.  The new calling site has semantic
adjustment needed, but you do not know that yet.  You need to
find out that is the cause of the breakage by looking at the
merge commit C and the history leading to it.

How would you do that?

Both "git diff A C" and "git diff B C" would be an enormous patch.
Each of them essentially shows the whole change on each branch
since they diverged.  The developers may have well behaved to
create good commits that follow the "commit small, commit often,
commit well contained units" mantra, and each individual commit
leading from Z to A and from Z to B may be easy to review and
understand, but looking at these small and easily reviewable
steps alone would not let you spot the breakage.  You need to
have a global picture of what the upper branch did (and
among many, one of them is to change the semantics of that
particular function) and look first at the huge "diff A C"
(which shows the change the lower branch introduces), and see if
that huge change is consistent with what have been done between
Z and A.

If you linearlize the history by rebasing the lower branch on
top of upper, instead of merging, the bug becomes much easier to
find and understand.  Your history would instead be:

    ---Z---o---X'--...---o---A---o---o---Y'--...---o---B'--D'

and there is a single commit Y' between A and B' that introduced
the new calling site that still uses the new semantics of the
function that was already in A.  "git show Y'" will be a much
smaller patch than "git diff A C" and it is much easier to deal
with.
-
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]

  Powered by Linux