# [PATCH v12 10/13] bisect--helper: `check_and_set_terms` shell function in C

22 messages from 2016-08-10 to 2016-08-13. Participants: Pranit Bauva, Junio C Hamano, Christian Couder.
Thread: https://gitlist.dev/t/43048

## Pranit Bauva, 2016-08-10 21:57

Subject: [PATCH v12 10/13] bisect--helper: `check_and_set_terms` shell function in C
Message-ID: <010201567675ae5e-2126c4c2-d76f-4e88-8efe-3936d3aad98d-000000@eu-west-1.amazonses.com>
URL: https://gitlist.dev/e/010201567675ae5e-2126c4c2-d76f-4e88-8efe-3936d3aad98d-000000%40eu-west-1.amazonses.com
In-Reply-To: <010201567675adc1-17e27495-6b36-40d1-836d-814da029fcc4-000000@eu-west-1.amazonses.com>

```
Reimplement the `check_and_set_terms` shell function in C and add
`check-and-set-terms` subcommand to `git bisect--helper` to call it from
git-bisect.sh

Using `--check-and-set-terms` subcommand is a temporary measure to port
shell function in C so as to use the existing test suite. As more
functions are ported, this subcommand will be retired but its
implementation will be called by some other methods.

check_and_set_terms() sets and receives two global variables namely
TERM_GOOD and TERM_BAD in the shell script. Luckily the file BISECT_TERMS
also contains the value of those variables so its appropriate to evoke the
method get_terms() after calling the subcommand so that it retrieves the
value of TERM_GOOD and TERM_BAD from the file BISECT_TERMS. The two
global variables are passed as arguments to the subcommand.

Also introduce bisect_terms_reset() to empty the contents of `term_good`
and `term_bad` of `struct bisect_terms`.

Also introduce set_terms() to copy the `term_good` and `term_bad` into
`struct bisect_terms` and write it out to the file BISECT_TERMS.

Mentored-by: Lars Schneider <larsxschneider@gmail.com>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
---
 builtin/bisect--helper.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++-
 git-bisect.sh            | 36 ++++-----------------------------
 2 files changed, 55 insertions(+), 33 deletions(-)

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 9f70edb..5c4350f 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -23,6 +23,7 @@ static const char * const git_bisect_helper_usage[] = {
 	N_("git bisect--helper --bisect-clean-state"),
 	N_("git bisect--helper --bisect-reset [<commit>]"),
 	N_("git bisect--helper --bisect-write <state> <revision> <TERM_GOOD> <TERM_BAD> [<nolog>]"),
+	N_("git bisect--helper --bisect-check-and-set-terms <command> <TERM_GOOD> <TERM_BAD>"),
 	NULL
 };
 
@@ -43,6 +44,12 @@ static void bisect_terms_release(struct bisect_terms *terms)
 	strbuf_release(&terms->term_bad);
 }
 
+static void bisect_terms_reset(struct bisect_terms *term)
+{
+	strbuf_reset(&term->term_good);
+	strbuf_reset(&term->term_bad);
+}
+
 /*
  * Check whether the string `term` belongs to the set of strings
  * included in the variable arguments.
@@ -253,6 +260,39 @@ static int bisect_write(const char *state, const char *rev,
 	return 0;
 }
 
+static int set_terms(struct bisect_terms *terms, const char *bad,
+		     const char *good)
+{
+	bisect_terms_reset(terms);
+	strbuf_addstr(&terms->term_good, good);
+	strbuf_addstr(&terms->term_bad, bad);
+	return write_terms(terms->term_bad.buf, terms->term_good.buf);
+}
+
+static int check_and_set_terms(struct bisect_terms *terms, const char *cmd)
+{
+	int has_term_file = !is_empty_or_missing_file(git_path_bisect_terms());
+
+	if (one_of(cmd, "skip", "start", "terms", NULL))
+		return 0;
+
+	if (has_term_file &&
+	    strcmp(cmd, terms->term_bad.buf) &&
+	    strcmp(cmd, terms->term_good.buf))
+		return error(_("Invalid command: you're currently in a "
+				"%s/%s bisect"), terms->term_bad.buf,
+				terms->term_good.buf);
+
+	if (!has_term_file) {
+		if (one_of(cmd, "bad", "good", NULL))
+			return set_terms(terms, "bad", "good");
+		if (one_of(cmd, "new", "old", NULL))
+			return set_terms(terms, "new", "old");
+	}
+
+	return 0;
+}
+
 int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 {
 	enum {
@@ -261,7 +301,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 		BISECT_CLEAN_STATE,
 		BISECT_RESET,
 		CHECK_EXPECTED_REVS,
-		BISECT_WRITE
+		BISECT_WRITE,
+		CHECK_AND_SET_TERMS
 	} cmdmode = 0;
 	int no_checkout = 0, res = 0;
 	struct option options[] = {
@@ -277,6 +318,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 			 N_("check for expected revs"), CHECK_EXPECTED_REVS),
 		OPT_CMDMODE(0, "bisect-write", &cmdmode,
 			 N_("write out the bisection state in BISECT_LOG"), BISECT_WRITE),
+		OPT_CMDMODE(0, "check-and-set-terms", &cmdmode,
+			 N_("check and set terms in a bisection state"), CHECK_AND_SET_TERMS),
 		OPT_BOOL(0, "no-checkout", &no_checkout,
 			 N_("update BISECT_HEAD instead of checking out the current commit")),
 		OPT_END()
@@ -320,6 +363,13 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 		strbuf_addstr(&terms.term_bad, argv[3]);
 		res = bisect_write(argv[0], argv[1], &terms, nolog);
 		break;
+	case CHECK_AND_SET_TERMS:
+		if (argc != 3)
+			die(_("--check-and-set-terms requires 3 arguments"));
+		strbuf_addstr(&terms.term_good, argv[1]);
+		strbuf_addstr(&terms.term_bad, argv[2]);
+		res = check_and_set_terms(&terms, argv[0]);
+		break;
 	default:
 		die("BUG: unknown subcommand '%d'", cmdmode);
 	}
diff --git a/git-bisect.sh b/git-bisect.sh
index b9896a4..a41e69b 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -239,7 +239,8 @@ bisect_skip() {
 bisect_state() {
 	bisect_autostart
 	state=$1
-	check_and_set_terms $state
+	git bisect--helper --check-and-set-terms $state $TERM_GOOD $TERM_BAD || exit
+	get_terms
 	case "$#,$state" in
 	0,*)
 		die "$(gettext "Please call 'bisect_state' with at least one argument.")" ;;
@@ -390,7 +391,8 @@ bisect_replay () {
 			command="$bisect"
 		fi
 		get_terms
-		check_and_set_terms "$command"
+		git bisect--helper --check-and-set-terms "$command" "$TERM_GOOD" "$TERM_BAD" || exit
+		get_terms
 		case "$command" in
 		start)
 			cmd="bisect_start $rev"
@@ -480,36 +482,6 @@ get_terms () {
 	fi
 }
 
-check_and_set_terms () {
-	cmd="$1"
-	case "$cmd" in
-	skip|start|terms) ;;
-	*)
-		if test -s "$GIT_DIR/BISECT_TERMS" && test "$cmd" != "$TERM_BAD" && test "$cmd" != "$TERM_GOOD"
-		then
-			die "$(eval_gettext "Invalid command: you're currently in a \$TERM_BAD/\$TERM_GOOD bisect.")"
-		fi
-		case "$cmd" in
-		bad|good)
-			if ! test -s "$GIT_DIR/BISECT_TERMS"
-			then
-				TERM_BAD=bad
-				TERM_GOOD=good
-				git bisect--helper --write-terms "$TERM_BAD" "$TERM_GOOD" || exit
-			fi
-			;;
-		new|old)
-			if ! test -s "$GIT_DIR/BISECT_TERMS"
-			then
-				TERM_BAD=new
-				TERM_GOOD=old
-				git bisect--helper --write-terms "$TERM_BAD" "$TERM_GOOD" || exit
-			fi
-			;;
-		esac ;;
-	esac
-}
-
 bisect_voc () {
 	case "$1" in
 	bad) echo "bad|new" ;;

--
https://github.com/git/git/pull/281

```

## Pranit Bauva, 2016-08-10 21:57

Subject: [PATCH v12 03/13] bisect--helper: `write_terms` shell function in C
Message-ID: <010201567675ae4e-51e1afee-f0e1-4b71-b307-382de3660b9a-000000@eu-west-1.amazonses.com>
URL: https://gitlist.dev/e/010201567675ae4e-51e1afee-f0e1-4b71-b307-382de3660b9a-000000%40eu-west-1.amazonses.com
In-Reply-To: <010201567675adc1-17e27495-6b36-40d1-836d-814da029fcc4-000000@eu-west-1.amazonses.com>

```
Reimplement the `write_terms` shell function in C and add a `write-terms`
subcommand to `git bisect--helper` to call it from git-bisect.sh . Also
remove the subcommand `--check-term-format` as it can now be called from
inside the function write_terms() C implementation.

Also `|| exit` is added when calling write-terms subcommand from
git-bisect.sh so as to exit whenever there is an error.

Using `--write-terms` 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 its implementation will
be called by some other method.

Mentored-by: Lars Schneider <larsxschneider@gmail.com>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
---
 builtin/bisect--helper.c | 36 +++++++++++++++++++++++++++++-------
 git-bisect.sh            | 22 +++++++---------------
 2 files changed, 36 insertions(+), 22 deletions(-)

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index a47f1f2..30e1031 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -4,9 +4,11 @@
 #include "bisect.h"
 #include "refs.h"
 
+static GIT_PATH_FUNC(git_path_bisect_terms, "BISECT_TERMS")
+
 static const char * const git_bisect_helper_usage[] = {
 	N_("git bisect--helper --next-all [--no-checkout]"),
-	N_("git bisect--helper --check-term-format <term> <orig_term>"),
+	N_("git bisect--helper --write-terms <bad_term> <good_term>"),
 	NULL
 };
 
@@ -57,18 +59,38 @@ static int check_term_format(const char *term, const char *orig_term)
 	return 0;
 }
 
+static int write_terms(const char *bad, const char *good)
+{
+	FILE *fp;
+	int res;
+
+	if (!strcmp(bad, good))
+		return error(_("please use two different terms"));
+
+	if (check_term_format(bad, "bad") || check_term_format(good, "good"))
+		return -1;
+
+	fp = fopen(git_path_bisect_terms(), "w");
+	if (!fp)
+		return error_errno(_("could not open the file BISECT_TERMS"));
+
+	res = fprintf(fp, "%s\n%s\n", bad, good);
+	res |= fclose(fp);
+	return (res < 0) ? -1 : 0;
+}
+
 int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 {
 	enum {
 		NEXT_ALL = 1,
-		CHECK_TERM_FMT
+		WRITE_TERMS
 	} cmdmode = 0;
 	int no_checkout = 0;
 	struct option options[] = {
 		OPT_CMDMODE(0, "next-all", &cmdmode,
 			 N_("perform 'git bisect next'"), NEXT_ALL),
-		OPT_CMDMODE(0, "check-term-format", &cmdmode,
-			 N_("check format of the term"), CHECK_TERM_FMT),
+		OPT_CMDMODE(0, "write-terms", &cmdmode,
+			 N_("write the terms to .git/BISECT_TERMS"), WRITE_TERMS),
 		OPT_BOOL(0, "no-checkout", &no_checkout,
 			 N_("update BISECT_HEAD instead of checking out the current commit")),
 		OPT_END()
@@ -83,10 +105,10 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 	switch (cmdmode) {
 	case NEXT_ALL:
 		return bisect_next_all(prefix, no_checkout);
-	case CHECK_TERM_FMT:
+	case WRITE_TERMS:
 		if (argc != 2)
-			die(_("--check-term-format requires two arguments"));
-		return check_term_format(argv[0], argv[1]);
+			die(_("--write-terms requires two arguments"));
+		return write_terms(argv[0], argv[1]);
 	default:
 		die("BUG: unknown subcommand '%d'", cmdmode);
 	}
diff --git a/git-bisect.sh b/git-bisect.sh
index 7d7965d..cd39bd0 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -210,7 +210,7 @@ bisect_start() {
 	eval "$eval true" &&
 	if test $must_write_terms -eq 1
 	then
-		write_terms "$TERM_BAD" "$TERM_GOOD"
+		git bisect--helper --write-terms "$TERM_BAD" "$TERM_GOOD"
 	fi &&
 	echo "git bisect start$orig_args" >>"$GIT_DIR/BISECT_LOG" || exit
 	#
@@ -557,18 +557,6 @@ get_terms () {
 	fi
 }
 
-write_terms () {
-	TERM_BAD=$1
-	TERM_GOOD=$2
-	if test "$TERM_BAD" = "$TERM_GOOD"
-	then
-		die "$(gettext "please use two different terms")"
-	fi
-	git bisect--helper --check-term-format "$TERM_BAD" bad || exit
-	git bisect--helper --check-term-format "$TERM_GOOD" good || exit
-	printf '%s\n%s\n' "$TERM_BAD" "$TERM_GOOD" >"$GIT_DIR/BISECT_TERMS"
-}
-
 check_and_set_terms () {
 	cmd="$1"
 	case "$cmd" in
@@ -582,13 +570,17 @@ check_and_set_terms () {
 		bad|good)
 			if ! test -s "$GIT_DIR/BISECT_TERMS"
 			then
-				write_terms bad good
+				TERM_BAD=bad
+				TERM_GOOD=good
+				git bisect--helper --write-terms "$TERM_BAD" "$TERM_GOOD" || exit
 			fi
 			;;
 		new|old)
 			if ! test -s "$GIT_DIR/BISECT_TERMS"
 			then
-				write_terms new old
+				TERM_BAD=new
+				TERM_GOOD=old
+				git bisect--helper --write-terms "$TERM_BAD" "$TERM_GOOD" || exit
 			fi
 			;;
 		esac ;;

--
https://github.com/git/git/pull/281

```

## Pranit Bauva, 2016-08-10 21:57

Subject: [PATCH v12 07/13] bisect--helper: `bisect_reset` shell function in C
Message-ID: <010201567675ae5e-bc1b6bb5-b677-4993-af0e-62e2d4d20033-000000@eu-west-1.amazonses.com>
URL: https://gitlist.dev/e/010201567675ae5e-bc1b6bb5-b677-4993-af0e-62e2d4d20033-000000%40eu-west-1.amazonses.com
In-Reply-To: <010201567675adc1-17e27495-6b36-40d1-836d-814da029fcc4-000000@eu-west-1.amazonses.com>

