{"thread":{"id":"61193","subject":"'git bisect run' not fully automatic when a merge base must be tested","startedAt":"2024-03-24T16:57:48Z","lastAt":"2024-03-24T21:42:17Z","messageCount":3,"participants":["Philippe Blain","Christian Couder","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"491363","messageId":"6ee4b8d8-5acb-3d3c-28e0-be972945e8d7@gmail.com","threadId":"61193","inReplyTo":null,"subject":"'git bisect run' not fully automatic when a merge base must be tested","fromName":"Philippe Blain","fromEmail":"levraiphilippeblain@gmail.com","sentAt":"2024-03-24T16:57:45Z","receivedAt":"2024-03-24T16:57:48Z","isPatch":false,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"Hello,\n\nI have encountered a situation where 'git bisect run' is not fully automatic,\ni.e. it stops before finding the first bad commit with the message:\n\"a merge base must be tested\".\n\nIs there a reason why it could not run the provided script \non the merge-base that it checks out ?\n\nFor context (and a reproducer), I was bisecting the \n'git commit --verbose --trailer' regression I reported in [1].\n\nI was running the bisection on a machine without curl installed, and so I \nwas building with 'NO_CURL' and thus needed to cherry-pick my 30bced3a67 \n(imap-send: add missing \"strbuf.h\" include under NO_CURL, 2024-02-02) at most steps\nof the bisection. So I was running this command:\n\ngit bisect reset && git bisect start &&\ngit bisect bad v2.43.2 && git bisect good v2.42.1 &&\ngit bisect run ~/bisect-git.sh\n\nwith this script to drive the bisection:\n\n=== ~/bisect-git.sh ===\n#!/bin/bash\n\ngit cherry-pick --allow-empty 30bced3a67\nif   make -j NO_CURL=1\nthen\n    # run project specific test and report its status\n    ~/bisect-trailers.sh\n    status=$?\nelse\n    # tell the caller this is untestable\n    status=125\nfi\n\n# return control\nexit $status\n==== end of ~/bisect-git.sh ===\n\nand this one to reproduce the regression:\n\n=== ~/bisect-trailer.sh ===\nset -e\n\nexport PATH=\"/path/to/git/bin-wrappers/:$PATH\"\n\nrepo=${TMPDIR:-/tmp}/test-trailers\nrm -rf \"$repo\" \n\nunset $(git rev-parse --local-env-vars)\n\ngit init \"$repo\"\ncd \"$repo\"\ndate>file\ngit add file\nexport GIT_EDITOR='cat file'\ngit commit --verbose -em \"file\" --trailer=\"key: value\" > /dev/null\ngit show | \\grep -q value\n=== end of ~/bisect-trailer.sh ===\n\nThis results in the bisection stopping at:\n\n\tHEAD is now at 4a14ccd05d imap-send: add missing \"strbuf.h\" include under NO_CURL\n\tBisecting: a merge base must be tested\n\t[d57c671a511d885a5cd390e3d6064c37af524a91] treewide: remove unnecessary includes in source files\n\tbisect run success\n\nwith the following bisect log:\n\ngit bisect start\n# status: waiting for both good and bad commits\n# bad: [efb050becb6bc703f76382e1f1b6273100e6ace3] Git 2.43.2\ngit bisect bad efb050becb6bc703f76382e1f1b6273100e6ace3\n# status: waiting for good commit(s), bad commit known\n# good: [61a22ddaf0626111193a17ac12f366bd6d167dff] Git 2.42.1\ngit bisect good 61a22ddaf0626111193a17ac12f366bd6d167dff\n# good: [5edbcead426056b54286499149244ae4cbf8b5f7] Merge branch 'bc/racy-4gb-files'\ngit bisect good 5edbcead426056b54286499149244ae4cbf8b5f7\n# good: [5baedc68b02c1b43b307d436edac702ac3e7b89d] Merge branch 'jk/bisect-reset-fix' into maint-2.43\ngit bisect good 5baedc68b02c1b43b307d436edac702ac3e7b89d\n# good: [2873a9686cf59ecbf851cea8c41e6ee545195423] Merge branch 'rs/rebase-use-strvec-pushf' into maint-2.43\ngit bisect good 2873a9686cf59ecbf851cea8c41e6ee545195423\n# bad: [03bc5976514f706889fceea623f35133014ebe78] imap-send: add missing \"strbuf.h\" include under NO_CURL\ngit bisect bad 03bc5976514f706889fceea623f35133014ebe78\n# bad: [9ae3c6dceb187af1ae09649dc5c61bb05a7013d9] imap-send: add missing \"strbuf.h\" include under NO_CURL\ngit bisect bad 9ae3c6dceb187af1ae09649dc5c61bb05a7013d9\n# good: [007488839cabbb5bc6777924ae03c4edeb1b6110] imap-send: add missing \"strbuf.h\" include under NO_CURL\ngit bisect good 007488839cabbb5bc6777924ae03c4edeb1b6110\n\nThis was the first time that I did a bisection with 'git bisect run'\nthat was not fully automated so it threw me off a bit.\n\nI tried it again today and realized that if I modify my ~/bisect-git.sh\nto use 'git cherry-pick --no-commit --allow-empty' instead, then the\nbisection runs to completion and finds the bad commit. So my understanding\nis that cherry-picking a commit during the bisection (without --no-commit)\nalters the commit graph and might not be the best idea... Looking at the man\npage the example does use 'git merge --no-commit' so I should have known better.\n\nBut the question remains: in general shouldn't 'git bisect run' do everything\nautomatically and not require the user to do something manually ?\n\n\nAnd a side note to finish:\nInitially I did not use 'unset $(git rev-parse --local-env-vars)' in\nmy ~/bisect-trailers.sh, and I was running the bisection in a secondary\nworktree of my clone of git.git. This did not work at all since the\n'git init' and 'git commit --verbose -em \"file\" --trailer=\"key: value\"'\nsteps were running _in my secondary worktree_ instead of in $TMPDIR,\nbecause of how the repository detection works in worktrees and\nexports some variables (I think...) This was also very confusing\nand I'm wondering if we should add a note somewhere about that \nin the documentation, although it seems specific to bisecting git.git...\n\nCheers,\nPhilippe\n\n[1] https://lore.kernel.org/git/8b4738ad-62cd-789e-712e-bd45a151b4ac@gmail.com/\n"},{"id":"491372","messageId":"CAP8UFD2dNix6OMzCR5Ao8YLD-OVbcOfZOP90G7-QRx=OtRAoYw@mail.gmail.com","threadId":"61193","inReplyTo":"6ee4b8d8-5acb-3d3c-28e0-be972945e8d7@gmail.com","subject":"Re: 'git bisect run' not fully automatic when a merge base must be tested","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2024-03-24T17:57:48Z","receivedAt":"2024-03-24T17:58:02Z","isPatch":false,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Sun, Mar 24, 2024 at 5:57 PM Philippe Blain\n<levraiphilippeblain@gmail.com> wrote:\n>\n> Hello,\n>\n> I have encountered a situation where 'git bisect run' is not fully automatic,\n> i.e. it stops before finding the first bad commit with the message:\n> \"a merge base must be tested\".\n>\n> Is there a reason why it could not run the provided script\n> on the merge-base that it checks out ?\n\nThe git-bisect-lk2009.txt file has a \"Checking merge bases\" section\nthat has some explanations about why merge bases must sometimes be\ntested and why a bisection should stop if a merge base is \"bad\".\n\nNow I am not sure how `git bisect run` behaves in practice when a\nmerge base should be tested. It's possible it never actually worked,\nor perhaps there was no test case in our tests checking this and it\nstopped working at one point.\n\n> For context (and a reproducer), I was bisecting the\n> 'git commit --verbose --trailer' regression I reported in [1].\n>\n> I was running the bisection on a machine without curl installed, and so I\n> was building with 'NO_CURL' and thus needed to cherry-pick my 30bced3a67\n> (imap-send: add missing \"strbuf.h\" include under NO_CURL, 2024-02-02) at most steps\n> of the bisection. So I was running this command:\n>\n> git bisect reset && git bisect start &&\n> git bisect bad v2.43.2 && git bisect good v2.42.1 &&\n> git bisect run ~/bisect-git.sh\n>\n> with this script to drive the bisection:\n>\n> === ~/bisect-git.sh ===\n> #!/bin/bash\n>\n> git cherry-pick --allow-empty 30bced3a67\n\nIt might not be a good idea to change the current branch when a\nbisection happens. I think it would be better to apply a patch using\n`git apply` before the tests and to remove it using `git apply -R`\nafter the tests.\n\n> if   make -j NO_CURL=1\n> then\n>     # run project specific test and report its status\n>     ~/bisect-trailers.sh\n>     status=$?\n> else\n>     # tell the caller this is untestable\n>     status=125\n> fi\n>\n> # return control\n> exit $status\n> ==== end of ~/bisect-git.sh ===\n>\n> and this one to reproduce the regression:\n>\n> === ~/bisect-trailer.sh ===\n> set -e\n>\n> export PATH=\"/path/to/git/bin-wrappers/:$PATH\"\n>\n> repo=${TMPDIR:-/tmp}/test-trailers\n> rm -rf \"$repo\"\n>\n> unset $(git rev-parse --local-env-vars)\n>\n> git init \"$repo\"\n> cd \"$repo\"\n> date>file\n> git add file\n> export GIT_EDITOR='cat file'\n> git commit --verbose -em \"file\" --trailer=\"key: value\" > /dev/null\n> git show | \\grep -q value\n> === end of ~/bisect-trailer.sh ===\n>\n> This results in the bisection stopping at:\n>\n>         HEAD is now at 4a14ccd05d imap-send: add missing \"strbuf.h\" include under NO_CURL\n>         Bisecting: a merge base must be tested\n>         [d57c671a511d885a5cd390e3d6064c37af524a91] treewide: remove unnecessary includes in source files\n>         bisect run success\n>\n> with the following bisect log:\n>\n> git bisect start\n> # status: waiting for both good and bad commits\n> # bad: [efb050becb6bc703f76382e1f1b6273100e6ace3] Git 2.43.2\n> git bisect bad efb050becb6bc703f76382e1f1b6273100e6ace3\n> # status: waiting for good commit(s), bad commit known\n> # good: [61a22ddaf0626111193a17ac12f366bd6d167dff] Git 2.42.1\n> git bisect good 61a22ddaf0626111193a17ac12f366bd6d167dff\n> # good: [5edbcead426056b54286499149244ae4cbf8b5f7] Merge branch 'bc/racy-4gb-files'\n> git bisect good 5edbcead426056b54286499149244ae4cbf8b5f7\n> # good: [5baedc68b02c1b43b307d436edac702ac3e7b89d] Merge branch 'jk/bisect-reset-fix' into maint-2.43\n> git bisect good 5baedc68b02c1b43b307d436edac702ac3e7b89d\n> # good: [2873a9686cf59ecbf851cea8c41e6ee545195423] Merge branch 'rs/rebase-use-strvec-pushf' into maint-2.43\n> git bisect good 2873a9686cf59ecbf851cea8c41e6ee545195423\n> # bad: [03bc5976514f706889fceea623f35133014ebe78] imap-send: add missing \"strbuf.h\" include under NO_CURL\n> git bisect bad 03bc5976514f706889fceea623f35133014ebe78\n> # bad: [9ae3c6dceb187af1ae09649dc5c61bb05a7013d9] imap-send: add missing \"strbuf.h\" include under NO_CURL\n> git bisect bad 9ae3c6dceb187af1ae09649dc5c61bb05a7013d9\n> # good: [007488839cabbb5bc6777924ae03c4edeb1b6110] imap-send: add missing \"strbuf.h\" include under NO_CURL\n> git bisect good 007488839cabbb5bc6777924ae03c4edeb1b6110\n>\n> This was the first time that I did a bisection with 'git bisect run'\n> that was not fully automated so it threw me off a bit.\n>\n> I tried it again today and realized that if I modify my ~/bisect-git.sh\n> to use 'git cherry-pick --no-commit --allow-empty' instead, then the\n> bisection runs to completion and finds the bad commit. So my understanding\n> is that cherry-picking a commit during the bisection (without --no-commit)\n> alters the commit graph and might not be the best idea...\n\nYeah, right.\n\nI think the merge bases should only be checked at the beginning of a\nbisection. So it is strange that it happened so late after it was\nstarted.\n\n> Looking at the man\n> page the example does use 'git merge --no-commit' so I should have known better.\n>\n> But the question remains: in general shouldn't 'git bisect run' do everything\n> automatically and not require the user to do something manually ?\n\nI agree but it might be a bug because git bisect just doesn't expect\nthe current branch to change while a bisection is going on.\n\n> And a side note to finish:\n> Initially I did not use 'unset $(git rev-parse --local-env-vars)' in\n> my ~/bisect-trailers.sh, and I was running the bisection in a secondary\n> worktree of my clone of git.git. This did not work at all since the\n> 'git init' and 'git commit --verbose -em \"file\" --trailer=\"key: value\"'\n> steps were running _in my secondary worktree_ instead of in $TMPDIR,\n> because of how the repository detection works in worktrees and\n> exports some variables (I think...) This was also very confusing\n> and I'm wondering if we should add a note somewhere about that\n> in the documentation, although it seems specific to bisecting git.git...\n\nPerhaps we could also have examples of good git bisect run scripts\nsomewhere that we can point Git developers to.\n"},{"id":"491377","messageId":"xmqq34sfqpcf.fsf@gitster.g","threadId":"61193","inReplyTo":"CAP8UFD2dNix6OMzCR5Ao8YLD-OVbcOfZOP90G7-QRx=OtRAoYw@mail.gmail.com","subject":"Re: 'git bisect run' not fully automatic when a merge base must be tested","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-03-24T21:42:08Z","receivedAt":"2024-03-24T21:42:17Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder <christian.couder@gmail.com> writes:\n\n> I think the merge bases should only be checked at the beginning of a\n> bisection. So it is strange that it happened so late after it was\n> started.\n\nUsing cherry-pick to alter the shape of the history being bisected\nmay confuse the algorithm to find the next step to test.  But if it\nis not \"confused\" but \"due to dynamic expansion of the search space,\nwe ended up in a forked history that requires a merge-base test\"\nthat is a legitimate condition, then I tend to agree with Philippe\nthat it should be automatically tested during \"bisect run\".\n"}]}