{"thread":{"id":"55512","subject":"Using --term-* with bisect breaks skip","startedAt":"2021-04-18T15:15:06Z","lastAt":"2021-04-22T05:16:38Z","messageCount":11,"participants":["Trygve Aaberge","Bagas Sanjaya","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"422214","messageId":"20210418151459.GC10839@aaberge.net","threadId":"55512","inReplyTo":null,"subject":"Using --term-* with bisect breaks skip","fromName":"Trygve Aaberge","fromEmail":"trygveaa@gmail.com","sentAt":"2021-04-18T15:14:59Z","receivedAt":"2021-04-18T15:15:06Z","isPatch":false,"sender":{"key":"trygveaa@gmail.com","avatar":"https://avatars.githubusercontent.com/u/601966?v=4"},"body":"What did you do before the bug happened? (Steps to reproduce your issue)\n1. git bisect start --term-new=fixed --term-old=unfixed master HEAD~10\n2. git bisect skip\n\nWhat did you expect to happen? (Expected behavior)\nGit should mark the commit as skipped and change HEAD to a different commit.\n\nWhat happened instead? (Actual behavior)\nThe commit was marked as skipped, but HEAD was not changed.\n\nWhat's different between what you expected and what actually happened?\nAfter running bisect skip, HEAD was still at the same commit as before,\ninstead of having changed to a new that I can test. The usual output about\nsteps left to test and the new commit was also missing, skip did not output\nanything.\n\nAnything else you want to add:\n- If I don't provide any --term-* options, skip works as expected.\n- The revisions provided in the reproduction steps doesn't matter, they're\n just an example.\n- I tried running from the next branch, and it happened there too.\n\n[System Info]\ngit version:\ngit version 2.31.1\ncpu: x86_64\nno commit associated with this build\nsizeof-long: 8\nsizeof-size_t: 8\nshell-path: /bin/sh\nuname: Linux 5.10.29-1-lts #1 SMP Sat, 10 Apr 2021 14:40:41 +0000 x86_64\ncompiler info: gnuc: 10.2\nlibc info: glibc: 2.33\n$SHELL (typically, interactive shell): /usr/bin/zsh\n\n\n[Enabled Hooks]\n\n\n-- \nTrygve Aaberge\n"},{"id":"422233","messageId":"c29ef929-4744-a498-540d-1b7df0afa0a0@gmail.com","threadId":"55512","inReplyTo":"20210418151459.GC10839@aaberge.net","subject":"Re: Using --term-* with bisect breaks skip","fromName":"Bagas Sanjaya","fromEmail":"bagasdotme@gmail.com","sentAt":"2021-04-19T06:58:20Z","receivedAt":"2021-04-19T06:58:28Z","isPatch":false,"sender":{"key":"bagasdotme@gmail.com","avatar":"https://avatars.githubusercontent.com/u/40219486?v=4"},"body":"On 18/04/21 22.14, Trygve Aaberge wrote:\n> What's different between what you expected and what actually happened?\n> After running bisect skip, HEAD was still at the same commit as before,\n> instead of having changed to a new that I can test. The usual output about\n> steps left to test and the new commit was also missing, skip did not output\n> anything.\nI can reproduce the issue, thanks.\n> Anything else you want to add:\n> - If I don't provide any --term-* options, skip works as expected.\nThe issue still persists without --term-* options on my computer.\n\nTo reproduce in git.git:\n 1. git bisect start\n 2. git bisect new v2.31.0\n 3. git bisect old v2.30.0\n 4. git bisect skip\n\n[CC] I'd CCed Miriam and Pranit (who submitted git-bisect in C) and Jeff\nfor the issue.\n\n-- \nAn old man doll... just what I always wanted! - Clara\n"},{"id":"422235","messageId":"20210419083926.GD10839@aaberge.net","threadId":"55512","inReplyTo":"c29ef929-4744-a498-540d-1b7df0afa0a0@gmail.com","subject":"Re: Using --term-* with bisect breaks skip","fromName":"Trygve Aaberge","fromEmail":"trygveaa@gmail.com","sentAt":"2021-04-19T08:39:26Z","receivedAt":"2021-04-19T08:39:34Z","isPatch":false,"sender":{"key":"trygveaa@gmail.com","avatar":"https://avatars.githubusercontent.com/u/601966?v=4"},"body":"On Mon, Apr 19, 2021 at 13:58:20 +0700, Bagas Sanjaya wrote:\n> The issue still persists without --term-* options on my computer.\n> \n> To reproduce in git.git:\n> 1. git bisect start\n> 2. git bisect new v2.31.0\n> 3. git bisect old v2.30.0\n> 4. git bisect skip\n\nRight, the issue persists with those commands here too. So it apparently\nhappens as long as you don't use good/bad, but with good/bad it works as\nexpected.\n\n-- \nTrygve Aaberge\n"},{"id":"422310","messageId":"f5368d12-81d9-8591-53f3-44e2e31a89dd@gmail.com","threadId":"55512","inReplyTo":"20210419083926.GD10839@aaberge.net","subject":"Re: Using --term-* with bisect breaks skip","fromName":"Bagas Sanjaya","fromEmail":"bagasdotme@gmail.com","sentAt":"2021-04-19T12:50:07Z","receivedAt":"2021-04-19T12:50:14Z","isPatch":false,"sender":{"key":"bagasdotme@gmail.com","avatar":"https://avatars.githubusercontent.com/u/40219486?v=4"},"body":"On 19/04/21 15.39, Trygve Aaberge wrote:\n> Right, the issue persists with those commands here too. So it apparently\n> happens as long as you don't use good/bad, but with good/bad it works as\n> expected.\n> \nSeems interesting, git bisect skip only works on good/bad terms but not on\nother terms (and not even on old/new)\n\nI saw similar test on t/t6030-bisect-porcelain.sh (hint: find test\nthat test for skip) , but I don't know whether that test will pass.\nI CC'ed Christian Couder for now.\n\n-- \nAn old man doll... just what I always wanted! - Clara\n"},{"id":"422321","messageId":"xmqqlf9e9k9d.fsf@gitster.g","threadId":"55512","inReplyTo":"20210418151459.GC10839@aaberge.net","subject":"Re: Using --term-* with bisect breaks skip","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-04-19T18:55:42Z","receivedAt":"2021-04-19T18:55:46Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Trygve Aaberge writes:\n\n> What did you do before the bug happened? (Steps to reproduce your issue)\n> 1. git bisect start --term-new=fixed --term-old=unfixed master HEAD~10\n> 2. git bisect skip\n>\n> What did you expect to happen? (Expected behavior)\n> Git should mark the commit as skipped and change HEAD to a different commit.\n>\n> What happened instead? (Actual behavior)\n> The commit was marked as skipped, but HEAD was not changed.\n>\n> What's different between what you expected and what actually happened?\n> After running bisect skip, HEAD was still at the same commit as before,\n> instead of having changed to a new that I can test. The usual output about\n> steps left to test and the new commit was also missing, skip did not output\n> anything.\n>\n> Anything else you want to add:\n> - If I don't provide any --term-* options, skip works as expected.\n> - The revisions provided in the reproduction steps doesn't matter, they're\n> just an example.\n> - I tried running from the next branch, and it happened there too.\n>\n> [System Info]\n> git version:\n> git version 2.31.1\n\nThanks for a report.\n\nI suspect that this is a fairly recent regression. With Git 2.30.2,\n\"bisect skip\" does flip HEAD and mark the commit to as untestable.\n\nCan you \"bisect\" the problem? There aren't that many commits that\ntouched bisection code during the period.\n\n$ git log --format='%aN %s' --no-merges v2.30.2..v2.31.1 -- \\\n bisect.c builtin/bisect--helper.c git-bisect.sh\nJeff King bisect: peel annotated tags to commits\nRené Scharfe use CALLOC_ARRAY\nJohannes Sixt replace \"parameters\" by \"arguments\" in error messages\nPranit Bauva bisect--helper: retire `--check-and-set-terms` subcommand\nPranit Bauva bisect--helper: reimplement `bisect_skip` shell function in C\nPranit Bauva bisect--helper: retire `--bisect-auto-next` subcommand\nPranit Bauva bisect--helper: use `res` instead of return in BISECT_RESET case option\nPranit Bauva bisect--helper: retire `--bisect-write` subcommand\nPranit Bauva bisect--helper: reimplement `bisect_replay` shell function in C\nPranit Bauva bisect--helper: reimplement `bisect_log` shell function in C\nMartin Ågren hash-lookup: rename from sha1-lookup\n\n"},{"id":"422326","messageId":"20210419193203.GE10839@aaberge.net","threadId":"55512","inReplyTo":"xmqqlf9e9k9d.fsf@gitster.g","subject":"Re: Using --term-* with bisect breaks skip","fromName":"Trygve Aaberge","fromEmail":"trygveaa@gmail.com","sentAt":"2021-04-19T19:32:03Z","receivedAt":"2021-04-19T19:32:10Z","isPatch":false,"sender":{"key":"trygveaa@gmail.com","avatar":"https://avatars.githubusercontent.com/u/601966?v=4"},"body":"On Mon, Apr 19, 2021 at 11:55:42 -0700, Junio C Hamano wrote:\n> Can you \"bisect\" the problem? There aren't that many commits that\n> touched bisection code during the period.\n\nYes, I ran it now. The commit introducing the regression is:\n\ncommit e4c7b33747dccc6600bc528b51e91c11b474eb04\nAuthor: Pranit Bauva \nDate: Wed Feb 3 22:54:37 2021 +0100\n\n bisect--helper: reimplement `bisect_skip` shell function in C\n \n Reimplement the `bisect_skip()` shell function in C and also add\n `bisect-skip` subcommand to `git bisect--helper` to call it from\n git-bisect.sh\n \n Using `--bisect-skip` subcommand is a temporary measure to port shell\n function to C so as to use the existing test suite.\n \n Mentored-by: Lars Schneider \n Mentored-by: Christian Couder \n Mentored-by: Johannes Schindelin \n Signed-off-by: Pranit Bauva \n Signed-off-by: Tanushree Tumane \n Signed-off-by: Miriam Rubio \n Signed-off-by: Junio C Hamano \n\n-- \nTrygve Aaberge\n"},{"id":"422404","messageId":"20210420123435.35936-1-bagasdotme@gmail.com","threadId":"55512","inReplyTo":"20210418151459.GC10839@aaberge.net","subject":"[PATCH] test: add test for git bisect skip with --term* arguments","fromName":"Bagas Sanjaya","fromEmail":"bagasdotme@gmail.com","sentAt":"2021-04-20T12:34:36Z","receivedAt":"2021-04-20T12:35:18Z","isPatch":true,"sender":{"key":"bagasdotme@gmail.com","avatar":"https://avatars.githubusercontent.com/u/40219486?v=4"},"body":"The current git bisect breakage happens when skipping in the middle of\nbisect session which is started with --term-new and --term-old\narguments. This test expect that HEAD changes after skipping, in other\nwords HEAD before and after skipping must be different.\n\nSigned-off-by: Bagas Sanjaya \n---\nNOTE: this patch is not intended for integrating into git.git, but\nrather this patch is written to demonstrate this breakage. I hope that\nthe test can be added to t6030-bisect-porcelain.sh, and make this patch\nredundant. \n\n t/t6031-bisect-skip.sh | 36 ++++++++++++++++++++++++++++++++++++\n 1 file changed, 36 insertions(+)\n create mode 100755 t/t6031-bisect-skip.sh\n\ndiff --git a/t/t6031-bisect-skip.sh b/t/t6031-bisect-skip.sh\nnew file mode 100755\nindex 0000000000..0dfc6f0928\n--- /dev/null\n+++ b/t/t6031-bisect-skip.sh\n@@ -0,0 +1,36 @@\n+#!/bin/sh\n+\n+#\n+# Copyright (c) 2021 Bagas Sanjaya\n+#\n+\n+test_description='Tests git bisect --skip'\n+\n+exec > test &&\n+\t\tgit add test && git commit -m $i\n+\tdone\n+}\n+\n+init\n+\n+test_expect_success 'test moving HEAD when skip bisecting' '\n+\tgit bisect start --term-new=ok --term-old=whoops HEAD HEAD~9 &&\n+\tHASH_SKIPPED_FROM=$(git rev-parse --verify HEAD) &&\n+\tgit bisect skip &&\n+\tHASH_SKIPPED_TO=$(git rev-parse --verify HEAD) &&\n+\ttest $HASH_SKIPPED_FROM != $HASH_SKIPPED_TO\n+'\n-- \n2.25.1\n\n"},{"id":"422457","messageId":"xmqqa6ps7rrc.fsf@gitster.g","threadId":"55512","inReplyTo":"20210420123435.35936-1-bagasdotme@gmail.com","subject":"Re: [PATCH] test: add test for git bisect skip with --term* arguments","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-04-20T18:08:55Z","receivedAt":"2021-04-20T18:08:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Bagas Sanjaya writes:\n\n> NOTE: this patch is not intended for integrating into git.git, but\n> rather this patch is written to demonstrate this breakage. I hope that\n> the test can be added to t6030-bisect-porcelain.sh, and make this patch\n> redundant. \n\nThen perhaps add it to where you think it belongs to before you\npost?\n\nIn any case, let's critique the patch a bit for future reference,\nwith a hope that you'd be contributing more ;-).\n\n> t/t6031-bisect-skip.sh | 36 ++++++++++++++++++++++++++++++++++++\n> 1 file changed, 36 insertions(+)\n> create mode 100755 t/t6031-bisect-skip.sh\n\nGood; many new people forget to make these executable.\n\n> +test_description='Tests git bisect --skip'\n> +\n> +exec +GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n> +export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n> +\n> +. ./test-lib.sh\n> +\n> +# hash variables\n> +HASH_SKIPPED_FROM=\n> +HASH_SKIPPED_TO=\n> +\n> +# initialize testing repo\n> +init() {\n\nStyle.\n\n> +\tfor i in `seq 1 20`; do\n\nUse $(test_seq ...)\n\n> +\t\techo $i >> test &&\n\nStyle\n\n> +\t\tgit add test && git commit -m $i\n> +\tdone\n> +}\n> +\n> +init\n\nWe do that in the first \"test_expect_success setup '... code ...'\" block\nto catch breakages in the initialization.\n\n> +test_expect_success 'test moving HEAD when skip bisecting' '\n> +\tgit bisect start --term-new=ok --term-old=whoops HEAD HEAD~9 &&\n> +\tHASH_SKIPPED_FROM=$(git rev-parse --verify HEAD) &&\n> +\tgit bisect skip &&\n> +\tHASH_SKIPPED_TO=$(git rev-parse --verify HEAD) &&\n> +\ttest $HASH_SKIPPED_FROM != $HASH_SKIPPED_TO\n> +'\n\nMissing \"test_done\".\n\nThanks.\n"},{"id":"422524","messageId":"20210421040808.14185-1-bagasdotme@gmail.com","threadId":"55512","inReplyTo":"20210418151459.GC10839@aaberge.net","subject":"[PATCH v2] test: add test for git bisect skip with --term* arguments","fromName":"Bagas Sanjaya","fromEmail":"bagasdotme@gmail.com","sentAt":"2021-04-21T04:08:10Z","receivedAt":"2021-04-21T04:14:59Z","isPatch":true,"sender":{"key":"bagasdotme@gmail.com","avatar":"https://avatars.githubusercontent.com/u/40219486?v=4"},"body":"Trygve Aaberge reported the current git bisect breakage on [1].\nAfter starting bisection with --term-new and --term-old arguments to git\nbisect start, skipping with git bisect skip does not change HEAD as\nexpected.\n\nLet's add the test to catch this breakage.\n\n[1]: https://lore.kernel.org/git/20210418151459.GC10839@aaberge.net/\n\nSigned-off-by: Bagas Sanjaya \n---\n\nChanges from v1:\n * style changes requested by Junio\n * rename test script and edit test description to be more descriptive\n * remove exec >test &&\n+\t\tgit add test && git commit -m \"commit $i\" &&\n+\t\ttest_tick\n+\tdone\n+'\n+\n+# actual bisection test\n+test_expect_success 'test moving HEAD when skip bisecting' '\n+\tgit bisect start --term-new=ok --term-old=whoops HEAD HEAD~9 &&\n+\tHASH_SKIPPED_FROM=$(git rev-parse --verify HEAD) &&\n+\tgit bisect skip &&\n+\tHASH_SKIPPED_TO=$(git rev-parse --verify HEAD) &&\n+\ttest $HASH_SKIPPED_FROM != $HASH_SKIPPED_TO\n+'\n+\n+test_done\n\nbase-commit: b0c09ab8796fb736efa432b8e817334f3e5ee75a\n-- \n2.25.1\n\n"},{"id":"422605","messageId":"xmqqa6przh08.fsf@gitster.g","threadId":"55512","inReplyTo":"20210421040808.14185-1-bagasdotme@gmail.com","subject":"Re: [PATCH v2] test: add test for git bisect skip with --term* arguments","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-04-21T17:25:59Z","receivedAt":"2021-04-21T17:26:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Bagas Sanjaya writes:\n\n> Trygve Aaberge reported the current git bisect breakage on [1].\n> After starting bisection with --term-new and --term-old arguments to git\n> bisect start, skipping with git bisect skip does not change HEAD as\n> expected.\n>\n> Let's add the test to catch this breakage.\n\nI expected, as you said in your first version, that it would be\nadded as part of an existing test script for \"git bisect\". And I\nsuspect that you can reuse the history the existing tests in the\nscript already use, so you won't have to add the first \"initialize\"\npiece. If the tested sequence should work but does not yet work due\nto lack of a fix to a known bug, the test should be marked as\ntest_expect_failure instead of test_expect_success.\n\n> * style changes requested by Junio\n\nHeh, I didn't request anything. I merely pointed out the parts that\nviolate the style guide, so if anything, the guide requested you and\nI was just a messenger ;-).\n\nThanks.\n"},{"id":"422665","messageId":"3bbc9350-60cd-9fb2-117a-add3dc5cf77e@gmail.com","threadId":"55512","inReplyTo":"xmqqa6przh08.fsf@gitster.g","subject":"Re: [PATCH v2] test: add test for git bisect skip with --term* arguments","fromName":"Bagas Sanjaya","fromEmail":"bagasdotme@gmail.com","sentAt":"2021-04-22T05:16:30Z","receivedAt":"2021-04-22T05:16:38Z","isPatch":true,"sender":{"key":"bagasdotme@gmail.com","avatar":"https://avatars.githubusercontent.com/u/40219486?v=4"},"body":"On 22/04/21 00.25, Junio C Hamano wrote:\n> I expected, as you said in your first version, that it would be\n> added as part of an existing test script for \"git bisect\". And I\n> suspect that you can reuse the history the existing tests in the\n> script already use, so you won't have to add the first \"initialize\"\n> piece. If the tested sequence should work but does not yet work due\n> to lack of a fix to a known bug, the test should be marked as\n> test_expect_failure instead of test_expect_success.\n\nSo will this patch be queued? Or should I send the patch adding this\ntest to t6030 instead of separate test script here?\n\n-- \nAn old man doll... just what I always wanted! - Clara\n"}]}