```
Reimplement `bisect_reset` shell function in C and add a `--bisect-reset`
subcommand to `git bisect--helper` to call it from git-bisect.sh .

Using `bisect_reset` subcommand is a temporary measure to port shell
functions to C so as to use the existing test suite. As more functions
are ported, this subcommand would be retired but its implementation will
be called by some other method.

Note: --bisect-clean-state subcommand has not been retired as there are
still a function namely `bisect_start()` which still uses this
subcommand.

Mentored-by: Lars Schneider <larsxschneider@gmail.com>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
---
 builtin/bisect--helper.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++-
 git-bisect.sh            | 28 ++--------------------------
 2 files changed, 48 insertions(+), 27 deletions(-)

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 3fffa78..409e268 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -4,6 +4,8 @@
 #include "bisect.h"
 #include "refs.h"
 #include "dir.h"
+#include "argv-array.h"
+#include "run-command.h"
 
 static GIT_PATH_FUNC(git_path_bisect_terms, "BISECT_TERMS")
 static GIT_PATH_FUNC(git_path_bisect_expected_rev, "BISECT_EXPECTED_REV")
@@ -13,11 +15,13 @@ static GIT_PATH_FUNC(git_path_bisect_names, "BISECT_NAMES")
 static GIT_PATH_FUNC(git_path_bisect_run, "BISECT_RUN")
 static GIT_PATH_FUNC(git_path_head_name, "head-name")
 static GIT_PATH_FUNC(git_path_bisect_start, "BISECT_START")
+static GIT_PATH_FUNC(git_path_bisect_head, "BISECT_HEAD")
 
 static const char * const git_bisect_helper_usage[] = {
 	N_("git bisect--helper --next-all [--no-checkout]"),
 	N_("git bisect--helper --write-terms <bad_term> <good_term>"),
 	N_("git bisect--helper --bisect-clean-state"),
+	N_("git bisect--helper --bisect-reset [<commit>]"),
 	NULL
 };
 
@@ -125,12 +129,47 @@ static int bisect_clean_state(void)
 	return result;
 }
 
+static int bisect_reset(const char *commit)
+{
+	struct strbuf branch = STRBUF_INIT;
+
+	if (!commit) {
+		if (strbuf_read_file(&branch, git_path_bisect_start(), 0) < 1) {
+			printf("We are not bisecting.\n");
+			return 0;
+		}
+		strbuf_rtrim(&branch);
+	} else {
+		struct object_id oid;
+		if (get_oid(commit, &oid))
+			return error(_("'%s' is not a valid commit"), commit);
+		strbuf_addstr(&branch, commit);
+	}
+
+	if (!file_exists(git_path_bisect_head())) {
+		struct argv_array argv = ARGV_ARRAY_INIT;
+		argv_array_pushl(&argv, "checkout", branch.buf, "--", NULL);
+		if (run_command_v_opt(argv.argv, RUN_GIT_CMD)) {
+			error(_("Could not check out original HEAD '%s'. Try "
+				"'git bisect reset <commit>'."), branch.buf);
+			strbuf_release(&branch);
+			argv_array_clear(&argv);
+			return -1;
+		}
+		argv_array_clear(&argv);
+	}
+
+	strbuf_release(&branch);
+	return bisect_clean_state();
+}
+
 int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 {
 	enum {
 		NEXT_ALL = 1,
 		WRITE_TERMS,
-		BISECT_CLEAN_STATE
+		BISECT_CLEAN_STATE,
+		BISECT_RESET
 	} cmdmode = 0;
 	int no_checkout = 0;
 	struct option options[] = {
@@ -140,6 +179,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 			 N_("write the terms to .git/BISECT_TERMS"), WRITE_TERMS),
 		OPT_CMDMODE(0, "bisect-clean-state", &cmdmode,
 			 N_("cleanup the bisection state"), BISECT_CLEAN_STATE),
+		OPT_CMDMODE(0, "bisect-reset", &cmdmode,
+			 N_("reset the bisection state"), BISECT_RESET),
 		OPT_BOOL(0, "no-checkout", &no_checkout,
 			 N_("update BISECT_HEAD instead of checking out the current commit")),
 		OPT_END()
@@ -162,6 +203,10 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 		if (argc != 0)
 			die(_("--bisect-clean-state requires no arguments"));
 		return bisect_clean_state();
+	case BISECT_RESET:
+		if (argc > 1)
+			die(_("--bisect-reset requires either zero or one arguments"));
+		return bisect_reset(argc ? argv[0] : NULL);
 	default:
 		die("BUG: unknown subcommand '%d'", cmdmode);
 	}
diff --git a/git-bisect.sh b/git-bisect.sh
index bbc57d2..18580b7 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -409,35 +409,11 @@ bisect_visualize() {
 	eval '"$@"' --bisect -- $(cat "$GIT_DIR/BISECT_NAMES")
 }
 
-bisect_reset() {
-	test -s "$GIT_DIR/BISECT_START" || {
-		gettextln "We are not bisecting."
-		return
-	}
-	case "$#" in
-	0) branch=$(cat "$GIT_DIR/BISECT_START") ;;
-	1) git rev-parse --quiet --verify "$1^{commit}" >/dev/null || {
-			invalid="$1"
-			die "$(eval_gettext "'\$invalid' is not a valid commit")"
-		}
-		branch="$1" ;;
-	*)
-		usage ;;
-	esac
-
-	if ! test -f "$GIT_DIR/BISECT_HEAD" && ! git checkout "$branch" --
-	then
-		die "$(eval_gettext "Could not check out original HEAD '\$branch'.
-Try 'git bisect reset <commit>'.")"
-	fi
-	git bisect--helper --bisect-clean-state || exit
-}
-
 bisect_replay () {
 	file="$1"
 	test "$#" -eq 1 || die "$(gettext "No logfile given")"
 	test -r "$file" || die "$(eval_gettext "cannot read \$file for replaying")"
-	bisect_reset
+	git bisect--helper --bisect-reset || exit
 	while read git bisect command rev
 	do
 		test "$git $bisect" = "git bisect" || test "$git" = "git-bisect" || continue
@@ -627,7 +603,7 @@ case "$#" in
 	visualize|view)
 		bisect_visualize "$@" ;;
 	reset)
-		bisect_reset "$@" ;;
+		git bisect--helper --bisect-reset "$@" ;;
 	replay)
 		bisect_replay "$@" ;;
 	log)

--
https://github.com/git/git/pull/281

```

## Pranit Bauva, 2016-08-10 21:57

Subject: [PATCH v12 02/13] bisect: rewrite `check_term_format` shell function in C
Message-ID: <010201567675ae45-39c80adf-8b87-4a92-a908-23ffa48db947-000000@eu-west-1.amazonses.com>
URL: https://gitlist.dev/e/010201567675ae45-39c80adf-8b87-4a92-a908-23ffa48db947-000000%40eu-west-1.amazonses.com
In-Reply-To: <010201567675adc1-17e27495-6b36-40d1-836d-814da029fcc4-000000@eu-west-1.amazonses.com>

```
Reimplement the `check_term_format` shell function in C and add
a `--check-term-format` subcommand to `git bisect--helper` to call it
from git-bisect.sh

Using `--check-term-format` 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 its
implementation will be called by some other method/subcommand. For
eg. In conversion of write_terms() of git-bisect.sh, the subcommand will
be removed and instead check_term_format() will be called in its C
implementation while a new subcommand will be introduced for write_terms().

Helped-by: Johannes Schindelein <Johannes.Schindelein@gmx.de>
Mentored-by: Lars Schneider <larsxschneider@gmail.com>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
---
 builtin/bisect--helper.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++-
 git-bisect.sh            | 31 ++-----------------------
 2 files changed, 61 insertions(+), 30 deletions(-)

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 8111c91..a47f1f2 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -2,19 +2,73 @@
 #include "cache.h"
 #include "parse-options.h"
 #include "bisect.h"
+#include "refs.h"
 
 static const char * const git_bisect_helper_usage[] = {
 	N_("git bisect--helper --next-all [--no-checkout]"),
+	N_("git bisect--helper --check-term-format <term> <orig_term>"),
 	NULL
 };
 
+/*
+ * Check whether the string `term` belongs to the set of strings
+ * included in the variable arguments.
+ */
+LAST_ARG_MUST_BE_NULL
+static int one_of(const char *term, ...)
+{
+	int res = 0;
+	va_list matches;
+	const char *match;
+
+	va_start(matches, term);
+	while (!res && (match = va_arg(matches, const char *)))
+		res = !strcmp(term, match);
+	va_end(matches);
+
+	return res;
+}
+
+static int check_term_format(const char *term, const char *orig_term)
+{
+	int res;
+	char *new_term = xstrfmt("refs/bisect/%s", term);
+
+	res = check_refname_format(new_term, 0);
+	free(new_term);
+
+	if (res)
+		return error(_("'%s' is not a valid term"), term);
+
+	if (one_of(term, "help", "start", "skip", "next", "reset",
+			"visualize", "replay", "log", "run", NULL))
+		return error(_("can't use the builtin command '%s' as a term"), term);
+
+	/*
+	 * In theory, nothing prevents swapping completely good and bad,
+	 * but this situation could be confusing and hasn't been tested
+	 * enough. Forbid it for now.
+	 */
+
+	if ((strcmp(orig_term, "bad") && one_of(term, "bad", "new", NULL)) ||
+		 (strcmp(orig_term, "good") && one_of(term, "good", "old", NULL)))
+		return error(_("can't change the meaning of the term '%s'"), term);
+
+	return 0;
+}
+
 int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 {
-	enum { NEXT_ALL = 1 } cmdmode = 0;
+	enum {
+		NEXT_ALL = 1,
+		CHECK_TERM_FMT
+	} cmdmode = 0;
 	int no_checkout = 0;
 	struct option options[] = {
 		OPT_CMDMODE(0, "next-all", &cmdmode,
 			 N_("perform 'git bisect next'"), NEXT_ALL),
+		OPT_CMDMODE(0, "check-term-format", &cmdmode,
+			 N_("check format of the term"), CHECK_TERM_FMT),
 		OPT_BOOL(0, "no-checkout", &no_checkout,
 			 N_("update BISECT_HEAD instead of checking out the current commit")),
 		OPT_END()
@@ -29,6 +83,10 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 	switch (cmdmode) {
 	case NEXT_ALL:
 		return bisect_next_all(prefix, no_checkout);
+	case CHECK_TERM_FMT:
+		if (argc != 2)
+			die(_("--check-term-format requires two arguments"));
+		return check_term_format(argv[0], argv[1]);
 	default:
 		die("BUG: unknown subcommand '%d'", cmdmode);
 	}
diff --git a/git-bisect.sh b/git-bisect.sh
index 5d1cb00..7d7965d 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -564,38 +564,11 @@ write_terms () {
 	then
 		die "$(gettext "please use two different terms")"
 	fi
-	check_term_format "$TERM_BAD" bad
-	check_term_format "$TERM_GOOD" good
+	git bisect--helper --check-term-format "$TERM_BAD" bad || exit
+	git bisect--helper --check-term-format "$TERM_GOOD" good || exit
 	printf '%s\n%s\n' "$TERM_BAD" "$TERM_GOOD" >"$GIT_DIR/BISECT_TERMS"
 }
 
-check_term_format () {
-	term=$1
-	git check-ref-format refs/bisect/"$term" ||
-	die "$(eval_gettext "'\$term' is not a valid term")"
-	case "$term" in
-	help|start|terms|skip|next|reset|visualize|replay|log|run)
-		die "$(eval_gettext "can't use the builtin command '\$term' as a term")"
-		;;
-	bad|new)
-		if test "$2" != bad
-		then
-			# In theory, nothing prevents swapping
-			# completely good and bad, but this situation
-			# could be confusing and hasn't been tested
-			# enough. Forbid it for now.
-			die "$(eval_gettext "can't change the meaning of term '\$term'")"
-		fi
-		;;
-	good|old)
-		if test "$2" != good
-		then
-			die "$(eval_gettext "can't change the meaning of term '\$term'")"
-		fi
-		;;
-	esac
-}
-
 check_and_set_terms () {
 	cmd="$1"
 	case "$cmd" in

--
https://github.com/git/git/pull/281

```

## Pranit Bauva, 2016-08-10 21:57

Subject: [PATCH v12 13/13] bisect--helper: `bisect_start` shell function partially in C
Message-ID: <010201567675ae72-0e0eeb11-e488-4562-8837-2443cbfe0792-000000@eu-west-1.amazonses.com>
URL: https://gitlist.dev/e/010201567675ae72-0e0eeb11-e488-4562-8837-2443cbfe0792-000000%40eu-west-1.amazonses.com
In-Reply-To: <010201567675adc1-17e27495-6b36-40d1-836d-814da029fcc4-000000@eu-west-1.amazonses.com>

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

Using `--bisect-start` subcommand is a temporary measure to port shell
function in 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.

Mentored-by: Lars Schneider <larsxschneider@gmail.com>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
---
 builtin/bisect--helper.c | 254 ++++++++++++++++++++++++++++++++++++++++++++++-
 git-bisect.sh            | 133 +------------------------
 2 files changed, 254 insertions(+), 133 deletions(-)

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index f912010..80116ba 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -7,6 +7,7 @@
 #include "argv-array.h"
 #include "run-command.h"
 #include "prompt.h"
+#include "quote.h"
 
 static GIT_PATH_FUNC(git_path_bisect_terms, "BISECT_TERMS")
 static GIT_PATH_FUNC(git_path_bisect_expected_rev, "BISECT_EXPECTED_REV")
@@ -27,6 +28,8 @@ static const char * const git_bisect_helper_usage[] = {
 	N_("git bisect--helper --bisect-check-and-set-terms <command> <TERM_GOOD> <TERM_BAD>"),
 	N_("git bisect--helper --bisect-next-check [<term>] <TERM_GOOD> <TERM_BAD"),
 	N_("git bisect--helper --bisect-terms [--term-good | --term-old | --term-bad | --term-new]"),
+	N_("git bisect--helper --bisect start [--term-{old,good}=<term> --term-{new,bad}=<term>]"
+					      "[--no-checkout] [<bad> [<good>...]] [--] [<paths>...]"),
 	NULL
 };
 
@@ -431,6 +434,244 @@ static int bisect_terms(struct bisect_terms *terms, const char **argv, int argc)
 	return 0;
 }
 
