Commit e4c7b33747 ("bisect--helper: reimplement `bisect_skip` shell
function in C", 2021-02-03), as part of the shell-to-C conversion,
forgot to read the 'terms' file (.git/BISECT_TERMS) during the new
'bisect skip' command implementation. As a result, the 'bisect skip'
command will use the default 'bad'/'good' terms. If the bisection
terms have been set to non-default values (for example by the
'bisect start' command), then the 'bisect skip' command will fail.
In order to correct this problem, we insert a call to the get_terms()
function, which reads the non-default terms from that file (if set),
in the '--bisect-skip' command implementation of 'bisect--helper'.
Also, add a test[1] to protect against potential future regression.
[1] https://lore.kernel.org/git/xmqqim45h585.fsf@gitster.g/T/#m207791568054b0f8cf1a3942878ea36293273c7d
Reported-by: Trygve Aaberge <trygveaa@xxxxxxxxx>
Signed-off-by: Bagas Sanjaya <bagasdotme@xxxxxxxxx>
Signed-off-by: Ramsay Jones <ramsay@xxxxxxxxxxxxxxxxxxxx>
---
Hi Junio,
This v2 patch was also created directly on top of commit e4c7b33747 and,
once again, applied to current 'master' (@311531c9de). (I was a little
surprised that it applied clean, given the changes to t6030-*.sh in the
meantime! ;).
I didn't bother with the range-diff[*], since it wasn't very illuminating
(just the addition of a few trailing lines in the commit message and the
addition of the test).
I don't know if I did the 'Co-authorship' bit correctly; I nearly used an
'Co-authored-by:' rather than an 'Signed-off-by:' for Bagas, but I seem to
remember that caused some push-back recently. Anyway, if I messed that up,
please just modify however you see fit.
ATB,
Ramsay Jones
[*] Once I got it to work, that is; for some reason, I was trying smaller
creation-factor values rather than larger! ;-)
builtin/bisect--helper.c | 1 +
t/t6030-bisect-porcelain.sh | 11 +++++++++++
2 files changed, 12 insertions(+)
diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 7ad9b4d55b..49c07f0710 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -1129,6 +1129,7 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
break;
case BISECT_SKIP:
set_terms(&terms, "bad", "good");
+ get_terms(&terms);
res = bisect_skip(&terms, argv, argc);
break;
default:
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index aa226381be..b707296ae2 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -919,6 +919,17 @@ test_expect_success 'bisect start takes options and revs in any order' '
test_cmp expected actual
'
+# Bisect is started with --term-new and --term-old arguments,
+# then skip. The HEAD should be changed.
+test_expect_success 'bisect skip works with --term*' '
+ git bisect reset &&
+ git bisect start --term-new=fixed --term-old=unfixed HEAD $HASH1 &&
+ hash_skipped_from=$(git rev-parse --verify HEAD) &&
+ git bisect skip &&
+ hash_skipped_to=$(git rev-parse --verify HEAD) &&
+ test "$hash_skipped_from" != "$hash_skipped_to"
+'
+
test_expect_success 'git bisect reset cleans bisection state properly' '
git bisect reset &&
git bisect start &&
--
2.31.0
