Why does git-bisect have to be run from the top level directory of the working tree?

Looking at some commits in the project, I see one by Marcel M. Cary ([email protected])

He says in a commit (it happens to be about git-pull but I think it is relevant)

"git pull" fails because POSIX shells have a notion of current working directory that is different from getcwd(). The shell stores this path in PWD. As a result, "cd ../" can be interpreted differently in a shell script than chdir("../") in a C program. The shell interprets "../" by essentially stripping the last textual path component from PWD, whereas C chdir() follows the ".." link in the current directory on the filesystem. When PWD is a symlink, these are different destinations. As a result, Git's C commands find the correct top-level working tree, and shell scripts do not.

https://github.com/git/git/commit/08fc0608657ee91bc85276667804c36a93138c7d

SO I'd say part of the reason is because git-bisect is a shell script which can't be trusted to find the toplevel on its own (when symlinks are involved).

The bisecting process needs to check out different revisions of your project. If a particular revision does not contain the current folder, then the current folder will be removed.

In that case, your shell could end up sitting in a folder which is no longer on the filesystem! Git will then be unable to find the toplevel's .git folder and so the bisect process cannot continue without intervention.

A demonstration:

$ git rev-parse --show-toplevel
/path/to/project
$ mkdir tmp
$ cd tmp
$ rmdir ../tmp
$ git rev-parse --show-toplevel
fatal: Unable to read current working directory: No such file or directory

Of course this same problem can occur when doing git checkout, and it can be easily fixed after the fact, e.g. with cd .. (willoller explains why that works in the shell but not in git).

But since bisecting is a process it makes sense to avoid this situation before we begin, especially if we want to use automation such as git bisect run.

As a result, Git's C commands find the correct top-level working tree, and shell scripts do not.

Well, with Git 2.21 (Feb. 2019), git bisect continues its transition from a shell script to C.

See commit 06f5608, commit 450ebb7, commit 129a6cf, commit 4fbdbd5, commit e3b1e3b, commit 0f30233, commit 5e82c3d (02 Jan 2019) by Pranit Bauva (pranitbauva1997).
Helped-by: Ramsay Jones (jeffhostetler), and Stephan Beyer (sbeyer).
^{(Merged by Junio C Hamano -- gitster -- in commit 09a9c1f, 07 Feb 2019)}

bisect--helper: bisect_start shell function partially in C

Reimplement the bisect_start shell function partially in C and add bisect-start subcommand to git bisect--helper to call it from git-bisect.sh .

The last part is not converted because it calls another shell function. bisect_start shell function will be completed after the bisect_next shell function is ported in C.

This is not yet complete, but a side-effect of that migration will be the ability to execute git bisect from a subfolder.

Git 2.23 further improves that conversion to C.
See commit 7877ac3 (21 May 2019) by Johannes Schindelin (dscho).
^{(Merged by Junio C Hamano -- gitster -- in commit 5b476dc, 17 Jun 2019)}

With Git 2.25 (Q1 2020), bisect_reset benefits from a fix.

See commit 51a0a4e (09 Dec 2019) by Tanushree Tumane (tanushree27).
^{(Merged by Junio C Hamano -- gitster -- in commit 4bfc9cc, 25 Dec 2019)}

bisect--helper: avoid use-after-free

^{Mentored-by: Johannes Schindelin}
^{Mentored-by: Christian Couder}
^{Signed-off-by: Tanushree Tumane}
^{Signed-off-by: Miriam Rubio}

