'git bisect run' not fully automatic when a merge base must be tested

'git bisect run' not fully automatic when a merge base must be tested

[Philippe Blain]

Hello,

I have encountered a situation where 'git bisect run' is not fully automatic,
i.e. it stops before finding the first bad commit with the message:
"a merge base must be tested".

Is there a reason why it could not run the provided script 
on the merge-base that it checks out ?

For context (and a reproducer), I was bisecting the 
'git commit --verbose --trailer' regression I reported in [1].

I was running the bisection on a machine without curl installed, and so I 
was building with 'NO_CURL' and thus needed to cherry-pick my 30bced3a67 
(imap-send: add missing "strbuf.h" include under NO_CURL, 2024-02-02) at most steps
of the bisection. So I was running this command:

git bisect reset && git bisect start &&
git bisect bad v2.43.2 && git bisect good v2.42.1 &&
git bisect run ~/bisect-git.sh

with this script to drive the bisection:

=== ~/bisect-git.sh ===
#!/bin/bash

git cherry-pick --allow-empty 30bced3a67
if   make -j NO_CURL=1
then
    # run project specific test and report its status
    ~/bisect-trailers.sh
    status=$?
else
    # tell the caller this is untestable
    status=125
fi

# return control
exit $status
==== end of ~/bisect-git.sh ===

and this one to reproduce the regression:

=== ~/bisect-trailer.sh ===
set -e

export PATH="/path/to/git/bin-wrappers/:$PATH"

repo=${TMPDIR:-/tmp}/test-trailers
rm -rf "$repo" 

unset $(git rev-parse --local-env-vars)

git init "$repo"
cd "$repo"
date>file
git add file
export GIT_EDITOR='cat file'
git commit --verbose -em "file" --trailer="key: value" > /dev/null
git show | \grep -q value
=== end of ~/bisect-trailer.sh ===

This results in the bisection stopping at:

	HEAD is now at 4a14ccd05d imap-send: add missing "strbuf.h" include under NO_CURL
	Bisecting: a merge base must be tested
	[d57c671a511d885a5cd390e3d6064c37af524a91] treewide: remove unnecessary includes in source files
	bisect run success

with the following bisect log:

git bisect start
# status: waiting for both good and bad commits
# bad: [efb050becb6bc703f76382e1f1b6273100e6ace3] Git 2.43.2
git bisect bad efb050becb6bc703f76382e1f1b6273100e6ace3
# status: waiting for good commit(s), bad commit known
# good: [61a22ddaf0626111193a17ac12f366bd6d167dff] Git 2.42.1
git bisect good 61a22ddaf0626111193a17ac12f366bd6d167dff
# good: [5edbcead426056b54286499149244ae4cbf8b5f7] Merge branch 'bc/racy-4gb-files'
git bisect good 5edbcead426056b54286499149244ae4cbf8b5f7
# good: [5baedc68b02c1b43b307d436edac702ac3e7b89d] Merge branch 'jk/bisect-reset-fix' into maint-2.43
git bisect good 5baedc68b02c1b43b307d436edac702ac3e7b89d
# good: [2873a9686cf59ecbf851cea8c41e6ee545195423] Merge branch 'rs/rebase-use-strvec-pushf' into maint-2.43
git bisect good 2873a9686cf59ecbf851cea8c41e6ee545195423
# bad: [03bc5976514f706889fceea623f35133014ebe78] imap-send: add missing "strbuf.h" include under NO_CURL
git bisect bad 03bc5976514f706889fceea623f35133014ebe78
# bad: [9ae3c6dceb187af1ae09649dc5c61bb05a7013d9] imap-send: add missing "strbuf.h" include under NO_CURL
git bisect bad 9ae3c6dceb187af1ae09649dc5c61bb05a7013d9
# good: [007488839cabbb5bc6777924ae03c4edeb1b6110] imap-send: add missing "strbuf.h" include under NO_CURL
git bisect good 007488839cabbb5bc6777924ae03c4edeb1b6110

This was the first time that I did a bisection with 'git bisect run'
that was not fully automated so it threw me off a bit.