+static int bisect_start(struct bisect_terms *terms, int no_checkout,
+			const char **argv, int argc)
+{
+	int i, has_double_dash = 0, must_write_terms = 0, bad_seen = 0;
+	int flags, pathspec_pos;
+	struct string_list revs = STRING_LIST_INIT_DUP;
+	struct string_list states = STRING_LIST_INIT_DUP;
+	struct strbuf start_head = STRBUF_INIT;
+	struct strbuf bisect_names = STRBUF_INIT;
+	struct strbuf orig_args = STRBUF_INIT;
+	const char *head;
+	unsigned char sha1[20];
+	FILE *fp;
+	struct object_id oid;
+
+	if (is_bare_repository())
+		no_checkout = 1;
+
+	for (i = 0; i < argc; i++) {
+		char *commit_id = xstrfmt("%s^{commit}", argv[i]);
+		if (!strcmp(argv[i], "--")) {
+			has_double_dash = 1;
+			break;
+		}
+		else if (!strcmp(argv[i], "--no-checkout"))
+			no_checkout = 1;
+		else if (!strcmp(argv[i], "--term-good") ||
+			 !strcmp(argv[i], "--term-old")) {
+			must_write_terms = 1;
+			strbuf_reset(&terms->term_good);
+			strbuf_addstr(&terms->term_good, argv[++i]);
+		}
+		else if (skip_prefix(argv[i], "--term-good=", &argv[i])) {
+			must_write_terms = 1;
+			strbuf_reset(&terms->term_good);
+			strbuf_addstr(&terms->term_good, argv[i]);
+		}
+		else if (skip_prefix(argv[i], "--term-old=", &argv[i])) {
+			must_write_terms = 1;
+			strbuf_reset(&terms->term_good);
+			strbuf_addstr(&terms->term_good, argv[i]);
+		}
+		else if (!strcmp(argv[i], "--term-bad") ||
+			 !strcmp(argv[i], "--term-new")) {
+			must_write_terms = 1;
+			strbuf_reset(&terms->term_bad);
+			strbuf_addstr(&terms->term_bad, argv[++i]);
+		}
+		else if (skip_prefix(argv[i], "--term-bad=", &argv[i])) {
+			must_write_terms = 1;
+			strbuf_reset(&terms->term_bad);
+			strbuf_addstr(&terms->term_bad, argv[i]);
+		}
+		else if (skip_prefix(argv[i], "--term-new=", &argv[i])) {
+			must_write_terms = 1;
+			strbuf_reset(&terms->term_good);
+			strbuf_addstr(&terms->term_good, argv[i]);
+		}
+		else if (starts_with(argv[i], "--") &&
+			 !one_of(argv[i], "--term-good", "--term-bad", NULL)) {
+			string_list_clear(&revs, 0);
+			string_list_clear(&states, 0);
+			die(_("unrecognised option: '%s'"), argv[i]);
+		}
+		else if (get_oid(argv[i], &oid) && !has_double_dash) {
+			string_list_clear(&revs, 0);
+			string_list_clear(&states, 0);
+			free(commit_id);
+			die(_("'%s' does not appear to be a valid revision"), argv[i]);
+		}
+		else {
+			free(commit_id);
+			string_list_append(&revs, oid_to_hex(&oid));
+		}
+	}
+	pathspec_pos = i;
+
+	/*
+	 * The user ran "git bisect start <sha1> <sha1>", hence did not
+	 * explicitly specify the terms, but we are already starting to
+	 * set references named with the default terms, and won't be able
+	 * to change afterwards.
+	 */
+	must_write_terms |= !!revs.nr;
+	for (i = 0; i < revs.nr; i++) {
+		if (bad_seen)
+			string_list_append(&states, terms->term_good.buf);
+		else {
+			bad_seen = 1;
+			string_list_append(&states, terms->term_bad.buf);
+		}
+	}
+
+	/*
+	 * Verify HEAD
+	 */
+	head = resolve_ref_unsafe("HEAD", 0, sha1, &flags);
+	if (!head) {
+		if (get_sha1("HEAD", sha1)) {
+			string_list_clear(&revs, 0);
+			string_list_clear(&states, 0);
+			die(_("Bad HEAD - I need a HEAD"));
+		}
+	}
+	if (!is_empty_or_missing_file(git_path_bisect_start())) {
+		/* Reset to the rev from where we started */
+		strbuf_read_file(&start_head, git_path_bisect_start(), 0);
+		strbuf_trim(&start_head);
+		if (!no_checkout) {
+			struct argv_array argv = ARGV_ARRAY_INIT;
+			argv_array_pushl(&argv, "checkout", start_head.buf,
+					 "--", NULL);
+			if (run_command_v_opt(argv.argv, RUN_GIT_CMD)) {
+				error(_("checking out '%s' failed. Try 'git "
+					"bisect start <valid-branch>'."),
+				      start_head.buf);
+				strbuf_release(&start_head);
+				string_list_clear(&revs, 0);
+				string_list_clear(&states, 0);
+				return -1;
+			}
+		}
+	} else {
+		if (starts_with(head, "refs/heads/") ||
+		    !get_oid_hex(head, &oid) || ref_exists(head)) {
+			/*
+			 * This error message should only be triggered by
+			 * cogito usage, and cogito users should understand
+			 * it relates to cg-seek.
+			 */
+			if (!is_empty_or_missing_file(git_path_head_name())) {
+				strbuf_release(&start_head);
+				string_list_clear(&revs, 0);
+				string_list_clear(&states, 0);
+				die(_("won't bisect on cg-seek'ed tree"));
+			}
+			if (starts_with(head, "refs/heads/")) {
+				strbuf_reset(&start_head);
+				strbuf_addstr(&start_head, head + 11);
+			}
+			else {
+				strbuf_reset(&start_head);
+				strbuf_addstr(&start_head, sha1_to_hex(sha1));
+			}
+		} else {
+			strbuf_release(&start_head);
+			string_list_clear(&revs, 0);
+			string_list_clear(&states, 0);
+			die(_("Bad HEAD - strange symbolic ref"));
+		}
+	}
+
+	/*
+	 * Get rid of any old bisect state.
+	 */
+	if (bisect_clean_state()) {
+		return -1;
+	}
+	/*
+	 * In case of mistaken revs or checkout error, or signals received,
+	 * "bisect_auto_next" below may exit or misbehave.
+	 * We have to trap this to be able to clean up using
+	 * "bisect_clean_state".
+	 */
+
+	/*
+	 * Write new start state
+	 */
+	if (write_file(git_path_bisect_start(), "%s\n", start_head.buf)) {
+		bisect_clean_state();
+		strbuf_release(&start_head);
+		string_list_clear(&revs, 0);
+		string_list_clear(&states, 0);
+		return -1;
+	}
+
+	if (no_checkout) {
+		get_oid(start_head.buf, &oid);
+		if (update_ref(NULL, "BISECT_HEAD", oid.hash, NULL, 0,
+			       UPDATE_REFS_MSG_ON_ERR)) {
+			bisect_clean_state();
+			strbuf_release(&start_head);
+			string_list_clear(&revs, 0);
+			string_list_clear(&states, 0);
+			return -1;
+		}
+	}
+	strbuf_release(&start_head);
+
+	if (pathspec_pos < argc - 1)
+		sq_quote_argv(&bisect_names, argv + pathspec_pos, 0);
+	write_file(git_path_bisect_names(), "%s\n", bisect_names.buf);
+	strbuf_release(&bisect_names);
+
+	for (i = 0; i < states.nr; i++) {
+		if (bisect_write(states.items[i].string,
+				  revs.items[i].string, terms, 1)) {
+			bisect_clean_state();
+			string_list_clear(&revs, 0);
+			string_list_clear(&states, 0);
+			return -1;
+		}
+	}
+	string_list_clear(&revs, 0);
+	string_list_clear(&states, 0);
+
+	if (must_write_terms)
+		if (write_terms(terms->term_bad.buf, terms->term_good.buf)) {
+			bisect_clean_state();
+			return -1;
+		}
+
+	fp = fopen(git_path_bisect_log(), "a");
+	if (!fp) {
+		bisect_clean_state();
+		return -1;
+	}
+	if (fprintf(fp, "git bisect start") < 1) {
+		bisect_clean_state();
+		return -1;
+	}
+	sq_quote_argv(&orig_args, argv, 0);
+	if (fprintf(fp, "%s", orig_args.buf) < 0) {
+		bisect_clean_state();
+		strbuf_release(&orig_args);
+		return -1;
+	}
+	strbuf_release(&orig_args);
+	if (fprintf(fp, "\n") < 1) {
+		fclose(fp);
+		bisect_clean_state();
+		return -1;
+	}
+	fclose(fp);
+
+	return 0;
+}
+
 int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 {
 	enum {
@@ -442,7 +683,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 		BISECT_WRITE,
 		CHECK_AND_SET_TERMS,
 		BISECT_NEXT_CHECK,
-		BISECT_TERMS
+		BISECT_TERMS,
+		BISECT_START
 	} cmdmode = 0;
 	int no_checkout = 0, res = 0;
 	struct option options[] = {
@@ -464,6 +706,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 			 N_("check whether bad or good terms exist"), BISECT_NEXT_CHECK),
 		OPT_CMDMODE(0, "bisect-terms", &cmdmode,
 			 N_("print out the bisect terms"), BISECT_TERMS),
+		OPT_CMDMODE(0, "bisect-start", &cmdmode,
+			 N_("start the bisect session"), BISECT_START),
 		OPT_BOOL(0, "no-checkout", &no_checkout,
 			 N_("update BISECT_HEAD instead of checking out the current commit")),
 		OPT_END()
@@ -472,7 +716,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 	bisect_terms_init(&terms);
 
 	argc = parse_options(argc, argv, prefix, options,
-			     git_bisect_helper_usage, PARSE_OPT_KEEP_UNKNOWN);
+			     git_bisect_helper_usage,
+			     PARSE_OPT_KEEP_DASHDASH | PARSE_OPT_KEEP_UNKNOWN);
 
 	if (!cmdmode)
 		usage_with_options(git_bisect_helper_usage, options);
@@ -526,6 +771,11 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 			die(_("--bisect-terms requires 0 or 1 argument"));
 		res = bisect_terms(&terms, argv, argc);
 		break;
+	case BISECT_START:
+		strbuf_addstr(&terms.term_good, "good");
+		strbuf_addstr(&terms.term_bad, "bad");
+		res = bisect_start(&terms, no_checkout, argv, argc);
+		break;
 	default:
 		die("BUG: unknown subcommand '%d'", cmdmode);
 	}
diff --git a/git-bisect.sh b/git-bisect.sh
index aea97c5f..ee504ca 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -72,122 +72,7 @@ bisect_autostart() {
 }
 
 bisect_start() {
-	#
-	# Check for one bad and then some good revisions.
-	#
-	has_double_dash=0
-	for arg; do
-		case "$arg" in --) has_double_dash=1; break ;; esac
-	done
-	orig_args=$(git rev-parse --sq-quote "$@")
-	bad_seen=0
-	eval=''
-	must_write_terms=0
-	revs=''
-	if test "z$(git rev-parse --is-bare-repository)" != zfalse
-	then
-		mode=--no-checkout
-	else
-		mode=''
-	fi
-	while [ $# -gt 0 ]; do
-		arg="$1"
-		case "$arg" in
-		--)
-			shift
-			break
-		;;
-		--no-checkout)
-			mode=--no-checkout
-			shift ;;
-		--term-good|--term-old)
-			shift
-			must_write_terms=1
-			TERM_GOOD=$1
-			shift ;;
-		--term-good=*|--term-old=*)
-			must_write_terms=1
-			TERM_GOOD=${1#*=}
-			shift ;;
-		--term-bad|--term-new)
-			shift
-			must_write_terms=1
-			TERM_BAD=$1
-			shift ;;
-		--term-bad=*|--term-new=*)
-			must_write_terms=1
-			TERM_BAD=${1#*=}
-			shift ;;
-		--*)
-			die "$(eval_gettext "unrecognised option: '\$arg'")" ;;
-		*)
-			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"
-			shift
-			;;
-		esac
-	done
-
-	for rev in $revs
-	do
-		# The user ran "git bisect start <sha1>
-		# <sha1>", hence did not explicitly specify
-		# the terms, but we are already starting to
-		# set references named with the default terms,
-		# and won't be able to change afterwards.
-		must_write_terms=1
-
-		case $bad_seen in
-		0) state=$TERM_BAD ; bad_seen=1 ;;
-		*) state=$TERM_GOOD ;;
-		esac
-		eval="$eval git bisect--helper --bisect-write '$state' '$rev' '$TERM_GOOD' '$TERM_BAD' 'nolog' &&"
-	done
-	#
-	# Verify HEAD.
-	#
-	head=$(GIT_DIR="$GIT_DIR" git symbolic-ref -q HEAD) ||
-	head=$(GIT_DIR="$GIT_DIR" git rev-parse --verify HEAD) ||
-	die "$(gettext "Bad HEAD - I need a HEAD")"
-
-	#
-	# Check if we are bisecting.
-	#
-	start_head=''
-	if test -s "$GIT_DIR/BISECT_START"
-	then
-		# Reset to the rev from where we started.
-		start_head=$(cat "$GIT_DIR/BISECT_START")
-		if test "z$mode" != "z--no-checkout"
-		then
-			git checkout "$start_head" -- ||
-			die "$(eval_gettext "Checking out '\$start_head' failed. Try 'git bisect reset <valid-branch>'.")"
-		fi
-	else
-		# Get rev from where we start.
-		case "$head" in
-		refs/heads/*|$_x40)
-			# This error message should only be triggered by
-			# cogito usage, and cogito users should understand
-			# it relates to cg-seek.
-			[ -s "$GIT_DIR/head-name" ] &&
-				die "$(gettext "won't bisect on cg-seek'ed tree")"
-			start_head="${head#refs/heads/}"
-			;;
-		*)
-			die "$(gettext "Bad HEAD - strange symbolic ref")"
-			;;
-		esac
-	fi
-
-	#
-	# Get rid of any old bisect state.
-	#
-	git bisect--helper --bisect-clean-state || exit
+	git bisect--helper --bisect-start $@ || exit
 
 	#
 	# Change state.
@@ -198,24 +83,10 @@ bisect_start() {
 	#
 	trap 'git bisect--helper --bisect-clean-state' 0
 	trap 'exit 255' 1 2 3 15
-
-	#
-	# Write new start state.
-	#
-	echo "$start_head" >"$GIT_DIR/BISECT_START" && {
-		test "z$mode" != "z--no-checkout" ||
-		git update-ref --no-deref BISECT_HEAD "$start_head"
-	} &&
-	git rev-parse --sq-quote "$@" >"$GIT_DIR/BISECT_NAMES" &&
-	eval "$eval true" &&
-	if test $must_write_terms -eq 1
-	then
-		git bisect--helper --write-terms "$TERM_BAD" "$TERM_GOOD"
-	fi &&
-	echo "git bisect start$orig_args" >>"$GIT_DIR/BISECT_LOG" || exit
 	#
 	# Check if we can proceed to the next bisect state.
 	#
+	get_terms
 	bisect_auto_next
 
 	trap '-' 0

--
https://github.com/git/git/pull/281

```

## Pranit Bauva, 2016-08-10 21:57

Subject: [PATCH v12 01/13] bisect--helper: use OPT_CMDMODE instead of OPT_BOOL
Message-ID: <010201567675adc1-17e27495-6b36-40d1-836d-814da029fcc4-000000@eu-west-1.amazonses.com>
URL: https://gitlist.dev/e/010201567675adc1-17e27495-6b36-40d1-836d-814da029fcc4-000000%40eu-west-1.amazonses.com
In-Reply-To: <0102015640423c26-2060fd70-c90d-4de3-ae8c-1801ad160b1c-000000@eu-west-1.amazonses.com>

```
`--next-all` is meant to be used as a subcommand to support multiple
"operation mode" though the current implementation does not contain any
other subcommand along side with `--next-all` but further commits will
include some more subcommands.

Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Mentored-by: Lars Schneider <larsxschneider@gmail.com>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
---
 builtin/bisect--helper.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 3324229..8111c91 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -10,11 +10,11 @@ static const char * const git_bisect_helper_usage[] = {
 
 int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 {
-	int next_all = 0;
+	enum { NEXT_ALL = 1 } cmdmode = 0;
 	int no_checkout = 0;
 	struct option options[] = {
-		OPT_BOOL(0, "next-all", &next_all,
-			 N_("perform 'git bisect next'")),
+		OPT_CMDMODE(0, "next-all", &cmdmode,
+			 N_("perform 'git bisect next'"), NEXT_ALL),
 		OPT_BOOL(0, "no-checkout", &no_checkout,
 			 N_("update BISECT_HEAD instead of checking out the current commit")),
 		OPT_END()
@@ -23,9 +23,14 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 	argc = parse_options(argc, argv, prefix, options,
 			     git_bisect_helper_usage, 0);
 
-	if (!next_all)
+	if (!cmdmode)
 		usage_with_options(git_bisect_helper_usage, options);
 
-	/* next-all */
-	return bisect_next_all(prefix, no_checkout);
+	switch (cmdmode) {
+	case NEXT_ALL:
+		return bisect_next_all(prefix, no_checkout);
+	default:
+		die("BUG: unknown subcommand '%d'", cmdmode);
+	}
+	return 0;
 }

