Christian Couder <chriscool@xxxxxxxxxxxxx> 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?
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.
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?
I think it also needs to call unapply_fixup when "bisect reset" is given
to clean up, but this is more about a possible approach and not about a
adding a polished and finished shiny new toy, so...
-- >8 --
bisect: use canned "fixup patch"
This allows you to have $GIT_DIR/bisect-fixup directory, populated with
"fixup" patch files. When revisions between A and B (inclusive) cannot be
tested for your bisection purpose because they have other unrelated
breakage whose fix is already known, you can store a fix-up patch in the
file whose name is A-B (the commit object name of A, a dash, then the
commit object name of B). When "git bisect" suggests you to test a
revision that falls in that range, the patch will automatically be
applied to the working tree, so that you can test with the known fix
already in place.
Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
---
git-bisect.sh | 36 ++++++++++++++++++++++++++
t/t6035-bisect-fixup.sh | 64 +++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 100 insertions(+), 0 deletions(-)
diff --git c/git-bisect.sh w/git-bisect.sh
index 0d0e278..600d9aa 100755
--- c/git-bisect.sh
+++ w/git-bisect.sh
@@ -338,13 +338,48 @@ exit_if_skipped_commits () {
fi
}
+apply_fixup() {
+ >"$GIT_DIR/BISECT_FIXUP"
+ (
+ cd "$GIT_DIR" 2>/dev/null &&
+ find bisect-fixup -type f -print
+ ) |
+ while read _fixup
+ do
+ _btm=$(expr "$_fixup" : 'bisect-fixup/\([0-9a-f]*\)-[0-9a-f]*$') &&
+ _btm=$(git rev-parse --quiet --verify "$_btm" 2>/dev/null) &&
+ _top=$(expr "$_fixup" : 'bisect-fixup/[0-9a-f]*-\([0-9a-f]*\)$') &&
+ _top=$(git rev-parse --quiet --verify "$_top" 2>/dev/null) ||
+ continue
+ if test "z$(git merge-base $_btm "$1")" = z$_btm &&
+ test "z$(git merge-base "$1" $_top)" = "z$1"
+ then
+ echo "Applying $_fixup..."
+ git apply "$GIT_DIR/$_fixup" || exit 1
+ echo >>"$GIT_DIR/BISECT_FIXUP" "$_fixup"
+ fi
+ done
+}
+
+unapply_fixup() {
+ test -r "$GIT_DIR/BISECT_FIXUP" || return 0
+ @@PERL@@ -e "print reverse <>" "$GIT_DIR/BISECT_FIXUP" |
+ while read _fixup
+ do
+ echo "Unapplying $_fixup..."
+ git apply -R "$GIT_DIR/$_fixup" || exit
+ done
+}
+
bisect_checkout() {
+ unapply_fixup || exit ;# should not happen but try to be careful
_rev="$1"
_msg="$2"
echo "Bisecting: $_msg"
mark_expected_rev "$_rev"
git checkout -q "$_rev" || exit
git show-branch "$_rev"
+ apply_fixup "$_rev" || exit
}
is_among() {
@@ -532,6 +567,7 @@ bisect_clean_state() {
do
git update-ref -d $ref $hash || exit
done
+ rm -f "$GIT_DIR/BISECT_FIXUP" &&
rm -f "$GIT_DIR/BISECT_EXPECTED_REV" &&
rm -f "$GIT_DIR/BISECT_ANCESTORS_OK" &&
rm -f "$GIT_DIR/BISECT_LOG" &&
diff --git c/t/t6035-bisect-fixup.sh w/t/t6035-bisect-fixup.sh
new file mode 100755
index 0000000..4745b73
--- /dev/null
+++ w/t/t6035-bisect-fixup.sh
@@ -0,0 +1,64 @@
+#!/bin/sh
+# Copyright (c) 2008, Junio C Hamano
+
+test_description='bisect fixup'
+
+. ./test-lib.sh
+
+cat >test-fixup <<\EOF
+diff a/elif b/elif
+--- a/elif
++++ b/elif
+@@ -1 +1 @@
+-0
++Z
+EOF
+
+test_expect_success setup '
+ echo 0 >elif
+ git add elif
+ for i in 1 2 3 4 5 6 7 8 9
+ do
+ test_tick &&
+ echo $i >file &&
+ git add file &&
+ git commit -m $i &&
+ git tag t$i
+ done &&
+
+ mkdir ".git/bisect-fixup" &&
+
+ btm=$(git rev-parse --short t3) &&
+ top=$(git rev-parse --short t5) &&
+ cat test-fixup >".git/bisect-fixup/$btm-$top" &&
+
+ btm=$(git rev-parse --short t7) &&
+ top=$(git rev-parse --short t8) &&
+ sed -e "s|Z|A|" test-fixup >".git/bisect-fixup/$btm-$top"
+'
+
+test_expect_success 'bisect fixup' '
+ t1=$(git rev-parse t1) &&
+ t5=$(git rev-parse t5) &&
+ t6=$(git rev-parse t6) &&
+ t7=$(git rev-parse t7) &&
+ t9=$(git rev-parse t9) &&
+
+ git bisect start &&
+ test $(git rev-parse HEAD) = $t9 &&
+ git bisect bad t9 &&
+ test $(git rev-parse HEAD) = $t9 &&
+ git bisect good t1 &&
+ test $(git rev-parse HEAD) = $t5 &&
+ grep Z elif &&
+
+ git bisect good &&
+ test $(git rev-parse HEAD) = $t7 &&
+ grep A elif &&
+
+ git bisect bad &&
+ test $(git rev-parse HEAD) = $t6 &&
+ grep 0 elif
+'
+
+test_done
--
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