I tried it again today and realized that if I modify my ~/bisect-git.sh
to use 'git cherry-pick --no-commit --allow-empty' instead, then the
bisection runs to completion and finds the bad commit. So my understanding
is that cherry-picking a commit during the bisection (without --no-commit)
alters the commit graph and might not be the best idea... Looking at the man
page the example does use 'git merge --no-commit' so I should have known better.

But the question remains: in general shouldn't 'git bisect run' do everything
automatically and not require the user to do something manually ?


And a side note to finish:
Initially I did not use 'unset $(git rev-parse --local-env-vars)' in
my ~/bisect-trailers.sh, and I was running the bisection in a secondary
worktree of my clone of git.git. This did not work at all since the
'git init' and 'git commit --verbose -em "file" --trailer="key: value"'
steps were running _in my secondary worktree_ instead of in $TMPDIR,
because of how the repository detection works in worktrees and
exports some variables (I think...) This was also very confusing
and I'm wondering if we should add a note somewhere about that 
in the documentation, although it seems specific to bisecting git.git...

Cheers,
Philippe

[1] https://lore.kernel.org/git/8b4738ad-62cd-789e-712e-bd45a151b4ac@gmail.com/

Re: 'git bisect run' not fully automatic when a merge base must be tested

[Christian Couder]

On Sun, Mar 24, 2024 at 5:57 PM Philippe Blain
<levraiphilippeblain@gmail.com> wrote:
>
> Hello,
>
> I have encountered a situation where 'git bisect run' is not fully automatic,
> i.e. it stops before finding the first bad commit with the message:
> "a merge base must be tested".
>
> Is there a reason why it could not run the provided script
> on the merge-base that it checks out ?

The git-bisect-lk2009.txt file has a "Checking merge bases" section
that has some explanations about why merge bases must sometimes be
tested and why a bisection should stop if a merge base is "bad".

Now I am not sure how `git bisect run` behaves in practice when a
merge base should be tested. It's possible it never actually worked,
or perhaps there was no test case in our tests checking this and it
stopped working at one point.

> For context (and a reproducer), I was bisecting the
> 'git commit --verbose --trailer' regression I reported in [1].
>
> I was running the bisection on a machine without curl installed, and so I
> was building with 'NO_CURL' and thus needed to cherry-pick my 30bced3a67
> (imap-send: add missing "strbuf.h" include under NO_CURL, 2024-02-02) at most steps
> of the bisection. So I was running this command:
>
> git bisect reset && git bisect start &&
> git bisect bad v2.43.2 && git bisect good v2.42.1 &&
> git bisect run ~/bisect-git.sh
>
> with this script to drive the bisection:
>
> === ~/bisect-git.sh ===
> #!/bin/bash
>
> git cherry-pick --allow-empty 30bced3a67

It might not be a good idea to change the current branch when a
bisection happens. I think it would be better to apply a patch using
`git apply` before the tests and to remove it using `git apply -R`
after the tests.