--
https://github.com/git/git/pull/281

```

## Pranit Bauva, 2016-08-10 21:57

Subject: [PATCH v12 12/13] bisect--helper: `get_terms` & `bisect_terms` shell function in C
Message-ID: <010201567675ae6a-d29ad308-fef5-44f2-8edf-6f7138e6cdf1-000000@eu-west-1.amazonses.com>
URL: https://gitlist.dev/e/010201567675ae6a-d29ad308-fef5-44f2-8edf-6f7138e6cdf1-000000%40eu-west-1.amazonses.com
In-Reply-To: <010201567675adc1-17e27495-6b36-40d1-836d-814da029fcc4-000000@eu-west-1.amazonses.com>

```
Reimplement the `get_terms` and `bisect_terms` shell function in C and
add `bisect-terms` subcommand to `git bisect--helper` to call it from
git-bisect.sh .

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

Mentored-by: Lars Schneider <larsxschneider@gmail.com>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
---
 builtin/bisect--helper.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++--
 git-bisect.sh            | 35 ++--------------------------
 2 files changed, 59 insertions(+), 35 deletions(-)

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index b6e9973..f912010 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -26,6 +26,7 @@ static const char * const git_bisect_helper_usage[] = {
 	N_("git bisect--helper --bisect-write <state> <revision> <TERM_GOOD> <TERM_BAD> [<nolog>]"),
 	N_("git bisect--helper --bisect-check-and-set-terms <command> <TERM_GOOD> <TERM_BAD>"),
 	N_("git bisect--helper --bisect-next-check [<term>] <TERM_GOOD> <TERM_BAD"),
+	N_("git bisect--helper --bisect-terms [--term-good | --term-old | --term-bad | --term-new]"),
 	NULL
 };
 
@@ -384,6 +385,52 @@ static int bisect_next_check(const struct bisect_terms *terms,
 	return 0;
 }
 
+static int get_terms(struct bisect_terms *terms)
+{
+	FILE *fp;
+	int res;
+	fp = fopen(git_path_bisect_terms(), "r");
+	if (!fp)
+		return -1;
+
+	bisect_terms_reset(terms);
+	res = strbuf_getline(&terms->term_bad, fp) == EOF ||
+	      strbuf_getline(&terms->term_good, fp) == EOF;
+
+	fclose(fp);
+	return res ? -1 : 0;
+}
+
+static int bisect_terms(struct bisect_terms *terms, const char **argv, int argc)
+{
+	int i;
+
+	if (get_terms(terms)) {
+		fprintf(stderr, N_("no terms defined\n"));
+		return -1;
+	}
+	if (argc == 0) {
+		printf(N_("Your current terms are %s for the old state\nand "
+		       "%s for the new state.\n"), terms->term_good.buf,
+		       terms->term_bad.buf);
+		return 0;
+	}
+
+	for (i = 0; i < argc; i++) {
+		if (!strcmp(argv[i], "--term-good"))
+			printf(N_("%s\n"), terms->term_good.buf);
+		else if (!strcmp(argv[i], "--term-bad"))
+			printf(N_("%s\n"), terms->term_bad.buf);
+		else
+			printf(N_("invalid argument %s for 'git bisect "
+				  "terms'.\nSupported options are: "
+				  "--term-good|--term-old and "
+				  "--term-bad|--term-new."), argv[i]);
+	}
+
+	return 0;
+}
+
 int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 {
 	enum {
@@ -394,7 +441,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 		CHECK_EXPECTED_REVS,
 		BISECT_WRITE,
 		CHECK_AND_SET_TERMS,
-		BISECT_NEXT_CHECK
+		BISECT_NEXT_CHECK,
+		BISECT_TERMS
 	} cmdmode = 0;
 	int no_checkout = 0, res = 0;
 	struct option options[] = {
@@ -414,6 +462,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 			 N_("check and set terms in a bisection state"), CHECK_AND_SET_TERMS),
 		OPT_CMDMODE(0, "bisect-next-check", &cmdmode,
 			 N_("check whether bad or good terms exist"), BISECT_NEXT_CHECK),
+		OPT_CMDMODE(0, "bisect-terms", &cmdmode,
+			 N_("print out the bisect terms"), BISECT_TERMS),
 		OPT_BOOL(0, "no-checkout", &no_checkout,
 			 N_("update BISECT_HEAD instead of checking out the current commit")),
 		OPT_END()
@@ -422,7 +472,7 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 	bisect_terms_init(&terms);
 
 	argc = parse_options(argc, argv, prefix, options,
-			     git_bisect_helper_usage, 0);
+			     git_bisect_helper_usage, PARSE_OPT_KEEP_UNKNOWN);
 
 	if (!cmdmode)
 		usage_with_options(git_bisect_helper_usage, options);
@@ -471,6 +521,11 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 		strbuf_addstr(&terms.term_bad, argv[1]);
 		res = bisect_next_check(&terms, argc == 3 ? argv[2] : NULL);
 		break;
+	case BISECT_TERMS:
+		if (argc > 1)
+			die(_("--bisect-terms requires 0 or 1 argument"));
+		res = bisect_terms(&terms, argv, argc);
+		break;
 	default:
 		die("BUG: unknown subcommand '%d'", cmdmode);
 	}
diff --git a/git-bisect.sh b/git-bisect.sh
index c2d6319..aea97c5f 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -355,7 +355,7 @@ bisect_replay () {
 		"$TERM_GOOD"|"$TERM_BAD"|skip)
 			git bisect--helper --bisect-write "$command" "$rev" "$TERM_GOOD" "$TERM_BAD" || exit;;
 		terms)
-			bisect_terms $rev ;;
+			git bisect--helper --bisect-terms $rev  || exit;;
 		*)
 			die "$(gettext "?? what are you talking about?")" ;;
 		esac
@@ -437,37 +437,6 @@ get_terms () {
 	fi
 }
 
-bisect_terms () {
-	get_terms
-	if ! test -s "$GIT_DIR/BISECT_TERMS"
-	then
-		die "$(gettext "no terms defined")"
-	fi
-	case "$#" in
-	0)
-		gettextln "Your current terms are $TERM_GOOD for the old state
-and $TERM_BAD for the new state."
-		;;
-	1)
-		arg=$1
-		case "$arg" in
-			--term-good|--term-old)
-				printf '%s\n' "$TERM_GOOD"
-				;;
-			--term-bad|--term-new)
-				printf '%s\n' "$TERM_BAD"
-				;;
-			*)
-				die "$(eval_gettext "invalid argument \$arg for 'git bisect terms'.
-Supported options are: --term-good|--term-old and --term-bad|--term-new.")"
-				;;
-		esac
-		;;
-	*)
-		usage ;;
-	esac
-}
-
 case "$#" in
 0)
 	usage ;;
@@ -498,7 +467,7 @@ case "$#" in
 	run)
 		bisect_run "$@" ;;
 	terms)
-		bisect_terms "$@" ;;
+		git bisect--helper --bisect-terms "$@" || exit;;
 	*)
 		usage ;;
 	esac

--
https://github.com/git/git/pull/281

```

## Pranit Bauva, 2016-08-10 21:57

Subject: [PATCH v12 04/13] bisect--helper: `bisect_clean_state` shell function in C
Message-ID: <010201567675ae50-4349bd9e-54a4-44e5-aabb-1d928126b237-000000@eu-west-1.amazonses.com>
URL: https://gitlist.dev/e/010201567675ae50-4349bd9e-54a4-44e5-aabb-1d928126b237-000000%40eu-west-1.amazonses.com
In-Reply-To: <010201567675adc1-17e27495-6b36-40d1-836d-814da029fcc4-000000@eu-west-1.amazonses.com>

```
Reimplement `bisect_clean_state` shell function in C and add a
`bisect-clean-state` subcommand to `git bisect--helper` to call it from
git-bisect.sh .

Using `--bisect-clean-state` subcommand is a 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 but its implementation  will
be called by bisect_reset() and bisect_start().

Mentored-by: Lars Schneider <larsxschneider@gmail.com>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
---
 builtin/bisect--helper.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++-
 git-bisect.sh            | 26 +++--------------------
 2 files changed, 57 insertions(+), 24 deletions(-)

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 30e1031..3fffa78 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -3,12 +3,21 @@
 #include "parse-options.h"
 #include "bisect.h"
 #include "refs.h"
+#include "dir.h"
 
 static GIT_PATH_FUNC(git_path_bisect_terms, "BISECT_TERMS")
+static GIT_PATH_FUNC(git_path_bisect_expected_rev, "BISECT_EXPECTED_REV")
+static GIT_PATH_FUNC(git_path_bisect_ancestors_ok, "BISECT_ANCESTORS_OK")
+static GIT_PATH_FUNC(git_path_bisect_log, "BISECT_LOG")
+static GIT_PATH_FUNC(git_path_bisect_names, "BISECT_NAMES")
+static GIT_PATH_FUNC(git_path_bisect_run, "BISECT_RUN")
+static GIT_PATH_FUNC(git_path_head_name, "head-name")
+static GIT_PATH_FUNC(git_path_bisect_start, "BISECT_START")
 
 static const char * const git_bisect_helper_usage[] = {
 	N_("git bisect--helper --next-all [--no-checkout]"),
 	N_("git bisect--helper --write-terms <bad_term> <good_term>"),
+	N_("git bisect--helper --bisect-clean-state"),
 	NULL
 };
 
@@ -79,11 +88,49 @@ static int write_terms(const char *bad, const char *good)
 	return (res < 0) ? -1 : 0;
 }
 
+static int mark_for_removal(const char *refname, const struct object_id *oid,
+			    int flag, void *cb_data)
+{
+	struct string_list *refs = cb_data;
+	char *ref = xstrfmt("refs/bisect/%s", refname);
+	string_list_append(refs, ref);
+	return 0;
+}
+
+static int bisect_clean_state(void)
+{
+	int result = 0;
+
+	/* There may be some refs packed during bisection */
+	struct string_list refs_for_removal = STRING_LIST_INIT_NODUP;
+	for_each_ref_in("refs/bisect/", mark_for_removal, (void *) &refs_for_removal);
+	string_list_append(&refs_for_removal, xstrdup("BISECT_HEAD"));
+	result = delete_refs(&refs_for_removal);
+	refs_for_removal.strdup_strings = 1;
+	string_list_clear(&refs_for_removal, 0);
+	unlink_or_warn(git_path_bisect_expected_rev());
+	unlink_or_warn(git_path_bisect_ancestors_ok());
+	unlink_or_warn(git_path_bisect_log());
+	unlink_or_warn(git_path_bisect_names());
+	unlink_or_warn(git_path_bisect_run());
+	unlink_or_warn(git_path_bisect_terms());
+	/* Cleanup head-name if it got left by an old version of git-bisect */
+	unlink_or_warn(git_path_head_name());
+	/*
+	 * Cleanup BISECT_START last to support the --no-checkout option
+	 * introduced in the commit 4796e823a.
+	 */
+	unlink_or_warn(git_path_bisect_start());
+
+	return result;
+}
+
 int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 {
 	enum {
 		NEXT_ALL = 1,
-		WRITE_TERMS
+		WRITE_TERMS,
+		BISECT_CLEAN_STATE
 	} cmdmode = 0;
 	int no_checkout = 0;
 	struct option options[] = {
@@ -91,6 +138,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 			 N_("perform 'git bisect next'"), NEXT_ALL),
 		OPT_CMDMODE(0, "write-terms", &cmdmode,
 			 N_("write the terms to .git/BISECT_TERMS"), WRITE_TERMS),
+		OPT_CMDMODE(0, "bisect-clean-state", &cmdmode,
+			 N_("cleanup the bisection state"), BISECT_CLEAN_STATE),
 		OPT_BOOL(0, "no-checkout", &no_checkout,
 			 N_("update BISECT_HEAD instead of checking out the current commit")),
 		OPT_END()
@@ -109,6 +158,10 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 		if (argc != 2)
 			die(_("--write-terms requires two arguments"));
 		return write_terms(argv[0], argv[1]);
+	case BISECT_CLEAN_STATE:
+		if (argc != 0)
+			die(_("--bisect-clean-state requires no arguments"));
+		return bisect_clean_state();
 	default:
 		die("BUG: unknown subcommand '%d'", cmdmode);
 	}
