Git Mailing List Archive mirror
 help / color / mirror / Atom feed
From: Christian Couder <christian.couder@gmail.com>
To: Philippe Blain <levraiphilippeblain@gmail.com>
Cc: Git mailing list <git@vger.kernel.org>
Subject: Re: 'git bisect run' not fully automatic when a merge base must be tested
Date: Sun, 24 Mar 2024 18:57:48 +0100	[thread overview]
Message-ID: <CAP8UFD2dNix6OMzCR5Ao8YLD-OVbcOfZOP90G7-QRx=OtRAoYw@mail.gmail.com> (raw)
In-Reply-To: <6ee4b8d8-5acb-3d3c-28e0-be972945e8d7@gmail.com>

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.

  reply	other threads:[~2024-03-24 17:58 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-24 16:57 'git bisect run' not fully automatic when a merge base must be tested Philippe Blain
2024-03-24 17:57 ` Christian Couder [this message]
2024-03-24 21:42   ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAP8UFD2dNix6OMzCR5Ao8YLD-OVbcOfZOP90G7-QRx=OtRAoYw@mail.gmail.com' \
    --to=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=levraiphilippeblain@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).