> if   make -j NO_CURL=1
> then
>     # run project specific test and report its status
>     ~/bisect-trailers.sh
>     status=$?
> else
>     # tell the caller this is untestable
>     status=125
> fi
>
> # return control
> exit $status
> ==== end of ~/bisect-git.sh ===
>
> and this one to reproduce the regression:
>
> === ~/bisect-trailer.sh ===
> set -e
>
> export PATH="/path/to/git/bin-wrappers/:$PATH"
>
> repo=${TMPDIR:-/tmp}/test-trailers
> rm -rf "$repo"
>
> unset $(git rev-parse --local-env-vars)
>
> git init "$repo"
> cd "$repo"
> date>file
> git add file
> export GIT_EDITOR='cat file'
> git commit --verbose -em "file" --trailer="key: value" > /dev/null
> git show | \grep -q value
> === end of ~/bisect-trailer.sh ===
>
> This results in the bisection stopping at:
>
>         HEAD is now at 4a14ccd05d imap-send: add missing "strbuf.h" include under NO_CURL
>         Bisecting: a merge base must be tested
>         [d57c671a511d885a5cd390e3d6064c37af524a91] treewide: remove unnecessary includes in source files
>         bisect run success
>
> with the following bisect log:
>
> git bisect start
> # status: waiting for both good and bad commits
> # bad: [efb050becb6bc703f76382e1f1b6273100e6ace3] Git 2.43.2
> git bisect bad efb050becb6bc703f76382e1f1b6273100e6ace3
> # status: waiting for good commit(s), bad commit known
> # good: [61a22ddaf0626111193a17ac12f366bd6d167dff] Git 2.42.1
> git bisect good 61a22ddaf0626111193a17ac12f366bd6d167dff
> # good: [5edbcead426056b54286499149244ae4cbf8b5f7] Merge branch 'bc/racy-4gb-files'
> git bisect good 5edbcead426056b54286499149244ae4cbf8b5f7
> # good: [5baedc68b02c1b43b307d436edac702ac3e7b89d] Merge branch 'jk/bisect-reset-fix' into maint-2.43
> git bisect good 5baedc68b02c1b43b307d436edac702ac3e7b89d
> # good: [2873a9686cf59ecbf851cea8c41e6ee545195423] Merge branch 'rs/rebase-use-strvec-pushf' into maint-2.43
> git bisect good 2873a9686cf59ecbf851cea8c41e6ee545195423
> # bad: [03bc5976514f706889fceea623f35133014ebe78] imap-send: add missing "strbuf.h" include under NO_CURL
> git bisect bad 03bc5976514f706889fceea623f35133014ebe78
> # bad: [9ae3c6dceb187af1ae09649dc5c61bb05a7013d9] imap-send: add missing "strbuf.h" include under NO_CURL
> git bisect bad 9ae3c6dceb187af1ae09649dc5c61bb05a7013d9
> # good: [007488839cabbb5bc6777924ae03c4edeb1b6110] imap-send: add missing "strbuf.h" include under NO_CURL
> git bisect good 007488839cabbb5bc6777924ae03c4edeb1b6110
>
> This was the first time that I did a bisection with 'git bisect run'
> that was not fully automated so it threw me off a bit.
>
> I tried it again today and realized that if I modify my ~/bisect-git.sh
> to use 'git cherry-pick --no-commit --allow-empty' instead, then the
> bisection runs to completion and finds the bad commit. So my understanding
> is that cherry-picking a commit during the bisection (without --no-commit)
> alters the commit graph and might not be the best idea...

Yeah, right.

I think the merge bases should only be checked at the beginning of a
bisection. So it is strange that it happened so late after it was
started.

> Looking at the man
> page the example does use 'git merge --no-commit' so I should have known better.
>
> But the question remains: in general shouldn't 'git bisect run' do everything
> automatically and not require the user to do something manually ?

I agree but it might be a bug because git bisect just doesn't expect
the current branch to change while a bisection is going on.

> And a side note to finish:
> Initially I did not use 'unset $(git rev-parse --local-env-vars)' in
> my ~/bisect-trailers.sh, and I was running the bisection in a secondary
> worktree of my clone of git.git. This did not work at all since the
> 'git init' and 'git commit --verbose -em "file" --trailer="key: value"'
> steps were running _in my secondary worktree_ instead of in $TMPDIR,
> because of how the repository detection works in worktrees and
> exports some variables (I think...) This was also very confusing
> and I'm wondering if we should add a note somewhere about that
> in the documentation, although it seems specific to bisecting git.git...

Perhaps we could also have examples of good git bisect run scripts
somewhere that we can point Git developers to.

Re: 'git bisect run' not fully automatic when a merge base must be tested

[Junio C Hamano]

Christian Couder <christian.couder@gmail.com> writes:

> I think the merge bases should only be checked at the beginning of a
> bisection. So it is strange that it happened so late after it was
> started.

Using cherry-pick to alter the shape of the history being bisected
may confuse the algorithm to find the next step to test.  But if it
is not "confused" but "due to dynamic expansion of the search space,
we ended up in a forked history that requires a merge-base test"
that is a legitimate condition, then I tend to agree with Philippe
that it should be automatically tested during "bisect run".