diff --git a/git-bisect.sh b/git-bisect.sh
index cd39bd0..bbc57d2 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -187,7 +187,7 @@ bisect_start() {
 	#
 	# Get rid of any old bisect state.
 	#
-	bisect_clean_state || exit
+	git bisect--helper --bisect-clean-state || exit
 
 	#
 	# Change state.
@@ -196,7 +196,7 @@ bisect_start() {
 	# We have to trap this to be able to clean up using
 	# "bisect_clean_state".
 	#
-	trap 'bisect_clean_state' 0
+	trap 'git bisect--helper --bisect-clean-state' 0
 	trap 'exit 255' 1 2 3 15
 
 	#
@@ -430,27 +430,7 @@ bisect_reset() {
 		die "$(eval_gettext "Could not check out original HEAD '\$branch'.
 Try 'git bisect reset <commit>'.")"
 	fi
-	bisect_clean_state
-}
-
-bisect_clean_state() {
-	# There may be some refs packed during bisection.
-	git for-each-ref --format='%(refname) %(objectname)' refs/bisect/\* |
-	while read ref hash
-	do
-		git update-ref -d $ref $hash || exit
-	done
-	rm -f "$GIT_DIR/BISECT_EXPECTED_REV" &&
-	rm -f "$GIT_DIR/BISECT_ANCESTORS_OK" &&
-	rm -f "$GIT_DIR/BISECT_LOG" &&
-	rm -f "$GIT_DIR/BISECT_NAMES" &&
-	rm -f "$GIT_DIR/BISECT_RUN" &&
-	rm -f "$GIT_DIR/BISECT_TERMS" &&
-	# Cleanup head-name if it got left by an old version of git-bisect
-	rm -f "$GIT_DIR/head-name" &&
-	git update-ref -d --no-deref BISECT_HEAD &&
-	# clean up BISECT_START last
-	rm -f "$GIT_DIR/BISECT_START"
+	git bisect--helper --bisect-clean-state || exit
 }
 
 bisect_replay () {

--
https://github.com/git/git/pull/281

```

## Pranit Bauva, 2016-08-10 21:57

Subject: [PATCH v12 08/13] bisect--helper: `is_expected_rev` & `check_expected_revs` shell function in C
Message-ID: <010201567675ae5e-b4ebeeb9-290f-4376-a2b3-daced1c78efe-000000@eu-west-1.amazonses.com>
URL: https://gitlist.dev/e/010201567675ae5e-b4ebeeb9-290f-4376-a2b3-daced1c78efe-000000%40eu-west-1.amazonses.com
In-Reply-To: <010201567675adc1-17e27495-6b36-40d1-836d-814da029fcc4-000000@eu-west-1.amazonses.com>

```
Reimplement `is_expected_rev` & `check_expected_revs` shell function in
C and add a `--check-expected-revs` subcommand to `git bisect--helper` to
call it from git-bisect.sh .

Using `--check-expected-revs` subcommand is a temporary measure to port
shell functions to C so as to use the existing test suite. As more
functions are ported, this subcommand would be retired but its
implementation will be called by some other method.

Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Mentored-by: Lars Schneider <larsxschneider@gmail.com>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
---
 builtin/bisect--helper.c | 33 ++++++++++++++++++++++++++++++++-
 git-bisect.sh            | 20 ++------------------
 2 files changed, 34 insertions(+), 19 deletions(-)

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 409e268..c03042f 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -163,13 +163,40 @@ static int bisect_reset(const char *commit)
 	return bisect_clean_state();
 }
 
+static int is_expected_rev(const char *expected_hex)
+{
+	struct strbuf actual_hex = STRBUF_INIT;
+	int res = 0;
+	if (strbuf_read_file(&actual_hex, git_path_bisect_expected_rev(), 0) >= 0) {
+		strbuf_trim(&actual_hex);
+		res = !strcmp(actual_hex.buf, expected_hex);
+	}
+	strbuf_release(&actual_hex);
+	return res;
+}
+
+static int check_expected_revs(const char **revs, int rev_nr)
+{
+	int i;
+
+	for (i = 0; i < rev_nr; i++) {
+		if (!is_expected_rev(revs[i])) {
+			unlink_or_warn(git_path_bisect_ancestors_ok());
+			unlink_or_warn(git_path_bisect_expected_rev());
+			return 0;
+		}
+	}
+	return 0;
+}
+
 int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 {
 	enum {
 		NEXT_ALL = 1,
 		WRITE_TERMS,
 		BISECT_CLEAN_STATE,
-		BISECT_RESET
+		BISECT_RESET,
+		CHECK_EXPECTED_REVS
 	} cmdmode = 0;
 	int no_checkout = 0;
 	struct option options[] = {
@@ -181,6 +208,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 			 N_("cleanup the bisection state"), BISECT_CLEAN_STATE),
 		OPT_CMDMODE(0, "bisect-reset", &cmdmode,
 			 N_("reset the bisection state"), BISECT_RESET),
+		OPT_CMDMODE(0, "check-expected-revs", &cmdmode,
+			 N_("check for expected revs"), CHECK_EXPECTED_REVS),
 		OPT_BOOL(0, "no-checkout", &no_checkout,
 			 N_("update BISECT_HEAD instead of checking out the current commit")),
 		OPT_END()
@@ -207,6 +236,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 		if (argc > 1)
 			die(_("--bisect-reset requires either zero or one arguments"));
 		return bisect_reset(argc ? argv[0] : NULL);
+	case CHECK_EXPECTED_REVS:
+		return check_expected_revs(argv, argc);
 	default:
 		die("BUG: unknown subcommand '%d'", cmdmode);
 	}
diff --git a/git-bisect.sh b/git-bisect.sh
index 18580b7..4f6545e 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -238,22 +238,6 @@ bisect_write() {
 	test -n "$nolog" || echo "git bisect $state $rev" >>"$GIT_DIR/BISECT_LOG"
 }
 
-is_expected_rev() {
-	test -f "$GIT_DIR/BISECT_EXPECTED_REV" &&
-	test "$1" = $(cat "$GIT_DIR/BISECT_EXPECTED_REV")
-}
-
-check_expected_revs() {
-	for _rev in "$@"; do
-		if ! is_expected_rev "$_rev"
-		then
-			rm -f "$GIT_DIR/BISECT_ANCESTORS_OK"
-			rm -f "$GIT_DIR/BISECT_EXPECTED_REV"
-			return
-		fi
-	done
-}
-
 bisect_skip() {
 	all=''
 	for arg in "$@"
@@ -280,7 +264,7 @@ bisect_state() {
 		rev=$(git rev-parse --verify $(bisect_head)) ||
 			die "$(gettext "Bad rev input: $(bisect_head)")"
 		bisect_write "$state" "$rev"
-		check_expected_revs "$rev" ;;
+		git bisect--helper --check-expected-revs "$rev" ;;
 	2,"$TERM_BAD"|*,"$TERM_GOOD"|*,skip)
 		shift
 		hash_list=''
@@ -294,7 +278,7 @@ bisect_state() {
 		do
 			bisect_write "$state" "$rev"
 		done
-		check_expected_revs $hash_list ;;
+		git bisect--helper --check-expected-revs $hash_list ;;
 	*,"$TERM_BAD")
 		die "$(eval_gettext "'git bisect \$TERM_BAD' can take only one argument.")" ;;
 	*)

--
https://github.com/git/git/pull/281

```

## Pranit Bauva, 2016-08-10 21:57

Subject: [PATCH v12 05/13] t6030: explicitly test for bisection cleanup
Message-ID: <010201567675ae4c-1fd14181-7d42-4099-ac1c-1fb5c1902ce4-000000@eu-west-1.amazonses.com>
URL: https://gitlist.dev/e/010201567675ae4c-1fd14181-7d42-4099-ac1c-1fb5c1902ce4-000000%40eu-west-1.amazonses.com
In-Reply-To: <010201567675adc1-17e27495-6b36-40d1-836d-814da029fcc4-000000@eu-west-1.amazonses.com>

```
Add test to explicitly check that 'git bisect reset' is working as
expected. This is already covered implicitly by the test suite.

Mentored-by: Lars Schneider <larsxschneider@gmail.com>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>

---
I faced this problem while converting `bisect_clean_state` and the tests
where showing breakages but it wasn't clear as to where exactly are they
breaking. This will patch  will help in that. Also I tested the test
coverage of the test suite before this patch and it covers this (I did
this by purposely changing names of files in git-bisect.sh and running
the test suite).
---
 t/t6030-bisect-porcelain.sh | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index e74662b..a17f7a6 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -894,4 +894,21 @@ test_expect_success 'bisect start takes options and revs in any order' '
 	test_cmp expected actual
 '
 
+test_expect_success 'git bisect reset cleans bisection state properly' '
+	git bisect reset &&
+	git bisect start &&
+	git bisect good $HASH1 &&
+	git bisect bad $HASH4 &&
+	git bisect reset &&
+	test -z "$(git for-each-ref "refs/bisect/*")" &&
+	test_path_is_missing "$GIT_DIR/BISECT_EXPECTED_REV" &&
+	test_path_is_missing "$GIT_DIR/BISECT_ANCESTORS_OK" &&
+	test_path_is_missing "$GIT_DIR/BISECT_LOG" &&
+	test_path_is_missing "$GIT_DIR/BISECT_RUN" &&
+	test_path_is_missing "$GIT_DIR/BISECT_TERMS" &&
+	test_path_is_missing "$GIT_DIR/head-name" &&
+	test_path_is_missing "$GIT_DIR/BISECT_HEAD" &&
+	test_path_is_missing "$GIT_DIR/BISECT_START"
+'
+
 test_done

--
https://github.com/git/git/pull/281

```

## Pranit Bauva, 2016-08-10 22:19

Subject: Re: [PATCH v12 01/13] bisect--helper: use OPT_CMDMODE instead of OPT_BOOL
Message-ID: <CAFZEwPNMOE4yBu8mRfYNW=w2FbkC03=C2W44TxUC0TF9sSx46g@mail.gmail.com>
URL: https://gitlist.dev/e/CAFZEwPNMOE4yBu8mRfYNW%3Dw2FbkC03%3DC2W44TxUC0TF9sSx46g%40mail.gmail.com
In-Reply-To: <010201567675adc1-17e27495-6b36-40d1-836d-814da029fcc4-000000@eu-west-1.amazonses.com>

```
Here[1] is the link for interdiff. Sorry could not send a cover patch
or put it in here. I am under a proxy which blocks IMAP/SMTP
connections and gmail wraps the lines.

[1]: http://paste.ubuntu.com/22794990/

Regards,
Pranit Bauva

```

## Pranit Bauva, 2016-08-10 21:57

Subject: [PATCH v12 09/13] bisect--helper: `bisect_write` shell function in C
Message-ID: <010201567675ae66-dc43cdd8-01cf-42b6-9823-9dfa79367008-000000@eu-west-1.amazonses.com>
URL: https://gitlist.dev/e/010201567675ae66-dc43cdd8-01cf-42b6-9823-9dfa79367008-000000%40eu-west-1.amazonses.com
In-Reply-To: <010201567675adc1-17e27495-6b36-40d1-836d-814da029fcc4-000000@eu-west-1.amazonses.com>

```
Reimplement the `bisect_write` shell function in C and add a
`bisect-write` subcommand to `git bisect--helper` to call it from
git-bisect.sh

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

Note: bisect_write() uses two variables namely TERM_GOOD and TERM_BAD
from the global shell script thus we need to pass it to the subcommand
using the arguments. We then store them in a struct bisect_terms and
pass the memory address around functions.

This patch also introduces new methods namely bisect_state_init() and
bisect_terms_release() for easy memory management for the struct
bisect_terms.

Mentored-by: Lars Schneider <larsxschneider@gmail.com>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
---
 builtin/bisect--helper.c | 97 ++++++++++++++++++++++++++++++++++++++++++++----
 git-bisect.sh            | 25 ++-----------
 2 files changed, 94 insertions(+), 28 deletions(-)

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index c03042f..9f70edb 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -22,9 +22,27 @@ static const char * const git_bisect_helper_usage[] = {
 	N_("git bisect--helper --write-terms <bad_term> <good_term>"),
 	N_("git bisect--helper --bisect-clean-state"),
 	N_("git bisect--helper --bisect-reset [<commit>]"),
+	N_("git bisect--helper --bisect-write <state> <revision> <TERM_GOOD> <TERM_BAD> [<nolog>]"),
 	NULL
 };
 
+struct bisect_terms {
+	struct strbuf term_good;
+	struct strbuf term_bad;
+};
+
+static void bisect_terms_init(struct bisect_terms *terms)
+{
+	strbuf_init(&terms->term_good, 0);
+	strbuf_init(&terms->term_bad, 0);
+}
+
+static void bisect_terms_release(struct bisect_terms *terms)
+{
+	strbuf_release(&terms->term_good);
+	strbuf_release(&terms->term_bad);
+}
+
 /*
  * Check whether the string `term` belongs to the set of strings
  * included in the variable arguments.
@@ -189,6 +207,52 @@ static int check_expected_revs(const char **revs, int rev_nr)
 	return 0;
 }
 
+static int bisect_write(const char *state, const char *rev,
+			const struct bisect_terms *terms, int nolog)
+{
+	struct strbuf tag = STRBUF_INIT;
+	struct strbuf commit_name = STRBUF_INIT;
+	struct object_id oid;
+	struct commit *commit;
+	struct pretty_print_context pp = {0};
+	FILE *fp;
+
+	if (!strcmp(state, terms->term_bad.buf))
+		strbuf_addf(&tag, "refs/bisect/%s", state);
+	else if (one_of(state, terms->term_good.buf, "skip", NULL))
+		strbuf_addf(&tag, "refs/bisect/%s-%s", state, rev);
+	else
+		return error(_("Bad bisect_write argument: %s"), state);
+
+	if (get_oid(rev, &oid)) {
+		strbuf_release(&tag);
+		return error(_("couldn't get the oid of the rev '%s'"), rev);
+	}
+
+	if (update_ref(NULL, tag.buf, oid.hash, NULL, 0,
+		       UPDATE_REFS_MSG_ON_ERR)) {
+		strbuf_release(&tag);
+		return -1;
+	}
+	strbuf_release(&tag);
+
+	fp = fopen(git_path_bisect_log(), "a");
+	if (!fp)
+		return error_errno(_("couldn't open the file '%s'"), git_path_bisect_log());
+
+	commit = lookup_commit_reference(oid.hash);
+	format_commit_message(commit, "%s", &commit_name, &pp);
+	fprintf(fp, "# %s: [%s] %s\n", state, sha1_to_hex(oid.hash),
+		commit_name.buf);
+	strbuf_release(&commit_name);
+
+	if (!nolog)
+		fprintf(fp, "git bisect %s %s\n", state, rev);
+
+	fclose(fp);
+	return 0;
+}
+
 int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 {
 	enum {
@@ -196,9 +260,10 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 		WRITE_TERMS,
 		BISECT_CLEAN_STATE,
 		BISECT_RESET,
-		CHECK_EXPECTED_REVS
+		CHECK_EXPECTED_REVS,
+		BISECT_WRITE
 	} cmdmode = 0;
-	int no_checkout = 0;
+	int no_checkout = 0, res = 0;
 	struct option options[] = {
 		OPT_CMDMODE(0, "next-all", &cmdmode,
 			 N_("perform 'git bisect next'"), NEXT_ALL),
@@ -210,10 +275,14 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 			 N_("reset the bisection state"), BISECT_RESET),
 		OPT_CMDMODE(0, "check-expected-revs", &cmdmode,
 			 N_("check for expected revs"), CHECK_EXPECTED_REVS),
+		OPT_CMDMODE(0, "bisect-write", &cmdmode,
+			 N_("write out the bisection state in BISECT_LOG"), BISECT_WRITE),
 		OPT_BOOL(0, "no-checkout", &no_checkout,
 			 N_("update BISECT_HEAD instead of checking out the current commit")),
 		OPT_END()
 	};
+	struct bisect_terms terms;
+	bisect_terms_init(&terms);
 
 	argc = parse_options(argc, argv, prefix, options,
 			     git_bisect_helper_usage, 0);
@@ -222,24 +291,38 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 		usage_with_options(git_bisect_helper_usage, options);
 
 	switch (cmdmode) {
+	int nolog;
 	case NEXT_ALL:
 		return bisect_next_all(prefix, no_checkout);
 	case WRITE_TERMS:
 		if (argc != 2)
 			die(_("--write-terms requires two arguments"));
-		return write_terms(argv[0], argv[1]);
+		res = write_terms(argv[0], argv[1]);
+		break;
 	case BISECT_CLEAN_STATE:
 		if (argc != 0)
 			die(_("--bisect-clean-state requires no arguments"));
-		return bisect_clean_state();
+		res = bisect_clean_state();
+		break;
 	case BISECT_RESET:
 		if (argc > 1)
 			die(_("--bisect-reset requires either zero or one arguments"));
-		return bisect_reset(argc ? argv[0] : NULL);
+		res = bisect_reset(argc ? argv[0] : NULL);
+		break;
 	case CHECK_EXPECTED_REVS:
-		return check_expected_revs(argv, argc);
+		res = check_expected_revs(argv, argc);
+		break;
+	case BISECT_WRITE:
+		if (argc != 4 && argc != 5)
+			die(_("--bisect-write requires either 4 or 5 arguments"));
+		nolog = (argc == 5) && !strcmp(argv[4], "nolog");
+		strbuf_addstr(&terms.term_good, argv[2]);
+		strbuf_addstr(&terms.term_bad, argv[3]);
+		res = bisect_write(argv[0], argv[1], &terms, nolog);
+		break;
 	default:
 		die("BUG: unknown subcommand '%d'", cmdmode);
 	}
-	return 0;
+	bisect_terms_release(&terms);
+	return res;
 }
diff --git a/git-bisect.sh b/git-bisect.sh
index 4f6545e..b9896a4 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -145,7 +145,7 @@ bisect_start() {
 		0) state=$TERM_BAD ; bad_seen=1 ;;
 		*) state=$TERM_GOOD ;;
 		esac
-		eval="$eval bisect_write '$state' '$rev' 'nolog' &&"
+		eval="$eval git bisect--helper --bisect-write '$state' '$rev' '$TERM_GOOD' '$TERM_BAD' 'nolog' &&"
 	done
 	#
 	# Verify HEAD.
@@ -221,23 +221,6 @@ bisect_start() {
 	trap '-' 0
 }
 
-bisect_write() {
-	state="$1"
-	rev="$2"
-	nolog="$3"
-	case "$state" in
-		"$TERM_BAD")
-			tag="$state" ;;
-		"$TERM_GOOD"|skip)
-			tag="$state"-"$rev" ;;
-		*)
-			die "$(eval_gettext "Bad bisect_write argument: \$state")" ;;
-	esac
-	git update-ref "refs/bisect/$tag" "$rev" || exit
-	echo "# $state: $(git show-branch $rev)" >>"$GIT_DIR/BISECT_LOG"
-	test -n "$nolog" || echo "git bisect $state $rev" >>"$GIT_DIR/BISECT_LOG"
-}
-
 bisect_skip() {
 	all=''
 	for arg in "$@"
@@ -263,7 +246,7 @@ bisect_state() {
 	1,"$TERM_BAD"|1,"$TERM_GOOD"|1,skip)
 		rev=$(git rev-parse --verify $(bisect_head)) ||
 			die "$(gettext "Bad rev input: $(bisect_head)")"
-		bisect_write "$state" "$rev"
+		git bisect--helper --bisect-write "$state" "$rev" "$TERM_GOOD" "$TERM_BAD" || exit
 		git bisect--helper --check-expected-revs "$rev" ;;
 	2,"$TERM_BAD"|*,"$TERM_GOOD"|*,skip)
 		shift
@@ -276,7 +259,7 @@ bisect_state() {
 		done
 		for rev in $hash_list
 		do
-			bisect_write "$state" "$rev"
+			git bisect--helper --bisect-write "$state" "$rev" "$TERM_GOOD" "$TERM_BAD" || exit
 		done
 		git bisect--helper --check-expected-revs $hash_list ;;
 	*,"$TERM_BAD")
@@ -413,7 +396,7 @@ bisect_replay () {
 			cmd="bisect_start $rev"
 			eval "$cmd" ;;
 		"$TERM_GOOD"|"$TERM_BAD"|skip)
-			bisect_write "$command" "$rev" ;;
+			git bisect--helper --bisect-write "$command" "$rev" "$TERM_GOOD" "$TERM_BAD" || exit;;
 		terms)
 			bisect_terms $rev ;;
 		*)

--
https://github.com/git/git/pull/281

```

## Pranit Bauva, 2016-08-10 21:57

Subject: [PATCH v12 11/13] bisect--helper: `bisect_next_check` & bisect_voc shell function in C
Message-ID: <010201567675ae68-15d79b9a-83fc-44ee-8607-07c2d0191c59-000000@eu-west-1.amazonses.com>
URL: https://gitlist.dev/e/010201567675ae68-15d79b9a-83fc-44ee-8607-07c2d0191c59-000000%40eu-west-1.amazonses.com
In-Reply-To: <010201567675adc1-17e27495-6b36-40d1-836d-814da029fcc4-000000@eu-west-1.amazonses.com>

```
Reimplement `bisect_next_check` shell function in C and add
`bisect-next-check` subcommand to `git bisect--helper` to call it from
git-bisect.sh .

Also reimplement `bisect_voc` shell function in C and call it from
`bisect_next_check` implementation in C.

Using `--bisect-next-check` 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 but its implementation will
be called by some other methods.

Mentored-by: Lars Schneider <larsxschneider@gmail.com>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
---
 builtin/bisect--helper.c | 103 ++++++++++++++++++++++++++++++++++++++++++++++-
 git-bisect.sh            |  60 ++-------------------------
 2 files changed, 106 insertions(+), 57 deletions(-)

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 5c4350f..b6e9973 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -6,6 +6,7 @@
 #include "dir.h"
 #include "argv-array.h"
 #include "run-command.h"
+#include "prompt.h"
 
 static GIT_PATH_FUNC(git_path_bisect_terms, "BISECT_TERMS")
 static GIT_PATH_FUNC(git_path_bisect_expected_rev, "BISECT_EXPECTED_REV")
@@ -24,6 +25,7 @@ static const char * const git_bisect_helper_usage[] = {
 	N_("git bisect--helper --bisect-reset [<commit>]"),
 	N_("git bisect--helper --bisect-write <state> <revision> <TERM_GOOD> <TERM_BAD> [<nolog>]"),
 	N_("git bisect--helper --bisect-check-and-set-terms <command> <TERM_GOOD> <TERM_BAD>"),
+	N_("git bisect--helper --bisect-next-check [<term>] <TERM_GOOD> <TERM_BAD"),
 	NULL
 };
 
@@ -293,6 +295,95 @@ static int check_and_set_terms(struct bisect_terms *terms, const char *cmd)
 	return 0;
 }
 
+static int mark_good(const char *refname, const struct object_id *oid,
+		     int flag, void *cb_data)
+{
+	int *m_good = (int *)cb_data;
+	*m_good = 0;
+	return 1;
+}
+
+static char *bisect_voc(char *revision_type)
+{
+	if (!strcmp(revision_type, "bad"))
+		return "bad|new";
+	if (!strcmp(revision_type, "good"))
+		return "good|old";
+
+	return NULL;
+}
+
+static int bisect_next_check(const struct bisect_terms *terms,
+			     const char *current_term)
+{
+	int missing_good = 1, missing_bad = 1;
+	char *bad_ref = xstrfmt("refs/bisect/%s", terms->term_bad.buf);
+	char *good_glob = xstrfmt("%s-*", terms->term_good.buf);
+	char *bad_syn, *good_syn;
+
+	if (ref_exists(bad_ref))
+		missing_bad = 0;
+	free(bad_ref);
+
+	for_each_glob_ref_in(mark_good, good_glob, "refs/bisect/",
+			     (void *) &missing_good);
+	free(good_glob);
+
+	if (!missing_good && !missing_bad)
+		return 0;
+
+	if (!current_term)
+		return -1;
+
+	if (missing_good && !missing_bad && current_term &&
+	    !strcmp(current_term, terms->term_good.buf)) {
+		char *yesno;
+		/*
+		 * have bad (or new) but not good (or old). We could bisect
+		 * although this is less optimum.
+		 */
+		fprintf(stderr, N_("Warning: bisecting only with a %s commit\n"),
+			terms->term_bad.buf);
+		if (!isatty(0))
+			return 0;
+		/*
+		 * TRANSLATORS: Make sure to include [Y] and [n] in your
+		 * translation. The program will only accept English input
+		 * at this point.
+		 */
+		yesno = git_prompt(_("Are you sure [Y/n]? "), PROMPT_ECHO);
+		if (starts_with(yesno, "N") || starts_with(yesno, "n"))
+			return -1;
+
+		return 0;
+	}
+	bad_syn = xstrdup(bisect_voc("bad"));
+	good_syn = xstrdup(bisect_voc("good"));
+	if (!is_empty_or_missing_file(git_path_bisect_start())) {
+		error(_("You need to give me at least one %s and "
+			"%s revision. You can use \"git bisect %s\" "
+			"and \"git bisect %s\" for that. \n"),
+			bad_syn, good_syn, bad_syn, good_syn);
+		free(bad_syn);
+		free(good_syn);
+		return -1;
+	}
+	else {
+		error(_("You need to start by \"git bisect start\". You "
+			"then need to give me at least one %s and %s "
+			"revision. You can use \"git bisect %s\" and "
+			"\"git bisect %s\" for that.\n"),
+			good_syn, bad_syn, bad_syn, good_syn);
+		free(bad_syn);
+		free(good_syn);
+		return -1;
+	}
+	free(bad_syn);
+	free(good_syn);
+
+	return 0;
+}
+
 int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 {
 	enum {
@@ -302,7 +393,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 		BISECT_RESET,
 		CHECK_EXPECTED_REVS,
 		BISECT_WRITE,
-		CHECK_AND_SET_TERMS
+		CHECK_AND_SET_TERMS,
+		BISECT_NEXT_CHECK
 	} cmdmode = 0;
 	int no_checkout = 0, res = 0;
 	struct option options[] = {
@@ -320,6 +412,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 			 N_("write out the bisection state in BISECT_LOG"), BISECT_WRITE),
 		OPT_CMDMODE(0, "check-and-set-terms", &cmdmode,
 			 N_("check and set terms in a bisection state"), CHECK_AND_SET_TERMS),
+		OPT_CMDMODE(0, "bisect-next-check", &cmdmode,
+			 N_("check whether bad or good terms exist"), BISECT_NEXT_CHECK),
 		OPT_BOOL(0, "no-checkout", &no_checkout,
 			 N_("update BISECT_HEAD instead of checking out the current commit")),
 		OPT_END()
@@ -370,6 +464,13 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 		strbuf_addstr(&terms.term_bad, argv[2]);
 		res = check_and_set_terms(&terms, argv[0]);
 		break;
+	case BISECT_NEXT_CHECK:
+		if (argc != 2 && argc != 3)
+			die(_("--bisect-next-check requires 2 or 3 arguments"));
+		strbuf_addstr(&terms.term_good, argv[0]);
+		strbuf_addstr(&terms.term_bad, argv[1]);
+		res = bisect_next_check(&terms, argc == 3 ? argv[2] : NULL);
+		break;
 	default:
 		die("BUG: unknown subcommand '%d'", cmdmode);
 	}
