Re: [PATCH 1/9 v4] bisect: add "git bisect replace" subcommand

From: Christian Couder <chriscool@tuxfamily.org>
Date: Nov 13, 2008, 17:50 UTC
Message-ID: <200811131850.40951.chriscool@tuxfamily.org>
In-Reply-To: <7v63mshsb7.fsf@gitster.siamese.dyndns.org>

Le jeudi 13 novembre 2008, Junio C Hamano a écrit :

Show 46 quoted lines

> Christian Couder <chriscool@tuxfamily.org> writes:
> > Le mercredi 12 novembre 2008, Junio C Hamano a écrit :
> > ...
> >
> >> When you want to hunt for a bug, it is certainly possible that your
> >> tests fail for a bug that is unrelated to what you are hunting for for
> >> a range of commits.  Borrowing from your picture:
> >>
> >>     ...--O--A--X1--X2--...--Xn--B--...
> >>
> >> non of the commit marked as Xi may not be testable.
> >>
> >> But at that point, will you really spend time to rebuild history
> >> between A and B by fixing an unrelated bug that hinders your bisect,
> >> so that you can have a parallel history that is bisectable?  I doubt
> >> anybody would.
> >
> > I think kernel developers and perhaps others do that somehow. I mean,
> > there is the following text in the git-bisect(1) documentation:
> >
> > "
> > You may often find that during bisect you want to have near-constant
> > tweaks (e.g., s/#define DEBUG 0/#define DEBUG 1/ in a header file, or
> > "revision that does not have this commit needs this patch applied to
> > work around other problem this bisection is not interested in") applied
> > to the revision being tested.
> >
> > To cope with such a situation, after the inner git-bisect finds the
> > next revision to test, with the "run" script, you can apply that tweak
> > before compiling,...
> > "
> >
> > So we suggest that people patch at bisect time in case of problems. But
> > I think creating a parallel branch should be better in the long run,
> > because you can easily keep the work you did to make things easier to
> > bisect and you can easily share it with other people working with you.
>
> I strongly disagree.
>
> Maybe you hit X2 which does have a breakage, and you would need to patch
> up that one before being able to test for your bug, but after you say
> good or bad on that one, the next pick will be far away from that Xi
> segment. You will test many more revisions before you come back to X3 or
> X5.  Why should we force the users to fix all the commits in the segment
> "just in case" somebody's bisect falls into the range before that
> actually happens?

The users would not be _forced_ at all to use "git bisect replace", they can ignore it if they want. And even if other coworkers use it, they are not forced at all to use it themself. If they get none of the "bisect-replace-*" branches, the behavior of git bisect will not change at all.

> In other words, unless the breakage you are hunting for exists between
> point A and B that you cannot bisect for that other breakage, you won't
> need to patch-up _every single revision_ in the range for the breakage.
> Doing so beforehand is wasteful.

I think that it depends on many factors. More precisely, it depends on how easy it is to fully test (because to make sure that the bug your are bisecting is between A and B you need to test A and B, so you waste some testing) vs how easy it is to patch up every single revision between A and B.

And I think it can be really easy to patch up every revision between A and B, you might need only something like:

$ git checkout -b patch-up B $ git rebase -i A^

and then squash the last commit into the first one, in the list "git rebase -i" gives you. It may even be easy to automate this with code like this (completely untested, and it assumes git rebase -i accept a script from stdin which may not work right now):

create_replace_branch() {
	_a="$1"
	_b="$2"
	git checkout -b bisect-replace-$_b "$_b" || exit
	git rev-list ^"$_a"^ "$_b" | {
		while read sha1
		do
			case $sha1 in
				$_a) echo "pick $_a"; echo "squash $_b" ;;
				$_b) ;;
				*) echo "pick $sha1 ;;
			esac
		done
	} | git rebase -i "$_a"^
}

Show 8 quoted lines

> And if you know the range of A..B and the fix, the procedure to follow
> the suggestion you quoted above from the doc can even be automated
> relatively easily.  Your "run" script would need to do two merge-base to
> see if the version to be tested falls inside the range, and if so apply
> the known fix before testing (and clean it up afterwards).
>
> Come to think of it, you do not even need to have a custom run script.
> How about an approach illustrated by this patch?

This is interesting, but the fix up patch might not apply cleanly on all the commits in the range, and there is no simple way to share these patches (and changes to them) in a team. Perhaps more importantly, there is also no simple way to look at the result from applying the patch or to manipulate it with other git commands.

I mean it was decided to store changes in Git as blob, trees and commits, not patches, so why would we store these changes as patches?

Regards, Christian.