In 5e82c3dd22a ("bisect--helper: bisect_reset shell function in C", 2019-01-02, Git v2.21.0-rc0 -- merge), the [git bisect](https://git-scm.com/docs/git-bisect) reset subcommand was ported to C.
When the call to git checkout failed, an error message (could not check out original HEAD) was reported to the user.

However, this error message used the strbuf that had just been released already.
Let's switch that around: first use it, then release it.

That continues with Git 2.26 (Q1 2020), where the underlying machinery of "git bisect--helper" is being refactored into pieces that are more easily reused.

See commit 6c69f22, commit 9ec598e, commit 45b6370, commit cdd4dc2, commit e8e3ce6, commit ce58b5d, commit 7613ec5 (17 Feb 2020) by Pranit Bauva (pranitbauva1997).
See commit 680e8a0, commit b8e3b2f, commit 16538bf (17 Feb 2020) by Miriam Rubio (``).
See commit bfacfce, commit 292731c (17 Feb 2020) by Tanushree Tumane (tanushree27).
^{(Merged by Junio C Hamano -- gitster -- in commit 25063e2, 05 Mar 2020)}

For instance:

bisect--helper: introduce new decide_next() function

^{Mentored-by: Christian Couder}
^{Signed-off-by: Tanushree Tumane}
^{Signed-off-by: Miriam Rubio}

Let's refactor code from bisect_next_check() into a new decide_next() helper function.

This removes some goto statements and makes the code simpler, clearer and easier to understand.

While at it bad_ref and good_glob are not const any more to void casting them inside free().

Before Git 2.28 (Q3 2020), the code to parse "git bisect start " command line was lax in validating the arguments.

See commit 4d9005f (20 May 2020) by Carlo Marcelo Arenas Belón (carenas).
^{(Merged by Junio C Hamano -- gitster -- in commit 63e50b8, 09 Jun 2020)}

bisect--helper: avoid segfault with bad syntax in start --term-*

^{Signed-off-by: Carlo Marcelo Arenas Belón}
^{Acked-by: Christian Couder}

06f5608c14 ("bisect--helper: bisect_start shell function partially in C", 2019-01-02, Git v2.21.0-rc0 -- merge) adds a lax parser for [git bisect start](https://git-scm.com/docs/git-bisect) which could result in a segfault under a bad syntax call for start with custom terms.

Detect if there are enough arguments left in the command line to use for --term-{old,good,new,bad} and abort with the same syntax error the original implementation will show if not.

While at it, remove an unnecessary (and incomplete) check for unknown arguments and make sure to add a test to avoid regressions.

Before Git 2.29 (Q4 2020), "git bisect start X Y^(man), when X and Y are not valid committish object names, should take X and Y as pathspec, but didn't.

See commit 73c6de0 (25 Sep 2020) by Christian Couder (chriscool).
^{(Merged by Junio C Hamano -- gitster -- in commit 03a0182, 04 Oct 2020)}

bisect: don't use invalid oid as rev when starting

^{Signed-off-by: Christian Couder}
^{Signed-off-by: Johannes Schindelin}

In 06f5608c14 ("bisect--helper: bisect_start shell function partially in C", 2019-01-02, Git v2.21.0-rc0 -- merge), we changed the following shell code:
-      rev=$(git rev-parse -q --verify "$arg^{commit}") || {
-              test $has_double_dash -eq 1 &&
-              die "$(eval_gettext "'\$arg' does not appear to be a valid revision")"
-              break
-      }
-      revs="$revs $rev"
into:
+      char *commit_id = xstrfmt("%s^{commit}", arg);
+      if (get_oid(commit_id, &oid) && has_double_dash)
+              die(_("'%s' does not appear to be a valid "
+                    "revision"), arg);
+
+      string_list_append(&revs, oid_to_hex(&oid));
+      free(commit_id);
In case of an invalid "arg" when "has_double_dash" is false, the old code would "break" out of the argument loop.

In the new C code though, oid_to_hex(&oid) is unconditonally appended to "revs". This is wrong first because "oid" is junk as get_oid(commit_id, &oid) failed, and second because it doesn't break out of the argument loop.

Not breaking out of the argument loop means that "arg" is then not treated as a path restriction (which is wrong).

With Git 2.29 (Q4 2020), the rewrite of the "git bisect"^(man) script in C continues.

See commit 517ecb3, commit 09535f0 (24 Sep 2020) by Pranit Bauva (pranitbauva1997).
See commit c7a7f48 (24 Sep 2020), and commit 7b4de74, commit 3027676, commit ef5aef5 (28 Aug 2020) by Miriam Rubio (mirucam).
^{(Merged by Junio C Hamano -- gitster -- in commit f4cc68c, 04 Oct 2020)}

bisect--helper: reimplement bisect_next and bisect_auto_next shell functions in C

^{Mentored-by: Lars Schneider}
^{Mentored-by: Christian Couder}
^{Mentored-by: Johannes Schindelin}
^{Signed-off-by: Pranit Bauva}
^{Signed-off-by: Tanushree Tumane}
^{Signed-off-by: Miriam Rubio}

Reimplement the bisect_next() and the bisect_auto_next() shell functions in C and add the subcommands to [git](https://github.com/git/git/blob/517ecb3161daa4503f7638489fd44177b3659913/Documentation/git-.txt)<sup>([man](https://git-scm.com/docs/git-))</sup> bisect--helper to call them from git-bisect.sh .

bisect_auto_next() function returns an enum bisect_error type as whole [git bisect](https://github.com/git/git/blob/517ecb3161daa4503f7638489fd44177b3659913/Documentation/git-bisect.txt)<sup>([man](https://git-scm.com/docs/git-bisect))</sup> can exit with an error code when bisect_next() does.

Return an error when bisect_next() fails, that fix a bug on shell script version.

Using --bisect-next and --bisect-auto-next subcommands is a temporary measure to port shell function to C so as to use the existing test suite. As more functions are ported, --bisect-auto-next subcommand will be retired and will be called by some other methods.

With Git 2.30 (Q1 2021), the rewriting "git bisect"^(man) in C continues.

See commit b0f6494, commit 5c517fe, commit 9b437b0, commit 27257bc, commit 04774b4, commit e439607, commit 88ad372 (15 Oct 2020) by Pranit Bauva (pranitbauva1997).
^{(Merged by Junio C Hamano -- gitster -- in commit cfdc70b, 09 Nov 2020)}

bisect--helper: reimplement bisect_state & bisect_head shell functions in C

^{Mentored-by: Lars Schneider}
^{Mentored-by: Christian Couder}
^{Mentored-by: Johannes Schindelin}
^{Signed-off-by: Pranit Bauva}
^{Signed-off-by: Tanushree Tumane}
^{Signed-off-by: Miriam Rubio}
^{Reviewed-by: Johannes Schindelin}

Reimplement the bisect_state() shell functions in C and also add a subcommand --bisect-state to git-bisect--helper to call them from git-bisect.sh.

Using --bisect-state subcommand is a temporary measure to port shell function to C so as to use the existing test suite. As more functions are ported, this subcommand will be retired and will be called by some other methods.

bisect_head() is only called from bisect_state(), thus it is not required to introduce another subcommand.

Note that the eval in the changed line of git-bisect.sh cannot be dropped: it is necessary because the rev and the tail variables may contain multiple, quoted arguments that need to be passed to bisect--helper (without the quotes, naturally).

With Git 2.31 (Q1 2021): piecemeal of rewrite of "git bisect"^(man) in C continues.

See commit 97b8294, commit e4c7b33, commit 9feea34, commit b7a6f16, commit 68efed8, commit 2b1fd94, commit 97d5ba6 (03 Feb 2021) by Pranit Bauva (pranitbauva1997).
^{(Merged by Junio C Hamano -- gitster -- in commit 0871fb9, 17 Feb 2021)}

bisect--helper: retire --bisect-write subcommand

^{Mentored-by: Lars Schneider}
^{Mentored-by: Christian Couder}
^{Mentored-by: Johannes Schindelin}
^{Signed-off-by: Pranit Bauva}
^{Signed-off-by: Tanushree Tumane}
^{Signed-off-by: Miriam Rubio}

The --bisect-write subcommand is no longer used from the git-bisect.sh shell script.
Instead the function bisect_write() is directly called from the C implementation.

Why does git-bisect have to be run from the top level directory of the working tree?

bisect--helper: `bisect_start` shell function partially in C

`bisect--helper`: avoid use-after-free

`bisect--helper`: introduce new `decide_next()` function

`bisect--helper`: avoid segfault with bad syntax in `start --term-*`

`bisect`: don't use invalid oid as rev when starting

`bisect--helper`: reimplement `bisect_next` and `bisect_auto_next` shell functions in C

`bisect--helper`: reimplement `bisect_state` & `bisect_head` shell functions in C

`bisect--helper`: retire `--bisect-write` subcommand

Tags:

Git

Git Bisect

Related

Recent Posts

Why does git-bisect have to be run from the top level directory of the working tree?

bisect--helper: bisect_start shell function partially in C

bisect--helper: avoid use-after-free

bisect--helper: introduce new decide_next() function

bisect--helper: avoid segfault with bad syntax in start --term-*

bisect: don't use invalid oid as rev when starting

bisect--helper: reimplement bisect_next and bisect_auto_next shell functions in C

bisect--helper: reimplement bisect_state & bisect_head shell functions in C

bisect--helper: retire --bisect-write subcommand

Tags:

Git

Git Bisect

Related

bisect--helper: `bisect_start` shell function partially in C

`bisect--helper`: avoid use-after-free

`bisect--helper`: introduce new `decide_next()` function

`bisect--helper`: avoid segfault with bad syntax in `start --term-*`

`bisect`: don't use invalid oid as rev when starting

`bisect--helper`: reimplement `bisect_next` and `bisect_auto_next` shell functions in C

`bisect--helper`: reimplement `bisect_state` & `bisect_head` shell functions in C

`bisect--helper`: retire `--bisect-write` subcommand