diff --git a/git-bisect.sh b/git-bisect.sh
index a41e69b..c2d6319 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -271,59 +271,14 @@ bisect_state() {
 	bisect_auto_next
 }
 
-bisect_next_check() {
-	missing_good= missing_bad=
-	git show-ref -q --verify refs/bisect/$TERM_BAD || missing_bad=t
-	test -n "$(git for-each-ref "refs/bisect/$TERM_GOOD-*")" || missing_good=t
-
-	case "$missing_good,$missing_bad,$1" in
-	,,*)
-		: have both $TERM_GOOD and $TERM_BAD - ok
-		;;
-	*,)
-		# do not have both but not asked to fail - just report.
-		false
-		;;
-	t,,"$TERM_GOOD")
-		# have bad (or new) but not good (or old).  we could bisect although
-		# this is less optimum.
-		eval_gettextln "Warning: bisecting only with a \$TERM_BAD commit." >&2
-		if test -t 0
-		then
-			# TRANSLATORS: Make sure to include [Y] and [n] in your
-			# translation. The program will only accept English input
-			# at this point.
-			gettext "Are you sure [Y/n]? " >&2
-			read yesno
-			case "$yesno" in [Nn]*) exit 1 ;; esac
-		fi
-		: bisect without $TERM_GOOD...
-		;;
-	*)
-		bad_syn=$(bisect_voc bad)
-		good_syn=$(bisect_voc good)
-		if test -s "$GIT_DIR/BISECT_START"
-		then
-
-			eval_gettextln "You need to give me at least one \$bad_syn and one \$good_syn revision.
-(You can use \"git bisect \$bad_syn\" and \"git bisect \$good_syn\" for that.)" >&2
-		else
-			eval_gettextln "You need to start by \"git bisect start\".
-You then need to give me at least one \$good_syn and one \$bad_syn revision.
-(You can use \"git bisect \$bad_syn\" and \"git bisect \$good_syn\" for that.)" >&2
-		fi
-		exit 1 ;;
-	esac
-}
-
 bisect_auto_next() {
-	bisect_next_check && bisect_next || :
+	git bisect--helper --bisect-next-check $TERM_GOOD $TERM_BAD && bisect_next || :
 }
 
 bisect_next() {
 	case "$#" in 0) ;; *) usage ;; esac
 	bisect_autostart
