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

3

74

If one tries to run any of the git bisect commands from anywhere other than the root directory of the repository, one is told:

You need to run this command from the toplevel of the working tree.

Why is that? I know of no other git command that has this requirement, and I see no obvious reason that bisect should be special. The man page makes no mention of this restriction, either.

It's really not a big deal. I'm mostly just curious.

Valenevalenka answered 18/9, 2012 at 15:5 Comment(4)
I assume it's to make it clear that your whole working copy will be modified during the bisectHelmick
And to avoid the edge case of what to do if you're in a directory that gets removed. Then again, git doesn't track directories...Churchyard
@CharlesB, Arafangion, Both those points apply just as much to git-checkout as they do to git-bisect, do they not?Valenevalenka
@Churchyard it is true about any command changing wc - pull, merge, rebase, checkout, etc. Nothing special about bisect. I think it is just yet another misconception.Intact
C
70

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).

Clydesdale answered 22/9, 2012 at 8:4 Comment(2)
I was hoping for a better reason, but that's hardly your fault. Thank you very much for doing some research.Valenevalenka
Yeah a bit disappointing. Go ahead and dig in to the code; it's remarkably (and sometimes humorously) well-commented and the commit messages are excellent.Clydesdale
R
12

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.

Reyesreykjavik answered 15/8, 2016 at 2:6 Comment(0)
K
9

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 .

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.


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.


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.


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.


"git bisect"(man) reimplemented more in C during 2.30 timeframe did not take an annotated tag as a good/bad endpoint well.
This regression has been corrected with Git 2.31.1 (Q1 2021).

See commit 7730f85 (16 Mar 2021) by Jeff King (peff).
(Merged by Junio C Hamano -- gitster -- in commit 35381b1, 19 Mar 2021)

bisect: peel annotated tags to commits

Signed-off-by: Jeff King

This patch fixes a bug where 'git-bisect'(man) doesnt handle receiving annotated tags as "git bisect" good <tag>, etc.
It's a regression in 27257bc ("bisect--helper: reimplement bisect_state & bisect_head shell functions in C", 2020-10-15, Git v2.30.0-rc0 -- merge listed in batch #4).

The original shell code called:

sha=$(git rev-parse --verify "$rev^{commit}") ||
        die "$(eval_gettext "Bad rev input: \$rev")"

which will peel the input to a commit (or complain if that's not possible).
But the C code just calls get_oid(), which will yield the oid of the tag.

The fix is to peel to a commit.
The error message here is a little non-idiomatic for Git (since it starts with a capital).

New error message:

Bad rev input (not a commit): xxxx

Warning: "git bisect skip"(man) when custom words are used for new/old did not work in Git 2.31, which has been corrected with Git 2.32 (Q2 2021).

See commit 4cd66e7 (29 Apr 2021) by Ramsay Jones (ramsay-jones).
(Merged by Junio C Hamano -- gitster -- in commit 8ca4771, 11 May 2021)

bisect--helper: use BISECT_TERMS in 'bisect skip' command

Reported-by: Trygve Aaberge
Signed-off-by: Bagas Sanjaya
Signed-off-by: Ramsay Jones

Commit e4c7b33 ("bisect--helper: reimplement bisect_skip shell function in C", 2021-02-03, Git v2.31.0-rc0 -- merge listed in batch #9), as part of the shell-to-C conversion, forgot to read the 'terms' file (.git/BISECT_TERMS) during the new 'bisect skip' command implementation.
As a result, the 'bisect skip' command will use the default 'bad'/'good' terms.
If the bisection terms have been set to non-default values (for example by the 'bisect start' command), then the 'bisect skip' command will fail.

In order to correct this problem, we insert a call to the get_terms() function, which reads the non-default terms from that file (if set), in the '--bisect-skip' command implementation of 'bisect--helper'.

Also, add a test to protect against potential future regression.


With Git 2.33 (Q3 2021), "git bisect"(man) spawned git show-branch(man) only to pretty-print the title of the commit after checking out the next version to be tested; this has been rewritten in C.

See commit 1fcc40c (28 Jul 2021), and commit ffcb4e9 (27 Jul 2021) by Junio C Hamano (gitster).
(Merged by Junio C Hamano -- gitster -- in commit ae2d05d, 24 Aug 2021)

bisect: do not run show-branch just to show the current commit

In scripted versions of "git bisect"(man), we used "git show-branch"(man) to describe a single commit in the bisect log and also to the interactive user after checking out the next version to be tested.

The former use of "git show-branch" was lost when the helper function that wrote bisect log entries was rewritten at 0f30233 ("bisect--helper: bisect_write shell function in C", 2019-01-02, Git v2.21.0-rc0 -- merge) in C

But we've kept the latter ever since 0871984 (bisect: make , 2009-05-09, Git v1.6.4-rc0 -- merge) (bisect: make "git bisect" use new --next-all bisect-helper function, 2009-05-09) started using the faithful C-rewrite introduced at ef24c7c (bisect--helper: add , 2009-04-19, Git v1.6.4-rc0 -- merge) (bisect--helper: add "--next-exit" to output bisect results, 2009-04-19).

Showing "[<full hex>] <subject>" is simple enough with our helper pretty.c::format_commit_message() and spawning show-branch is an overkill.
Let's lose one external process.

Kata answered 3/3, 2019 at 20:59 Comment(2)
Your dedication to keep updating this answer for newer Git versions is impressive!Valenevalenka
@ParkerCoates Thank you. That rewrite in C is an effort which spans quite a few Git releases. (I did not contribute to it, I just report it here as it happens)Kata

© 2022 - 2024 — McMap. All rights reserved.