git bisect plus fixes (was: PATCH: Add --size-check=[error|warning])

git bisect plus fixes (was: PATCH: Add --size-check=[error|warning])

[Ralf Wildenhues]

[ adding the git list; this is
  http://thread.gmane.org/gmane.comp.gnu.binutils/52601/focus=1112779 ]

Hello,

* Ingo Molnar wrote on Mon, Mar 14, 2011 at 01:23:42PM CET:
> Also, i hope you are not suggesting to break projects just because
> they are not important to you personally? The fix is exceedingly
> simple to do for the binutils project - and impossible to do for the
> kernel project (because during bisection - which is a very powerful
> debugging tool - older versions of the source get checked out).

FWIW, I don't have an opinion on this particular binutils issue, but
it would be very helpful if 'git bisect' made it easy to denote
"when going back, you might also need some of these changes".
(I'd just use a patch -p1 with a here-file in the bisect script, but
that might not be enough for all practical use cases.)

This issue has come up several times with high-profile issues.

Thanks,
Ralf

[PATCH] git-bisect.txt: example for bisecting with hotfix

[Michael J Gruber]

Give an example on how to bisect when older revisions need a hotfix to
build, run or test. Triggered by the binutils/kernel issue at

http://thread.gmane.org/gmane.comp.gnu.binutils/52601/focus=1112779

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
Maybe this doc fix would do. Just tag the hotfix and tell people to cherry-pick it
like this. I don't think "git bisect --with-fix=hotfix" would be much simpler.
(culling kernel list from cc - don't apply this to the wrong tree :)

 Documentation/git-bisect.txt |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
index c39d957..25acf26 100644
--- a/Documentation/git-bisect.txt
+++ b/Documentation/git-bisect.txt
@@ -322,6 +322,17 @@ $ git bisect run sh -c "make || exit 125; ~/check_test_case.sh"
 +
 Does the same as the previous example, but on a single line.
 
+* Bisect with compatibility hotfix:
++
+------------
+$ git bisect start HEAD HEAD~10 --   # culprit is among the last 10
+$ git bisect run sh -c "git cherry-pick -n hotfix || exit 125; make || exit 125; ~/check_test_case.sh"
+------------
++
+Does the same as the previous example, but applies an additional patch
+before building. This is useful when your build or test environment changed so
+that older revisions may need a fix which newer ones have already.
+
 Author
 ------
 Written by Linus Torvalds <torvalds@osdl.org>
-- 
1.7.4.1.404.g62d316

Re: [PATCH] git-bisect.txt: example for bisecting with hotfix

[Junio C Hamano]

Michael J Gruber <git@drmicha.warpmail.net> writes:

> +* Bisect with compatibility hotfix:
> ++
> +------------
> +$ git bisect start HEAD HEAD~10 --   # culprit is among the last 10
> +$ git bisect run sh -c "git cherry-pick -n hotfix || exit 125; make || exit 125; ~/check_test_case.sh"
> +------------
> ++
> +Does the same as the previous example, but applies an additional patch
> +before building. This is useful when your build or test environment changed so
> +that older revisions may need a fix which newer ones have already.
> +

It is a good idea to add an example that shows it is perfectly Ok to muck
with the working tree before testing, but I don't see this patch as such.

First of all, doesn't bisect_checkout does its job with "checkout -q"
without "-f"?  How would that interact with running cherry-pick _every
time_ you check commit to be tested out?  In some situations it may work
OK by accident, but as an example that is likely to be cut&pasted I think
we should show a reasonably safer way than this.

There likely are more than one hot-fixes; it would make more sense to
illustrate merging a hot-fixes branch using "git merge --no-commit" than
using cherry-pick.

But the above are minor; the biggest issue I have with this patch is that
it breaks the train of thought for people who are reading from top to
bottom.

Look at what is there currently. It starts simple (single command run via
"run" interface), and demonstrates that anything complex can be easily
managed with a fully-spelled-out "one test wrapper that builds and then
runs test" example, to show the most generic way you can use. It then
introduces special exit codes in the script, as that form is easier to
read than a single command line.

It then shows, as a final aside, that it isn't strictly required to use a
wrapper script and you could use "sh -c" to wrap that in the command line,
which may be easier to use when (and only when IMO---if you will have
anything that needs debugging then you are better off with the first
approach) the command line you use for testing is trivial.

I think a new example you are adding would fit much better in the flow if
you replaced the example it refers to as "the previous example".  There
are that "previous example" that uses the "check_test_case.sh", and the
one before that one that uses "make test"; they are duplicates that do not
add much value and we would add value by dropping one of them.  It
probably is better to remove the "make test" one and keep the
"check_test_case.sh" one, as long as you explain "check_test_case.sh"
sufficiently well, because the latter is more generally applicable.

The new example would fit well as an illustration of what you _could_ have
in test.sh script when you need to do more elaborate set-up before testing
each revision, e.g., you tweak the working tree with hotfix before running
"make || exit 125", and clean that up after you tested. The core of the
new section would look like this:

	$ cat test.sh
	#!/bin/sh

	# tweak the working tree by merging the hot-fix branch
        # and then attempt a build
	if	git cherry-pick --no-commit hot-fix &&
        	make
	then
                # run project specific test and report its status
                ./test.sh
                status=$?
	else
		# tell the caller this is untestable
		status=125
	fi

	# undo the tweak to allow clean flipping to the next commit
        git reset --hard

	# return control
	exit $status

[PATCHv2 1/2] git-bisect.txt: streamline run presentation

[Michael J Gruber]

Streamline the presentation of "bisect run" by removing one example
which does not introduce new concepts.

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
 Documentation/git-bisect.txt |   34 ++++++++--------------------------
 1 files changed, 8 insertions(+), 26 deletions(-)

diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
index c39d957..47e8b1e 100644
--- a/Documentation/git-bisect.txt
+++ b/Documentation/git-bisect.txt
@@ -274,53 +274,35 @@ $ git bisect start HEAD origin --    # HEAD is bad, origin is good
 $ git bisect run make test           # "make test" builds and tests
 ------------
 
-* Automatically bisect a broken test suite:
-+
-------------
-$ cat ~/test.sh
-#!/bin/sh
-make || exit 125                   # this skips broken builds
-make test                          # "make test" runs the test suite
-$ git bisect start v1.3 v1.1 --    # v1.3 is bad, v1.1 is good
-$ git bisect run ~/test.sh
-------------
-+
-Here we use a "test.sh" custom script. In this script, if "make"
-fails, we skip the current commit.
-+
-It is safer to use a custom script outside the repository to prevent
-interactions between the bisect, make and test processes and the
-script.
-+
-"make test" should "exit 0", if the test suite passes, and
-"exit 1" otherwise.
-
 * Automatically bisect a broken test case:
 +
 ------------
 $ cat ~/test.sh
 #!/bin/sh
 make || exit 125                     # this skips broken builds
-~/check_test_case.sh                 # does the test case passes ?
+~/check_test_case.sh                 # does the test case pass?
 $ git bisect start HEAD HEAD~10 --   # culprit is among the last 10
 $ git bisect run ~/test.sh
 ------------
 +
-Here "check_test_case.sh" should "exit 0" if the test case passes,
+Here we use a "test.sh" custom script. In this script, if "make"
+fails, we skip the current commit.
+"check_test_case.sh" should "exit 0" if the test case passes,
 and "exit 1" otherwise.
 +
-It is safer if both "test.sh" and "check_test_case.sh" scripts are
+It is safer if both "test.sh" and "check_test_case.sh" are
 outside the repository to prevent interactions between the bisect,
 make and test processes and the scripts.
 
-* Automatically bisect a broken test suite:
+* Automatically bisect a broken test case:
 +
 ------------
 $ git bisect start HEAD HEAD~10 --   # culprit is among the last 10
 $ git bisect run sh -c "make || exit 125; ~/check_test_case.sh"
 ------------
 +
-Does the same as the previous example, but on a single line.
+This shows that you can do without a run script if you write the test
+on a single line.
 
 Author
 ------
-- 
1.7.4.1.404.g62d316

[PATCHv2 2/2] git-bisect.txt: example for bisecting with hot-fix

[Michael J Gruber]

Give an example on how to bisect when older revisions need a hot-fix to
build, run or test. Triggered by the binutils/kernel issue at

http://thread.gmane.org/gmane.comp.gnu.binutils/52601/focus=1112779

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
The example script is basically Junio's, with merge rather than cherry-pick.

 Documentation/git-bisect.txt |   33 +++++++++++++++++++++++++++++++++
 1 files changed, 33 insertions(+), 0 deletions(-)

diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
index 47e8b1e..989e223 100644
--- a/Documentation/git-bisect.txt
+++ b/Documentation/git-bisect.txt
@@ -294,6 +294,39 @@ It is safer if both "test.sh" and "check_test_case.sh" are
 outside the repository to prevent interactions between the bisect,
 make and test processes and the scripts.
 
+* Automatically bisect with temporary modifications (hot-fix):
++
+------------
+$ cat ~/test.sh
+#!/bin/sh
+
+# tweak the working tree by merging the hot-fix branch
+# and then attempt a build
+if	git merge --no-commit hot-fix &&
+	make
+then
+	# run project specific test and report its status
+	~/check_test_case.sh
+	status=$?
+else
+	# tell the caller this is untestable
+	status=125
+fi
+
+# undo the tweak to allow clean flipping to the next commit
+git reset --hard
+
+# return control
+exit $status
+------------
++
+This applies modifications from a hot-fix branch before each test run,
+e.g. in case your build or test environment changed so that older
+revisions may need a fix which newer ones have already. (Make sure the
+hot-fix branch is based off a commit which is contained in all revisions
+which you are bisecting, so that the merge does not pull in too much, or
+use `git cherry-pick` instead of `git merge`.)
+
 * Automatically bisect a broken test case:
 +
 ------------
-- 
1.7.4.1.404.g62d316

[PATCH] Document 'git bisect fix'.

[Ralf Wildenhues]

git bisect is sometimes less effective than it could be in projects
with long-lived but simple bugs (e.g., little-tested configurations).
Rather than skipping vast revision ranges, it might be easier to fix
them up from known bugfix branches.

'git bisect fix' teaches bisect about when some known bug was
introduced and when it was fixed, so that bisect can merge in
the fix when needed into new test candidates.
---

* Ralf Wildenhues wrote on Mon, Mar 14, 2011 at 02:16:23PM CET:
> it would be very helpful if 'git bisect' made it easy to denote
> "when going back, you might also need some of these changes".

Merging in a set of bugfix branches (branches with minimal fixes, based
right off of commits introducing some bug) before testing a particular
contender would be a good start.  Of course we don't want some bugfix
branch to be merged in if the known bug isn't yet in the current
contender, so as to not merge unrelated changes.

Cherry-pick things is another option, but the above seems a bit more
gittish to me, and works well with bugfix branches.  Also, data like
"bugzilla X was introduced by C1 and fixed by C2" is helpful (and
already available in some projects) anyway in a semi-automatic fashion.
You might even want to version it, or keep it in project meta-data.

If some bug was fixed by a merge only, the more general notation
"f_1 ^b_1 ^b_1' ..." could apply.

Here's a balloon doc patch to show what I mean.  Comments?
Is this too unlike how bisect works today?  Too dangerous?

Thanks, and please keep me in Cc:,
Ralf

 Documentation/git-bisect.txt |   20 ++++++++++++++++++++
 git-bisect.sh                |    4 +++-
 2 files changed, 23 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
index c39d957..9074cb3 100644
--- a/Documentation/git-bisect.txt
+++ b/Documentation/git-bisect.txt
@@ -25,6 +25,7 @@ on the subcommand:
  git bisect replay <logfile>
  git bisect log
  git bisect run <cmd>...
+ git bisect fix [(<range>|<rev>)...]
 
 This command uses 'git rev-list --bisect' to help drive the
 binary search process to find which change introduced a bug, given an
@@ -198,6 +199,25 @@ $ git bisect skip v2.5 v2.5..v2.6
 This tells the bisect process that the commits between `v2.5` included
 and `v2.6` included should be skipped.
 
+Fixing up known bugs
+~~~~~~~~~~~~~~~~~~~~
+
+If many revisions are broken due to some unrelated but known issue that
+is easily fixed, you might want to prefer fixing it up temporarily.
+If `<commit1>` introduces a bug fixed by `<commit2>`, instruct bisect
+to merge the latter before testing a commit that contains the former:
+
+------------
+$ git bisect fix <commit1>..<commit2>
+------------
+
+A single `<commit>` acts as if `<commit>^..<commit>` was specified.
+Fix statements can be repeated for every known bug, and are valid until
+the bisection state is cleaned up with reset.
+
+Any bisect action that causes a new commit to be chosen will try to merge
+the needed fixes and fail if they do not merge cleanly.
+
 
 Cutting down bisection by giving more parameters to bisect start
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
diff --git a/git-bisect.sh b/git-bisect.sh
index c21e33c..2b137f0 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -1,6 +1,6 @@
 #!/bin/sh
 
-USAGE='[help|start|bad|good|skip|next|reset|visualize|replay|log|run]'
+USAGE='[help|start|bad|good|skip|fix|next|reset|visualize|replay|log|run]'
 LONG_USAGE='git bisect help
         print this long help message.
 git bisect start [<bad> [<good>...]] [--] [<pathspec>...]
@@ -11,6 +11,8 @@ git bisect good [<rev>...]
         mark <rev>... known-good revisions.
 git bisect skip [(<rev>|<range>)...]
         mark <rev>... untestable revisions.
+git bisect fix [(<c1>..<c2>|<rev>)...]
+        mark descendants of <c1>/<rev^> as needing fixes <c2>/<rev>.
 git bisect next
         find next bisection to test and check it out.
 git bisect reset [<commit>]
-- 
1.7.4.1.231.ge4ce

Re: [PATCH] Document 'git bisect fix'.

[Yann Dirson]

Hi Ralf,

Maybe you should have made it more obvious that this patch is a RFC
for a proposed feature (say, in subject line).

>'git bisect fix' teaches bisect about when some known bug was
>introduced and when it was fixed, so that bisect can merge in
>the fix when needed into new test candidates.

This sounds like a great idea.  I do have myself to do conditional
cherry-picks from bisect scripts, to deal with this problem.

>If some bug was fixed by a merge only, the more general notation
>"f_1 ^b_1 ^b_1' ..." could apply.

Some more precise example may make this case more clear.


+Fixing up known bugs
+~~~~~~~~~~~~~~~~~~~~
+
+If many revisions are broken due to some unrelated but known issue that
+is easily fixed, you might want to prefer fixing it up temporarily.

It seems natural for "bisect run" to use this mechanism.  What about
the non-automated process ?  We may want to get the fixes applied, or
to only list available fixes to the user so he can "git merge" them
manually, or maybe an interactive selection mode ?  Probably something
to be chosen via some config variable and flags, in a separate patch
of the would-be series.  But what happens initially will be a good thing
to document.

+If `<commit1>` introduces a bug fixed by `<commit2>`, instruct bisect
+to merge the latter before testing a commit that contains the former:
+
+------------
+$ git bisect fix <commit1>..<commit2>
+------------

Usually, a bug also gets fixed by an official commit which does not
fulfill the constraint of being branched at the faulty one.  In this
case you don't want to merge the fix if such a fix is already included,
and thus you will need a way to specify "alternate fixes" to control
this.

+A single `<commit>` acts as if `<commit>^..<commit>` was specified.
+Fix statements can be repeated for every known bug, and are valid until
+the bisection state is cleaned up with reset.

That is on the safe side, but we may at some point want some sort of
"repository of fixes", where this info gets stored for easy reuse on
subsequent bisections.

+Any bisect action that causes a new commit to be chosen will try to merge
+the needed fixes and fail if they do not merge cleanly.

maybe "... similar to what happens when a bisect-run script terminates with exit code
greated than 127." ?

-- 
Yann Dirson - Bertin Technologies

Re: [PATCH] Document 'git bisect fix'.

[Christian Couder]

Hi,

On Mon, Mar 14, 2011 at 10:00 PM, Ralf Wildenhues
<Ralf.Wildenhues@gmx.de> wrote:
> git bisect is sometimes less effective than it could be in projects
> with long-lived but simple bugs (e.g., little-tested configurations).
> Rather than skipping vast revision ranges, it might be easier to fix
> them up from known bugfix branches.

It's already possible to deal with this problem by creating a new
branch where the bug is fixed, and then using "git replace", so that
the new branch is used instead of the old one.
Please search for "git replace" in this doc:

http://www.kernel.org/pub/software/scm/git/docs/git-bisect-lk2009.html

> 'git bisect fix' teaches bisect about when some known bug was
> introduced and when it was fixed, so that bisect can merge in
> the fix when needed into new test candidates.

Perhaps some people would find it easier to use what you suggest but
using git replace may be nicer because you have to create the new
branch once, so you need to fix merge or rebase problems only once.
And the new branch may be useful not only for bisecting, for example
to recreate old versions.

Thanks,
Christian.

Re: [PATCH] Document 'git bisect fix'.

[Michael J Gruber]

Christian Couder venit, vidit, dixit 16.03.2011 10:52:
> Hi,
> 
> On Mon, Mar 14, 2011 at 10:00 PM, Ralf Wildenhues
> <Ralf.Wildenhues@gmx.de> wrote:
>> git bisect is sometimes less effective than it could be in projects
>> with long-lived but simple bugs (e.g., little-tested configurations).
>> Rather than skipping vast revision ranges, it might be easier to fix
>> them up from known bugfix branches.
> 
> It's already possible to deal with this problem by creating a new
> branch where the bug is fixed, and then using "git replace", so that
> the new branch is used instead of the old one.
> Please search for "git replace" in this doc:
> 
> http://www.kernel.org/pub/software/scm/git/docs/git-bisect-lk2009.html
> 
>> 'git bisect fix' teaches bisect about when some known bug was
>> introduced and when it was fixed, so that bisect can merge in
>> the fix when needed into new test candidates.
> 
> Perhaps some people would find it easier to use what you suggest but
> using git replace may be nicer because you have to create the new
> branch once, so you need to fix merge or rebase problems only once.
> And the new branch may be useful not only for bisecting, for example
> to recreate old versions.

I'd say the replace method is perfect for transporting an existing fix
"back in time" when the range of non-bisectable commits is limited. But
since you have to replace the right (most recent) commit in that range
it is less convenient when you have a fix due to a changed/exotic build
environment or such which you do not want in your mainline.

Also, you have to rebase the whole history back to the commit which
introduced the problem - and that could be the root commit if the bisect
problems arise from a changed toolchain, like here.

Michael
P.S.: Did you cull cc on purpose or did gmane mess up? Readding AM, LT, TG

Re: [PATCH] Document 'git bisect fix'.

[Junio C Hamano]

Michael J Gruber <git@drmicha.warpmail.net> writes:

> Christian Couder venit, vidit, dixit 16.03.2011 10:52:
> ...
>> It's already possible to deal with this problem by creating a new
>> branch where the bug is fixed,...
>
> I'd say the replace method is perfect for transporting an existing fix
> "back in time" when the range of non-bisectable commits is limited. But
> since you have to replace the right (most recent) commit in that range
> it is less convenient when you have a fix due to a changed/exotic build
> environment or such which you do not want in your mainline.

I totally agree with Michael.  If somebody has _already_ used "replace" to
make an alternate history in which nobody made any mistake by masking each
and every bug fixed in the past, your bisect would be easier, but that is
nothing more than a theoretical daydreaming. Who in the right mind would
do that?

If you need fixes applied for unrelated bug to even trigger the bug you
are chasing, "replace" is not a practical option. You might even be the
first to notice that these "known fixes" mattered in the part of the
history you happen to be bisecting, and nobody sane would have prepared
such "replace" in the past just in case.

Treat "replace" as nothing more than a reimplementation of "grafts" done
right (i.e. can be transferred using the usual git transfer protocols); I
don't want to see its use advocated for applications it is not suited.  It
just confuses people.