-	bisect_next_check $TERM_GOOD
+	git bisect--helper --bisect-next-check $TERM_GOOD $TERM_BAD $TERM_GOOD|| exit
 
 	# Perform all bisection computation, display and checkout
 	git bisect--helper --next-all $(test -f "$GIT_DIR/BISECT_HEAD" && echo --no-checkout)
@@ -355,7 +310,7 @@ bisect_next() {
 }
 
 bisect_visualize() {
-	bisect_next_check fail
+	git bisect--helper --bisect-next-check $TERM_GOOD $TERM_BAD fail || exit
 
 	if test $# = 0
 	then
@@ -409,7 +364,7 @@ bisect_replay () {
 }
 
 bisect_run () {
-	bisect_next_check fail
+	git bisect--helper --bisect-next-check $TERM_GOOD $TERM_BAD fail || exit
 
 	while true
 	do
@@ -482,13 +437,6 @@ get_terms () {
 	fi
 }
 
-bisect_voc () {
-	case "$1" in
-	bad) echo "bad|new" ;;
-	good) echo "good|old" ;;
-	esac
-}
-
 bisect_terms () {
 	get_terms
 	if ! test -s "$GIT_DIR/BISECT_TERMS"

--
https://github.com/git/git/pull/281

```

## Pranit Bauva, 2016-08-10 21:57

Subject: [PATCH v12 06/13] wrapper: move is_empty_file() and rename it as is_empty_or_missing_file()
Message-ID: <010201567675ae57-614652e4-7e6c-4451-a4cf-aa73922fe552-000000@eu-west-1.amazonses.com>
URL: https://gitlist.dev/e/010201567675ae57-614652e4-7e6c-4451-a4cf-aa73922fe552-000000%40eu-west-1.amazonses.com
In-Reply-To: <010201567675adc1-17e27495-6b36-40d1-836d-814da029fcc4-000000@eu-west-1.amazonses.com>

```
is_empty_file() can help to refactor a lot of code. This will be very
helpful in porting "git bisect" to C.

Suggested-by: Torsten Bögershausen <tboegi@web.de>
Mentored-by: Lars Schneider <larsxschneider@gmail.com>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
---
 builtin/am.c | 20 ++------------------
 cache.h      |  3 +++
 wrapper.c    | 13 +++++++++++++
 3 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index 3dfe70b..6ee158f 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -30,22 +30,6 @@
 #include "mailinfo.h"
 
 /**
- * Returns 1 if the file is empty or does not exist, 0 otherwise.
- */
-static int is_empty_file(const char *filename)
-{
-	struct stat st;
-
-	if (stat(filename, &st) < 0) {
-		if (errno == ENOENT)
-			return 1;
-		die_errno(_("could not stat %s"), filename);
-	}
-
-	return !st.st_size;
-}
-
-/**
  * Returns the length of the first line of msg.
  */
 static int linelen(const char *msg)
@@ -1323,7 +1307,7 @@ static int parse_mail(struct am_state *state, const char *mail)
 		goto finish;
 	}
 
-	if (is_empty_file(am_path(state, "patch"))) {
+	if (is_empty_or_missing_file(am_path(state, "patch"))) {
 		printf_ln(_("Patch is empty. Was it split wrong?"));
 		die_user_resolve(state);
 	}
@@ -1911,7 +1895,7 @@ static void am_run(struct am_state *state, int resume)
 		resume = 0;
 	}
 
-	if (!is_empty_file(am_path(state, "rewritten"))) {
+	if (!is_empty_or_missing_file(am_path(state, "rewritten"))) {
 		assert(state->rebasing);
 		copy_notes_for_rebase(state);
 		run_post_rewrite_hook(state);
diff --git a/cache.h b/cache.h
index 6049f86..91e2f81 100644
--- a/cache.h
+++ b/cache.h
@@ -1870,4 +1870,7 @@ void sleep_millisec(int millisec);
  */
 void safe_create_dir(const char *dir, int share);
 
+/* Return 1 if the file is empty or does not exists, 0 otherwise. */
+extern int is_empty_or_missing_file(const char *filename);
+
 #endif /* CACHE_H */
diff --git a/wrapper.c b/wrapper.c
index 5dc4e15..e70e4d1 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -696,3 +696,16 @@ void sleep_millisec(int millisec)
 {
 	poll(NULL, 0, millisec);
 }
+
+int is_empty_or_missing_file(const char *filename)
+{
+	struct stat st;
+
+	if (stat(filename, &st) < 0) {
+		if (errno == ENOENT)
+			return 1;
+		die_errno(_("could not stat %s"), filename);
+	}
+
+	return !st.st_size;
+}

--
https://github.com/git/git/pull/281

```

## Junio C Hamano, 2016-08-12 18:11

Subject: Re: [PATCH v12 11/13] bisect--helper: `bisect_next_check` & bisect_voc shell function in C
Message-ID: <xmqq1t1tyi0z.fsf@gitster.mtv.corp.google.com>
URL: https://gitlist.dev/e/xmqq1t1tyi0z.fsf%40gitster.mtv.corp.google.com
In-Reply-To: <010201567675ae68-15d79b9a-83fc-44ee-8607-07c2d0191c59-000000@eu-west-1.amazonses.com>

```
Pranit Bauva <pranit.bauva@gmail.com> writes:

> +static char *bisect_voc(char *revision_type)
> +{
> +	if (!strcmp(revision_type, "bad"))
> +		return "bad|new";
> +	if (!strcmp(revision_type, "good"))
> +		return "good|old";
> +
> +	return NULL;
> +}

I think you can return "const char *" from the above function.  Then
you do not have to do xstrdup() on the return values to store in
bad_syn and good_syn, and you do not have to free(3) them.

> +static int bisect_next_check(const struct bisect_terms *terms,
> +			     const char *current_term)
> +{
> + ....
> +		fprintf(stderr, N_("Warning: bisecting only with a %s commit\n"),
> +			terms->term_bad.buf);

Hmph, is this N_() and not _()?

> + ....
> +	}
> +	bad_syn = xstrdup(bisect_voc("bad"));
> +	good_syn = xstrdup(bisect_voc("good"));
> + ....
> +	free(bad_syn);
> +	free(good_syn);


```

## Junio C Hamano, 2016-08-12 18:49

Subject: Re: [PATCH v12 11/13] bisect--helper: `bisect_next_check` & bisect_voc shell function in C
Message-ID: <xmqqpopdx1on.fsf@gitster.mtv.corp.google.com>
URL: https://gitlist.dev/e/xmqqpopdx1on.fsf%40gitster.mtv.corp.google.com
In-Reply-To: <xmqq1t1tyi0z.fsf@gitster.mtv.corp.google.com>

```
Junio C Hamano <gitster@pobox.com> writes:

> Pranit Bauva <pranit.bauva@gmail.com> writes:
>
>> +static int bisect_next_check(const struct bisect_terms *terms,
>> +			     const char *current_term)
>> +{
>> + ....
>> +		fprintf(stderr, N_("Warning: bisecting only with a %s commit\n"),
>> +			terms->term_bad.buf);
>
> Hmph, is this N_() and not _()?

I recall you saying that you are not familiar with i18n.  As it is a
good skill to have outside the context of this project, let's do a
quick primer. GSoC is a learning experience ;-).

There is a runtime function "gettext()" that takes a string, which
is typically a printf format string, and gives another string.  You
feed your message in the original language, meant to be used in the
C locale, and the function gives it translated into the end-user's
language, specified by environment variables like $LC_MESSAGES.

Since it is too cumbersome to write gettext() all the time, _()
exists as a short-hand for it.  So running this

	printf(_("Hello, world\n"));

would look up "Hello world\n" in database for the end-user's
language, and shows the translated message instead.

By scanning the source text, you can extract these constant strings
passed to gettext() or _().  This is done by the i18n coordinator
with the "msgmerge" program.  By doing so, we learn "Hello, world\n"
must be translated, and then ask i18n team to translate it to their
language.  The result is what we have in the l10n database.  They
are in po/*.po files in our source tree.

Sometimes, the message you want to be translated may be in a
variable, e.g.

        void greeting(const char *message)
        {
                printf(_(message));
        }

        int main(int ac, char **av)
        {
                int i;
                const char *message[] = {
                        "Hello, world\n",
                        "Goodbye, everybody\n",
                };
                for (i = 0; i < ARRAY_SIZE(message); i++)
                        greeting(message[i]);
        }

And scanning the source would not find "Hello, world\n" or "Goodby,
everybody\n" must be translated.  We do not want to do this:

                ...
                const char *message[] = {
*BAD*                   _("Hello, world\n"),
*BAD*                   _("Goodbye, everybody\n"),
                };
                ...

because we do *NOT* want to call gettext() there (we call the
function at runtime inside greeting() instead).  We use N_()
to mark such messages, like so:

                ...
                const char *message[] = {
*GOOD*                  N_("Hello, world\n"),
*GOOD*                  N_("Goodbye, everybody\n"),
                };
                ...

The N_() macro is a no-op at runtime.  It just is there so that
"msgmerge" can notice that the constant string there is something
that needs to be thrown into the l10n database.

As a concrete example:

> @@ -24,6 +25,7 @@ static const char * const git_bisect_helper_usage[] = {
>  	N_("git bisect--helper --bisect-reset [<commit>]"),
>  	N_("git bisect--helper --bisect-write <state> <revision> <TERM_GOOD> <TERM_BAD> [<nolog>]"),
>  	N_("git bisect--helper --bisect-check-and-set-terms <command> <TERM_GOOD> <TERM_BAD>"),
> +	N_("git bisect--helper --bisect-next-check [<term>] <TERM_GOOD> <TERM_BAD"),
>  	NULL
>  };

this is such a use of N_().  You want to keep untranslated message
in the git_bisect_helper_usage[] array, to be given to gettext(), or
more likely its short-hand _(), when these usage strings are used,
and make sure these strings will be in the l10n database so that
translators will give you translations to them to be used at
runtime.

> +		/*
> +		 * have bad (or new) but not good (or old). We could bisect
> +		 * although this is less optimum.
> +		 */
> +		fprintf(stderr, N_("Warning: bisecting only with a %s commit\n"),
> +			terms->term_bad.buf);

This one wants to call gettext() function to give fprintf() the
translated warning message.  It should be _().

> +		/*
> +		 * TRANSLATORS: Make sure to include [Y] and [n] in your
> +		 * translation. The program will only accept English input
> +		 * at this point.
> +		 */
> +		yesno = git_prompt(_("Are you sure [Y/n]? "), PROMPT_ECHO);

Just like this one.

```

## Junio C Hamano, 2016-08-12 19:24

Subject: Re: [PATCH v12 04/13] bisect--helper: `bisect_clean_state` shell function in C
Message-ID: <xmqqlh01x02d.fsf@gitster.mtv.corp.google.com>
URL: https://gitlist.dev/e/xmqqlh01x02d.fsf%40gitster.mtv.corp.google.com
In-Reply-To: <010201567675ae50-4349bd9e-54a4-44e5-aabb-1d928126b237-000000@eu-west-1.amazonses.com>

```
Pranit Bauva <pranit.bauva@gmail.com> writes:

> +static int bisect_clean_state(void)
> +{
> +	int result = 0;
> +
> +	/* There may be some refs packed during bisection */
> +	struct string_list refs_for_removal = STRING_LIST_INIT_NODUP;
> +	for_each_ref_in("refs/bisect/", mark_for_removal, (void *) &refs_for_removal);
> +	string_list_append(&refs_for_removal, xstrdup("BISECT_HEAD"));
> +	result = delete_refs(&refs_for_removal);

I think this function has changed its signature recently.  I am
planning to tag 2.10-rc0 this weekend, and it may be a good time to
rebase the series on to an updated codebase.

```

## Junio C Hamano, 2016-08-12 19:25

Subject: Re: [PATCH v12 13/13] bisect--helper: `bisect_start` shell function partially in C
Message-ID: <xmqqh9apx00v.fsf@gitster.mtv.corp.google.com>
URL: https://gitlist.dev/e/xmqqh9apx00v.fsf%40gitster.mtv.corp.google.com
In-Reply-To: <010201567675ae72-0e0eeb11-e488-4562-8837-2443cbfe0792-000000@eu-west-1.amazonses.com>

```
Pranit Bauva <pranit.bauva@gmail.com> writes:

> + ...
> +	/*
> +	 * Write new start state
> +	 */
> +	if (write_file(git_path_bisect_start(), "%s\n", start_head.buf)) {

I think this function has changed its signature recently.  I am
planning to tag 2.10-rc0 this weekend, and it may be a good time to
rebase the series on to an updated codebase.

Thanks.

```

## Pranit Bauva, 2016-08-13 06:32

Subject: Re: [PATCH v12 11/13] bisect--helper: `bisect_next_check` & bisect_voc shell function in C
Message-ID: <CAFZEwPMw-cWnHnHGLgCkD8LKshc-irV9p2PV9s9Nk61QWKdrfQ@mail.gmail.com>
URL: https://gitlist.dev/e/CAFZEwPMw-cWnHnHGLgCkD8LKshc-irV9p2PV9s9Nk61QWKdrfQ%40mail.gmail.com
In-Reply-To: <xmqqpopdx1on.fsf@gitster.mtv.corp.google.com>

```
Hey Junio,

On Sat, Aug 13, 2016 at 12:19 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Pranit Bauva <pranit.bauva@gmail.com> writes:
>>
>>> +static int bisect_next_check(const struct bisect_terms *terms,
>>> +                         const char *current_term)
>>> +{
>>> + ....
>>> +            fprintf(stderr, N_("Warning: bisecting only with a %s commit\n"),
>>> +                    terms->term_bad.buf);
>>
>> Hmph, is this N_() and not _()?
>
> I recall you saying that you are not familiar with i18n.  As it is a
> good skill to have outside the context of this project, let's do a
> quick primer. GSoC is a learning experience ;-).

True! :)

> There is a runtime function "gettext()" that takes a string, which
> is typically a printf format string, and gives another string.  You
> feed your message in the original language, meant to be used in the
> C locale, and the function gives it translated into the end-user's
> language, specified by environment variables like $LC_MESSAGES.
>
> Since it is too cumbersome to write gettext() all the time, _()
> exists as a short-hand for it.  So running this
>
>         printf(_("Hello, world\n"));
>
> would look up "Hello world\n" in database for the end-user's
> language, and shows the translated message instead.
>
> By scanning the source text, you can extract these constant strings
> passed to gettext() or _().  This is done by the i18n coordinator
> with the "msgmerge" program.  By doing so, we learn "Hello, world\n"
> must be translated, and then ask i18n team to translate it to their
> language.  The result is what we have in the l10n database.  They
> are in po/*.po files in our source tree.
>
> Sometimes, the message you want to be translated may be in a
> variable, e.g.
>
>         void greeting(const char *message)
>         {
>                 printf(_(message));
>         }
>
>         int main(int ac, char **av)
>         {
>                 int i;
>                 const char *message[] = {
>                         "Hello, world\n",
>                         "Goodbye, everybody\n",
>                 };
>                 for (i = 0; i < ARRAY_SIZE(message); i++)
>                         greeting(message[i]);
>         }
>
> And scanning the source would not find "Hello, world\n" or "Goodby,
> everybody\n" must be translated.  We do not want to do this:
>
>                 ...
>                 const char *message[] = {
> *BAD*                   _("Hello, world\n"),
> *BAD*                   _("Goodbye, everybody\n"),
>                 };
>                 ...
>
> because we do *NOT* want to call gettext() there (we call the
> function at runtime inside greeting() instead).  We use N_()
> to mark such messages, like so:
>
>                 ...
>                 const char *message[] = {
> *GOOD*                  N_("Hello, world\n"),
> *GOOD*                  N_("Goodbye, everybody\n"),
>                 };
>                 ...
>
> The N_() macro is a no-op at runtime.  It just is there so that
> "msgmerge" can notice that the constant string there is something
> that needs to be thrown into the l10n database.
>
> As a concrete example:
>
>> @@ -24,6 +25,7 @@ static const char * const git_bisect_helper_usage[] = {
>>       N_("git bisect--helper --bisect-reset [<commit>]"),
>>       N_("git bisect--helper --bisect-write <state> <revision> <TERM_GOOD> <TERM_BAD> [<nolog>]"),
>>       N_("git bisect--helper --bisect-check-and-set-terms <command> <TERM_GOOD> <TERM_BAD>"),
>> +     N_("git bisect--helper --bisect-next-check [<term>] <TERM_GOOD> <TERM_BAD"),
>>       NULL
>>  };
>
> this is such a use of N_().  You want to keep untranslated message
> in the git_bisect_helper_usage[] array, to be given to gettext(), or
> more likely its short-hand _(), when these usage strings are used,
> and make sure these strings will be in the l10n database so that
> translators will give you translations to them to be used at
> runtime.
>
>> +             /*
>> +              * have bad (or new) but not good (or old). We could bisect
>> +              * although this is less optimum.
>> +              */
>> +             fprintf(stderr, N_("Warning: bisecting only with a %s commit\n"),
>> +                     terms->term_bad.buf);
>
> This one wants to call gettext() function to give fprintf() the
> translated warning message.  It should be _().
>
>> +             /*
>> +              * TRANSLATORS: Make sure to include [Y] and [n] in your
>> +              * translation. The program will only accept English input
>> +              * at this point.
>> +              */
>> +             yesno = git_prompt(_("Are you sure [Y/n]? "), PROMPT_ECHO);
>
> Just like this one.

Thanks a lot for taking time to explain this. I had searched a bit but
couldn't find the difference between _() and N_()!

Regards,
Pranit Bauva

```

## Pranit Bauva, 2016-08-13 06:33

Subject: Re: [PATCH v12 13/13] bisect--helper: `bisect_start` shell function partially in C
Message-ID: <CAFZEwPMoAi9aPYOrE8bk23yB=bC2rjCM8fAXwBFBTvOtbE+uAg@mail.gmail.com>
URL: https://gitlist.dev/e/CAFZEwPMoAi9aPYOrE8bk23yB%3DbC2rjCM8fAXwBFBTvOtbE%2BuAg%40mail.gmail.com
In-Reply-To: <xmqqh9apx00v.fsf@gitster.mtv.corp.google.com>

```
Hey Junio,

On Sat, Aug 13, 2016 at 12:55 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Pranit Bauva <pranit.bauva@gmail.com> writes:
>
>> + ...
>> +     /*
>> +      * Write new start state
>> +      */
>> +     if (write_file(git_path_bisect_start(), "%s\n", start_head.buf)) {
>
> I think this function has changed its signature recently.  I am
> planning to tag 2.10-rc0 this weekend, and it may be a good time to
> rebase the series on to an updated codebase.
>
> Thanks.

Yes sure will do that and send an updated version.

Regards,
Pranit Bauva

```

## Christian Couder, 2016-08-13 07:34

Subject: Re: [PATCH v12 13/13] bisect--helper: `bisect_start` shell function partially in C
Message-ID: <CAP8UFD1pUOUKKMQLsyTMtSBUG3s340vOjrGqRpC-CGzuyQXwzA@mail.gmail.com>
URL: https://gitlist.dev/e/CAP8UFD1pUOUKKMQLsyTMtSBUG3s340vOjrGqRpC-CGzuyQXwzA%40mail.gmail.com
In-Reply-To: <010201567675ae72-0e0eeb11-e488-4562-8837-2443cbfe0792-000000@eu-west-1.amazonses.com>

```
On Wed, Aug 10, 2016 at 11:57 PM, Pranit Bauva <pranit.bauva@gmail.com> wrote:
>
> @@ -431,6 +434,244 @@ static int bisect_terms(struct bisect_terms *terms, const char **argv, int argc)
>         return 0;
>  }
>
> +static int bisect_start(struct bisect_terms *terms, int no_checkout,
> +                       const char **argv, int argc)
> +{
> +       int i, has_double_dash = 0, must_write_terms = 0, bad_seen = 0;
> +       int flags, pathspec_pos;
> +       struct string_list revs = STRING_LIST_INIT_DUP;
> +       struct string_list states = STRING_LIST_INIT_DUP;
> +       struct strbuf start_head = STRBUF_INIT;
> +       struct strbuf bisect_names = STRBUF_INIT;
> +       struct strbuf orig_args = STRBUF_INIT;
> +       const char *head;
> +       unsigned char sha1[20];
> +       FILE *fp;
> +       struct object_id oid;
> +
> +       if (is_bare_repository())
> +               no_checkout = 1;
> +
> +       for (i = 0; i < argc; i++) {
> +               char *commit_id = xstrfmt("%s^{commit}", argv[i]);
> +               if (!strcmp(argv[i], "--")) {
> +                       has_double_dash = 1;
> +                       break;
> +               }

In the shell code there is a loop dedicated to checking if there is a
double dash in the arguments before the real argument parsing loop.
There is a reason for that.
If you do it in the same loop, has_double_dash will not be set when
the arguments that are before the double dash will be parsed.

> +               else if (!strcmp(argv[i], "--no-checkout"))
> +                       no_checkout = 1;
> +               else if (!strcmp(argv[i], "--term-good") ||
> +                        !strcmp(argv[i], "--term-old")) {
> +                       must_write_terms = 1;
> +                       strbuf_reset(&terms->term_good);
> +                       strbuf_addstr(&terms->term_good, argv[++i]);
> +               }
> +               else if (skip_prefix(argv[i], "--term-good=", &argv[i])) {

(Maybe you could put the "else if (...) {" on the same line as the "}" above.)

> +                       must_write_terms = 1;
> +                       strbuf_reset(&terms->term_good);
> +                       strbuf_addstr(&terms->term_good, argv[i]);
> +               }
> +               else if (skip_prefix(argv[i], "--term-old=", &argv[i])) {
> +                       must_write_terms = 1;
> +                       strbuf_reset(&terms->term_good);
> +                       strbuf_addstr(&terms->term_good, argv[i]);
> +               }
> +               else if (!strcmp(argv[i], "--term-bad") ||
> +                        !strcmp(argv[i], "--term-new")) {
> +                       must_write_terms = 1;
> +                       strbuf_reset(&terms->term_bad);
> +                       strbuf_addstr(&terms->term_bad, argv[++i]);
> +               }
> +               else if (skip_prefix(argv[i], "--term-bad=", &argv[i])) {
> +                       must_write_terms = 1;
> +                       strbuf_reset(&terms->term_bad);
> +                       strbuf_addstr(&terms->term_bad, argv[i]);
> +               }
> +               else if (skip_prefix(argv[i], "--term-new=", &argv[i])) {
> +                       must_write_terms = 1;
> +                       strbuf_reset(&terms->term_good);
> +                       strbuf_addstr(&terms->term_good, argv[i]);
> +               }
> +               else if (starts_with(argv[i], "--") &&
> +                        !one_of(argv[i], "--term-good", "--term-bad", NULL)) {
> +                       string_list_clear(&revs, 0);
> +                       string_list_clear(&states, 0);
> +                       die(_("unrecognised option: '%s'"), argv[i]);
> +               }
> +               else if (get_oid(argv[i], &oid) && !has_double_dash) {

And here checking "!has_double_dash" has no meaning if you check for a
double dash in the same loop, because there is a "break" after
has_double_dash is set above.

> +                       string_list_clear(&revs, 0);
> +                       string_list_clear(&states, 0);
> +                       free(commit_id);
> +                       die(_("'%s' does not appear to be a valid revision"), argv[i]);
> +               }
> +               else {
> +                       free(commit_id);
> +                       string_list_append(&revs, oid_to_hex(&oid));
> +               }
> +       }
> +       pathspec_pos = i;
> +

```

## Pranit Bauva, 2016-08-13 13:50

Subject: Re: [PATCH v12 13/13] bisect--helper: `bisect_start` shell function partially in C
Message-ID: <CAFZEwPMn=qUWbkdBfYgo8g8CdPNLX-B2xEBvZ8-otk3=2qjxNQ@mail.gmail.com>
URL: https://gitlist.dev/e/CAFZEwPMn%3DqUWbkdBfYgo8g8CdPNLX-B2xEBvZ8-otk3%3D2qjxNQ%40mail.gmail.com
In-Reply-To: <CAP8UFD1pUOUKKMQLsyTMtSBUG3s340vOjrGqRpC-CGzuyQXwzA@mail.gmail.com>

```
Hey Christian,

On Sat, Aug 13, 2016 at 1:04 PM, Christian Couder
<christian.couder@gmail.com> wrote:
> On Wed, Aug 10, 2016 at 11:57 PM, Pranit Bauva <pranit.bauva@gmail.com> wrote:
>>
>> @@ -431,6 +434,244 @@ static int bisect_terms(struct bisect_terms *terms, const char **argv, int argc)
>>         return 0;
>>  }
>>
>> +static int bisect_start(struct bisect_terms *terms, int no_checkout,
>> +                       const char **argv, int argc)
>> +{
>> +       int i, has_double_dash = 0, must_write_terms = 0, bad_seen = 0;
>> +       int flags, pathspec_pos;
>> +       struct string_list revs = STRING_LIST_INIT_DUP;
>> +       struct string_list states = STRING_LIST_INIT_DUP;
>> +       struct strbuf start_head = STRBUF_INIT;
>> +       struct strbuf bisect_names = STRBUF_INIT;
>> +       struct strbuf orig_args = STRBUF_INIT;
>> +       const char *head;
>> +       unsigned char sha1[20];
>> +       FILE *fp;
>> +       struct object_id oid;
>> +
>> +       if (is_bare_repository())
>> +               no_checkout = 1;
>> +
>> +       for (i = 0; i < argc; i++) {
>> +               char *commit_id = xstrfmt("%s^{commit}", argv[i]);
>> +               if (!strcmp(argv[i], "--")) {
>> +                       has_double_dash = 1;
>> +                       break;
>> +               }
>
> In the shell code there is a loop dedicated to checking if there is a
> double dash in the arguments before the real argument parsing loop.
> There is a reason for that.
> If you do it in the same loop, has_double_dash will not be set when
> the arguments that are before the double dash will be parsed.

I had tried that before. But somehow I couldn't get it to run so I
reverted back. I have now successfully tried it and its working. You
can checkout the PR[1].

>> +               else if (!strcmp(argv[i], "--no-checkout"))
>> +                       no_checkout = 1;
>> +               else if (!strcmp(argv[i], "--term-good") ||
>> +                        !strcmp(argv[i], "--term-old")) {
>> +                       must_write_terms = 1;
>> +                       strbuf_reset(&terms->term_good);
>> +                       strbuf_addstr(&terms->term_good, argv[++i]);
>> +               }
>> +               else if (skip_prefix(argv[i], "--term-good=", &argv[i])) {
>
> (Maybe you could put the "else if (...) {" on the same line as the "}" above.)
>
>> +                       must_write_terms = 1;
>> +                       strbuf_reset(&terms->term_good);
>> +                       strbuf_addstr(&terms->term_good, argv[i]);
>> +               }
>> +               else if (skip_prefix(argv[i], "--term-old=", &argv[i])) {
>> +                       must_write_terms = 1;
>> +                       strbuf_reset(&terms->term_good);
>> +                       strbuf_addstr(&terms->term_good, argv[i]);
>> +               }
>> +               else if (!strcmp(argv[i], "--term-bad") ||
>> +                        !strcmp(argv[i], "--term-new")) {
>> +                       must_write_terms = 1;
>> +                       strbuf_reset(&terms->term_bad);
>> +                       strbuf_addstr(&terms->term_bad, argv[++i]);
>> +               }
>> +               else if (skip_prefix(argv[i], "--term-bad=", &argv[i])) {
>> +                       must_write_terms = 1;
>> +                       strbuf_reset(&terms->term_bad);
>> +                       strbuf_addstr(&terms->term_bad, argv[i]);
>> +               }
>> +               else if (skip_prefix(argv[i], "--term-new=", &argv[i])) {
>> +                       must_write_terms = 1;
>> +                       strbuf_reset(&terms->term_good);
>> +                       strbuf_addstr(&terms->term_good, argv[i]);
>> +               }
>> +               else if (starts_with(argv[i], "--") &&
>> +                        !one_of(argv[i], "--term-good", "--term-bad", NULL)) {
>> +                       string_list_clear(&revs, 0);
>> +                       string_list_clear(&states, 0);
>> +                       die(_("unrecognised option: '%s'"), argv[i]);
>> +               }
>> +               else if (get_oid(argv[i], &oid) && !has_double_dash) {
>
> And here checking "!has_double_dash" has no meaning if you check for a
> double dash in the same loop, because there is a "break" after
> has_double_dash is set above.

I think this is the situation in the shell script too.

>> +                       string_list_clear(&revs, 0);
>> +                       string_list_clear(&states, 0);
>> +                       free(commit_id);
>> +                       die(_("'%s' does not appear to be a valid revision"), argv[i]);
>> +               }
>> +               else {
>> +                       free(commit_id);
>> +                       string_list_append(&revs, oid_to_hex(&oid));
>> +               }
>> +       }
>> +       pathspec_pos = i;
>> +

Regards,
Pranit Bauva

```