# [PATCH v16 Part II 7/8] bisect--helper: `bisect_start` shell function partially in C

50 messages from 2017-10-27 to 2019-01-07. Participants: Pranit Bauva, Martin Ågren, Junio C Hamano, Stephan Beyer, Ramsay Jones, SZEDER Gábor, Johannes Schindelin, Pranit Bauva via GitGitGadget, Tanushree Tumane via GitGitGadget, TANUSHREE TUMANE.
Thread: https://gitlist.dev/t/47045

## Pranit Bauva, 2017-10-27 15:06

Subject: [PATCH v16 Part II 7/8] bisect--helper: `bisect_start` shell function partially in C
Message-ID: <0102015f5e5ee2f5-3d106a61-7f79-4973-851f-f55a691a6189-000000@eu-west-1.amazonses.com>
URL: https://gitlist.dev/e/0102015f5e5ee2f5-3d106a61-7f79-4973-851f-f55a691a6189-000000%40eu-west-1.amazonses.com
In-Reply-To: <0102015f5e5ee171-f30f4868-886f-47a1-a4e4-b4936afc545d-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.

Also introduce a method `bisect_append_log_quoted` to keep things short
and crisp.

Helped-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
Helped-by: Stephan Beyer <s-beyer@gmx.net>
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 | 228 ++++++++++++++++++++++++++++++++++++++++++++++-
 git-bisect.sh            | 132 +--------------------------
 2 files changed, 228 insertions(+), 132 deletions(-)

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index ab0580ce0089a..4ac175c49e80c 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")
@@ -14,6 +15,8 @@ static GIT_PATH_FUNC(git_path_bisect_ancestors_ok, "BISECT_ANCESTORS_OK")
 static GIT_PATH_FUNC(git_path_bisect_start, "BISECT_START")
 static GIT_PATH_FUNC(git_path_bisect_head, "BISECT_HEAD")
 static GIT_PATH_FUNC(git_path_bisect_log, "BISECT_LOG")
+static GIT_PATH_FUNC(git_path_head_name, "head-name")
+static GIT_PATH_FUNC(git_path_bisect_names, "BISECT_NAMES")
 
 static const char * const git_bisect_helper_usage[] = {
 	N_("git bisect--helper --next-all [--no-checkout]"),
@@ -24,6 +27,8 @@ static const char * const git_bisect_helper_usage[] = {
 	N_("git bisect--helper --bisect-check-and-set-terms <command> <good_term> <bad_term>"),
 	N_("git bisect--helper --bisect-next-check [<term>] <good_term> <bad_term>"),
 	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
 };
 
@@ -401,6 +406,217 @@ static int bisect_terms(struct bisect_terms *terms, const char **argv, int argc)
 	return 0;
 }
 
+static int bisect_append_log_quoted(const char **argv)
+{
+	int retval = 0;
+	FILE *fp = fopen(git_path_bisect_log(), "a");
+	struct strbuf orig_args = STRBUF_INIT;
+
+	if (!fp)
+		return -1;
+
+	if (fprintf(fp, "git bisect start") < 1)
+		goto fail;
+
+	sq_quote_argv(&orig_args, argv, 0);
+	if (fprintf(fp, "%s\n", orig_args.buf) < 1)
+		goto fail;
+
+	goto finish;
+
+fail:
+	retval = -1;
+finish:
+	fclose(fp);
+	strbuf_release(&orig_args);
+	return retval;
+}
+
+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, retval = 0;
+	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 object_id head_oid;
+	struct object_id oid;
+	const char *head;
+
+	if (is_bare_repository())
+		no_checkout = 1;
+
+	/*
+	 * Check for one bad and then some good revisions
+	 */
+	for (i = 0; i < argc; i++) {
+		if (!strcmp(argv[i], "--")) {
+			has_double_dash = 1;
+			break;
+		}
+	}
+
+	for (i = 0; i < argc; i++) {
+		const char *arg = argv[i];
+		if (!strcmp(argv[i], "--")) {
+			break;
+		} else if (!strcmp(arg, "--no-checkout")) {
+			no_checkout = 1;
+		} else if (!strcmp(arg, "--term-good") ||
+			 !strcmp(arg, "--term-old")) {
+			must_write_terms = 1;
+			free((void *) terms->term_good);
+			terms->term_good = xstrdup(argv[++i]);
+		} else if (skip_prefix(arg, "--term-good=", &arg) ||
+			   skip_prefix(arg, "--term-old=", &arg)) {
+			must_write_terms = 1;
+			free((void *) terms->term_good);
+			terms->term_good = xstrdup(arg);
+		} else if (!strcmp(arg, "--term-bad") ||
+			 !strcmp(arg, "--term-new")) {
+			must_write_terms = 1;
+			free((void *) terms->term_bad);
+			terms->term_bad = xstrdup(argv[++i]);
+		} else if (skip_prefix(arg, "--term-bad=", &arg) ||
+			   skip_prefix(arg, "--term-new=", &arg)) {
+			must_write_terms = 1;
+			free((void *) terms->term_bad);
+			terms->term_bad = xstrdup(arg);
+		} else if (starts_with(arg, "--") &&
+			 !one_of(arg, "--term-good", "--term-bad", NULL)) {
+			return error(_("unrecognised option: '%s'"), arg);
+		} else {
+			char *commit_id = xstrfmt("%s^{commit}", arg);
+			if (get_oid(commit_id, &oid) && has_double_dash)
+				die(_("'%s' does not appear to be a valid "
+				      "revision"), arg);
+
+			string_list_append(&revs, oid_to_hex(&oid));
+			free(commit_id);
+		}
+	}
+	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);
+		} else {
+			bad_seen = 1;
+			string_list_append(&states, terms->term_bad);
+		}
+	}
+
+	/*
+	 * Verify HEAD
+	 */
+	head = resolve_ref_unsafe("HEAD", 0, &head_oid, &flags);
+	if (!head)
+		if (get_oid("HEAD", &head_oid))
+			return error(_("Bad HEAD - I need a HEAD"));
+
+	/*
+	 * Check if we are bisecting
+	 */
+	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);
+				goto fail;
+			}
+		}
+	} else {
+		/* Get the rev from where we start. */
+		if (!get_oid(head, &head_oid) &&
+		    !starts_with(head, "refs/heads/")) {
+			strbuf_reset(&start_head);
+			strbuf_addstr(&start_head, sha1_to_hex(head_oid.hash));
+		} else if (!get_oid(head, &head_oid) &&
+			   skip_prefix(head, "refs/heads/", &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()))
+				return error(_("won't bisect on cg-seek'ed tree"));
+			strbuf_addstr(&start_head, head);
+		} else {
+			return error(_("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
+	 */
+	write_file(git_path_bisect_start(), "%s\n", start_head.buf);
+
+	if (no_checkout) {
+		get_oid(start_head.buf, &oid);
+		if (update_ref(NULL, "BISECT_HEAD", &oid, NULL, 0,
+			       UPDATE_REFS_MSG_ON_ERR))
+			goto fail;
+	}
+
+	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);
+
+	for (i = 0; i < states.nr; i++)
+		if (bisect_write(states.items[i].string,
+				 revs.items[i].string, terms, 1))
+			goto fail;
+
+	if (must_write_terms)
+		if (write_terms(terms->term_bad, terms->term_good))
+			goto fail;
+
+	retval = bisect_append_log_quoted(argv);
+	if (retval)
+		goto fail;
+
+	goto finish;
+
+fail:
+	retval = -1;
+finish:
+	string_list_clear(&revs, 0);
+	string_list_clear(&states, 0);
+	strbuf_release(&start_head);
+	strbuf_release(&bisect_names);
+	return retval;
+}
+
 int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 {
 	enum {
@@ -412,7 +628,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[] = {
@@ -434,6 +651,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()
@@ -441,7 +660,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 	struct bisect_terms 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);
@@ -489,6 +709,10 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 			return error(_("--bisect-terms requires 0 or 1 argument"));
 		res = bisect_terms(&terms, argv, argc);
 		break;
+	case BISECT_START:
+		set_terms(&terms, "bad", "good");
+		res = bisect_start(&terms, no_checkout, argv, argc);
+		break;
 	default:
 		return error("BUG: unknown subcommand '%d'", cmdmode);
 	}
diff --git a/git-bisect.sh b/git-bisect.sh
index 1bc5749f78484..9d2f8936198d2 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -71,122 +71,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.
@@ -199,22 +84,9 @@ bisect_start() {
 	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" || exit
-	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/420

```

## Pranit Bauva, 2017-10-27 15:06

Subject: [PATCH v16 Part II 2/8] bisect--helper: `bisect_write` shell function in C
Message-ID: <0102015f5e5ee22c-ea9c3a38-5d42-4dce-a54b-45c59768a70b-000000@eu-west-1.amazonses.com>
URL: https://gitlist.dev/e/0102015f5e5ee22c-ea9c3a38-5d42-4dce-a54b-45c59768a70b-000000%40eu-west-1.amazonses.com
In-Reply-To: <0102015f5e5ee171-f30f4868-886f-47a1-a4e4-b4936afc545d-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.

Add a log_commit() helper function to write the contents of the commit message
header to a file which will be re-used in future parts of the code as
well.

Also introduce a function free_terms() to free the memory of `struct
bisect_terms` and set_terms() to set the values of members in `struct
bisect_terms`.

Helped-by: Ramsay Jones <ramsay@ramsayjones.plus.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 | 107 +++++++++++++++++++++++++++++++++++++++++++++--
 git-bisect.sh            |  25 ++---------
 2 files changed, 108 insertions(+), 24 deletions(-)

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 12754448f7b6a..6295f53c850a8 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -12,15 +12,37 @@ 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_start, "BISECT_START")
 static GIT_PATH_FUNC(git_path_bisect_head, "BISECT_HEAD")
+static GIT_PATH_FUNC(git_path_bisect_log, "BISECT_LOG")
 
 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>]"),
+	N_("git bisect--helper --bisect-write <state> <revision> <good_term> <bad_term> [<nolog>]"),
 	NULL
 };
 
+struct bisect_terms {
+	const char *term_good;
+	const char *term_bad;
+};
+
+static void free_terms(struct bisect_terms *terms)
+{
+	if (!terms->term_good)
+		free((void *) terms->term_good);
+	if (!terms->term_bad)
+		free((void *) terms->term_bad);
+}
+
+static void set_terms(struct bisect_terms *terms, const char *bad,
+		      const char *good)
+{
+	terms->term_good = xstrdup(good);
+	terms->term_bad = xstrdup(bad);
+}
+
 /*
  * Check whether the string `term` belongs to the set of strings
  * included in the variable arguments.
@@ -146,6 +168,72 @@ static int bisect_reset(const char *commit)
 	return bisect_clean_state();
 }
 
+static void log_commit(FILE *fp, char *fmt, const char *state,
+		       struct commit *commit)
+{
+	struct pretty_print_context pp = {0};
+	struct strbuf commit_msg = STRBUF_INIT;
+	char *label = xstrfmt(fmt, state);
+
+	format_commit_message(commit, "%s", &commit_msg, &pp);
+
+	fprintf(fp, "# %s: [%s] %s\n", label, oid_to_hex(&commit->object.oid),
+		commit_msg.buf);
+
+	strbuf_release(&commit_msg);
+	free(label);
+}
+
+static int bisect_write(const char *state, const char *rev,
+			const struct bisect_terms *terms, int nolog)
+{
+	struct strbuf tag = STRBUF_INIT;
+	struct object_id oid;
+	struct commit *commit;
+	FILE *fp = NULL;
+	int retval = 0;
+
+	if (!strcmp(state, terms->term_bad)) {
+		strbuf_addf(&tag, "refs/bisect/%s", state);
+	} else if (one_of(state, terms->term_good, "skip", NULL)) {
+		strbuf_addf(&tag, "refs/bisect/%s-%s", state, rev);
+	} else {
+		error(_("Bad bisect_write argument: %s"), state);
+		goto fail;
+	}
+
+	if (get_oid(rev, &oid)) {
+		error(_("couldn't get the oid of the rev '%s'"), rev);
+		goto fail;
+	}
+
+	if (update_ref(NULL, tag.buf, &oid, NULL, 0,
+		       UPDATE_REFS_MSG_ON_ERR))
+		goto fail;
+
+	fp = fopen(git_path_bisect_log(), "a");
+	if (!fp) {
+		error_errno(_("couldn't open the file '%s'"), git_path_bisect_log());
+		goto fail;
+	}
+
+	commit = lookup_commit_reference(&oid);
+	log_commit(fp, "%s", state, commit);
+
+	if (!nolog)
+		fprintf(fp, "git bisect %s %s\n", state, rev);
+
+	goto finish;
+
+fail:
+	retval = -1;
+finish:
+	if (fp)
+		fclose(fp);
+	strbuf_release(&tag);
+	return retval;
+}
+
 int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 {
 	enum {
@@ -153,9 +241,10 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 		WRITE_TERMS,
 		BISECT_CLEAN_STATE,
 		CHECK_EXPECTED_REVS,
-		BISECT_RESET
+		BISECT_RESET,
+		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),
@@ -167,10 +256,13 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 			 N_("check for expected revs"), CHECK_EXPECTED_REVS),
 		OPT_CMDMODE(0, "bisect-reset", &cmdmode,
 			 N_("reset the bisection state"), BISECT_RESET),
+		OPT_CMDMODE(0, "bisect-write", &cmdmode,
+			 N_("update the refs according to the bisection state and may write it to 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;
 
 	argc = parse_options(argc, argv, prefix, options,
 			     git_bisect_helper_usage, 0);
@@ -179,6 +271,7 @@ 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:
@@ -196,8 +289,16 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 		if (argc > 1)
 			return error(_("--bisect-reset requires either no argument or a commit"));
 		return bisect_reset(argc ? argv[0] : NULL);
+	case BISECT_WRITE:
+		if (argc != 4 && argc != 5)
+			return error(_("--bisect-write requires either 4 or 5 arguments"));
+		nolog = (argc == 5) && !strcmp(argv[4], "nolog");
+		set_terms(&terms, argv[3], argv[2]);
+		res = bisect_write(argv[0], argv[1], &terms, nolog);
+		break;
 	default:
 		return error("BUG: unknown subcommand '%d'", cmdmode);
 	}
-	return 0;
+	free_terms(&terms);
+	return res;
 }
diff --git a/git-bisect.sh b/git-bisect.sh
index 4d403242482cf..1eab94ec943a1 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -144,7 +144,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.
@@ -220,23 +220,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() {
 		bisected_head=$(bisect_head)
 		rev=$(git rev-parse --verify "$bisected_head") ||
 			die "$(eval_gettext "Bad rev input: \$bisected_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/420

```

## Pranit Bauva, 2017-10-27 15:06

Subject: [PATCH v16 Part II 3/8] wrapper: move is_empty_file() and rename it as is_empty_or_missing_file()
Message-ID: <0102015f5e5ee273-03ff1203-7c84-4260-bda7-c27a87646517-000000@eu-west-1.amazonses.com>
URL: https://gitlist.dev/e/0102015f5e5ee273-03ff1203-7c84-4260-bda7-c27a87646517-000000%40eu-west-1.amazonses.com
In-Reply-To: <0102015f5e5ee171-f30f4868-886f-47a1-a4e4-b4936afc545d-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 4b6f1534f8840..2afaaabcdbecf 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -34,22 +34,6 @@
 #include "packfile.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)
@@ -1298,7 +1282,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."));
 		die_user_resolve(state);
 	}
@@ -1880,7 +1864,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 25adcf681531c..f05056a2d7fa1 100644
--- a/cache.h
+++ b/cache.h
@@ -1958,4 +1958,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 61aba0b5c1bec..21c0fee2db459 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -690,3 +690,16 @@ int xgethostname(char *buf, size_t len)
 		buf[len - 1] = 0;
 	return ret;
 }
+
+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/420

```

## Pranit Bauva, 2017-10-27 15:06

Subject: [PATCH v16 Part II 5/8] bisect--helper: `bisect_next_check` shell function in C
Message-ID: <0102015f5e5ee2ca-bf824205-86e6-4259-b7d7-a68e2fbb087d-000000@eu-west-1.amazonses.com>
URL: https://gitlist.dev/e/0102015f5e5ee2ca-bf824205-86e6-4259-b7d7-a68e2fbb087d-000000%40eu-west-1.amazonses.com
In-Reply-To: <0102015f5e5ee171-f30f4868-886f-47a1-a4e4-b4936afc545d-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 .

`bisect_voc` shell function is no longer useful now and is replaced by
using a char *[] of "new|bad" and "good|old" values.

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.

Helped-by: Stephan Beyer <s-beyer@gmx.net>
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 | 91 +++++++++++++++++++++++++++++++++++++++++++++++-
 git-bisect.sh            | 60 +++----------------------------
 2 files changed, 94 insertions(+), 57 deletions(-)

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 65abf8a70c6d9..0f9c3e63821b8 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")
@@ -21,6 +22,7 @@ static const char * const git_bisect_helper_usage[] = {
 	N_("git bisect--helper --bisect-reset [<commit>]"),
 	N_("git bisect--helper --bisect-write <state> <revision> <good_term> <bad_term> [<nolog>]"),
 	N_("git bisect--helper --bisect-check-and-set-terms <command> <good_term> <bad_term>"),
+	N_("git bisect--helper --bisect-next-check [<term>] <good_term> <bad_term>"),
 	NULL
 };
 
@@ -44,6 +46,11 @@ static void set_terms(struct bisect_terms *terms, const char *bad,
 	terms->term_bad = xstrdup(bad);
 }
 
+static const char *voc[] = {
+	"bad|new",
+	"good|old"
+};
+
 /*
  * Check whether the string `term` belongs to the set of strings
  * included in the variable arguments.
@@ -264,6 +271,79 @@ 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 int bisect_next_check(const struct bisect_terms *terms,
+			     const char *current_term)
+{
+	int missing_good = 1, missing_bad = 1, retval = 0;
+	const char *bad_ref = xstrfmt("refs/bisect/%s", terms->term_bad);
+	const char *good_glob = xstrfmt("%s-*", terms->term_good);
+
+	if (ref_exists(bad_ref))
+		missing_bad = 0;
+
+	for_each_glob_ref_in(mark_good, good_glob, "refs/bisect/",
+			     (void *) &missing_good);
+
+	if (!missing_good && !missing_bad)
+		goto finish;
+
+	if (!current_term)
+		goto fail;
+
+	if (missing_good && !missing_bad && current_term &&
+	    !strcmp(current_term, terms->term_good)) {
+		char *yesno;
+		/*
+		 * have bad (or new) but not good (or old). We could bisect
+		 * although this is less optimum.
+		 */
+		fprintf(stderr, _("Warning: bisecting only with a %s commit\n"),
+			terms->term_bad);
+		if (!isatty(0))
+			goto finish;
+		/*
+		 * 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"))
+			goto fail;
+
+		goto finish;
+	}
+	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"),
+			voc[0], voc[1], voc[0], voc[1]);
+		goto fail;
+	} 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"),
+			voc[1], voc[0], voc[1], voc[0]);
+		goto fail;
+	}
+	goto finish;
+
+fail:
+	retval = -1;
+finish:
+	free((void *) good_glob);
+	free((void *) bad_ref);
+	return retval;
+}
+
 int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 {
 	enum {
@@ -273,7 +353,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 		CHECK_EXPECTED_REVS,
 		BISECT_RESET,
 		BISECT_WRITE,
-		CHECK_AND_SET_TERMS
+		CHECK_AND_SET_TERMS,
+		BISECT_NEXT_CHECK
 	} cmdmode = 0;
 	int no_checkout = 0, res = 0;
 	struct option options[] = {
@@ -291,6 +372,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 			 N_("update the refs according to the bisection state and may write it to 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()
@@ -335,6 +418,12 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 		set_terms(&terms, argv[2], argv[1]);
 		res = check_and_set_terms(&terms, argv[0]);
 		break;
+	case BISECT_NEXT_CHECK:
+		if (argc != 2 && argc != 3)
+			return error(_("--bisect-next-check requires 2 or 3 arguments"));
+		set_terms(&terms, argv[1], argv[0]);
+		res = bisect_next_check(&terms, argc == 3 ? argv[2] : NULL);
+		break;
 	default:
 		return error("BUG: unknown subcommand '%d'", cmdmode);
 	}
diff --git a/git-bisect.sh b/git-bisect.sh
index 0447b73aa588b..c21fafc62d71f 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/420

```

## Pranit Bauva, 2017-10-27 15:06

Subject: [PATCH v16 Part II 8/8] t6030: make various test to pass GETTEXT_POISON tests
Message-ID: <0102015f5e5ee2f8-9c3b0a3a-ba4f-4a50-8791-69fa5e336b7e-000000@eu-west-1.amazonses.com>
URL: https://gitlist.dev/e/0102015f5e5ee2f8-9c3b0a3a-ba4f-4a50-8791-69fa5e336b7e-000000%40eu-west-1.amazonses.com
In-Reply-To: <0102015f5e5ee171-f30f4868-886f-47a1-a4e4-b4936afc545d-000000@eu-west-1.amazonses.com>

```
Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
---
 t/t6030-bisect-porcelain.sh | 120 ++++++++++++++++++++++----------------------
 1 file changed, 60 insertions(+), 60 deletions(-)

diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index 55835ee4a4715..f9e61c6540e57 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -133,21 +133,21 @@ test_expect_success 'bisect reset removes bisect state after --no-checkout' '
 
 test_expect_success 'bisect start: back in good branch' '
 	git branch > branch.output &&
-	grep "* other" branch.output > /dev/null &&
+	test_i18ngrep "* other" branch.output > /dev/null &&
 	git bisect start $HASH4 $HASH1 -- &&
 	git bisect good &&
 	git bisect start $HASH4 $HASH1 -- &&
 	git bisect bad &&
 	git bisect reset &&
 	git branch > branch.output &&
-	grep "* other" branch.output > /dev/null
+	test_i18ngrep "* other" branch.output > /dev/null
 '
 
 test_expect_success 'bisect start: no ".git/BISECT_START" created if junk rev' '
 	git bisect reset &&
 	test_must_fail git bisect start $HASH4 foo -- &&
 	git branch > branch.output &&
-	grep "* other" branch.output > /dev/null &&
+	test_i18ngrep "* other" branch.output > /dev/null &&
 	test_must_fail test -e .git/BISECT_START
 '
 
@@ -158,14 +158,14 @@ test_expect_success 'bisect start: existing ".git/BISECT_START" not modified if
 	test_must_fail git bisect start $HASH4 foo -- &&
 	git branch > branch.output &&
 	test_i18ngrep "* (no branch, bisect started on other)" branch.output > /dev/null &&
-	test_cmp saved .git/BISECT_START
+	test_i18ncmp saved .git/BISECT_START
 '
 test_expect_success 'bisect start: no ".git/BISECT_START" if mistaken rev' '
 	git bisect start $HASH4 $HASH1 -- &&
 	git bisect good &&
 	test_must_fail git bisect start $HASH1 $HASH4 -- &&
 	git branch > branch.output &&
-	grep "* other" branch.output > /dev/null &&
+	test_i18ngrep "* other" branch.output > /dev/null &&
 	test_must_fail test -e .git/BISECT_START
 '
 
@@ -174,7 +174,7 @@ test_expect_success 'bisect start: no ".git/BISECT_START" if checkout error' '
 	test_must_fail git bisect start $HASH4 $HASH1 -- &&
 	git branch &&
 	git branch > branch.output &&
-	grep "* other" branch.output > /dev/null &&
+	test_i18ngrep "* other" branch.output > /dev/null &&
 	test_must_fail test -e .git/BISECT_START &&
 	test -z "$(git for-each-ref "refs/bisect/*")" &&
 	git checkout HEAD hello
@@ -189,7 +189,7 @@ test_expect_success 'bisect skip: successful result' '
 	git bisect start $HASH4 $HASH1 &&
 	git bisect skip &&
 	git bisect bad > my_bisect_log.txt &&
-	grep "$HASH2 is the first bad commit" my_bisect_log.txt
+	test_i18ngrep "$HASH2 is the first bad commit" my_bisect_log.txt
 '
 
 # $HASH1 is good, $HASH4 is bad, we skip $HASH3 and $HASH2
@@ -200,11 +200,11 @@ test_expect_success 'bisect skip: cannot tell between 3 commits' '
 	git bisect start $HASH4 $HASH1 &&
 	git bisect skip &&
 	test_expect_code 2 git bisect skip >my_bisect_log.txt &&
-	grep "first bad commit could be any of" my_bisect_log.txt &&
+	test_i18ngrep "first bad commit could be any of" my_bisect_log.txt &&
 	! grep $HASH1 my_bisect_log.txt &&
-	grep $HASH2 my_bisect_log.txt &&
-	grep $HASH3 my_bisect_log.txt &&
-	grep $HASH4 my_bisect_log.txt
+	test_i18ngrep $HASH2 my_bisect_log.txt &&
+	test_i18ngrep $HASH3 my_bisect_log.txt &&
+	test_i18ngrep $HASH4 my_bisect_log.txt
 '
 
 # $HASH1 is good, $HASH4 is bad, we skip $HASH3
@@ -216,11 +216,11 @@ test_expect_success 'bisect skip: cannot tell between 2 commits' '
 	git bisect start $HASH4 $HASH1 &&
 	git bisect skip &&
 	test_expect_code 2 git bisect good >my_bisect_log.txt &&
-	grep "first bad commit could be any of" my_bisect_log.txt &&
+	test_i18ngrep "first bad commit could be any of" my_bisect_log.txt &&
 	! grep $HASH1 my_bisect_log.txt &&
 	! grep $HASH2 my_bisect_log.txt &&
-	grep $HASH3 my_bisect_log.txt &&
-	grep $HASH4 my_bisect_log.txt
+	test_i18ngrep $HASH3 my_bisect_log.txt &&
+	test_i18ngrep $HASH4 my_bisect_log.txt
 '
 
 # $HASH1 is good, $HASH4 is both skipped and bad, we skip $HASH3
@@ -235,11 +235,11 @@ test_expect_success 'bisect skip: with commit both bad and skipped' '
 	git bisect good $HASH1 &&
 	git bisect skip &&
 	test_expect_code 2 git bisect good >my_bisect_log.txt &&
-	grep "first bad commit could be any of" my_bisect_log.txt &&
+	test_i18ngrep "first bad commit could be any of" my_bisect_log.txt &&
 	! grep $HASH1 my_bisect_log.txt &&
-	! grep $HASH2 my_bisect_log.txt &&
-	grep $HASH3 my_bisect_log.txt &&
-	grep $HASH4 my_bisect_log.txt
+	! rep $HASH2 my_bisect_log.txt &&
+	test_i18ngrep $HASH3 my_bisect_log.txt &&
+	test_i18ngrep $HASH4 my_bisect_log.txt
 '
 
 # We want to automatically find the commit that
@@ -254,7 +254,7 @@ test_expect_success \
      git bisect good $HASH1 &&
      git bisect bad $HASH4 &&
      git bisect run ./test_script.sh > my_bisect_log.txt &&
-     grep "$HASH3 is the first bad commit" my_bisect_log.txt &&
+     test_i18ngrep "$HASH3 is the first bad commit" my_bisect_log.txt &&
      git bisect reset'
 
 # We want to automatically find the commit that
@@ -267,7 +267,7 @@ test_expect_success \
      chmod +x test_script.sh &&
      git bisect start $HASH4 $HASH1 &&
      git bisect run ./test_script.sh > my_bisect_log.txt &&
-     grep "$HASH4 is the first bad commit" my_bisect_log.txt &&
+     test_i18ngrep "$HASH4 is the first bad commit" my_bisect_log.txt &&
      git bisect reset'
 
 # $HASH1 is good, $HASH5 is bad, we skip $HASH3
@@ -280,14 +280,14 @@ test_expect_success 'bisect skip: add line and then a new test' '
 	git bisect start $HASH5 $HASH1 &&
 	git bisect skip &&
 	git bisect good > my_bisect_log.txt &&
-	grep "$HASH5 is the first bad commit" my_bisect_log.txt &&
+	test_i18ngrep "$HASH5 is the first bad commit" my_bisect_log.txt &&
 	git bisect log > log_to_replay.txt &&
 	git bisect reset
 '
 
 test_expect_success 'bisect skip and bisect replay' '
 	git bisect replay log_to_replay.txt > my_bisect_log.txt &&
-	grep "$HASH5 is the first bad commit" my_bisect_log.txt &&
+	test_i18ngrep "$HASH5 is the first bad commit" my_bisect_log.txt &&
 	git bisect reset
 '
 
@@ -307,11 +307,11 @@ test_expect_success 'bisect run & skip: cannot tell between 2' '
 		false
 	else
 		test $? -eq 2 &&
-		grep "first bad commit could be any of" my_bisect_log.txt &&
+		test_i18ngrep "first bad commit could be any of" my_bisect_log.txt &&
 		! grep $HASH3 my_bisect_log.txt &&
 		! grep $HASH6 my_bisect_log.txt &&
-		grep $HASH4 my_bisect_log.txt &&
-		grep $HASH5 my_bisect_log.txt
+		test_i18ngrep $HASH4 my_bisect_log.txt &&
+		test_i18ngrep $HASH5 my_bisect_log.txt
 	fi
 '
 
@@ -328,7 +328,7 @@ test_expect_success 'bisect run & skip: find first bad' '
 	chmod +x test_script.sh &&
 	git bisect start $HASH7 $HASH1 &&
 	git bisect run ./test_script.sh > my_bisect_log.txt &&
-	grep "$HASH6 is the first bad commit" my_bisect_log.txt
+	test_i18ngrep "$HASH6 is the first bad commit" my_bisect_log.txt
 '
 
 test_expect_success 'bisect skip only one range' '
@@ -337,7 +337,7 @@ test_expect_success 'bisect skip only one range' '
 	git bisect skip $HASH1..$HASH5 &&
 	test "$HASH6" = "$(git rev-parse --verify HEAD)" &&
 	test_must_fail git bisect bad > my_bisect_log.txt &&
-	grep "first bad commit could be any of" my_bisect_log.txt
+	test_i18ngrep "first bad commit could be any of" my_bisect_log.txt
 '
 
 test_expect_success 'bisect skip many ranges' '
@@ -346,7 +346,7 @@ test_expect_success 'bisect skip many ranges' '
 	git bisect skip $HASH2 $HASH2.. ..$HASH5 &&
 	test "$HASH6" = "$(git rev-parse --verify HEAD)" &&
 	test_must_fail git bisect bad > my_bisect_log.txt &&
-	grep "first bad commit could be any of" my_bisect_log.txt
+	test_i18ngrep "first bad commit could be any of" my_bisect_log.txt
 '
 
 test_expect_success 'bisect starting with a detached HEAD' '
@@ -378,7 +378,7 @@ test_expect_success 'bisect does not create a "bisect" branch' '
 	rev_hash6=$(git rev-parse --verify HEAD) &&
 	test "$rev_hash6" = "$HASH6" &&
 	git bisect good > my_bisect_log.txt &&
-	grep "$HASH7 is the first bad commit" my_bisect_log.txt &&
+	test_i18ngrep "$HASH7 is the first bad commit" my_bisect_log.txt &&
 	git bisect reset &&
 	rev_hash6=$(git rev-parse --verify bisect) &&
 	test "$rev_hash6" = "$HASH6" &&
@@ -405,26 +405,26 @@ test_expect_success 'side branch creation' '
 test_expect_success 'good merge base when good and bad are siblings' '
 	git bisect start "$HASH7" "$SIDE_HASH7" > my_bisect_log.txt &&
 	test_i18ngrep "merge base must be tested" my_bisect_log.txt &&
-	grep $HASH4 my_bisect_log.txt &&
+	test_i18ngrep $HASH4 my_bisect_log.txt &&
 	git bisect good > my_bisect_log.txt &&
 	! grep "merge base must be tested" my_bisect_log.txt &&
-	grep $HASH6 my_bisect_log.txt &&
+	test_i18ngrep $HASH6 my_bisect_log.txt &&
 	git bisect reset
 '
 test_expect_success 'skipped merge base when good and bad are siblings' '
 	git bisect start "$SIDE_HASH7" "$HASH7" > my_bisect_log.txt &&
 	test_i18ngrep "merge base must be tested" my_bisect_log.txt &&
-	grep $HASH4 my_bisect_log.txt &&
+	test_i18ngrep $HASH4 my_bisect_log.txt &&
 	git bisect skip > my_bisect_log.txt 2>&1 &&
-	grep "warning" my_bisect_log.txt &&
-	grep $SIDE_HASH6 my_bisect_log.txt &&
+	test_i18ngrep "warning" my_bisect_log.txt &&
+	test_i18ngrep $SIDE_HASH6 my_bisect_log.txt &&
 	git bisect reset
 '
 
 test_expect_success 'bad merge base when good and bad are siblings' '
 	git bisect start "$HASH7" HEAD > my_bisect_log.txt &&
 	test_i18ngrep "merge base must be tested" my_bisect_log.txt &&
-	grep $HASH4 my_bisect_log.txt &&
+	test_i18ngrep $HASH4 my_bisect_log.txt &&
 	test_must_fail git bisect bad > my_bisect_log.txt 2>&1 &&
 	test_i18ngrep "merge base $HASH4 is bad" my_bisect_log.txt &&
 	test_i18ngrep "fixed between $HASH4 and \[$SIDE_HASH7\]" my_bisect_log.txt &&
@@ -454,8 +454,8 @@ test_expect_success 'many merge bases creation' '
 	B_HASH=$(git rev-parse --verify HEAD) &&
 	git merge-base --all "$A_HASH" "$B_HASH" > merge_bases.txt &&
 	test_line_count = 2 merge_bases.txt &&
-	grep "$HASH5" merge_bases.txt &&
-	grep "$SIDE_HASH5" merge_bases.txt
+	test_i18ngrep "$HASH5" merge_bases.txt &&
+	test_i18ngrep "$SIDE_HASH5" merge_bases.txt
 '
 
 test_expect_success 'good merge bases when good and bad are siblings' '
@@ -465,11 +465,11 @@ test_expect_success 'good merge bases when good and bad are siblings' '
 	test_i18ngrep "merge base must be tested" my_bisect_log2.txt &&
 	{
 		{
-			grep "$SIDE_HASH5" my_bisect_log.txt &&
-			grep "$HASH5" my_bisect_log2.txt
+			test_i18ngrep "$SIDE_HASH5" my_bisect_log.txt &&
+			test_i18ngrep "$HASH5" my_bisect_log2.txt
 		} || {
-			grep "$SIDE_HASH5" my_bisect_log2.txt &&
-			grep "$HASH5" my_bisect_log.txt
+			test_i18ngrep "$SIDE_HASH5" my_bisect_log2.txt &&
+			test_i18ngrep "$HASH5" my_bisect_log.txt
 		}
 	} &&
 	git bisect reset
@@ -478,7 +478,7 @@ test_expect_success 'good merge bases when good and bad are siblings' '
 test_expect_success 'optimized merge base checks' '
 	git bisect start "$HASH7" "$SIDE_HASH7" > my_bisect_log.txt &&
 	test_i18ngrep "merge base must be tested" my_bisect_log.txt &&
-	grep "$HASH4" my_bisect_log.txt &&
+	test_i18ngrep "$HASH4" my_bisect_log.txt &&
 	git bisect good > my_bisect_log2.txt &&
 	test -f ".git/BISECT_ANCESTORS_OK" &&
 	test "$HASH6" = $(git rev-parse --verify HEAD) &&
@@ -527,7 +527,7 @@ test_expect_success 'restricting bisection on one dir' '
 	para1=$(git rev-parse --verify HEAD) &&
 	test "$para1" = "$PARA_HASH1" &&
 	git bisect bad > my_bisect_log.txt &&
-	grep "$PARA_HASH1 is the first bad commit" my_bisect_log.txt
+	test_i18ngrep "$PARA_HASH1 is the first bad commit" my_bisect_log.txt
 '
 
 test_expect_success 'restricting bisection on one dir and a file' '
@@ -545,7 +545,7 @@ test_expect_success 'restricting bisection on one dir and a file' '
 	para1=$(git rev-parse --verify HEAD) &&
 	test "$para1" = "$PARA_HASH1" &&
 	git bisect good > my_bisect_log.txt &&
-	grep "$PARA_HASH4 is the first bad commit" my_bisect_log.txt
+	test_i18ngrep "$PARA_HASH4 is the first bad commit" my_bisect_log.txt
 '
 
 test_expect_success 'skipping away from skipped commit' '
@@ -576,7 +576,7 @@ test_expect_success 'test bisection on bare repo - --no-checkout specified' '
 			"test \$(git rev-list BISECT_HEAD ^$HASH2 --max-count=1 | wc -l) = 0" \
 			>../nocheckout.log
 	) &&
-	grep "$HASH3 is the first bad commit" nocheckout.log
+	test_i18ngrep "$HASH3 is the first bad commit" nocheckout.log
 '
 
 
@@ -591,7 +591,7 @@ test_expect_success 'test bisection on bare repo - --no-checkout defaulted' '
 			"test \$(git rev-list BISECT_HEAD ^$HASH2 --max-count=1 | wc -l) = 0" \
 			>../defaulted.log
 	) &&
-	grep "$HASH3 is the first bad commit" defaulted.log
+	test_i18ngrep "$HASH3 is the first bad commit" defaulted.log
 '
 
 #
@@ -633,7 +633,7 @@ EOF
 test_expect_success 'bisect fails if tree is broken on start commit' '
 	git bisect reset &&
 	test_must_fail git bisect start BROKEN_HASH7 BROKEN_HASH4 2>error.txt &&
-	test_cmp expected.missing-tree.default error.txt
+	test_i18ncmp expected.missing-tree.default error.txt
 '
 
 test_expect_success 'bisect fails if tree is broken on trial commit' '
@@ -641,7 +641,7 @@ test_expect_success 'bisect fails if tree is broken on trial commit' '
 	test_must_fail git bisect start BROKEN_HASH9 BROKEN_HASH4 2>error.txt &&
 	git reset --hard broken &&
 	git checkout broken &&
-	test_cmp expected.missing-tree.default error.txt
+	test_i18ncmp expected.missing-tree.default error.txt
 '
 
 check_same()
@@ -726,7 +726,7 @@ test_expect_success 'bisect log: successful result' '
 	git bisect start $HASH4 $HASH2 &&
 	git bisect good &&
 	git bisect log >bisect-log.txt &&
-	test_cmp expected.bisect-log bisect-log.txt &&
+	test_i18ncmp expected.bisect-log bisect-log.txt &&
 	git bisect reset
 '
 
@@ -746,7 +746,7 @@ test_expect_success 'bisect log: only skip commits left' '
 	git bisect start $HASH4 $HASH2 &&
 	test_must_fail git bisect skip &&
 	git bisect log >bisect-skip-log.txt &&
-	test_cmp expected.bisect-skip-log bisect-skip-log.txt &&
+	test_i18ncmp expected.bisect-skip-log bisect-skip-log.txt &&
 	git bisect reset
 '
 
@@ -780,14 +780,14 @@ test_expect_success 'bisect start with one new and old' '
 	git bisect new $HASH4 &&
 	git bisect new &&
 	git bisect new >bisect_result &&
-	grep "$HASH2 is the first new commit" bisect_result &&
+	test_i18ngrep "$HASH2 is the first new commit" bisect_result &&
 	git bisect log >log_to_replay.txt &&
 	git bisect reset
 '
 
 test_expect_success 'bisect replay with old and new' '
 	git bisect replay log_to_replay.txt >bisect_result &&
-	grep "$HASH2 is the first new commit" bisect_result &&
+	test_i18ngrep "$HASH2 is the first new commit" bisect_result &&
 	git bisect reset
 '
 
@@ -811,10 +811,10 @@ test_expect_success 'bisect terms shows good/bad after start' '
 	git bisect start HEAD $HASH1 &&
 	git bisect terms --term-good >actual &&
 	echo good >expected &&
-	test_cmp expected actual &&
+	test_i18ncmp expected actual &&
 	git bisect terms --term-bad >actual &&
 	echo bad >expected &&
-	test_cmp expected actual
+	test_i18ncmp expected actual
 '
 
 test_expect_success 'bisect start with one term1 and term2' '
@@ -824,14 +824,14 @@ test_expect_success 'bisect start with one term1 and term2' '
 	git bisect term1 $HASH4 &&
 	git bisect term1 &&
 	git bisect term1 >bisect_result &&
-	grep "$HASH2 is the first term1 commit" bisect_result &&
+	test_i18ngrep "$HASH2 is the first term1 commit" bisect_result &&
 	git bisect log >log_to_replay.txt &&
 	git bisect reset
 '
 
 test_expect_success 'bisect replay with term1 and term2' '
 	git bisect replay log_to_replay.txt >bisect_result &&
-	grep "$HASH2 is the first term1 commit" bisect_result &&
+	test_i18ngrep "$HASH2 is the first term1 commit" bisect_result &&
 	git bisect reset
 '
 
@@ -840,7 +840,7 @@ test_expect_success 'bisect start term1 term2' '
 	git bisect start --term-new term1 --term-old term2 $HASH4 $HASH1 &&
 	git bisect term1 &&
 	git bisect term1 >bisect_result &&
-	grep "$HASH2 is the first term1 commit" bisect_result &&
+	test_i18ngrep "$HASH2 is the first term1 commit" bisect_result &&
 	git bisect log >log_to_replay.txt &&
 	git bisect reset
 '
@@ -878,10 +878,10 @@ test_expect_success 'bisect start --term-* does store terms' '
 	test_i18ncmp expected actual &&
 	git bisect terms --term-bad >actual &&
 	echo one >expected &&
-	test_cmp expected actual &&
+	test_i18ncmp expected actual &&
 	git bisect terms --term-good >actual &&
 	echo two >expected &&
-	test_cmp expected actual
+	test_i18ncmp expected actual
 '
 
 test_expect_success 'bisect start takes options and revs in any order' '
@@ -891,7 +891,7 @@ test_expect_success 'bisect start takes options and revs in any order' '
 		$HASH1 --term-good three -- &&
 	(git bisect terms --term-bad && git bisect terms --term-good) >actual &&
 	printf "%s\n%s\n" bad-term three >expected &&
-	test_cmp expected actual
+	test_i18ncmp expected actual
 '
 
 test_expect_success 'git bisect reset cleans bisection state properly' '

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

```

## Pranit Bauva, 2017-10-27 15:06

Subject: [PATCH v16 Part II 4/8] bisect--helper: `check_and_set_terms` shell function in C
Message-ID: <0102015f5e5ee273-de8fa1d2-1d59-4990-93f5-d36a6cb3f45a-000000@eu-west-1.amazonses.com>
URL: https://gitlist.dev/e/0102015f5e5ee273-de8fa1d2-1d59-4990-93f5-d36a6cb3f45a-000000%40eu-west-1.amazonses.com
In-Reply-To: <0102015f5e5ee171-f30f4868-886f-47a1-a4e4-b4936afc545d-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.

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 | 41 ++++++++++++++++++++++++++++++++++++++++-
 git-bisect.sh            | 36 ++++--------------------------------
 2 files changed, 44 insertions(+), 33 deletions(-)

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 6295f53c850a8..65abf8a70c6d9 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -20,6 +20,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> <good_term> <bad_term> [<nolog>]"),
+	N_("git bisect--helper --bisect-check-and-set-terms <command> <good_term> <bad_term>"),
 	NULL
 };
 
@@ -234,6 +235,35 @@ static int bisect_write(const char *state, const char *rev,
 	return retval;
 }
 
+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) &&
+	    strcmp(cmd, terms->term_good))
+		return error(_("Invalid command: you're currently in a "
+				"%s/%s bisect"), terms->term_bad,
+				terms->term_good);
+
+	if (!has_term_file) {
+		if (one_of(cmd, "bad", "good", NULL)) {
+			free_terms(terms);
+			set_terms(terms, "bad", "good");
+			return write_terms(terms->term_bad, terms->term_good);
+		}
+		else if (one_of(cmd, "new", "old", NULL)) {
+			free_terms(terms);
+			set_terms(terms, "new", "old");
+			return write_terms(terms->term_bad, terms->term_good);
+		}
+	}
+
+	return 0;
+}
+
 int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 {
 	enum {
@@ -242,7 +272,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 		BISECT_CLEAN_STATE,
 		CHECK_EXPECTED_REVS,
 		BISECT_RESET,
-		BISECT_WRITE
+		BISECT_WRITE,
+		CHECK_AND_SET_TERMS
 	} cmdmode = 0;
 	int no_checkout = 0, res = 0;
 	struct option options[] = {
@@ -258,6 +289,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 			 N_("reset the bisection state"), BISECT_RESET),
 		OPT_CMDMODE(0, "bisect-write", &cmdmode,
 			 N_("update the refs according to the bisection state and may write it to 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()
@@ -296,6 +329,12 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 		set_terms(&terms, argv[3], argv[2]);
 		res = bisect_write(argv[0], argv[1], &terms, nolog);
 		break;
+	case CHECK_AND_SET_TERMS:
+		if (argc != 3)
+			return error(_("--check-and-set-terms requires 3 arguments"));
+		set_terms(&terms, argv[2], argv[1]);
+		res = check_and_set_terms(&terms, argv[0]);
+		break;
 	default:
 		return error("BUG: unknown subcommand '%d'", cmdmode);
 	}
diff --git a/git-bisect.sh b/git-bisect.sh
index 1eab94ec943a1..0447b73aa588b 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -238,7 +238,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 "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/420

```

## Pranit Bauva, 2017-10-27 15:06

Subject: [PATCH v16 Part II 1/8] bisect--helper: `bisect_reset` shell function in C
Message-ID: <0102015f5e5ee171-f30f4868-886f-47a1-a4e4-b4936afc545d-000000@eu-west-1.amazonses.com>
URL: https://gitlist.dev/e/0102015f5e5ee171-f30f4868-886f-47a1-a4e4-b4936afc545d-000000%40eu-west-1.amazonses.com
In-Reply-To: <0102015ecc65d695-22151d3b-752b-4c10-a3a3-b8ef52491664-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>

---
Hey,

This is the part 2 of the initial series[1] on bisect re-write. When I
submitted my patches of the part 1 of the bisect series, Ramsay informed
that he has been working on my previous patches and pointed me to his
patches. I have incorporated his changes into this series. Stephan did a
awesome job in reviewing my series and I have incorporated those changes
as well.

Ramsay's requested changes which have been incorporated:
 * bisect--helper: convert to struct object_id: I have took out the bits
   which were in this series and hand-picked them in this series.
 * bisect--helper: add a log_commit() helper function: This function
   might not seem to be very relevant in this part of the series, but it
   is going to be in future parts of the series as described in
   `188ea5855d93df` of Ramsay's git.git repo[2].
 * bisect--helper: reduce scope of a variable in `bisect_start`
 * bisect--helper: remove useless sub-expression in condition: I had
   already done this change as suggested by Stephan.
 * bisect--helper: set correct term from --term-new= option: I had
   already done this change as suggested by Stephan.
 * bisect--helper: remove redundant assignment to has_double_dash
 * bisect--helper: remove redundant goto's
 * bisect--helper: add some vertical whitespace
 * bisect--helper: fixup some coding style issues

Ramsay's changes which I had already done ([]-ed comments):
 * `bisect_write`: add a GIT_PATH_FUNC() removed earlier
 * `bisect_write`: lookup_commit_reference() now takes an object_id
 * `bisect_next_check`: addressed comments in Stephan's review and
   Junio's squash
 * `bisect_terms`: fixed up usage string and tested for --term-[good|old] and
   --term-[bad|new] in bisect-terms subcommand argument processing:
   instead of fixing up usage string, I just removed it as Stephan
   suggested.

Stephan's requested changes which have been incorporated:
 * Use goto's using `goto fail` and `goto finish`
 * Marking strings for translation
 * Stored res value and then returned it at the end
 * Implement set_terms() and free_terms() for proper memory handling for
   struct bisect_terms and fixed all leakages
 * Remove bisect_voc() function
 * Remove the bisect_term_usage string in bisect_terms()
 * Use return error() every where instead of die()
 * Improving readability of a message in bisect_terms()
 * bisect_start(): fix some typos, remove has_double_dash, join the two
   if branches, fix the bug when it should be terms->term_bad,
   decomposed it into smaller functions by introducing
   bisect_append_log()

A big thanks to Stephan and Ramsay for patiently reviewing my series and
helping me get this merged.

[1]:
https://public-inbox.org/git/0102015ecc65d695-22151d3b-752b-4c10-a3a3-b8ef52491664-000000@eu-west-1.amazonses.com/

[2]: git://repo.or.cz/git/raj.git
Regards,
Pranit Bauva
---
 builtin/bisect--helper.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++-
 git-bisect.sh            | 28 ++-------------------------
 2 files changed, 50 insertions(+), 27 deletions(-)

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 35d2105f941c6..12754448f7b6a 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -3,15 +3,21 @@
 #include "parse-options.h"
 #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")
 static GIT_PATH_FUNC(git_path_bisect_ancestors_ok, "BISECT_ANCESTORS_OK")
+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
 };
 
@@ -106,13 +112,48 @@ static void check_expected_revs(const char **revs, int rev_nr)
 	}
 }
 
+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)
+			return !printf(_("We are not bisecting.\n"));
+		strbuf_rtrim(&branch);
+	} else {
+		struct object_id oid;
+
+		if (get_oid_commit(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,
-		CHECK_EXPECTED_REVS
+		CHECK_EXPECTED_REVS,
+		BISECT_RESET
 	} cmdmode = 0;
 	int no_checkout = 0;
 	struct option options[] = {
@@ -124,6 +165,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 			 N_("cleanup the bisection state"), BISECT_CLEAN_STATE),
 		OPT_CMDMODE(0, "check-expected-revs", &cmdmode,
 			 N_("check for expected revs"), CHECK_EXPECTED_REVS),
+		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()
@@ -149,6 +192,10 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 	case CHECK_EXPECTED_REVS:
 		check_expected_revs(argv, argc);
 		return 0;
+	case BISECT_RESET:
+		if (argc > 1)
+			return error(_("--bisect-reset requires either no argument or a commit"));
+		return bisect_reset(argc ? argv[0] : NULL);
 	default:
 		return error("BUG: unknown subcommand '%d'", cmdmode);
 	}
diff --git a/git-bisect.sh b/git-bisect.sh
index 0138a8860e377..4d403242482cf 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -393,35 +393,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
@@ -611,7 +587,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/420

```

## Pranit Bauva, 2017-10-27 15:06

Subject: [PATCH v16 Part II 6/8] bisect--helper: `get_terms` & `bisect_terms` shell function in C
Message-ID: <0102015f5e5ee303-449f1b76-ff53-433c-8973-8b4850466810-000000@eu-west-1.amazonses.com>
URL: https://gitlist.dev/e/0102015f5e5ee303-449f1b76-ff53-433c-8973-8b4850466810-000000%40eu-west-1.amazonses.com
In-Reply-To: <0102015f5e5ee171-f30f4868-886f-47a1-a4e4-b4936afc545d-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.

Also use error() to report "no terms defined" and accordingly change the
test in t6030.

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    | 69 +++++++++++++++++++++++++++++++++++++++++++--
 git-bisect.sh               | 35 ++---------------------
 t/t6030-bisect-porcelain.sh |  2 +-
 3 files changed, 70 insertions(+), 36 deletions(-)

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 0f9c3e63821b8..ab0580ce0089a 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-write <state> <revision> <good_term> <bad_term> [<nolog>]"),
 	N_("git bisect--helper --bisect-check-and-set-terms <command> <good_term> <bad_term>"),
 	N_("git bisect--helper --bisect-next-check [<term>] <good_term> <bad_term>"),
+	N_("git bisect--helper --bisect-terms [--term-good | --term-old | --term-bad | --term-new]"),
 	NULL
 };
 
@@ -344,6 +345,62 @@ static int bisect_next_check(const struct bisect_terms *terms,
 	return retval;
 }
 
+static int get_terms(struct bisect_terms *terms)
+{
+	struct strbuf str = STRBUF_INIT;
+	FILE *fp = NULL;
+	int res = 0;
+
+	fp = fopen(git_path_bisect_terms(), "r");
+	if (!fp)
+		goto fail;
+
+	free_terms(terms);
+	strbuf_getline_lf(&str, fp);
+	terms->term_bad = strbuf_detach(&str, NULL);
+	strbuf_getline_lf(&str, fp);
+	terms->term_good = strbuf_detach(&str, NULL);
+	goto finish;
+
+fail:
+	res = -1;
+finish:
+	if (fp)
+		fclose(fp);
+	strbuf_release(&str);
+	return res;
+}
+
+static int bisect_terms(struct bisect_terms *terms, const char **argv, int argc)
+{
+	int i;
+
+	if (get_terms(terms))
+		return error(_("no terms defined"));
+
+	if (argc > 1)
+		return error(_("--bisect-term requires exactly one argument"));
+
+	if (argc == 0)
+		return !printf(_("Your current terms are %s for the old state\n"
+				 "and %s for the new state.\n"),
+				 terms->term_good, terms->term_bad);
+
+	for (i = 0; i < argc; i++) {
+		if (!strcmp(argv[i], "--term-good"))
+			printf(_("%s\n"), terms->term_good);
+		else if (!strcmp(argv[i], "--term-bad"))
+			printf(_("%s\n"), terms->term_bad);
+		else
+			error(_("BUG: invalid argument %s for 'git bisect terms'.\n"
+				  "Supported 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 {
@@ -354,7 +411,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 		BISECT_RESET,
 		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[] = {
@@ -374,6 +432,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()
@@ -381,7 +441,7 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 	struct bisect_terms 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);
@@ -424,6 +484,11 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 		set_terms(&terms, argv[1], argv[0]);
 		res = bisect_next_check(&terms, argc == 3 ? argv[2] : NULL);
 		break;
+	case BISECT_TERMS:
+		if (argc > 1)
+			return error(_("--bisect-terms requires 0 or 1 argument"));
+		res = bisect_terms(&terms, argv, argc);
+		break;
 	default:
 		return error("BUG: unknown subcommand '%d'", cmdmode);
 	}
diff --git a/git-bisect.sh b/git-bisect.sh
index c21fafc62d71f..1bc5749f78484 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
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index f84ff941c3624..55835ee4a4715 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -802,7 +802,7 @@ test_expect_success 'bisect terms needs 0 or 1 argument' '
 	test_must_fail git bisect terms only-one &&
 	test_must_fail git bisect terms 1 2 &&
 	test_must_fail git bisect terms 2>actual &&
-	echo "no terms defined" >expected &&
+	echo "error: no terms defined" >expected &&
 	test_i18ncmp expected actual
 '
 

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

```

## Pranit Bauva, 2017-10-27 15:32

Subject: Re: [PATCH v16 Part II 1/8] bisect--helper: `bisect_reset` shell function in C
Message-ID: <CAFZEwPPxuno5ey9qWVVE9=CU5f-BmBZjhQMrHOXS1tFG6r4LkQ@mail.gmail.com>
URL: https://gitlist.dev/e/CAFZEwPPxuno5ey9qWVVE9%3DCU5f-BmBZjhQMrHOXS1tFG6r4LkQ%40mail.gmail.com
In-Reply-To: <0102015f5e5ee171-f30f4868-886f-47a1-a4e4-b4936afc545d-000000@eu-west-1.amazonses.com>

```
Hey,

I forgot to mention. One can find the travis build here[1] which is passing.

[1]: https://travis-ci.org/git/git/builds/293725346

Regards,
Pranit Bauva

```

## Martin Ågren, 2017-10-27 17:28

Subject: Re: [PATCH v16 Part II 2/8] bisect--helper: `bisect_write` shell function in C
Message-ID: <20171027172845.15437-1-martin.agren@gmail.com>
URL: https://gitlist.dev/e/20171027172845.15437-1-martin.agren%40gmail.com
In-Reply-To: <0102015f5e5ee22c-ea9c3a38-5d42-4dce-a54b-45c59768a70b-000000@eu-west-1.amazonses.com>

```
On 27 October 2017 at 17:06, Pranit Bauva <pranit.bauva@gmail.com> wrote:
> +static void free_terms(struct bisect_terms *terms)
> +{
> +       if (!terms->term_good)
> +               free((void *) terms->term_good);
> +       if (!terms->term_bad)
> +               free((void *) terms->term_bad);
> +}

These look like no-ops. Remove `!` for correctness, or `if (...)` for
simplicity, since `free()` can handle NULL.

You leave the pointers dangling, but you're ok for now since this is the
last thing that happens in `cmd_bisect__helper()`. Your later patches
add more users, but they're also ok, since they immediately assign new
values.

In case you (and others) find it useful, the below is a patch I've been
sitting on for a while as part of a series to plug various memory-leaks.
`FREE_AND_NULL_CONST()` would be useful in precisely these situations.

-- >8 --
Subject: git-compat-util: add FREE_AND_NULL_CONST() wrapper

Commit 481df65 ("git-compat-util: add a FREE_AND_NULL() wrapper around
free(ptr); ptr = NULL", 2017-06-15) added `FREE_AND_NULL()`. One
advantage of this macro is that it reduces risk of copy-paste errors and
reviewer-fatigue, especially when cleaning up more than just a single
pointer.

However, `FREE_AND_NULL()` can not be used with a const-pointer:

  struct foo { const char *bar; }
  ...
  FREE_AND_NULL(baz.bar);

This causes the compiler to barf as `free()` is called: "error: passing
argument 1 of ‘free’ discards ‘const’ qualifier from pointer target
type". A naive attempt to remedy this might look like this:

  FREE_AND_NULL((void *)baz.bar);

Now the assignment is problematic: "error: lvalue required as left
operand of assignment".

Add a `FREE_AND_NULL_CONST()` wrapper macro which acts as
`FREE_AND_NULL()`, except it casts to `(void *)` when freeing. As a
demonstration, use this in two existing code paths that were identified
by some grepping. Future patches will use it to clean up struct-fields:

  FREE_AND_NULL_CONST(baz.bar);

This macro is a slightly bigger hammer than `FREE_AND_NULL` and has a
slightly larger potential for misuse. Document that `FREE_AND_NULL`
should be preferred.

Signed-off-by: Martin Ågren <martin.agren@gmail.com>
---
 argv-array.c      | 3 +--
 git-compat-util.h | 8 ++++++++
 submodule.c       | 3 +--
 3 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/argv-array.c b/argv-array.c
index 5d370fa33..433a5997a 100644
--- a/argv-array.c
+++ b/argv-array.c
@@ -59,8 +59,7 @@ void argv_array_pop(struct argv_array *array)
 {
 	if (!array->argc)
 		return;
-	free((char *)array->argv[array->argc - 1]);
-	array->argv[array->argc - 1] = NULL;
+	FREE_AND_NULL_CONST(array->argv[array->argc - 1]);
 	array->argc--;
 }
 
diff --git a/git-compat-util.h b/git-compat-util.h
index cedad4d58..ca3dcbc58 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -815,6 +815,14 @@ extern FILE *fopen_or_warn(const char *path, const char *mode);
  */
 #define FREE_AND_NULL(p) do { free(p); (p) = NULL; } while (0)
 
+/*
+ * FREE_AND_NULL_CONST(ptr) is like FREE_AND_NULL(ptr) except it casts
+ * to (void *) when calling free. This is useful when handling, e.g., a
+ * `const char *`, but it can also be misused. Prefer FREE_AND_NULL, and
+ * use this only when you need to and it is safe to do so.
+ */
+#define FREE_AND_NULL_CONST(p) do { free((void *)(p)); (p) = NULL; } while (0)
+
 #define ALLOC_ARRAY(x, alloc) (x) = xmalloc(st_mult(sizeof(*(x)), (alloc)))
 #define REALLOC_ARRAY(x, alloc) (x) = xrealloc((x), st_mult(sizeof(*(x)), (alloc)))
 
diff --git a/submodule.c b/submodule.c
index 63e7094e1..7759f9050 100644
--- a/submodule.c
+++ b/submodule.c
@@ -364,8 +364,7 @@ int parse_submodule_update_strategy(const char *value,
 {
 	enum submodule_update_type type;
 
-	free((void*)dst->command);
-	dst->command = NULL;
+	FREE_AND_NULL_CONST(dst->command);
 
 	type = parse_submodule_update_type(value);
 	if (type == SM_UPDATE_UNSPECIFIED)
-- 
2.15.0.rc2.5.g97dd00bb7


```

## Martin Ågren, 2017-10-27 17:35

Subject: Re: [PATCH v16 Part II 5/8] bisect--helper: `bisect_next_check` shell function in C
Message-ID: <CAN0heSpRc5VwuMgusoPBegUSsc3t8USK6hm2wd5CyqNpKUiObg@mail.gmail.com>
URL: https://gitlist.dev/e/CAN0heSpRc5VwuMgusoPBegUSsc3t8USK6hm2wd5CyqNpKUiObg%40mail.gmail.com
In-Reply-To: <0102015f5e5ee2ca-bf824205-86e6-4259-b7d7-a68e2fbb087d-000000@eu-west-1.amazonses.com>

```
On 27 October 2017 at 17:06, Pranit Bauva <pranit.bauva@gmail.com> wrote:
> +               /*
> +                * have bad (or new) but not good (or old). We could bisect
> +                * although this is less optimum.
> +                */
> +               fprintf(stderr, _("Warning: bisecting only with a %s commit\n"),
> +                       terms->term_bad);

Maybe this should use `warning()`?

> -               # 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

I wonder if we can somehow pick up the existing translation? It would
now be fuzzy, in some sense, but since the string was originally in a
different file, maybe the po-tools won't be able to discover the
fuzzyness? We could add a TRANSLATORS-comment, so that the translators
know that this string matches an old one. There are more strings like
that in this patch, and maybe in some others as well, I haven't looked.

(Adding Jiang to cc.)

```

## Junio C Hamano, 2017-10-27 17:40

Subject: Re: [PATCH v16 Part II 1/8] bisect--helper: `bisect_reset` shell function in C
Message-ID: <xmqqshe4bib0.fsf@gitster.mtv.corp.google.com>
URL: https://gitlist.dev/e/xmqqshe4bib0.fsf%40gitster.mtv.corp.google.com
In-Reply-To: <0102015f5e5ee171-f30f4868-886f-47a1-a4e4-b4936afc545d-000000@eu-west-1.amazonses.com>

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

> +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)
> +			return !printf(_("We are not bisecting.\n"));
> +		strbuf_rtrim(&branch);
> +	} else {
> +		struct object_id oid;
> +
> +		if (get_oid_commit(commit, &oid))
> +			return error(_("'%s' is not a valid commit"), commit);
> +		strbuf_addstr(&branch, commit);

The original checks "test -s BISECT_START" and complains, even when
an explicit commit is given.  With this change, when the user is not
bisecting, giving "git bisect reset master" goes ahead---it is
likely that BISECT_HEAD does not exist and we may hit "Could not
check out" error, but if BISECT_HEAD is left behind from a previous
run (which is likely completely unrelated to whatever the user
currently is doing), we'd end up doing quite a random thing, no?

> +	}
> +
> +	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;

How does this return value affect the value eventually given to
exit(3), called by somewhere in git.c that called this function?

The call graph would be

    common-main.c::main()
    -> git.c::cmd_main()
       -> handle_builtin()
          . exit(run_builtin())
          -> run_builtin()
             . status = p->fn()
             -> cmd_bisect__helper()
                . return bisect_reset()
                -> bisect_reset()
                   . return -1
             . if (status) return status;

So the -1 is returned throughout the callchain and exit(3) ends up
getting it---which is not quite right.  We shouldn't be giving
negative value to exit(3).  bisect_clean_state() and other helper
functions may already share the same issue.

> +		}
> +		argv_array_clear(&argv);
> +	}
> +
> +	strbuf_release(&branch);
> +	return bisect_clean_state();
> +}

```

## Junio C Hamano, 2017-10-27 18:19

Subject: Re: [PATCH v16 Part II 2/8] bisect--helper: `bisect_write` shell function in C
Message-ID: <xmqqo9osbghy.fsf@gitster.mtv.corp.google.com>
URL: https://gitlist.dev/e/xmqqo9osbghy.fsf%40gitster.mtv.corp.google.com
In-Reply-To: <0102015f5e5ee22c-ea9c3a38-5d42-4dce-a54b-45c59768a70b-000000@eu-west-1.amazonses.com>

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

> -		bisect_write "$state" "$rev"
> +		git bisect--helper --bisect-write "$state" "$rev" "$TERM_GOOD" "$TERM_BAD" || exit

I can see why two extra "terms" parameters need to be passed to this
helper at this step; looking at patches around 4/8 and 6/8 where C
code can directly find out what words are used for GOOD and BAD, we
should be able to lose these two extra parameters from this helper
by internally making a call to get_terms() from bisect_write() ;-)

```

## Martin Ågren, 2017-10-27 20:04

Subject: Re: [PATCH v16 Part II 6/8] bisect--helper: `get_terms` & `bisect_terms` shell function in C
Message-ID: <CAN0heSrWbgNEfeXj0D0SBiyC_9i++qecBcb5Kuf4WtS1wFxKDQ@mail.gmail.com>
URL: https://gitlist.dev/e/CAN0heSrWbgNEfeXj0D0SBiyC_9i%2B%2BqecBcb5Kuf4WtS1wFxKDQ%40mail.gmail.com
In-Reply-To: <0102015f5e5ee303-449f1b76-ff53-433c-8973-8b4850466810-000000@eu-west-1.amazonses.com>

```
On 27 October 2017 at 17:06, Pranit Bauva <pranit.bauva@gmail.com> wrote:
> +       for (i = 0; i < argc; i++) {
> +               if (!strcmp(argv[i], "--term-good"))
> +                       printf(_("%s\n"), terms->term_good);
> +               else if (!strcmp(argv[i], "--term-bad"))
> +                       printf(_("%s\n"), terms->term_bad);

You seem to have lost --term-old and --term-new. I also wonder why these
would need translating. You break GETTEXT_POISON here, then fix it in
patch 8/8.

I'm not even sure you need patch 8/8. If I drop these two `_()`, I can
run `git rebase -ix "make GETTEXT_POISON=Yes test"` on the entire series
without failure. Patch 8/8 also switches to `test_i18ngrep` for some
usages of `git branch` and for some checks on `.git/BISECT_START`. I'm
not sure that's needed.

> +               else
> +                       error(_("BUG: invalid argument %s for 'git bisect terms'.\n"
> +                                 "Supported options are: "
> +                                 "--term-good|--term-old and "
> +                                 "--term-bad|--term-new."), argv[i]);
> +       }

```

## Stephan Beyer, 2017-10-30 13:22

Subject: Re: [PATCH v16 Part II 1/8] bisect--helper: `bisect_reset` shell function in C
Message-ID: <7d978e14-2eec-23b9-c25f-e6b361101b03@gmx.net>
URL: https://gitlist.dev/e/7d978e14-2eec-23b9-c25f-e6b361101b03%40gmx.net
In-Reply-To: <0102015f5e5ee171-f30f4868-886f-47a1-a4e4-b4936afc545d-000000@eu-west-1.amazonses.com>

```
Hi Pranit,

On 10/27/2017 05:06 PM, Pranit Bauva wrote:
> A big thanks to Stephan and Ramsay for patiently reviewing my series and
> helping me get this merged.

You're welcome. ;)

In addition to the things mentioned by the other reviewers, only a minor
thing:

> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
> index 35d2105f941c6..12754448f7b6a 100644
> --- a/builtin/bisect--helper.c
> +++ b/builtin/bisect--helper.c
[...]> @@ -106,13 +112,48 @@ static void check_expected_revs(const char
**revs, int rev_nr)
>  	}
>  }
>  
> +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)
> +			return !printf(_("We are not bisecting.\n"));

This is weird; I had to look up the return value of printf first before
I could understand what you are doing ;) I think that it is meant as a
shortcut for

	printf(_("We are not bisecting.\n"));
	return 0;

but please also express it with these two lines. (Or what is the point
of returning a non-zero value only in the case when nothing could be
printed?)

Best,
Stephan

```

## Stephan Beyer, 2017-10-30 13:38

Subject: Re: [PATCH v16 Part II 2/8] bisect--helper: `bisect_write` shell function in C
Message-ID: <49b9e9e6-8aeb-9a14-9309-f9274fe13e7d@gmx.net>
URL: https://gitlist.dev/e/49b9e9e6-8aeb-9a14-9309-f9274fe13e7d%40gmx.net
In-Reply-To: <0102015f5e5ee22c-ea9c3a38-5d42-4dce-a54b-45c59768a70b-000000@eu-west-1.amazonses.com>

```
Also just a minor in addition to the others:

> @@ -153,9 +241,10 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
>  		WRITE_TERMS,
>  		BISECT_CLEAN_STATE,
>  		CHECK_EXPECTED_REVS,
> -		BISECT_RESET
> +		BISECT_RESET,
> +		BISECT_WRITE
>  	} cmdmode = 0;
> -	int no_checkout = 0;
> +	int no_checkout = 0, res = 0;

This last change is not necessary. You never use "res".

Stephan

```

## Stephan Beyer, 2017-10-30 16:24

Subject: Re: [PATCH v16 Part II 2/8] bisect--helper: `bisect_write` shell function in C
Message-ID: <ff95b58e-ed76-ebe3-fee9-7f73f1a0c868@gmx.net>
URL: https://gitlist.dev/e/ff95b58e-ed76-ebe3-fee9-7f73f1a0c868%40gmx.net
In-Reply-To: <49b9e9e6-8aeb-9a14-9309-f9274fe13e7d@gmx.net>

```
On 10/30/2017 02:38 PM, Stephan Beyer wrote:
> This last change is not necessary. You never use "res".

Whoops, ignore this!
I compared old and new diff and mixed something up, it seems.

Sorry,
Stephan

```

## Stephan Beyer, 2017-10-30 16:34

Subject: Re: [PATCH v16 Part II 6/8] bisect--helper: `get_terms` & `bisect_terms` shell function in C
Message-ID: <af5f503a-6c2c-436f-ae74-cfbb11cb7a7f@gmx.net>
URL: https://gitlist.dev/e/af5f503a-6c2c-436f-ae74-cfbb11cb7a7f%40gmx.net
In-Reply-To: <0102015f5e5ee303-449f1b76-ff53-433c-8973-8b4850466810-000000@eu-west-1.amazonses.com>

```
On 10/27/2017 05:06 PM, Pranit Bauva wrote:
> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
> index 0f9c3e63821b8..ab0580ce0089a 100644
> --- a/builtin/bisect--helper.c
> +++ b/builtin/bisect--helper.c
[...]
> +static int bisect_terms(struct bisect_terms *terms, const char **argv, int argc)
> +{
> +	int i;
> +
> +	if (get_terms(terms))
> +		return error(_("no terms defined"));
> +
> +	if (argc > 1)
> +		return error(_("--bisect-term requires exactly one argument"));
> +
> +	if (argc == 0)
> +		return !printf(_("Your current terms are %s for the old state\n"
> +				 "and %s for the new state.\n"),
> +				 terms->term_good, terms->term_bad);

Same as in 1/8: you probably want "printf(...); return 0;" except there
is a good reason.

> +
> +	for (i = 0; i < argc; i++) {
> +		if (!strcmp(argv[i], "--term-good"))
> +			printf(_("%s\n"), terms->term_good);
> +		else if (!strcmp(argv[i], "--term-bad"))
> +			printf(_("%s\n"), terms->term_bad);

The last two printfs: I think there is no point in translating "%s\n",
so using "%s\n" instead of _("%s\n") looks more reasonable.

> +		else
> +			error(_("BUG: invalid argument %s for 'git bisect terms'.\n"
> +				  "Supported options are: "
> +				  "--term-good|--term-old and "
> +				  "--term-bad|--term-new."), argv[i]);

Should this be "return error(...)"?

> +	}
> +
> +	return 0;
> +}
> +

Stephan

```

## Stephan Beyer, 2017-10-30 16:51

Subject: Re: [PATCH v16 Part II 7/8] bisect--helper: `bisect_start` shell function partially in C
Message-ID: <c26e1cab-a3d0-8cae-b18b-eaa02ce1240c@gmx.net>
URL: https://gitlist.dev/e/c26e1cab-a3d0-8cae-b18b-eaa02ce1240c%40gmx.net
In-Reply-To: <0102015f5e5ee2f5-3d106a61-7f79-4973-851f-f55a691a6189-000000@eu-west-1.amazonses.com>

```
Hi,

> +			return error(_("unrecognised option: '%s'"), arg);

Please write "unrecogni_z_ed".

Since the string for translation changed from
	"unrecognised option: '$arg'"
to
	"unrecognised option: '%s'"
anyway, it does not result in further work for the translators to
correct it to
	"unrecognized option: '%s'"

Stephan

```

## Pranit Bauva, 2017-10-30 17:26

Subject: Re: [PATCH v16 Part II 1/8] bisect--helper: `bisect_reset` shell function in C
Message-ID: <CAFZEwPPq30e_5Zp0UZ8UgTG415w3vLq_J6e=GFxThCZdCFjUxA@mail.gmail.com>
URL: https://gitlist.dev/e/CAFZEwPPq30e_5Zp0UZ8UgTG415w3vLq_J6e%3DGFxThCZdCFjUxA%40mail.gmail.com
In-Reply-To: <xmqqshe4bib0.fsf@gitster.mtv.corp.google.com>

```
Hey Junio,

On Fri, Oct 27, 2017 at 11:10 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Pranit Bauva <pranit.bauva@gmail.com> writes:
>
>> +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)
>> +                     return !printf(_("We are not bisecting.\n"));
>> +             strbuf_rtrim(&branch);
>> +     } else {
>> +             struct object_id oid;
>> +
>> +             if (get_oid_commit(commit, &oid))
>> +                     return error(_("'%s' is not a valid commit"), commit);
>> +             strbuf_addstr(&branch, commit);
>
> The original checks "test -s BISECT_START" and complains, even when
> an explicit commit is given.  With this change, when the user is not
> bisecting, giving "git bisect reset master" goes ahead---it is
> likely that BISECT_HEAD does not exist and we may hit "Could not
> check out" error, but if BISECT_HEAD is left behind from a previous
> run (which is likely completely unrelated to whatever the user
> currently is doing), we'd end up doing quite a random thing, no?

Yes. Thanks for mentioning this point. I don't quite remember things
right now about what made me do this change. There might have been
something which had made me do this change because this isn't just a
silly mistake. Any which ways, I couldn't recollect the reason (should
be more careful to put code comments).

>> +     }
>> +
>> +     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;
>
> How does this return value affect the value eventually given to
> exit(3), called by somewhere in git.c that called this function?
>
> The call graph would be
>
>     common-main.c::main()
>     -> git.c::cmd_main()
>        -> handle_builtin()
>           . exit(run_builtin())
>           -> run_builtin()
>              . status = p->fn()
>              -> cmd_bisect__helper()
>                 . return bisect_reset()
>                 -> bisect_reset()
>                    . return -1
>              . if (status) return status;
>
> So the -1 is returned throughout the callchain and exit(3) ends up
> getting it---which is not quite right.  We shouldn't be giving
> negative value to exit(3).  bisect_clean_state() and other helper
> functions may already share the same issue.

I had totally missed that exit() takes only single byte value and thus
only positive integers. I think changing it to "return 1;" will do.
There are a few places in the previous series which use "return -1;"
which would need to be changed. I will resend that series.

>> +             }
>> +             argv_array_clear(&argv);
>> +     }
>> +
>> +     strbuf_release(&branch);
>> +     return bisect_clean_state();
>> +}

Regards,
Pranit Bauva

```

## Pranit Bauva, 2017-10-30 17:27

Subject: Re: [PATCH v16 Part II 1/8] bisect--helper: `bisect_reset` shell function in C
Message-ID: <CAFZEwPN2GpFy6r1vRpS6oB-qC0r5ymGB_q0AJWueLPyGj7-uvw@mail.gmail.com>
URL: https://gitlist.dev/e/CAFZEwPN2GpFy6r1vRpS6oB-qC0r5ymGB_q0AJWueLPyGj7-uvw%40mail.gmail.com
In-Reply-To: <7d978e14-2eec-23b9-c25f-e6b361101b03@gmx.net>

```
Hey Stephan,

On Mon, Oct 30, 2017 at 6:52 PM, Stephan Beyer <s-beyer@gmx.net> wrote:
> Hi Pranit,
>> +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)
>> +                     return !printf(_("We are not bisecting.\n"));
>
> This is weird; I had to look up the return value of printf first before
> I could understand what you are doing ;) I think that it is meant as a
> shortcut for
>
>         printf(_("We are not bisecting.\n"));
>         return 0;
>
> but please also express it with these two lines. (Or what is the point
> of returning a non-zero value only in the case when nothing could be
> printed?)

I was just being a little lazy I suppose. I will stick to doing it in
two lines and avoiding fancy things.

Regards,
Pranit Bauva

```

## Pranit Bauva, 2017-10-30 17:28

Subject: Re: [PATCH v16 Part II 7/8] bisect--helper: `bisect_start` shell function partially in C
Message-ID: <CAFZEwPM5-e4DVApGPmeLr95t1YseSaswxitq5zY=EYpnucu-yg@mail.gmail.com>
URL: https://gitlist.dev/e/CAFZEwPM5-e4DVApGPmeLr95t1YseSaswxitq5zY%3DEYpnucu-yg%40mail.gmail.com
In-Reply-To: <c26e1cab-a3d0-8cae-b18b-eaa02ce1240c@gmx.net>

```
Hey Stephan,

On Mon, Oct 30, 2017 at 10:21 PM, Stephan Beyer <s-beyer@gmx.net> wrote:
> Hi,
>
>> +                     return error(_("unrecognised option: '%s'"), arg);
>
> Please write "unrecogni_z_ed".
>
> Since the string for translation changed from
>         "unrecognised option: '$arg'"
> to
>         "unrecognised option: '%s'"
> anyway, it does not result in further work for the translators to
> correct it to
>         "unrecognized option: '%s'"

Yeah Sure!

Regards,
Pranit Bauva

```

## Pranit Bauva, 2017-10-30 17:31

Subject: Re: [PATCH v16 Part II 6/8] bisect--helper: `get_terms` & `bisect_terms` shell function in C
Message-ID: <CAFZEwPMpsJAQP6ZGNUTi+A_27o7_mMRD8p=3RAf0fywjOjY0vA@mail.gmail.com>
URL: https://gitlist.dev/e/CAFZEwPMpsJAQP6ZGNUTi%2BA_27o7_mMRD8p%3D3RAf0fywjOjY0vA%40mail.gmail.com
In-Reply-To: <af5f503a-6c2c-436f-ae74-cfbb11cb7a7f@gmx.net>

```
Hey Stephan,

On Mon, Oct 30, 2017 at 10:04 PM, Stephan Beyer <s-beyer@gmx.net> wrote:
> On 10/27/2017 05:06 PM, Pranit Bauva wrote:
>> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
>> index 0f9c3e63821b8..ab0580ce0089a 100644
>> --- a/builtin/bisect--helper.c
>> +++ b/builtin/bisect--helper.c
> [...]
>> +static int bisect_terms(struct bisect_terms *terms, const char **argv, int argc)
>> +{
>> +     int i;
>> +
>> +     if (get_terms(terms))
>> +             return error(_("no terms defined"));
>> +
>> +     if (argc > 1)
>> +             return error(_("--bisect-term requires exactly one argument"));
>> +
>> +     if (argc == 0)
>> +             return !printf(_("Your current terms are %s for the old state\n"
>> +                              "and %s for the new state.\n"),
>> +                              terms->term_good, terms->term_bad);
>
> Same as in 1/8: you probably want "printf(...); return 0;" except there
> is a good reason.

No good reason. I will make the change.

>> +
>> +     for (i = 0; i < argc; i++) {
>> +             if (!strcmp(argv[i], "--term-good"))
>> +                     printf(_("%s\n"), terms->term_good);
>> +             else if (!strcmp(argv[i], "--term-bad"))
>> +                     printf(_("%s\n"), terms->term_bad);
>
> The last two printfs: I think there is no point in translating "%s\n",
> so using "%s\n" instead of _("%s\n") looks more reasonable.

Also this probably does weird things with GETTEXT_POISON. I am
investigating what's happening as Martin pointed out in other thread..

>> +             else
>> +                     error(_("BUG: invalid argument %s for 'git bisect terms'.\n"
>> +                               "Supported options are: "
>> +                               "--term-good|--term-old and "
>> +                               "--term-bad|--term-new."), argv[i]);
>
> Should this be "return error(...)"?

Yeah. I missed this

>> +     }
>> +
>> +     return 0;
>> +}
>> +
>
> Stephan

Regards,
Pranit Bauva

```

## Pranit Bauva, 2017-10-30 17:35

Subject: Re: [PATCH v16 Part II 2/8] bisect--helper: `bisect_write` shell function in C
Message-ID: <CAFZEwPP7dkWwRJD2ohDfnV_Phb0ga7YPZoVC920JPrQXLAGekw@mail.gmail.com>
URL: https://gitlist.dev/e/CAFZEwPP7dkWwRJD2ohDfnV_Phb0ga7YPZoVC920JPrQXLAGekw%40mail.gmail.com
In-Reply-To: <20171027172845.15437-1-martin.agren@gmail.com>

```
Hey Martin,

On Fri, Oct 27, 2017 at 10:58 PM, Martin Ågren <martin.agren@gmail.com> wrote:
> On 27 October 2017 at 17:06, Pranit Bauva <pranit.bauva@gmail.com> wrote:
>> +static void free_terms(struct bisect_terms *terms)
>> +{
>> +       if (!terms->term_good)
>> +               free((void *) terms->term_good);
>> +       if (!terms->term_bad)
>> +               free((void *) terms->term_bad);
>> +}
>
> These look like no-ops. Remove `!` for correctness, or `if (...)` for
> simplicity, since `free()` can handle NULL.

I probably forgot to do this here. I will make the change.

> You leave the pointers dangling, but you're ok for now since this is the
> last thing that happens in `cmd_bisect__helper()`. Your later patches
> add more users, but they're also ok, since they immediately assign new
> values.
>
> In case you (and others) find it useful, the below is a patch I've been
> sitting on for a while as part of a series to plug various memory-leaks.
> `FREE_AND_NULL_CONST()` would be useful in precisely these situations.

Honestly, I wouldn't be the best person to judge this.

Regards,
Pranit Bauva

```

## Pranit Bauva, 2017-10-30 17:38

Subject: Re: [PATCH v16 Part II 2/8] bisect--helper: `bisect_write` shell function in C
Message-ID: <CAFZEwPOThFTLzfiPWs_2NVQ3BbQ26asTOyvGp72L-_OEuTfujQ@mail.gmail.com>
URL: https://gitlist.dev/e/CAFZEwPOThFTLzfiPWs_2NVQ3BbQ26asTOyvGp72L-_OEuTfujQ%40mail.gmail.com
In-Reply-To: <xmqqo9osbghy.fsf@gitster.mtv.corp.google.com>

```
Hey Junio,

On Fri, Oct 27, 2017 at 11:49 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Pranit Bauva <pranit.bauva@gmail.com> writes:
>
>> -             bisect_write "$state" "$rev"
>> +             git bisect--helper --bisect-write "$state" "$rev" "$TERM_GOOD" "$TERM_BAD" || exit
>
> I can see why two extra "terms" parameters need to be passed to this
> helper at this step; looking at patches around 4/8 and 6/8 where C
> code can directly find out what words are used for GOOD and BAD, we
> should be able to lose these two extra parameters from this helper
> by internally making a call to get_terms() from bisect_write() ;-)

Yes quite true, but then after converting bisect_skip() we can
completely get rid of this line and then it won't be needed in the
ported C code.

PS: I have already ported that function but those patches are local as of now.

Regards,
Pranit Bauva

```

## Pranit Bauva, 2017-10-30 17:40

Subject: Re: [PATCH v16 Part II 5/8] bisect--helper: `bisect_next_check` shell function in C
Message-ID: <CAFZEwPPH+pZQ2OtQjbDqjz3L_WCU2UvRQx60WFg8y-jTNUKnQQ@mail.gmail.com>
URL: https://gitlist.dev/e/CAFZEwPPH%2BpZQ2OtQjbDqjz3L_WCU2UvRQx60WFg8y-jTNUKnQQ%40mail.gmail.com
In-Reply-To: <CAN0heSpRc5VwuMgusoPBegUSsc3t8USK6hm2wd5CyqNpKUiObg@mail.gmail.com>

```
Hey Martin,

On Fri, Oct 27, 2017 at 11:05 PM, Martin Ågren <martin.agren@gmail.com> wrote:
> On 27 October 2017 at 17:06, Pranit Bauva <pranit.bauva@gmail.com> wrote:
>> +               /*
>> +                * have bad (or new) but not good (or old). We could bisect
>> +                * although this is less optimum.
>> +                */
>> +               fprintf(stderr, _("Warning: bisecting only with a %s commit\n"),
>> +                       terms->term_bad);
>
> Maybe this should use `warning()`?

Yeah. That would be better.

>> -               # 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
>
> I wonder if we can somehow pick up the existing translation? It would
> now be fuzzy, in some sense, but since the string was originally in a
> different file, maybe the po-tools won't be able to discover the
> fuzzyness? We could add a TRANSLATORS-comment, so that the translators
> know that this string matches an old one. There are more strings like
> that in this patch, and maybe in some others as well, I haven't looked.
>
> (Adding Jiang to cc.)

Since I am re-rolling my previous series as well, I can make the
change in all patches.

Regards,
Pranit Bauva

```

## Pranit Bauva, 2017-10-30 17:45

Subject: Re: [PATCH v16 Part II 6/8] bisect--helper: `get_terms` & `bisect_terms` shell function in C
Message-ID: <CAFZEwPMp_sMNVaQxVJNuNGBt0S28J9Ho41ESLuFgsudc+VLbsw@mail.gmail.com>
URL: https://gitlist.dev/e/CAFZEwPMp_sMNVaQxVJNuNGBt0S28J9Ho41ESLuFgsudc%2BVLbsw%40mail.gmail.com
In-Reply-To: <CAN0heSrWbgNEfeXj0D0SBiyC_9i++qecBcb5Kuf4WtS1wFxKDQ@mail.gmail.com>

```
Hey Martin,

On Sat, Oct 28, 2017 at 1:34 AM, Martin Ågren <martin.agren@gmail.com> wrote:
> On 27 October 2017 at 17:06, Pranit Bauva <pranit.bauva@gmail.com> wrote:
>> +       for (i = 0; i < argc; i++) {
>> +               if (!strcmp(argv[i], "--term-good"))
>> +                       printf(_("%s\n"), terms->term_good);
>> +               else if (!strcmp(argv[i], "--term-bad"))
>> +                       printf(_("%s\n"), terms->term_bad);
>
> You seem to have lost --term-old and --term-new. I also wonder why these
> would need translating. You break GETTEXT_POISON here, then fix it in
> patch 8/8.
>
> I'm not even sure you need patch 8/8. If I drop these two `_()`, I can
> run `git rebase -ix "make GETTEXT_POISON=Yes test"` on the entire series
> without failure. Patch 8/8 also switches to `test_i18ngrep` for some
> usages of `git branch` and for some checks on `.git/BISECT_START`. I'm
> not sure that's needed.

Maybe. I am not quite familiar with what does GETTEXT_POISON exactly
does, so I will probably investigate further in this.

Regards,
Pranit Bauva

```

## Ramsay Jones, 2017-11-08 00:07

Subject: Re: [PATCH v16 Part II 1/8] bisect--helper: `bisect_reset` shell function in C
Message-ID: <272aaa88-99c2-83a8-7343-c65797566a14@ramsayjones.plus.com>
URL: https://gitlist.dev/e/272aaa88-99c2-83a8-7343-c65797566a14%40ramsayjones.plus.com
In-Reply-To: <0102015f5e5ee171-f30f4868-886f-47a1-a4e4-b4936afc545d-000000@eu-west-1.amazonses.com>

```


On 27/10/17 16:06, Pranit Bauva wrote:
> 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>
> 
> ---
[snip]

Sorry for not responding sooner, I've been a bit busy.

Unfortunately, I have only had time to skim the patches, but
I haven't noticed anything too serious.

>  builtin/bisect--helper.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++-
>  git-bisect.sh            | 28 ++-------------------------
>  2 files changed, 50 insertions(+), 27 deletions(-)
> 
> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
> index 35d2105f941c6..12754448f7b6a 100644
> --- a/builtin/bisect--helper.c
> +++ b/builtin/bisect--helper.c
> @@ -3,15 +3,21 @@
>  #include "parse-options.h"
>  #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")
>  static GIT_PATH_FUNC(git_path_bisect_ancestors_ok, "BISECT_ANCESTORS_OK")
> +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
>  };
>  
> @@ -106,13 +112,48 @@ static void check_expected_revs(const char **revs, int rev_nr)
>  	}
>  }
>  
> +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)
> +			return !printf(_("We are not bisecting.\n"));

I've no idea what this is about! If printf encounters an error, then
this will be equivalent to !-1. If printf does not encounter an error,
then this will be !<length of output> (whatever that may be, given that
the string is marked for translation).

I would suggest that you don't want to do that. ;-)

ATB,
Ramsay Jones


```

## Ramsay Jones, 2017-11-08 00:26

Subject: Re: [PATCH v16 Part II 2/8] bisect--helper: `bisect_write` shell function in C
Message-ID: <a6f8d81e-22de-8dd6-da11-3bda69f8da27@ramsayjones.plus.com>
URL: https://gitlist.dev/e/a6f8d81e-22de-8dd6-da11-3bda69f8da27%40ramsayjones.plus.com
In-Reply-To: <0102015f5e5ee22c-ea9c3a38-5d42-4dce-a54b-45c59768a70b-000000@eu-west-1.amazonses.com>

```


On 27/10/17 16:06, Pranit Bauva wrote:
> 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.
> 
> Add a log_commit() helper function to write the contents of the commit message
> header to a file which will be re-used in future parts of the code as
> well.
> 
> Also introduce a function free_terms() to free the memory of `struct
> bisect_terms` and set_terms() to set the values of members in `struct
> bisect_terms`.
> 
> Helped-by: Ramsay Jones <ramsay@ramsayjones.plus.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 | 107 +++++++++++++++++++++++++++++++++++++++++++++--
>  git-bisect.sh            |  25 ++---------
>  2 files changed, 108 insertions(+), 24 deletions(-)
> 
> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
> index 12754448f7b6a..6295f53c850a8 100644
> --- a/builtin/bisect--helper.c
> +++ b/builtin/bisect--helper.c
> @@ -12,15 +12,37 @@ 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_start, "BISECT_START")
>  static GIT_PATH_FUNC(git_path_bisect_head, "BISECT_HEAD")
> +static GIT_PATH_FUNC(git_path_bisect_log, "BISECT_LOG")
>  
>  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>]"),
> +	N_("git bisect--helper --bisect-write <state> <revision> <good_term> <bad_term> [<nolog>]"),
>  	NULL
>  };
>  
> +struct bisect_terms {
> +	const char *term_good;
> +	const char *term_bad;
> +};
> +
> +static void free_terms(struct bisect_terms *terms)
> +{
> +	if (!terms->term_good)
> +		free((void *) terms->term_good);> +	if (!terms->term_bad)
> +		free((void *) terms->term_bad);
You can pass a NULL pointer to free(), so the conditionals here
are not required.

> +}
> +
> +static void set_terms(struct bisect_terms *terms, const char *bad,
> +		      const char *good)
> +{
> +	terms->term_good = xstrdup(good);
> +	terms->term_bad = xstrdup(bad);
> +}
> +
>  /*
>   * Check whether the string `term` belongs to the set of strings
>   * included in the variable arguments.
> @@ -146,6 +168,72 @@ static int bisect_reset(const char *commit)
>  	return bisect_clean_state();
>  }
>  
> +static void log_commit(FILE *fp, char *fmt, const char *state,
> +		       struct commit *commit)
> +{
> +	struct pretty_print_context pp = {0};
> +	struct strbuf commit_msg = STRBUF_INIT;
> +	char *label = xstrfmt(fmt, state);
> +
> +	format_commit_message(commit, "%s", &commit_msg, &pp);
> +
> +	fprintf(fp, "# %s: [%s] %s\n", label, oid_to_hex(&commit->object.oid),
> +		commit_msg.buf);
> +
> +	strbuf_release(&commit_msg);
> +	free(label);
> +}
> +
> +static int bisect_write(const char *state, const char *rev,
> +			const struct bisect_terms *terms, int nolog)
> +{
> +	struct strbuf tag = STRBUF_INIT;
> +	struct object_id oid;
> +	struct commit *commit;
> +	FILE *fp = NULL;
> +	int retval = 0;
> +
> +	if (!strcmp(state, terms->term_bad)) {
> +		strbuf_addf(&tag, "refs/bisect/%s", state);
> +	} else if (one_of(state, terms->term_good, "skip", NULL)) {
> +		strbuf_addf(&tag, "refs/bisect/%s-%s", state, rev);
> +	} else {
> +		error(_("Bad bisect_write argument: %s"), state);
> +		goto fail;
> +	}
> +
> +	if (get_oid(rev, &oid)) {
> +		error(_("couldn't get the oid of the rev '%s'"), rev);
> +		goto fail;
> +	}
> +
> +	if (update_ref(NULL, tag.buf, &oid, NULL, 0,
> +		       UPDATE_REFS_MSG_ON_ERR))
> +		goto fail;
> +
> +	fp = fopen(git_path_bisect_log(), "a");
> +	if (!fp) {
> +		error_errno(_("couldn't open the file '%s'"), git_path_bisect_log());
> +		goto fail;
> +	}
> +
> +	commit = lookup_commit_reference(&oid);
> +	log_commit(fp, "%s", state, commit);
> +
> +	if (!nolog)
> +		fprintf(fp, "git bisect %s %s\n", state, rev);
> +
> +	goto finish;
> +
> +fail:
> +	retval = -1;
> +finish:
> +	if (fp)
> +		fclose(fp);
> +	strbuf_release(&tag);
> +	return retval;
> +}
> +
>  int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
>  {
>  	enum {
> @@ -153,9 +241,10 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
>  		WRITE_TERMS,
>  		BISECT_CLEAN_STATE,
>  		CHECK_EXPECTED_REVS,
> -		BISECT_RESET
> +		BISECT_RESET,
> +		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),
> @@ -167,10 +256,13 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
>  			 N_("check for expected revs"), CHECK_EXPECTED_REVS),
>  		OPT_CMDMODE(0, "bisect-reset", &cmdmode,
>  			 N_("reset the bisection state"), BISECT_RESET),
> +		OPT_CMDMODE(0, "bisect-write", &cmdmode,
> +			 N_("update the refs according to the bisection state and may write it to BISECT_LOG"), BISECT_WRITE),

Is the "... and may write it to" necessary? ;-)
[The original was just: "write out the bisection state in BISECT_LOG"]

ATB,
Ramsay Jones


```

## Ramsay Jones, 2017-11-08 00:37

Subject: Re: [PATCH v16 Part II 4/8] bisect--helper: `check_and_set_terms` shell function in C
Message-ID: <1af55a7a-d58c-6f36-3a2f-25c79cbfb84e@ramsayjones.plus.com>
URL: https://gitlist.dev/e/1af55a7a-d58c-6f36-3a2f-25c79cbfb84e%40ramsayjones.plus.com
In-Reply-To: <0102015f5e5ee273-de8fa1d2-1d59-4990-93f5-d36a6cb3f45a-000000@eu-west-1.amazonses.com>

```


On 27/10/17 16:06, Pranit Bauva wrote:
> 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.
> 
> 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 | 41 ++++++++++++++++++++++++++++++++++++++++-
>  git-bisect.sh            | 36 ++++--------------------------------
>  2 files changed, 44 insertions(+), 33 deletions(-)
> 
> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
> index 6295f53c850a8..65abf8a70c6d9 100644
> --- a/builtin/bisect--helper.c
> +++ b/builtin/bisect--helper.c
> @@ -20,6 +20,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> <good_term> <bad_term> [<nolog>]"),
> +	N_("git bisect--helper --bisect-check-and-set-terms <command> <good_term> <bad_term>"),
>  	NULL
>  };
>  
> @@ -234,6 +235,35 @@ static int bisect_write(const char *state, const char *rev,
>  	return retval;
>  }
>  
> +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) &&
> +	    strcmp(cmd, terms->term_good))
> +		return error(_("Invalid command: you're currently in a "
> +				"%s/%s bisect"), terms->term_bad,
> +				terms->term_good);
> +
> +	if (!has_term_file) {
> +		if (one_of(cmd, "bad", "good", NULL)) {
> +			free_terms(terms);
> +			set_terms(terms, "bad", "good");
> +			return write_terms(terms->term_bad, terms->term_good);
> +		}
> +		else if (one_of(cmd, "new", "old", NULL)) {

cuddle the else: '} else if (one_of(....)) {'
OR, simply remove the else.

ATB,
Ramsay Jones


```

## Ramsay Jones, 2017-11-08 00:48

Subject: Re: [PATCH v16 Part II 5/8] bisect--helper: `bisect_next_check` shell function in C
Message-ID: <8d59ede3-043d-6a48-b290-bf4d49e650f1@ramsayjones.plus.com>
URL: https://gitlist.dev/e/8d59ede3-043d-6a48-b290-bf4d49e650f1%40ramsayjones.plus.com
In-Reply-To: <0102015f5e5ee2ca-bf824205-86e6-4259-b7d7-a68e2fbb087d-000000@eu-west-1.amazonses.com>

```


On 27/10/17 16:06, Pranit Bauva wrote:
> 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 .
> 
> `bisect_voc` shell function is no longer useful now and is replaced by
> using a char *[] of "new|bad" and "good|old" values.
> 
> 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.
> 
> Helped-by: Stephan Beyer <s-beyer@gmx.net>
> 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 | 91 +++++++++++++++++++++++++++++++++++++++++++++++-
>  git-bisect.sh            | 60 +++----------------------------
>  2 files changed, 94 insertions(+), 57 deletions(-)
> 
> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
> index 65abf8a70c6d9..0f9c3e63821b8 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")
> @@ -21,6 +22,7 @@ static const char * const git_bisect_helper_usage[] = {
>  	N_("git bisect--helper --bisect-reset [<commit>]"),
>  	N_("git bisect--helper --bisect-write <state> <revision> <good_term> <bad_term> [<nolog>]"),
>  	N_("git bisect--helper --bisect-check-and-set-terms <command> <good_term> <bad_term>"),
> +	N_("git bisect--helper --bisect-next-check [<term>] <good_term> <bad_term>"),
>  	NULL
>  };
>  
> @@ -44,6 +46,11 @@ static void set_terms(struct bisect_terms *terms, const char *bad,
>  	terms->term_bad = xstrdup(bad);
>  }
>  
> +static const char *voc[] = {
> +	"bad|new",
> +	"good|old"
> +};
> +
In my version I had this instead:

+static const char *vocab_bad = "bad|new";
+static const char *vocab_good = "good|old";
+

which I prefer, because ...

>  /*
>   * Check whether the string `term` belongs to the set of strings
>   * included in the variable arguments.
> @@ -264,6 +271,79 @@ 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 int bisect_next_check(const struct bisect_terms *terms,
> +			     const char *current_term)
> +{
> +	int missing_good = 1, missing_bad = 1, retval = 0;
> +	const char *bad_ref = xstrfmt("refs/bisect/%s", terms->term_bad);
> +	const char *good_glob = xstrfmt("%s-*", terms->term_good);
> +
> +	if (ref_exists(bad_ref))
> +		missing_bad = 0;
> +
> +	for_each_glob_ref_in(mark_good, good_glob, "refs/bisect/",
> +			     (void *) &missing_good);
> +
> +	if (!missing_good && !missing_bad)
> +		goto finish;
> +
> +	if (!current_term)
> +		goto fail;
> +
> +	if (missing_good && !missing_bad && current_term &&
> +	    !strcmp(current_term, terms->term_good)) {
> +		char *yesno;
> +		/*
> +		 * have bad (or new) but not good (or old). We could bisect
> +		 * although this is less optimum.
> +		 */
> +		fprintf(stderr, _("Warning: bisecting only with a %s commit\n"),
> +			terms->term_bad);
> +		if (!isatty(0))
> +			goto finish;
> +		/*
> +		 * 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"))
> +			goto fail;
> +
> +		goto finish;
> +	}
> +	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"),
> +			voc[0], voc[1], voc[0], voc[1]);

... this is (arguably) easier to read:
+			vocab_bad, vocab_good, vocab_bad, vocab_good);

> +		goto fail;
> +	} 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"),
> +			voc[1], voc[0], voc[1], voc[0]);

ditto

ATB,
Ramsay Jones


```

## Ramsay Jones, 2017-11-08 00:59

Subject: Re: [PATCH v16 Part II 6/8] bisect--helper: `get_terms` & `bisect_terms` shell function in C
Message-ID: <aacc7970-6952-206e-5ef0-10e0cf3d7373@ramsayjones.plus.com>
URL: https://gitlist.dev/e/aacc7970-6952-206e-5ef0-10e0cf3d7373%40ramsayjones.plus.com
In-Reply-To: <0102015f5e5ee303-449f1b76-ff53-433c-8973-8b4850466810-000000@eu-west-1.amazonses.com>

```


On 27/10/17 16:06, Pranit Bauva wrote:
> 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.
> 
> Also use error() to report "no terms defined" and accordingly change the
> test in t6030.
> 
> 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    | 69 +++++++++++++++++++++++++++++++++++++++++++--
>  git-bisect.sh               | 35 ++---------------------
>  t/t6030-bisect-porcelain.sh |  2 +-
>  3 files changed, 70 insertions(+), 36 deletions(-)
> 
> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
> index 0f9c3e63821b8..ab0580ce0089a 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-write <state> <revision> <good_term> <bad_term> [<nolog>]"),
>  	N_("git bisect--helper --bisect-check-and-set-terms <command> <good_term> <bad_term>"),
>  	N_("git bisect--helper --bisect-next-check [<term>] <good_term> <bad_term>"),
> +	N_("git bisect--helper --bisect-terms [--term-good | --term-old | --term-bad | --term-new]"),
>  	NULL
>  };
>  
> @@ -344,6 +345,62 @@ static int bisect_next_check(const struct bisect_terms *terms,
>  	return retval;
>  }
>  
> +static int get_terms(struct bisect_terms *terms)
> +{
> +	struct strbuf str = STRBUF_INIT;
> +	FILE *fp = NULL;
> +	int res = 0;
> +
> +	fp = fopen(git_path_bisect_terms(), "r");
> +	if (!fp)
> +		goto fail;
> +
> +	free_terms(terms);
> +	strbuf_getline_lf(&str, fp);
> +	terms->term_bad = strbuf_detach(&str, NULL);
> +	strbuf_getline_lf(&str, fp);
> +	terms->term_good = strbuf_detach(&str, NULL);
> +	goto finish;
> +
> +fail:
> +	res = -1;
> +finish:
> +	if (fp)
> +		fclose(fp);
> +	strbuf_release(&str);
> +	return res;
> +}
> +
> +static int bisect_terms(struct bisect_terms *terms, const char **argv, int argc)
> +{
> +	int i;
> +
> +	if (get_terms(terms))
> +		return error(_("no terms defined"));
> +
> +	if (argc > 1)
> +		return error(_("--bisect-term requires exactly one argument"));
> +
> +	if (argc == 0)
> +		return !printf(_("Your current terms are %s for the old state\n"
> +				 "and %s for the new state.\n"),
> +				 terms->term_good, terms->term_bad);

Again, I don't think you want to do this. :-D

> +
> +	for (i = 0; i < argc; i++) {
> +		if (!strcmp(argv[i], "--term-good"))> +			printf(_("%s\n"), terms->term_good);
> +		else if (!strcmp(argv[i], "--term-bad"))
> +			printf(_("%s\n"), terms->term_bad);
> +		else

Here, you want to check for the pairs "--term-{good,old}" and
"--term-{bad,new}", so maybe this instead:

+		if (one_of(argv[i], "--term-good", "--term-old", NULL))
+			printf("%s\n", terms->term_good);
+		else if (one_of(argv[i], "--term-bad", "--term-new", NULL))
+			printf("%s\n", terms->term_bad);
+		else

> +			error(_("BUG: invalid argument %s for 'git bisect terms'.\n"
> +				  "Supported options are: "
> +				  "--term-good|--term-old and "
> +				  "--term-bad|--term-new."), argv[i]);
> +	}
> +
> +	return 0;
> +}
> +

ATB,
Ramsay Jones



```

## Stephan Beyer, 2017-11-12 19:19

Subject: Re: [PATCH v16 Part II 5/8] bisect--helper: `bisect_next_check` shell function in C
Message-ID: <d0994817-25be-25b8-565e-d81c5f18b210@gmx.net>
URL: https://gitlist.dev/e/d0994817-25be-25b8-565e-d81c5f18b210%40gmx.net
In-Reply-To: <0102015f5e5ee2ca-bf824205-86e6-4259-b7d7-a68e2fbb087d-000000@eu-west-1.amazonses.com>

```
Hi,

another minor:

On 10/27/2017 05:06 PM, Pranit Bauva wrote:
> @@ -264,6 +271,79 @@ 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 int bisect_next_check(const struct bisect_terms *terms,
> +			     const char *current_term)
> +{
> +	int missing_good = 1, missing_bad = 1, retval = 0;
> +	const char *bad_ref = xstrfmt("refs/bisect/%s", terms->term_bad);
> +	const char *good_glob = xstrfmt("%s-*", terms->term_good);
> +
> +	if (ref_exists(bad_ref))
> +		missing_bad = 0;
> +
> +	for_each_glob_ref_in(mark_good, good_glob, "refs/bisect/",
> +			     (void *) &missing_good);
> +
> +	if (!missing_good && !missing_bad)
> +		goto finish;
> +
> +	if (!current_term)
> +		goto fail;
> +
> +	if (missing_good && !missing_bad && current_term &&

This check for "current_term" is not necessary; it can be asserted to be
non-NULL, otherwise you would have jumped to "fail"

Stephan

```

## Stephan Beyer, 2017-11-12 19:27

Subject: Re: [PATCH v16 Part II 5/8] bisect--helper: `bisect_next_check` shell function in C
Message-ID: <81ae6057-d293-2dcd-64db-4dac41849131@gmx.net>
URL: https://gitlist.dev/e/81ae6057-d293-2dcd-64db-4dac41849131%40gmx.net
In-Reply-To: <0102015f5e5ee2ca-bf824205-86e6-4259-b7d7-a68e2fbb087d-000000@eu-west-1.amazonses.com>

```
> @@ -21,6 +22,7 @@ static const char * const git_bisect_helper_usage[] = {
>  	N_("git bisect--helper --bisect-reset [<commit>]"),
>  	N_("git bisect--helper --bisect-write <state> <revision> <good_term> <bad_term> [<nolog>]"),
>  	N_("git bisect--helper --bisect-check-and-set-terms <command> <good_term> <bad_term>"),
> +	N_("git bisect--helper --bisect-next-check [<term>] <good_term> <bad_term>"),

I think the order is wrong. It is <good_term> <bad_term> [<term>]

```

## Stephan Beyer, 2017-11-12 20:03

Subject: Re: [PATCH v16 Part II 5/8] bisect--helper: `bisect_next_check` shell function in C
Message-ID: <214cfc63-79a8-a46f-4440-5e223186f2ff@gmx.net>
URL: https://gitlist.dev/e/214cfc63-79a8-a46f-4440-5e223186f2ff%40gmx.net
In-Reply-To: <0102015f5e5ee2ca-bf824205-86e6-4259-b7d7-a68e2fbb087d-000000@eu-west-1.amazonses.com>

```
Hi again ;)

On 10/27/2017 05:06 PM, Pranit Bauva wrote:
> @@ -44,6 +46,11 @@ static void set_terms(struct bisect_terms *terms, const char *bad,
>  	terms->term_bad = xstrdup(bad);
>  }
>  
> +static const char *voc[] = {
> +	"bad|new",
> +	"good|old"
> +};
> +
>  /*
>   * Check whether the string `term` belongs to the set of strings
>   * included in the variable arguments.
> @@ -264,6 +271,79 @@ 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 int bisect_next_check(const struct bisect_terms *terms,
> +			     const char *current_term)
> +{
> +	int missing_good = 1, missing_bad = 1, retval = 0;
> +	const char *bad_ref = xstrfmt("refs/bisect/%s", terms->term_bad);
> +	const char *good_glob = xstrfmt("%s-*", terms->term_good);
> +
> +	if (ref_exists(bad_ref))
> +		missing_bad = 0;
> +
> +	for_each_glob_ref_in(mark_good, good_glob, "refs/bisect/",
> +			     (void *) &missing_good);
> +
> +	if (!missing_good && !missing_bad)
> +		goto finish;
> +
> +	if (!current_term)
> +		goto fail;
> +
> +	if (missing_good && !missing_bad && current_term &&
> +	    !strcmp(current_term, terms->term_good)) {
> +		char *yesno;
> +		/*
> +		 * have bad (or new) but not good (or old). We could bisect
> +		 * although this is less optimum.
> +		 */
> +		fprintf(stderr, _("Warning: bisecting only with a %s commit\n"),
> +			terms->term_bad);
> +		if (!isatty(0))
> +			goto finish;
> +		/*
> +		 * 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"))
> +			goto fail;
> +
> +		goto finish;
> +	}
> +	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"),
> +			voc[0], voc[1], voc[0], voc[1]);
> +		goto fail;
> +	} 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"),
> +			voc[1], voc[0], voc[1], voc[0]);
> +		goto fail;

In both of the above "error" calls, you should drop the final "\n"
because "error" does that already.

On the other hand, you have dropped the "\n"s of the orginal error
messages. So it should probably be

 _("You need to give me at least one %s and %s revision.\n"
   "You can use \"git bisect %s\" and \"git bisect %s\" for that.")

and

 _("You need to start by \"git bisect start\".\n"
   "You then need to give me at least one %s and %s revision.\n"
   "You can use \"git bisect %s\" and "\"git bisect %s\" for that.")

Stephan

```

## Junio C Hamano, 2017-11-13 03:56

Subject: Re: [PATCH v16 Part II 5/8] bisect--helper: `bisect_next_check` shell function in C
Message-ID: <xmqqineeal29.fsf@gitster.mtv.corp.google.com>
URL: https://gitlist.dev/e/xmqqineeal29.fsf%40gitster.mtv.corp.google.com
In-Reply-To: <214cfc63-79a8-a46f-4440-5e223186f2ff@gmx.net>

```
Stephan Beyer <s-beyer@gmx.net> writes:

> Hi again ;)
> ...
> In both of the above "error" calls, you should drop the final "\n"
> because "error" does that already.
>
> On the other hand, you have dropped the "\n"s of the orginal error
> messages. So it should probably be
>
>  _("You need to give me at least one %s and %s revision.\n"
>    "You can use \"git bisect %s\" and \"git bisect %s\" for that.")
>
> and
>
>  _("You need to start by \"git bisect start\".\n"
>    "You then need to give me at least one %s and %s revision.\n"
>    "You can use \"git bisect %s\" and "\"git bisect %s\" for that.")
>
> Stephan

Thanks for reviews (not just this patch, but for reviews on other
patches, too).

```

## SZEDER Gábor, 2018-02-16 01:22

Subject: Re: [PATCH v16 Part II 7/8] bisect--helper: `bisect_start` shell function partially in C
Message-ID: <20180216012243.14169-1-szeder.dev@gmail.com>
URL: https://gitlist.dev/e/20180216012243.14169-1-szeder.dev%40gmail.com
In-Reply-To: <0102015f5e5ee2f5-3d106a61-7f79-4973-851f-f55a691a6189-000000@eu-west-1.amazonses.com>

```
> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
> index ab0580ce0089a..4ac175c49e80c 100644
> --- a/builtin/bisect--helper.c
> +++ b/builtin/bisect--helper.c

> +	/*
> +	 * Check if we are bisecting
> +	 */
> +	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);
> +				goto fail;
> +			}
> +		}
> +	} else {
> +		/* Get the rev from where we start. */
> +		if (!get_oid(head, &head_oid) &&
> +		    !starts_with(head, "refs/heads/")) {
> +			strbuf_reset(&start_head);
> +			strbuf_addstr(&start_head, sha1_to_hex(head_oid.hash));

Please use oid_to_hex(&head_oid) instead of sha1_to_hex().


```

## Johannes Schindelin, 2018-11-23 10:13

Subject: Re: [PATCH v16 Part II 2/8] bisect--helper: `bisect_write` shell function in C
Message-ID: <nycvar.QRO.7.76.6.1811231111030.41@tvgsbejvaqbjf.bet>
URL: https://gitlist.dev/e/nycvar.QRO.7.76.6.1811231111030.41%40tvgsbejvaqbjf.bet
In-Reply-To: <CAFZEwPP7dkWwRJD2ohDfnV_Phb0ga7YPZoVC920JPrQXLAGekw@mail.gmail.com>

```
Hi Pranit,

(Cc:ing Tanushree because they will try to pick up this patch series as
part of the Outreachy program.)

On Mon, 30 Oct 2017, Pranit Bauva wrote:

> On Fri, Oct 27, 2017 at 10:58 PM, Martin Ågren <martin.agren@gmail.com> wrote:
> > On 27 October 2017 at 17:06, Pranit Bauva <pranit.bauva@gmail.com> wrote:
> >> +static void free_terms(struct bisect_terms *terms)
> >> +{
> >> +       if (!terms->term_good)
> >> +               free((void *) terms->term_good);
> >> +       if (!terms->term_bad)
> >> +               free((void *) terms->term_bad);
> >> +}
> >
> > These look like no-ops. Remove `!` for correctness, or `if (...)` for
> > simplicity, since `free()` can handle NULL.
> 
> I probably forgot to do this here. I will make the change.
> 
> > You leave the pointers dangling, but you're ok for now since this is the
> > last thing that happens in `cmd_bisect__helper()`. Your later patches
> > add more users, but they're also ok, since they immediately assign new
> > values.
> >
> > In case you (and others) find it useful, the below is a patch I've been
> > sitting on for a while as part of a series to plug various memory-leaks.
> > `FREE_AND_NULL_CONST()` would be useful in precisely these situations.
> 
> Honestly, I wouldn't be the best person to judge this.

Git's source code implicitly assumes that any `const` pointer refers to
memory owned by another code path. It is therefore probably not a good
idea to encourage `free()`ing a `const` pointer.

Which brings me back to the question: who really owns that allocated
memory to which `term_good` and `term_bad` refer?

Ciao,
Johannes
```

## Martin Ågren, 2018-11-23 12:22

Subject: Re: [PATCH v16 Part II 2/8] bisect--helper: `bisect_write` shell function in C
Message-ID: <CAN0heSpaW5C_dMh7gh0ezyzACBgZ2SYXP67E_3moeKNyTvu9xg@mail.gmail.com>
URL: https://gitlist.dev/e/CAN0heSpaW5C_dMh7gh0ezyzACBgZ2SYXP67E_3moeKNyTvu9xg%40mail.gmail.com
In-Reply-To: <nycvar.QRO.7.76.6.1811231111030.41@tvgsbejvaqbjf.bet>

```
On Fri, 23 Nov 2018 at 11:13, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> On Mon, 30 Oct 2017, Pranit Bauva wrote:
>
> > On Fri, Oct 27, 2017 at 10:58 PM, Martin Ågren <martin.agren@gmail.com> wrote:
> > > On 27 October 2017 at 17:06, Pranit Bauva <pranit.bauva@gmail.com> wrote:
> > >> +static void free_terms(struct bisect_terms *terms)
> > >> +{
> > >> +       if (!terms->term_good)
> > >> +               free((void *) terms->term_good);
> > >> +       if (!terms->term_bad)
> > >> +               free((void *) terms->term_bad);
> > >> +}

> > > You leave the pointers dangling, but you're ok for now since this is the
> > > last thing that happens in `cmd_bisect__helper()`. Your later patches
> > > add more users, but they're also ok, since they immediately assign new
> > > values.
> > >
> > > In case you (and others) find it useful, the below is a patch I've been
> > > sitting on for a while as part of a series to plug various memory-leaks.
> > > `FREE_AND_NULL_CONST()` would be useful in precisely these situations.
> >
> > Honestly, I wouldn't be the best person to judge this.
>
> Git's source code implicitly assumes that any `const` pointer refers to
> memory owned by another code path. It is therefore probably not a good
> idea to encourage `free()`ing a `const` pointer.

Yeah, I never submitted that patch as part of a real series. I remember
having a funky feeling about it, and whatever use-case I had for this
macro, I managed to solve the memory leak in some other way in a
rewrite.

Thanks for a sanity check.

Martin

```

## Johannes Schindelin, 2018-11-26 18:18

Subject: Re: [PATCH v16 Part II 2/8] bisect--helper: `bisect_write` shell function in C
Message-ID: <nycvar.QRO.7.76.6.1811261918170.41@tvgsbejvaqbjf.bet>
URL: https://gitlist.dev/e/nycvar.QRO.7.76.6.1811261918170.41%40tvgsbejvaqbjf.bet
In-Reply-To: <CAN0heSpaW5C_dMh7gh0ezyzACBgZ2SYXP67E_3moeKNyTvu9xg@mail.gmail.com>

```
Hi Martin,

On Fri, 23 Nov 2018, Martin Ågren wrote:

> On Fri, 23 Nov 2018 at 11:13, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> > On Mon, 30 Oct 2017, Pranit Bauva wrote:
> >
> > > On Fri, Oct 27, 2017 at 10:58 PM, Martin Ågren <martin.agren@gmail.com> wrote:
> > > > On 27 October 2017 at 17:06, Pranit Bauva <pranit.bauva@gmail.com> wrote:
> > > >> +static void free_terms(struct bisect_terms *terms)
> > > >> +{
> > > >> +       if (!terms->term_good)
> > > >> +               free((void *) terms->term_good);
> > > >> +       if (!terms->term_bad)
> > > >> +               free((void *) terms->term_bad);
> > > >> +}
> 
> > > > You leave the pointers dangling, but you're ok for now since this is the
> > > > last thing that happens in `cmd_bisect__helper()`. Your later patches
> > > > add more users, but they're also ok, since they immediately assign new
> > > > values.
> > > >
> > > > In case you (and others) find it useful, the below is a patch I've been
> > > > sitting on for a while as part of a series to plug various memory-leaks.
> > > > `FREE_AND_NULL_CONST()` would be useful in precisely these situations.
> > >
> > > Honestly, I wouldn't be the best person to judge this.
> >
> > Git's source code implicitly assumes that any `const` pointer refers to
> > memory owned by another code path. It is therefore probably not a good
> > idea to encourage `free()`ing a `const` pointer.
> 
> Yeah, I never submitted that patch as part of a real series. I remember
> having a funky feeling about it, and whatever use-case I had for this
> macro, I managed to solve the memory leak in some other way in a
> rewrite.
> 
> Thanks for a sanity check.

I am glad you agree, and it's just fair that I contribute a sanity check
on this here list when I have benefitted so many times from the same.

Ciao,
Johannes
```

## Pranit Bauva via GitGitGadget, 2019-01-02 15:38

Subject: [PATCH v17 1/7] bisect--helper: `bisect_reset` shell function in C
Message-ID: <338ebdc97a86295495fa9da638e0b96c2ea746ee.1546443509.git.gitgitgadget@gmail.com>
URL: https://gitlist.dev/e/338ebdc97a86295495fa9da638e0b96c2ea746ee.1546443509.git.gitgitgadget%40gmail.com
In-Reply-To: <pull.101.v17.git.gitgitgadget@gmail.com>

```
From: Pranit Bauva <pranit.bauva@gmail.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>
Mentored by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
Signed-off-by: Tanushree Tumane <tanushreetumane@gmail.com>
---
 builtin/bisect--helper.c | 51 +++++++++++++++++++++++++++++++++++++++-
 git-bisect.sh            | 28 ++--------------------
 2 files changed, 52 insertions(+), 27 deletions(-)

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 4b5fadcbe1..aa6495dc84 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -3,15 +3,21 @@
 #include "parse-options.h"
 #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")
 static GIT_PATH_FUNC(git_path_bisect_ancestors_ok, "BISECT_ANCESTORS_OK")
+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
 };
 
@@ -106,13 +112,50 @@ static void check_expected_revs(const char **revs, int rev_nr)
 	}
 }
 
+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(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)) {
+			strbuf_release(&branch);
+			argv_array_clear(&argv);
+			return error(_("could not check out original"
+				       " HEAD '%s'. Try 'git bisect"
+				       "reset <commit>'."), branch.buf);
+		}
+		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,
-		CHECK_EXPECTED_REVS
+		CHECK_EXPECTED_REVS,
+		BISECT_RESET
 	} cmdmode = 0;
 	int no_checkout = 0;
 	struct option options[] = {
@@ -124,6 +167,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 			 N_("cleanup the bisection state"), BISECT_CLEAN_STATE),
 		OPT_CMDMODE(0, "check-expected-revs", &cmdmode,
 			 N_("check for expected revs"), CHECK_EXPECTED_REVS),
+		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()
@@ -149,6 +194,10 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 	case CHECK_EXPECTED_REVS:
 		check_expected_revs(argv, argc);
 		return 0;
+	case BISECT_RESET:
+		if (argc > 1)
+			return error(_("--bisect-reset requires either no argument or a commit"));
+		return !!bisect_reset(argc ? argv[0] : NULL);
 	default:
 		return error("BUG: unknown subcommand '%d'", cmdmode);
 	}
diff --git a/git-bisect.sh b/git-bisect.sh
index 54cbfecc5a..afbfbc1f8e 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -393,35 +393,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
@@ -613,7 +589,7 @@ case "$#" in
 	visualize|view)
 		bisect_visualize "$@" ;;
 	reset)
-		bisect_reset "$@" ;;
+		git bisect--helper --bisect-reset "$@" ;;
 	replay)
 		bisect_replay "$@" ;;
 	log)
-- 
gitgitgadget


```

## Tanushree Tumane via GitGitGadget, 2019-01-02 15:38

Subject: [PATCH v17 0/7] git bisect: convert from shell to C
Message-ID: <pull.101.v17.git.gitgitgadget@gmail.com>
URL: https://gitlist.dev/e/pull.101.v17.git.gitgitgadget%40gmail.com
In-Reply-To: <0102015f5e5ee171-f30f4868-886f-47a1-a4e4-b4936afc545d-000000@eu-west-1.amazonses.com>

```
Changes since Pranit's v16:
===========================

bisect--helper: bisect_reset shell function in C
================================================

 * The return !printf(...); construct was considered unsafe and disentangled
   into printf(...); return 0;
 * It is more elegant to release branch and argv first, and then return
   error(...) than to error(...); release branch; clear argv; return -1;
 * Prefix !! to bisect_reset(...) function call in cmd_bisect__helper(...) 
   because its return value will be given to exit()

bisect--helper: bisect_write shell function in C
================================================

 * The option [--no-log] is passed before <state>.
 * Remove const from struct bisect_terms variables because const pointers
   refers to memory owned by another code path.
 * Use FREE_AND_NULL(...) in free_terms(...).
 * It makes more sense to free terms before set in set_terms(...) to avoid
   memory leak.
 * Remove fail: retval = -1; for cleaner code flow, replace goto fail; with 
   retval = -1; goto finish; or retval = error(...); goto finish;
 * It is more consistent to introduce --no-log as option like --no-checkout.
 * Initialize struct bisect_terms terms in cmd_bisect__helper(...).
 * The return res; in cmd_bisect__helper(...) may return -1, so prefixed !! 
   to handle such cases.

bisect--helper: check_and_set_terms shell function in C
=======================================================

 * free_terms() before set_terms() is obsolete as set_terms() frees terms
   before set.

bisect--helper: bisect_next_check shell function in C
=====================================================

 * [<term>] is first argument of bisect-next-check.
 * vocab_good and vocab_bad for easier readability.
 * Declare *_warning char pointers.
 * Remove fail: label, its only job was to set retval = -1;.

bisect--helper: get_terms & bisect_terms shell function in C
============================================================

 * Remove fail: label, its only job was to set res = -1;.
 * Parse the argv in cmd_bisect__helper() and pass option to bisect_terms().

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

 * Remove fail: label, its only job was to set retval = -1;.
 * Spell correction unrecognised to unrecognized.
 * if (revs.nr) must_write_terms = 1; is more readable than must_write_terms
   |= !!revs.nr;.
 * error() messages starts with small letters.
 * sha1_to_hex() is old practice.
 * sq_quote_argv() now requires only two args.
 * Instead of nesting ifs, use &&.

Pranit Bauva (7):
  bisect--helper: `bisect_reset` shell function in C
  bisect--helper: `bisect_write` shell function in C
  wrapper: move is_empty_file() and rename it as
    is_empty_or_missing_file()
  bisect--helper: `check_and_set_terms` shell function in C
  bisect--helper: `bisect_next_check` shell function in C
  bisect--helper: `get_terms` & `bisect_terms` shell function in C
  bisect--helper: `bisect_start` shell function partially in C

 builtin/am.c                |  20 +-
 builtin/bisect--helper.c    | 563 +++++++++++++++++++++++++++++++++++-
 cache.h                     |   3 +
 git-bisect.sh               | 314 ++------------------
 t/t6030-bisect-porcelain.sh |   2 +-
 wrapper.c                   |  13 +
 6 files changed, 595 insertions(+), 320 deletions(-)


base-commit: 7f4e64169352e03476b0ea64e7e2973669e491a2
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-101%2Ftanushree27%2Fgit-bisect_part2_fixup-v17
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-101/tanushree27/git-bisect_part2_fixup-v17
Pull-Request: https://github.com/gitgitgadget/git/pull/101

Range-diff vs v16:

 1:  f1e89ba517 ! 1:  338ebdc97a bisect--helper: `bisect_reset` shell function in C
     @@ -16,8 +16,9 @@
      
          Mentored-by: Lars Schneider <larsxschneider@gmail.com>
          Mentored-by: Christian Couder <chriscool@tuxfamily.org>
     +    Mentored by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
          Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
     -    Signed-off-by: Junio C Hamano <gitster@pobox.com>
     +    Signed-off-by: Tanushree Tumane <tanushreetumane@gmail.com>
      
       diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
       --- a/builtin/bisect--helper.c
     @@ -53,8 +54,10 @@
      +	struct strbuf branch = STRBUF_INIT;
      +
      +	if (!commit) {
     -+		if (strbuf_read_file(&branch, git_path_bisect_start(), 0) < 1)
     -+			return !printf(_("We are not bisecting.\n"));
     ++		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;
     @@ -69,11 +72,11 @@
      +
      +		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;
     ++			return error(_("could not check out original"
     ++				       " HEAD '%s'. Try 'git bisect"
     ++				       "reset <commit>'."), branch.buf);
      +		}
      +		argv_array_clear(&argv);
      +	}
     @@ -110,7 +113,7 @@
      +	case BISECT_RESET:
      +		if (argc > 1)
      +			return error(_("--bisect-reset requires either no argument or a commit"));
     -+		return bisect_reset(argc ? argv[0] : NULL);
     ++		return !!bisect_reset(argc ? argv[0] : NULL);
       	default:
       		return error("BUG: unknown subcommand '%d'", cmdmode);
       	}
 2:  da4ac60ad4 ! 2:  7d6291d9dd bisect--helper: `bisect_write` shell function in C
     @@ -27,8 +27,9 @@
          Helped-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
          Mentored-by: Lars Schneider <larsxschneider@gmail.com>
          Mentored-by: Christian Couder <chriscool@tuxfamily.org>
     +    Mentored-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
          Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
     -    Signed-off-by: Junio C Hamano <gitster@pobox.com>
     +    Signed-off-by: Tanushree Tumane <tanushreetumane@gmail.com>
      
       diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
       --- a/builtin/bisect--helper.c
     @@ -44,27 +45,27 @@
       	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> <good_term> <bad_term> [<nolog>]"),
     ++	N_("git bisect--helper --bisect-write [--no-log] <state> <revision> <good_term> <bad_term>"),
       	NULL
       };
       
      +struct bisect_terms {
     -+	const char *term_good;
     -+	const char *term_bad;
     ++	char *term_good;
     ++	char *term_bad;
      +};
      +
      +static void free_terms(struct bisect_terms *terms)
      +{
     -+	if (!terms->term_good)
     -+		free((void *) terms->term_good);
     -+	if (!terms->term_bad)
     -+		free((void *) terms->term_bad);
     ++	FREE_AND_NULL(terms->term_good);
     ++	FREE_AND_NULL(terms->term_bad);
      +}
      +
      +static void set_terms(struct bisect_terms *terms, const char *bad,
      +		      const char *good)
      +{
     ++	free((void *)terms->term_good);
      +	terms->term_good = xstrdup(good);
     ++	free((void *)terms->term_bad);
      +	terms->term_bad = xstrdup(bad);
      +}
      +
     @@ -105,35 +106,33 @@
      +	} else if (one_of(state, terms->term_good, "skip", NULL)) {
      +		strbuf_addf(&tag, "refs/bisect/%s-%s", state, rev);
      +	} else {
     -+		error(_("Bad bisect_write argument: %s"), state);
     -+		goto fail;
     ++		retval = error(_("Bad bisect_write argument: %s"), state);
     ++		goto finish;
      +	}
      +
      +	if (get_oid(rev, &oid)) {
     -+		error(_("couldn't get the oid of the rev '%s'"), rev);
     -+		goto fail;
     ++		retval = error(_("couldn't get the oid of the rev '%s'"), rev);
     ++		goto finish;
      +	}
      +
      +	if (update_ref(NULL, tag.buf, &oid, NULL, 0,
     -+		       UPDATE_REFS_MSG_ON_ERR))
     -+		goto fail;
     ++		       UPDATE_REFS_MSG_ON_ERR)) {
     ++		retval = -1;
     ++		goto finish;
     ++	}
      +
      +	fp = fopen(git_path_bisect_log(), "a");
      +	if (!fp) {
     -+		error_errno(_("couldn't open the file '%s'"), git_path_bisect_log());
     -+		goto fail;
     ++		retval = error_errno(_("couldn't open the file '%s'"), git_path_bisect_log());
     ++		goto finish;
      +	}
      +
     -+	commit = lookup_commit_reference(&oid);
     ++	commit = lookup_commit_reference(the_repository, &oid);
      +	log_commit(fp, "%s", state, commit);
      +
      +	if (!nolog)
      +		fprintf(fp, "git bisect %s %s\n", state, rev);
      +
     -+	goto finish;
     -+
     -+fail:
     -+	retval = -1;
      +finish:
      +	if (fp)
      +		fclose(fp);
     @@ -153,7 +152,7 @@
      +		BISECT_WRITE
       	} cmdmode = 0;
      -	int no_checkout = 0;
     -+	int no_checkout = 0, res = 0;
     ++	int no_checkout = 0, res = 0, nolog = 0;
       	struct option options[] = {
       		OPT_CMDMODE(0, "next-all", &cmdmode,
       			 N_("perform 'git bisect next'"), NEXT_ALL),
     @@ -162,31 +161,24 @@
       		OPT_CMDMODE(0, "bisect-reset", &cmdmode,
       			 N_("reset the bisection state"), BISECT_RESET),
      +		OPT_CMDMODE(0, "bisect-write", &cmdmode,
     -+			 N_("update the refs according to the bisection state and may write it to BISECT_LOG"), BISECT_WRITE),
     ++			 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_BOOL(0, "no-log", &nolog,
     ++			 N_("no log for BISECT_WRITE ")),
       		OPT_END()
       	};
     -+	struct bisect_terms terms;
     ++	struct bisect_terms terms = { .term_good = NULL, .term_bad = NULL };
       
       	argc = parse_options(argc, argv, prefix, options,
       			     git_bisect_helper_usage, 0);
     -@@
     - 		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 > 1)
       			return error(_("--bisect-reset requires either no argument or a commit"));
     - 		return bisect_reset(argc ? argv[0] : NULL);
     + 		return !!bisect_reset(argc ? argv[0] : NULL);
      +	case BISECT_WRITE:
      +		if (argc != 4 && argc != 5)
      +			return error(_("--bisect-write requires either 4 or 5 arguments"));
     -+		nolog = (argc == 5) && !strcmp(argv[4], "nolog");
      +		set_terms(&terms, argv[3], argv[2]);
      +		res = bisect_write(argv[0], argv[1], &terms, nolog);
      +		break;
     @@ -195,7 +187,7 @@
       	}
      -	return 0;
      +	free_terms(&terms);
     -+	return res;
     ++	return !!res;
       }
      
       diff --git a/git-bisect.sh b/git-bisect.sh
 3:  8874ae1a3a ! 3:  5f24e7cef2 wrapper: move is_empty_file() and rename it as is_empty_or_missing_file()
     @@ -9,14 +9,13 @@
          Mentored-by: Lars Schneider <larsxschneider@gmail.com>
          Mentored-by: Christian Couder <chriscool@tuxfamily.org>
          Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
     -    Signed-off-by: Junio C Hamano <gitster@pobox.com>
      
       diff --git a/builtin/am.c b/builtin/am.c
       --- a/builtin/am.c
       +++ b/builtin/am.c
      @@
     - #include "string-list.h"
       #include "packfile.h"
     + #include "repository.h"
       
      -/**
      - * Returns 1 if the file is empty or does not exist, 0 otherwise.
     @@ -61,7 +60,7 @@
       +++ b/cache.h
      @@
        */
     - void safe_create_dir(const char *dir, int share);
     + extern int print_sha1_ellipsis(void);
       
      +/* Return 1 if the file is empty or does not exists, 0 otherwise. */
      +extern int is_empty_or_missing_file(const char *filename);
 4:  7e9fdedc45 ! 4:  4e97a08a10 bisect--helper: `check_and_set_terms` shell function in C
     @@ -20,8 +20,9 @@
      
          Mentored-by: Lars Schneider <larsxschneider@gmail.com>
          Mentored-by: Christian Couder <chriscool@tuxfamily.org>
     +    Mentored-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
          Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
     -    Signed-off-by: Junio C Hamano <gitster@pobox.com>
     +    Signed-off-by: Tanushree Tumane <tanushreetumane@gmail.com>
      
       diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
       --- a/builtin/bisect--helper.c
     @@ -29,7 +30,7 @@
      @@
       	N_("git bisect--helper --bisect-clean-state"),
       	N_("git bisect--helper --bisect-reset [<commit>]"),
     - 	N_("git bisect--helper --bisect-write <state> <revision> <good_term> <bad_term> [<nolog>]"),
     + 	N_("git bisect--helper --bisect-write [--no-log] <state> <revision> <good_term> <bad_term>"),
      +	N_("git bisect--helper --bisect-check-and-set-terms <command> <good_term> <bad_term>"),
       	NULL
       };
     @@ -53,12 +54,10 @@
      +
      +	if (!has_term_file) {
      +		if (one_of(cmd, "bad", "good", NULL)) {
     -+			free_terms(terms);
      +			set_terms(terms, "bad", "good");
      +			return write_terms(terms->term_bad, terms->term_good);
      +		}
     -+		else if (one_of(cmd, "new", "old", NULL)) {
     -+			free_terms(terms);
     ++		if (one_of(cmd, "new", "old", NULL)) {
      +			set_terms(terms, "new", "old");
      +			return write_terms(terms->term_bad, terms->term_good);
      +		}
     @@ -78,17 +77,17 @@
      +		BISECT_WRITE,
      +		CHECK_AND_SET_TERMS
       	} cmdmode = 0;
     - 	int no_checkout = 0, res = 0;
     + 	int no_checkout = 0, res = 0, nolog = 0;
       	struct option options[] = {
      @@
       			 N_("reset the bisection state"), BISECT_RESET),
       		OPT_CMDMODE(0, "bisect-write", &cmdmode,
     - 			 N_("update the refs according to the bisection state and may write it to BISECT_LOG"), BISECT_WRITE),
     + 			 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()
     + 		OPT_BOOL(0, "no-log", &nolog,
      @@
       		set_terms(&terms, argv[3], argv[2]);
       		res = bisect_write(argv[0], argv[1], &terms, nolog);
 5:  ca12543868 ! 5:  e9f400b1d6 bisect--helper: `bisect_next_check` shell function in C
     @@ -17,8 +17,9 @@
          Helped-by: Stephan Beyer <s-beyer@gmx.net>
          Mentored-by: Lars Schneider <larsxschneider@gmail.com>
          Mentored-by: Christian Couder <chriscool@tuxfamily.org>
     +    Mentored-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
          Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
     -    Signed-off-by: Junio C Hamano <gitster@pobox.com>
     +    Signed-off-by: Tanushree Tumane <tanushreetumane@gmail.com>
      
       diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
       --- a/builtin/bisect--helper.c
     @@ -33,9 +34,9 @@
       static GIT_PATH_FUNC(git_path_bisect_expected_rev, "BISECT_EXPECTED_REV")
      @@
       	N_("git bisect--helper --bisect-reset [<commit>]"),
     - 	N_("git bisect--helper --bisect-write <state> <revision> <good_term> <bad_term> [<nolog>]"),
     + 	N_("git bisect--helper --bisect-write [--no-log] <state> <revision> <good_term> <bad_term>"),
       	N_("git bisect--helper --bisect-check-and-set-terms <command> <good_term> <bad_term>"),
     -+	N_("git bisect--helper --bisect-next-check [<term>] <good_term> <bad_term>"),
     ++	N_("git bisect--helper --bisect-next-check <good_term> <bad_term> [<term>]"),
       	NULL
       };
       
     @@ -43,10 +44,8 @@
       	terms->term_bad = xstrdup(bad);
       }
       
     -+static const char *voc[] = {
     -+	"bad|new",
     -+	"good|old"
     -+};
     ++static const char *vocab_bad = "bad|new";
     ++static const char *vocab_good = "good|old";
      +
       /*
        * Check whether the string `term` belongs to the set of strings
     @@ -63,6 +62,15 @@
      +	return 1;
      +}
      +
     ++static const char *need_bad_and_good_revision_warning =
     ++	N_("You need to give me at least one %s and %s revision.\n"
     ++	   "You can use \"git bisect %s\" and \"git bisect %s\" for that.");
     ++
     ++static const char *need_bisect_start_warning =
     ++	N_("You need to start by \"git bisect start\".\n"
     ++	   "You then need to give me at least one %s and %s revision.\n"
     ++	   "You can use \"git bisect %s\" and \"git bisect %s\" for that.");
     ++
      +static int bisect_next_check(const struct bisect_terms *terms,
      +			     const char *current_term)
      +{
     @@ -79,18 +87,19 @@
      +	if (!missing_good && !missing_bad)
      +		goto finish;
      +
     -+	if (!current_term)
     -+		goto fail;
     ++	if (!current_term) {
     ++		retval = -1;
     ++		goto finish;
     ++	}
      +
     -+	if (missing_good && !missing_bad && current_term &&
     ++	if (missing_good && !missing_bad &&
      +	    !strcmp(current_term, terms->term_good)) {
      +		char *yesno;
      +		/*
      +		 * have bad (or new) but not good (or old). We could bisect
      +		 * although this is less optimum.
      +		 */
     -+		fprintf(stderr, _("Warning: bisecting only with a %s commit\n"),
     -+			terms->term_bad);
     ++		warning(_("bisecting only with a %s commit"), terms->term_bad);
      +		if (!isatty(0))
      +			goto finish;
      +		/*
     @@ -100,28 +109,17 @@
      +		 */
      +		yesno = git_prompt(_("Are you sure [Y/n]? "), PROMPT_ECHO);
      +		if (starts_with(yesno, "N") || starts_with(yesno, "n"))
     -+			goto fail;
     -+
     ++			retval = -1;
      +		goto finish;
      +	}
      +	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"),
     -+			voc[0], voc[1], voc[0], voc[1]);
     -+		goto fail;
     ++		retval = error(_(need_bad_and_good_revision_warning),
     ++			       vocab_bad, vocab_good, vocab_bad, vocab_good);
      +	} 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"),
     -+			voc[1], voc[0], voc[1], voc[0]);
     -+		goto fail;
     ++		retval = error(_(need_bisect_start_warning),
     ++			       vocab_good, vocab_bad, vocab_good, vocab_bad);
      +	}
     -+	goto finish;
      +
     -+fail:
     -+	retval = -1;
      +finish:
      +	free((void *) good_glob);
      +	free((void *) bad_ref);
     @@ -139,17 +137,17 @@
      +		CHECK_AND_SET_TERMS,
      +		BISECT_NEXT_CHECK
       	} cmdmode = 0;
     - 	int no_checkout = 0, res = 0;
     + 	int no_checkout = 0, res = 0, nolog = 0;
       	struct option options[] = {
      @@
     - 			 N_("update the refs according to the bisection state and may write it to BISECT_LOG"), BISECT_WRITE),
     + 			 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()
     + 		OPT_BOOL(0, "no-log", &nolog,
      @@
       		set_terms(&terms, argv[2], argv[1]);
       		res = check_and_set_terms(&terms, argv[0]);
     @@ -245,8 +243,8 @@
      -	bisect_next_check fail
      +	git bisect--helper --bisect-next-check $TERM_GOOD $TERM_BAD fail || exit
       
     - 	while true
     - 	do
     + 	test -n "$*" || die "$(gettext "bisect run failed: no command provided.")"
     + 
      @@
       	fi
       }
 6:  db7b281c8c ! 6:  f2b1706e5b bisect--helper: `get_terms` & `bisect_terms` shell function in C
     @@ -14,18 +14,24 @@
          Also use error() to report "no terms defined" and accordingly change the
          test in t6030.
      
     +    We need to use PARSE_OPT_KEEP_UNKNOWN here to allow for parameters that
     +    look like options (e.g --term-good) but should not be parsed by
     +    cmd_bisect__helper(). This change is safe because all other cmdmodes have
     +    strict argc checks already.
     +
          Mentored-by: Lars Schneider <larsxschneider@gmail.com>
          Mentored-by: Christian Couder <chriscool@tuxfamily.org>
     +    Mentored-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
          Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
     -    Signed-off-by: Junio C Hamano <gitster@pobox.com>
     +    Signed-off-by: Tanushree Tumane <tanushreetumane@gmail.com>
      
       diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
       --- a/builtin/bisect--helper.c
       +++ b/builtin/bisect--helper.c
      @@
     - 	N_("git bisect--helper --bisect-write <state> <revision> <good_term> <bad_term> [<nolog>]"),
     + 	N_("git bisect--helper --bisect-write [--no-log] <state> <revision> <good_term> <bad_term>"),
       	N_("git bisect--helper --bisect-check-and-set-terms <command> <good_term> <bad_term>"),
     - 	N_("git bisect--helper --bisect-next-check [<term>] <good_term> <bad_term>"),
     + 	N_("git bisect--helper --bisect-next-check <good_term> <bad_term> [<term>]"),
      +	N_("git bisect--helper --bisect-terms [--term-good | --term-old | --term-bad | --term-new]"),
       	NULL
       };
     @@ -41,18 +47,17 @@
      +	int res = 0;
      +
      +	fp = fopen(git_path_bisect_terms(), "r");
     -+	if (!fp)
     -+		goto fail;
     ++	if (!fp) {
     ++		res = -1;
     ++		goto finish;
     ++	}
      +
      +	free_terms(terms);
      +	strbuf_getline_lf(&str, fp);
      +	terms->term_bad = strbuf_detach(&str, NULL);
      +	strbuf_getline_lf(&str, fp);
      +	terms->term_good = strbuf_detach(&str, NULL);
     -+	goto finish;
      +
     -+fail:
     -+	res = -1;
      +finish:
      +	if (fp)
      +		fclose(fp);
     @@ -60,32 +65,26 @@
      +	return res;
      +}
      +
     -+static int bisect_terms(struct bisect_terms *terms, const char **argv, int argc)
     ++static int bisect_terms(struct bisect_terms *terms, const char *option)
      +{
     -+	int i;
     -+
      +	if (get_terms(terms))
      +		return error(_("no terms defined"));
      +
     -+	if (argc > 1)
     -+		return error(_("--bisect-term requires exactly one argument"));
     -+
     -+	if (argc == 0)
     -+		return !printf(_("Your current terms are %s for the old state\n"
     -+				 "and %s for the new state.\n"),
     -+				 terms->term_good, terms->term_bad);
     -+
     -+	for (i = 0; i < argc; i++) {
     -+		if (!strcmp(argv[i], "--term-good"))
     -+			printf(_("%s\n"), terms->term_good);
     -+		else if (!strcmp(argv[i], "--term-bad"))
     -+			printf(_("%s\n"), terms->term_bad);
     -+		else
     -+			error(_("BUG: invalid argument %s for 'git bisect terms'.\n"
     -+				  "Supported options are: "
     -+				  "--term-good|--term-old and "
     -+				  "--term-bad|--term-new."), argv[i]);
     ++	if (option == NULL) {
     ++		printf(_("Your current terms are %s for the old state\n"
     ++			 "and %s for the new state.\n"),
     ++		       terms->term_good, terms->term_bad);
     ++		return 0;
      +	}
     ++	if (one_of(option, "--term-good", "--term-old", NULL))
     ++		printf("%s\n", terms->term_good);
     ++	else if (one_of(option, "--term-bad", "--term-new", NULL))
     ++		printf("%s\n", terms->term_bad);
     ++	else
     ++		return error(_("invalid argument %s for 'git bisect terms'.\n"
     ++			       "Supported options are: "
     ++			       "--term-good|--term-old and "
     ++			       "--term-bad|--term-new."), option);
      +
      +	return 0;
      +}
     @@ -101,7 +100,7 @@
      +		BISECT_NEXT_CHECK,
      +		BISECT_TERMS
       	} cmdmode = 0;
     - 	int no_checkout = 0, res = 0;
     + 	int no_checkout = 0, res = 0, nolog = 0;
       	struct option options[] = {
      @@
       			 N_("check and set terms in a bisection state"), CHECK_AND_SET_TERMS),
     @@ -111,9 +110,9 @@
      +			 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()
     + 		OPT_BOOL(0, "no-log", &nolog,
      @@
     - 	struct bisect_terms terms;
     + 	struct bisect_terms terms = { .term_good = NULL, .term_bad = NULL };
       
       	argc = parse_options(argc, argv, prefix, options,
      -			     git_bisect_helper_usage, 0);
     @@ -128,7 +127,7 @@
      +	case BISECT_TERMS:
      +		if (argc > 1)
      +			return error(_("--bisect-terms requires 0 or 1 argument"));
     -+		res = bisect_terms(&terms, argv, argc);
     ++		res = bisect_terms(&terms, argc == 1 ? argv[0] : NULL);
      +		break;
       	default:
       		return error("BUG: unknown subcommand '%d'", cmdmode);
     @@ -142,7 +141,7 @@
       			git bisect--helper --bisect-write "$command" "$rev" "$TERM_GOOD" "$TERM_BAD" || exit;;
       		terms)
      -			bisect_terms $rev ;;
     -+			git bisect--helper --bisect-terms $rev  || exit;;
     ++			git bisect--helper --bisect-terms $rev || exit;;
       		*)
       			die "$(gettext "?? what are you talking about?")" ;;
       		esac
 7:  e14e913797 ! 7:  87a033b858 bisect--helper: `bisect_start` shell function partially in C
     @@ -6,7 +6,7 @@
          `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
     +    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.
      
     @@ -18,12 +18,17 @@
          Also introduce a method `bisect_append_log_quoted` to keep things short
          and crisp.
      
     +    Note that we are a bit lax about command-line parsing because the helper
     +    is not supposed to be called by the user directly (but only from the git
     +    bisect script).
     +
          Helped-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
          Helped-by: Stephan Beyer <s-beyer@gmx.net>
          Mentored-by: Lars Schneider <larsxschneider@gmail.com>
          Mentored-by: Christian Couder <chriscool@tuxfamily.org>
     +    Mentored-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
          Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
     -    Signed-off-by: Junio C Hamano <gitster@pobox.com>
     +    Signed-off-by: Tanushree Tumane <tanushreetumane@gmail.com>
      
       diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
       --- a/builtin/bisect--helper.c
     @@ -47,10 +52,10 @@
       	N_("git bisect--helper --next-all [--no-checkout]"),
      @@
       	N_("git bisect--helper --bisect-check-and-set-terms <command> <good_term> <bad_term>"),
     - 	N_("git bisect--helper --bisect-next-check [<term>] <good_term> <bad_term>"),
     + 	N_("git bisect--helper --bisect-next-check <good_term> <bad_term> [<term>]"),
       	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>...]"),
     ++					     "[--no-checkout] [<bad> [<good>...]] [--] [<paths>...]"),
       	NULL
       };
       
     @@ -67,17 +72,15 @@
      +	if (!fp)
      +		return -1;
      +
     -+	if (fprintf(fp, "git bisect start") < 1)
     -+		goto fail;
     ++	if (fprintf(fp, "git bisect start") < 1) {
     ++		retval = -1;
     ++		goto finish;
     ++	}
      +
     -+	sq_quote_argv(&orig_args, argv, 0);
     ++	sq_quote_argv(&orig_args, argv);
      +	if (fprintf(fp, "%s\n", orig_args.buf) < 1)
     -+		goto fail;
     ++		retval = -1;
      +
     -+	goto finish;
     -+
     -+fail:
     -+	retval = -1;
      +finish:
      +	fclose(fp);
      +	strbuf_release(&orig_args);
     @@ -138,7 +141,7 @@
      +			terms->term_bad = xstrdup(arg);
      +		} else if (starts_with(arg, "--") &&
      +			 !one_of(arg, "--term-good", "--term-bad", NULL)) {
     -+			return error(_("unrecognised option: '%s'"), arg);
     ++			return error(_("unrecognized option: '%s'"), arg);
      +		} else {
      +			char *commit_id = xstrfmt("%s^{commit}", arg);
      +			if (get_oid(commit_id, &oid) && has_double_dash)
     @@ -157,7 +160,8 @@
      +	 * set references named with the default terms, and won't be able
      +	 * to change afterwards.
      +	 */
     -+	must_write_terms |= !!revs.nr;
     ++	if (revs.nr)
     ++		must_write_terms = 1;
      +	for (i = 0; i < revs.nr; i++) {
      +		if (bad_seen) {
      +			string_list_append(&states, terms->term_good);
     @@ -173,7 +177,7 @@
      +	head = resolve_ref_unsafe("HEAD", 0, &head_oid, &flags);
      +	if (!head)
      +		if (get_oid("HEAD", &head_oid))
     -+			return error(_("Bad HEAD - I need a HEAD"));
     ++			return error(_("bad HEAD - I need a HEAD"));
      +
      +	/*
      +	 * Check if we are bisecting
     @@ -188,10 +192,11 @@
      +			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);
     -+				goto fail;
     ++				retval = error(_("checking out '%s' failed."
     ++						 " Try 'git bisect start "
     ++						 "<valid-branch>'."),
     ++					       start_head.buf);
     ++				goto finish;
      +			}
      +		}
      +	} else {
     @@ -199,7 +204,7 @@
      +		if (!get_oid(head, &head_oid) &&
      +		    !starts_with(head, "refs/heads/")) {
      +			strbuf_reset(&start_head);
     -+			strbuf_addstr(&start_head, sha1_to_hex(head_oid.hash));
     ++			strbuf_addstr(&start_head, oid_to_hex(&head_oid));
      +		} else if (!get_oid(head, &head_oid) &&
      +			   skip_prefix(head, "refs/heads/", &head)) {
      +			/*
     @@ -211,7 +216,7 @@
      +				return error(_("won't bisect on cg-seek'ed tree"));
      +			strbuf_addstr(&start_head, head);
      +		} else {
     -+			return error(_("Bad HEAD - strange symbolic ref"));
     ++			return error(_("bad HEAD - strange symbolic ref"));
      +		}
      +	}
      +
     @@ -236,31 +241,33 @@
      +	if (no_checkout) {
      +		get_oid(start_head.buf, &oid);
      +		if (update_ref(NULL, "BISECT_HEAD", &oid, NULL, 0,
     -+			       UPDATE_REFS_MSG_ON_ERR))
     -+			goto fail;
     ++			       UPDATE_REFS_MSG_ON_ERR)) {
     ++			retval = -1;
     ++			goto finish;
     ++		}
      +	}
      +
      +	if (pathspec_pos < argc - 1)
     -+		sq_quote_argv(&bisect_names, argv + pathspec_pos, 0);
     ++		sq_quote_argv(&bisect_names, argv + pathspec_pos);
      +	write_file(git_path_bisect_names(), "%s\n", bisect_names.buf);
      +
      +	for (i = 0; i < states.nr; i++)
      +		if (bisect_write(states.items[i].string,
     -+				 revs.items[i].string, terms, 1))
     -+			goto fail;
     ++				 revs.items[i].string, terms, 1)) {
     ++			retval = -1;
     ++			goto finish;
     ++		}
      +
     -+	if (must_write_terms)
     -+		if (write_terms(terms->term_bad, terms->term_good))
     -+			goto fail;
     ++	if (must_write_terms && write_terms(terms->term_bad,
     ++					    terms->term_good)) {
     ++		retval = -1;
     ++		goto finish;
     ++	}
      +
      +	retval = bisect_append_log_quoted(argv);
      +	if (retval)
     -+		goto fail;
     -+
     -+	goto finish;
     ++		retval = -1;
      +
     -+fail:
     -+	retval = -1;
      +finish:
      +	string_list_clear(&revs, 0);
      +	string_list_clear(&states, 0);
     @@ -280,7 +287,7 @@
      +		BISECT_TERMS,
      +		BISECT_START
       	} cmdmode = 0;
     - 	int no_checkout = 0, res = 0;
     + 	int no_checkout = 0, res = 0, nolog = 0;
       	struct option options[] = {
      @@
       			 N_("check whether bad or good terms exist"), BISECT_NEXT_CHECK),
     @@ -290,9 +297,9 @@
      +			 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()
     + 		OPT_BOOL(0, "no-log", &nolog,
      @@
     - 	struct bisect_terms terms;
     + 	struct bisect_terms terms = { .term_good = NULL, .term_bad = NULL };
       
       	argc = parse_options(argc, argv, prefix, options,
      -			     git_bisect_helper_usage, PARSE_OPT_KEEP_UNKNOWN);
     @@ -303,7 +310,7 @@
       		usage_with_options(git_bisect_helper_usage, options);
      @@
       			return error(_("--bisect-terms requires 0 or 1 argument"));
     - 		res = bisect_terms(&terms, argv, argc);
     + 		res = bisect_terms(&terms, argc == 1 ? argv[0] : NULL);
       		break;
      +	case BISECT_START:
      +		set_terms(&terms, "bad", "good");
 8:  161bbaced1 < -:  ---------- t6030: make various test to pass GETTEXT_POISON tests

-- 
gitgitgadget

```

## Pranit Bauva via GitGitGadget, 2019-01-02 15:38

Subject: [PATCH v17 2/7] bisect--helper: `bisect_write` shell function in C
Message-ID: <7d6291d9dda2531646b2a64941338adb98f60dee.1546443509.git.gitgitgadget@gmail.com>
URL: https://gitlist.dev/e/7d6291d9dda2531646b2a64941338adb98f60dee.1546443509.git.gitgitgadget%40gmail.com
In-Reply-To: <pull.101.v17.git.gitgitgadget@gmail.com>

```
From: Pranit Bauva <pranit.bauva@gmail.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.

Add a log_commit() helper function to write the contents of the commit message
header to a file which will be re-used in future parts of the code as
well.

Also introduce a function free_terms() to free the memory of `struct
bisect_terms` and set_terms() to set the values of members in `struct
bisect_terms`.

Helped-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
Mentored-by: Lars Schneider <larsxschneider@gmail.com>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Mentored-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
Signed-off-by: Tanushree Tumane <tanushreetumane@gmail.com>
---
 builtin/bisect--helper.c | 105 +++++++++++++++++++++++++++++++++++++--
 git-bisect.sh            |  25 ++--------
 2 files changed, 106 insertions(+), 24 deletions(-)

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index aa6495dc84..df821be4b2 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -12,15 +12,37 @@ 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_start, "BISECT_START")
 static GIT_PATH_FUNC(git_path_bisect_head, "BISECT_HEAD")
+static GIT_PATH_FUNC(git_path_bisect_log, "BISECT_LOG")
 
 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>]"),
+	N_("git bisect--helper --bisect-write [--no-log] <state> <revision> <good_term> <bad_term>"),
 	NULL
 };
 
+struct bisect_terms {
+	char *term_good;
+	char *term_bad;
+};
+
+static void free_terms(struct bisect_terms *terms)
+{
+	FREE_AND_NULL(terms->term_good);
+	FREE_AND_NULL(terms->term_bad);
+}
+
+static void set_terms(struct bisect_terms *terms, const char *bad,
+		      const char *good)
+{
+	free((void *)terms->term_good);
+	terms->term_good = xstrdup(good);
+	free((void *)terms->term_bad);
+	terms->term_bad = xstrdup(bad);
+}
+
 /*
  * Check whether the string `term` belongs to the set of strings
  * included in the variable arguments.
@@ -148,6 +170,70 @@ static int bisect_reset(const char *commit)
 	return bisect_clean_state();
 }
 
+static void log_commit(FILE *fp, char *fmt, const char *state,
+		       struct commit *commit)
+{
+	struct pretty_print_context pp = {0};
+	struct strbuf commit_msg = STRBUF_INIT;
+	char *label = xstrfmt(fmt, state);
+
+	format_commit_message(commit, "%s", &commit_msg, &pp);
+
+	fprintf(fp, "# %s: [%s] %s\n", label, oid_to_hex(&commit->object.oid),
+		commit_msg.buf);
+
+	strbuf_release(&commit_msg);
+	free(label);
+}
+
+static int bisect_write(const char *state, const char *rev,
+			const struct bisect_terms *terms, int nolog)
+{
+	struct strbuf tag = STRBUF_INIT;
+	struct object_id oid;
+	struct commit *commit;
+	FILE *fp = NULL;
+	int retval = 0;
+
+	if (!strcmp(state, terms->term_bad)) {
+		strbuf_addf(&tag, "refs/bisect/%s", state);
+	} else if (one_of(state, terms->term_good, "skip", NULL)) {
+		strbuf_addf(&tag, "refs/bisect/%s-%s", state, rev);
+	} else {
+		retval = error(_("Bad bisect_write argument: %s"), state);
+		goto finish;
+	}
+
+	if (get_oid(rev, &oid)) {
+		retval = error(_("couldn't get the oid of the rev '%s'"), rev);
+		goto finish;
+	}
+
+	if (update_ref(NULL, tag.buf, &oid, NULL, 0,
+		       UPDATE_REFS_MSG_ON_ERR)) {
+		retval = -1;
+		goto finish;
+	}
+
+	fp = fopen(git_path_bisect_log(), "a");
+	if (!fp) {
+		retval = error_errno(_("couldn't open the file '%s'"), git_path_bisect_log());
+		goto finish;
+	}
+
+	commit = lookup_commit_reference(the_repository, &oid);
+	log_commit(fp, "%s", state, commit);
+
+	if (!nolog)
+		fprintf(fp, "git bisect %s %s\n", state, rev);
+
+finish:
+	if (fp)
+		fclose(fp);
+	strbuf_release(&tag);
+	return retval;
+}
+
 int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 {
 	enum {
@@ -155,9 +241,10 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 		WRITE_TERMS,
 		BISECT_CLEAN_STATE,
 		CHECK_EXPECTED_REVS,
-		BISECT_RESET
+		BISECT_RESET,
+		BISECT_WRITE
 	} cmdmode = 0;
-	int no_checkout = 0;
+	int no_checkout = 0, res = 0, nolog = 0;
 	struct option options[] = {
 		OPT_CMDMODE(0, "next-all", &cmdmode,
 			 N_("perform 'git bisect next'"), NEXT_ALL),
@@ -169,10 +256,15 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 			 N_("check for expected revs"), CHECK_EXPECTED_REVS),
 		OPT_CMDMODE(0, "bisect-reset", &cmdmode,
 			 N_("reset the bisection state"), BISECT_RESET),
+		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_BOOL(0, "no-log", &nolog,
+			 N_("no log for BISECT_WRITE ")),
 		OPT_END()
 	};
+	struct bisect_terms terms = { .term_good = NULL, .term_bad = NULL };
 
 	argc = parse_options(argc, argv, prefix, options,
 			     git_bisect_helper_usage, 0);
@@ -198,8 +290,15 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 		if (argc > 1)
 			return error(_("--bisect-reset requires either no argument or a commit"));
 		return !!bisect_reset(argc ? argv[0] : NULL);
+	case BISECT_WRITE:
+		if (argc != 4 && argc != 5)
+			return error(_("--bisect-write requires either 4 or 5 arguments"));
+		set_terms(&terms, argv[3], argv[2]);
+		res = bisect_write(argv[0], argv[1], &terms, nolog);
+		break;
 	default:
 		return error("BUG: unknown subcommand '%d'", cmdmode);
 	}
-	return 0;
+	free_terms(&terms);
+	return !!res;
 }
diff --git a/git-bisect.sh b/git-bisect.sh
index afbfbc1f8e..ebf445223c 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -144,7 +144,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.
@@ -220,23 +220,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() {
 		bisected_head=$(bisect_head)
 		rev=$(git rev-parse --verify "$bisected_head") ||
 			die "$(eval_gettext "Bad rev input: \$bisected_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 ;;
 		*)
-- 
gitgitgadget


```

## Pranit Bauva via GitGitGadget, 2019-01-02 15:38

Subject: [PATCH v17 4/7] bisect--helper: `check_and_set_terms` shell function in C
Message-ID: <4e97a08a10b2d23d83c1d1b53b78b39fcc6af6ad.1546443509.git.gitgitgadget@gmail.com>
URL: https://gitlist.dev/e/4e97a08a10b2d23d83c1d1b53b78b39fcc6af6ad.1546443509.git.gitgitgadget%40gmail.com
In-Reply-To: <pull.101.v17.git.gitgitgadget@gmail.com>

```
From: Pranit Bauva <pranit.bauva@gmail.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.

Mentored-by: Lars Schneider <larsxschneider@gmail.com>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Mentored-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
Signed-off-by: Tanushree Tumane <tanushreetumane@gmail.com>
---
 builtin/bisect--helper.c | 39 ++++++++++++++++++++++++++++++++++++++-
 git-bisect.sh            | 36 ++++--------------------------------
 2 files changed, 42 insertions(+), 33 deletions(-)

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index df821be4b2..02ea5a9611 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -20,6 +20,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 [--no-log] <state> <revision> <good_term> <bad_term>"),
+	N_("git bisect--helper --bisect-check-and-set-terms <command> <good_term> <bad_term>"),
 	NULL
 };
 
@@ -234,6 +235,33 @@ static int bisect_write(const char *state, const char *rev,
 	return retval;
 }
 
+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) &&
+	    strcmp(cmd, terms->term_good))
+		return error(_("Invalid command: you're currently in a "
+				"%s/%s bisect"), terms->term_bad,
+				terms->term_good);
+
+	if (!has_term_file) {
+		if (one_of(cmd, "bad", "good", NULL)) {
+			set_terms(terms, "bad", "good");
+			return write_terms(terms->term_bad, terms->term_good);
+		}
+		if (one_of(cmd, "new", "old", NULL)) {
+			set_terms(terms, "new", "old");
+			return write_terms(terms->term_bad, terms->term_good);
+		}
+	}
+
+	return 0;
+}
+
 int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 {
 	enum {
@@ -242,7 +270,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 		BISECT_CLEAN_STATE,
 		CHECK_EXPECTED_REVS,
 		BISECT_RESET,
-		BISECT_WRITE
+		BISECT_WRITE,
+		CHECK_AND_SET_TERMS
 	} cmdmode = 0;
 	int no_checkout = 0, res = 0, nolog = 0;
 	struct option options[] = {
@@ -258,6 +287,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 			 N_("reset the bisection state"), BISECT_RESET),
 		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_BOOL(0, "no-log", &nolog,
@@ -296,6 +327,12 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 		set_terms(&terms, argv[3], argv[2]);
 		res = bisect_write(argv[0], argv[1], &terms, nolog);
 		break;
+	case CHECK_AND_SET_TERMS:
+		if (argc != 3)
+			return error(_("--check-and-set-terms requires 3 arguments"));
+		set_terms(&terms, argv[2], argv[1]);
+		res = check_and_set_terms(&terms, argv[0]);
+		break;
 	default:
 		return error("BUG: unknown subcommand '%d'", cmdmode);
 	}
diff --git a/git-bisect.sh b/git-bisect.sh
index ebf445223c..9e993a8187 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -238,7 +238,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 "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"
@@ -482,36 +484,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" ;;
-- 
gitgitgadget


```

## Pranit Bauva via GitGitGadget, 2019-01-02 15:38

Subject: [PATCH v17 5/7] bisect--helper: `bisect_next_check` shell function in C
Message-ID: <e9f400b1d6e091798d9444046cfbcb67f7afd1b3.1546443509.git.gitgitgadget@gmail.com>
URL: https://gitlist.dev/e/e9f400b1d6e091798d9444046cfbcb67f7afd1b3.1546443509.git.gitgitgadget%40gmail.com
In-Reply-To: <pull.101.v17.git.gitgitgadget@gmail.com>

```
From: Pranit Bauva <pranit.bauva@gmail.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 .

`bisect_voc` shell function is no longer useful now and is replaced by
using a char *[] of "new|bad" and "good|old" values.

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.

Helped-by: Stephan Beyer <s-beyer@gmx.net>
Mentored-by: Lars Schneider <larsxschneider@gmail.com>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Mentored-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
Signed-off-by: Tanushree Tumane <tanushreetumane@gmail.com>
---
 builtin/bisect--helper.c | 88 +++++++++++++++++++++++++++++++++++++++-
 git-bisect.sh            | 60 ++-------------------------
 2 files changed, 91 insertions(+), 57 deletions(-)

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 02ea5a9611..38ae825b9a 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")
@@ -21,6 +22,7 @@ static const char * const git_bisect_helper_usage[] = {
 	N_("git bisect--helper --bisect-reset [<commit>]"),
 	N_("git bisect--helper --bisect-write [--no-log] <state> <revision> <good_term> <bad_term>"),
 	N_("git bisect--helper --bisect-check-and-set-terms <command> <good_term> <bad_term>"),
+	N_("git bisect--helper --bisect-next-check <good_term> <bad_term> [<term>]"),
 	NULL
 };
 
@@ -44,6 +46,9 @@ static void set_terms(struct bisect_terms *terms, const char *bad,
 	terms->term_bad = xstrdup(bad);
 }
 
+static const char *vocab_bad = "bad|new";
+static const char *vocab_good = "good|old";
+
 /*
  * Check whether the string `term` belongs to the set of strings
  * included in the variable arguments.
@@ -262,6 +267,78 @@ 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 const char *need_bad_and_good_revision_warning =
+	N_("You need to give me at least one %s and %s revision.\n"
+	   "You can use \"git bisect %s\" and \"git bisect %s\" for that.");
+
+static const char *need_bisect_start_warning =
+	N_("You need to start by \"git bisect start\".\n"
+	   "You then need to give me at least one %s and %s revision.\n"
+	   "You can use \"git bisect %s\" and \"git bisect %s\" for that.");
+
+static int bisect_next_check(const struct bisect_terms *terms,
+			     const char *current_term)
+{
+	int missing_good = 1, missing_bad = 1, retval = 0;
+	const char *bad_ref = xstrfmt("refs/bisect/%s", terms->term_bad);
+	const char *good_glob = xstrfmt("%s-*", terms->term_good);
+
+	if (ref_exists(bad_ref))
+		missing_bad = 0;
+
+	for_each_glob_ref_in(mark_good, good_glob, "refs/bisect/",
+			     (void *) &missing_good);
+
+	if (!missing_good && !missing_bad)
+		goto finish;
+
+	if (!current_term) {
+		retval = -1;
+		goto finish;
+	}
+
+	if (missing_good && !missing_bad &&
+	    !strcmp(current_term, terms->term_good)) {
+		char *yesno;
+		/*
+		 * have bad (or new) but not good (or old). We could bisect
+		 * although this is less optimum.
+		 */
+		warning(_("bisecting only with a %s commit"), terms->term_bad);
+		if (!isatty(0))
+			goto finish;
+		/*
+		 * 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"))
+			retval = -1;
+		goto finish;
+	}
+	if (!is_empty_or_missing_file(git_path_bisect_start())) {
+		retval = error(_(need_bad_and_good_revision_warning),
+			       vocab_bad, vocab_good, vocab_bad, vocab_good);
+	} else {
+		retval = error(_(need_bisect_start_warning),
+			       vocab_good, vocab_bad, vocab_good, vocab_bad);
+	}
+
+finish:
+	free((void *) good_glob);
+	free((void *) bad_ref);
+	return retval;
+}
+
 int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 {
 	enum {
@@ -271,7 +348,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 		CHECK_EXPECTED_REVS,
 		BISECT_RESET,
 		BISECT_WRITE,
-		CHECK_AND_SET_TERMS
+		CHECK_AND_SET_TERMS,
+		BISECT_NEXT_CHECK
 	} cmdmode = 0;
 	int no_checkout = 0, res = 0, nolog = 0;
 	struct option options[] = {
@@ -289,6 +367,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_BOOL(0, "no-log", &nolog,
@@ -333,6 +413,12 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 		set_terms(&terms, argv[2], argv[1]);
 		res = check_and_set_terms(&terms, argv[0]);
 		break;
+	case BISECT_NEXT_CHECK:
+		if (argc != 2 && argc != 3)
+			return error(_("--bisect-next-check requires 2 or 3 arguments"));
+		set_terms(&terms, argv[1], argv[0]);
+		res = bisect_next_check(&terms, argc == 3 ? argv[2] : NULL);
+		break;
 	default:
 		return error("BUG: unknown subcommand '%d'", cmdmode);
 	}
diff --git a/git-bisect.sh b/git-bisect.sh
index 9e993a8187..5ef3e25621 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
 
 	test -n "$*" || die "$(gettext "bisect run failed: no command provided.")"
 
@@ -484,13 +439,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"
-- 
gitgitgadget


```

## Pranit Bauva via GitGitGadget, 2019-01-02 15:38

Subject: [PATCH v17 7/7] bisect--helper: `bisect_start` shell function partially in C
Message-ID: <87a033b858d7e6acba2671339f6bdded03dcdc3a.1546443509.git.gitgitgadget@gmail.com>
URL: https://gitlist.dev/e/87a033b858d7e6acba2671339f6bdded03dcdc3a.1546443509.git.gitgitgadget%40gmail.com
In-Reply-To: <pull.101.v17.git.gitgitgadget@gmail.com>

```
From: Pranit Bauva <pranit.bauva@gmail.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.

Also introduce a method `bisect_append_log_quoted` to keep things short
and crisp.

Note that we are a bit lax about command-line parsing because the helper
is not supposed to be called by the user directly (but only from the git
bisect script).

Helped-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
Helped-by: Stephan Beyer <s-beyer@gmx.net>
Mentored-by: Lars Schneider <larsxschneider@gmail.com>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Mentored-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
Signed-off-by: Tanushree Tumane <tanushreetumane@gmail.com>
---
 builtin/bisect--helper.c | 230 ++++++++++++++++++++++++++++++++++++++-
 git-bisect.sh            | 132 +---------------------
 2 files changed, 230 insertions(+), 132 deletions(-)

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index d61edd3c91..22e669e3b1 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")
@@ -14,6 +15,8 @@ static GIT_PATH_FUNC(git_path_bisect_ancestors_ok, "BISECT_ANCESTORS_OK")
 static GIT_PATH_FUNC(git_path_bisect_start, "BISECT_START")
 static GIT_PATH_FUNC(git_path_bisect_head, "BISECT_HEAD")
 static GIT_PATH_FUNC(git_path_bisect_log, "BISECT_LOG")
+static GIT_PATH_FUNC(git_path_head_name, "head-name")
+static GIT_PATH_FUNC(git_path_bisect_names, "BISECT_NAMES")
 
 static const char * const git_bisect_helper_usage[] = {
 	N_("git bisect--helper --next-all [--no-checkout]"),
@@ -24,6 +27,8 @@ static const char * const git_bisect_helper_usage[] = {
 	N_("git bisect--helper --bisect-check-and-set-terms <command> <good_term> <bad_term>"),
 	N_("git bisect--helper --bisect-next-check <good_term> <bad_term> [<term>]"),
 	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
 };
 
@@ -389,6 +394,219 @@ static int bisect_terms(struct bisect_terms *terms, const char *option)
 	return 0;
 }
 
+static int bisect_append_log_quoted(const char **argv)
+{
+	int retval = 0;
+	FILE *fp = fopen(git_path_bisect_log(), "a");
+	struct strbuf orig_args = STRBUF_INIT;
+
+	if (!fp)
+		return -1;
+
+	if (fprintf(fp, "git bisect start") < 1) {
+		retval = -1;
+		goto finish;
+	}
+
+	sq_quote_argv(&orig_args, argv);
+	if (fprintf(fp, "%s\n", orig_args.buf) < 1)
+		retval = -1;
+
+finish:
+	fclose(fp);
+	strbuf_release(&orig_args);
+	return retval;
+}
+
+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, retval = 0;
+	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 object_id head_oid;
+	struct object_id oid;
+	const char *head;
+
+	if (is_bare_repository())
+		no_checkout = 1;
+
+	/*
+	 * Check for one bad and then some good revisions
+	 */
+	for (i = 0; i < argc; i++) {
+		if (!strcmp(argv[i], "--")) {
+			has_double_dash = 1;
+			break;
+		}
+	}
+
+	for (i = 0; i < argc; i++) {
+		const char *arg = argv[i];
+		if (!strcmp(argv[i], "--")) {
+			break;
+		} else if (!strcmp(arg, "--no-checkout")) {
+			no_checkout = 1;
+		} else if (!strcmp(arg, "--term-good") ||
+			 !strcmp(arg, "--term-old")) {
+			must_write_terms = 1;
+			free((void *) terms->term_good);
+			terms->term_good = xstrdup(argv[++i]);
+		} else if (skip_prefix(arg, "--term-good=", &arg) ||
+			   skip_prefix(arg, "--term-old=", &arg)) {
+			must_write_terms = 1;
+			free((void *) terms->term_good);
+			terms->term_good = xstrdup(arg);
+		} else if (!strcmp(arg, "--term-bad") ||
+			 !strcmp(arg, "--term-new")) {
+			must_write_terms = 1;
+			free((void *) terms->term_bad);
+			terms->term_bad = xstrdup(argv[++i]);
+		} else if (skip_prefix(arg, "--term-bad=", &arg) ||
+			   skip_prefix(arg, "--term-new=", &arg)) {
+			must_write_terms = 1;
+			free((void *) terms->term_bad);
+			terms->term_bad = xstrdup(arg);
+		} else if (starts_with(arg, "--") &&
+			 !one_of(arg, "--term-good", "--term-bad", NULL)) {
+			return error(_("unrecognized option: '%s'"), arg);
+		} else {
+			char *commit_id = xstrfmt("%s^{commit}", arg);
+			if (get_oid(commit_id, &oid) && has_double_dash)
+				die(_("'%s' does not appear to be a valid "
+				      "revision"), arg);
+
+			string_list_append(&revs, oid_to_hex(&oid));
+			free(commit_id);
+		}
+	}
+	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.
+	 */
+	if (revs.nr)
+		must_write_terms = 1;
+	for (i = 0; i < revs.nr; i++) {
+		if (bad_seen) {
+			string_list_append(&states, terms->term_good);
+		} else {
+			bad_seen = 1;
+			string_list_append(&states, terms->term_bad);
+		}
+	}
+
+	/*
+	 * Verify HEAD
+	 */
+	head = resolve_ref_unsafe("HEAD", 0, &head_oid, &flags);
+	if (!head)
+		if (get_oid("HEAD", &head_oid))
+			return error(_("bad HEAD - I need a HEAD"));
+
+	/*
+	 * Check if we are bisecting
+	 */
+	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)) {
+				retval = error(_("checking out '%s' failed."
+						 " Try 'git bisect start "
+						 "<valid-branch>'."),
+					       start_head.buf);
+				goto finish;
+			}
+		}
+	} else {
+		/* Get the rev from where we start. */
+		if (!get_oid(head, &head_oid) &&
+		    !starts_with(head, "refs/heads/")) {
+			strbuf_reset(&start_head);
+			strbuf_addstr(&start_head, oid_to_hex(&head_oid));
+		} else if (!get_oid(head, &head_oid) &&
+			   skip_prefix(head, "refs/heads/", &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()))
+				return error(_("won't bisect on cg-seek'ed tree"));
+			strbuf_addstr(&start_head, head);
+		} else {
+			return error(_("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
+	 */
+	write_file(git_path_bisect_start(), "%s\n", start_head.buf);
+
+	if (no_checkout) {
+		get_oid(start_head.buf, &oid);
+		if (update_ref(NULL, "BISECT_HEAD", &oid, NULL, 0,
+			       UPDATE_REFS_MSG_ON_ERR)) {
+			retval = -1;
+			goto finish;
+		}
+	}
+
+	if (pathspec_pos < argc - 1)
+		sq_quote_argv(&bisect_names, argv + pathspec_pos);
+	write_file(git_path_bisect_names(), "%s\n", bisect_names.buf);
+
+	for (i = 0; i < states.nr; i++)
+		if (bisect_write(states.items[i].string,
+				 revs.items[i].string, terms, 1)) {
+			retval = -1;
+			goto finish;
+		}
+
+	if (must_write_terms && write_terms(terms->term_bad,
+					    terms->term_good)) {
+		retval = -1;
+		goto finish;
+	}
+
+	retval = bisect_append_log_quoted(argv);
+	if (retval)
+		retval = -1;
+
+finish:
+	string_list_clear(&revs, 0);
+	string_list_clear(&states, 0);
+	strbuf_release(&start_head);
+	strbuf_release(&bisect_names);
+	return retval;
+}
+
 int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 {
 	enum {
@@ -400,7 +618,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, nolog = 0;
 	struct option options[] = {
@@ -422,6 +641,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_BOOL(0, "no-log", &nolog,
@@ -431,7 +652,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 	struct bisect_terms terms = { .term_good = NULL, .term_bad = NULL };
 
 	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);
@@ -477,6 +699,10 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 			return error(_("--bisect-terms requires 0 or 1 argument"));
 		res = bisect_terms(&terms, argc == 1 ? argv[0] : NULL);
 		break;
+	case BISECT_START:
+		set_terms(&terms, "bad", "good");
+		res = bisect_start(&terms, no_checkout, argv, argc);
+		break;
 	default:
 		return error("BUG: unknown subcommand '%d'", cmdmode);
 	}
diff --git a/git-bisect.sh b/git-bisect.sh
index bdb614e3c2..efee12b8b1 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -71,122 +71,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,23 +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" || exit
-	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
-- 
gitgitgadget

```

## Pranit Bauva via GitGitGadget, 2019-01-02 15:38

Subject: [PATCH v17 3/7] wrapper: move is_empty_file() and rename it as is_empty_or_missing_file()
Message-ID: <5f24e7cef26b4b1b6c87be03e5485b182040ece1.1546443509.git.gitgitgadget@gmail.com>
URL: https://gitlist.dev/e/5f24e7cef26b4b1b6c87be03e5485b182040ece1.1546443509.git.gitgitgadget%40gmail.com
In-Reply-To: <pull.101.v17.git.gitgitgadget@gmail.com>

```
From: Pranit Bauva <pranit.bauva@gmail.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 8f27f3375b..310eefe9e8 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -34,22 +34,6 @@
 #include "packfile.h"
 #include "repository.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.
  */
@@ -1220,7 +1204,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."));
 		die_user_resolve(state);
 	}
@@ -1803,7 +1787,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 ca36b44ee0..9ff7e56b74 100644
--- a/cache.h
+++ b/cache.h
@@ -1788,4 +1788,7 @@ void safe_create_dir(const char *dir, int share);
  */
 extern int print_sha1_ellipsis(void);
 
+/* 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 e4fa9d84cd..ea3cf64d4c 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -690,3 +690,16 @@ int xgethostname(char *buf, size_t len)
 		buf[len - 1] = 0;
 	return ret;
 }
+
+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;
+}
-- 
gitgitgadget


```

## Pranit Bauva via GitGitGadget, 2019-01-02 15:38

Subject: [PATCH v17 6/7] bisect--helper: `get_terms` & `bisect_terms` shell function in C
Message-ID: <f2b1706e5be93d8481dfd99fcb4c0a705fd36784.1546443509.git.gitgitgadget@gmail.com>
URL: https://gitlist.dev/e/f2b1706e5be93d8481dfd99fcb4c0a705fd36784.1546443509.git.gitgitgadget%40gmail.com
In-Reply-To: <pull.101.v17.git.gitgitgadget@gmail.com>

```
From: Pranit Bauva <pranit.bauva@gmail.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.

Also use error() to report "no terms defined" and accordingly change the
test in t6030.

We need to use PARSE_OPT_KEEP_UNKNOWN here to allow for parameters that
look like options (e.g --term-good) but should not be parsed by
cmd_bisect__helper(). This change is safe because all other cmdmodes have
strict argc checks already.

Mentored-by: Lars Schneider <larsxschneider@gmail.com>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Mentored-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
Signed-off-by: Tanushree Tumane <tanushreetumane@gmail.com>
---
 builtin/bisect--helper.c    | 62 +++++++++++++++++++++++++++++++++++--
 git-bisect.sh               | 35 ++-------------------
 t/t6030-bisect-porcelain.sh |  2 +-
 3 files changed, 63 insertions(+), 36 deletions(-)

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 38ae825b9a..d61edd3c91 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-write [--no-log] <state> <revision> <good_term> <bad_term>"),
 	N_("git bisect--helper --bisect-check-and-set-terms <command> <good_term> <bad_term>"),
 	N_("git bisect--helper --bisect-next-check <good_term> <bad_term> [<term>]"),
+	N_("git bisect--helper --bisect-terms [--term-good | --term-old | --term-bad | --term-new]"),
 	NULL
 };
 
@@ -339,6 +340,55 @@ static int bisect_next_check(const struct bisect_terms *terms,
 	return retval;
 }
 
+static int get_terms(struct bisect_terms *terms)
+{
+	struct strbuf str = STRBUF_INIT;
+	FILE *fp = NULL;
+	int res = 0;
+
+	fp = fopen(git_path_bisect_terms(), "r");
+	if (!fp) {
+		res = -1;
+		goto finish;
+	}
+
+	free_terms(terms);
+	strbuf_getline_lf(&str, fp);
+	terms->term_bad = strbuf_detach(&str, NULL);
+	strbuf_getline_lf(&str, fp);
+	terms->term_good = strbuf_detach(&str, NULL);
+
+finish:
+	if (fp)
+		fclose(fp);
+	strbuf_release(&str);
+	return res;
+}
+
+static int bisect_terms(struct bisect_terms *terms, const char *option)
+{
+	if (get_terms(terms))
+		return error(_("no terms defined"));
+
+	if (option == NULL) {
+		printf(_("Your current terms are %s for the old state\n"
+			 "and %s for the new state.\n"),
+		       terms->term_good, terms->term_bad);
+		return 0;
+	}
+	if (one_of(option, "--term-good", "--term-old", NULL))
+		printf("%s\n", terms->term_good);
+	else if (one_of(option, "--term-bad", "--term-new", NULL))
+		printf("%s\n", terms->term_bad);
+	else
+		return error(_("invalid argument %s for 'git bisect terms'.\n"
+			       "Supported options are: "
+			       "--term-good|--term-old and "
+			       "--term-bad|--term-new."), option);
+
+	return 0;
+}
+
 int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 {
 	enum {
@@ -349,7 +399,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 		BISECT_RESET,
 		BISECT_WRITE,
 		CHECK_AND_SET_TERMS,
-		BISECT_NEXT_CHECK
+		BISECT_NEXT_CHECK,
+		BISECT_TERMS
 	} cmdmode = 0;
 	int no_checkout = 0, res = 0, nolog = 0;
 	struct option options[] = {
@@ -369,6 +420,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_BOOL(0, "no-log", &nolog,
@@ -378,7 +431,7 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 	struct bisect_terms terms = { .term_good = NULL, .term_bad = NULL };
 
 	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);
@@ -419,6 +472,11 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 		set_terms(&terms, argv[1], argv[0]);
 		res = bisect_next_check(&terms, argc == 3 ? argv[2] : NULL);
 		break;
+	case BISECT_TERMS:
+		if (argc > 1)
+			return error(_("--bisect-terms requires 0 or 1 argument"));
+		res = bisect_terms(&terms, argc == 1 ? argv[0] : NULL);
+		break;
 	default:
 		return error("BUG: unknown subcommand '%d'", cmdmode);
 	}
diff --git a/git-bisect.sh b/git-bisect.sh
index 5ef3e25621..bdb614e3c2 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
@@ -439,37 +439,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 ;;
@@ -500,7 +469,7 @@ case "$#" in
 	run)
 		bisect_run "$@" ;;
 	terms)
-		bisect_terms "$@" ;;
+		git bisect--helper --bisect-terms "$@" || exit;;
 	*)
 		usage ;;
 	esac
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index f84ff941c3..55835ee4a4 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -802,7 +802,7 @@ test_expect_success 'bisect terms needs 0 or 1 argument' '
 	test_must_fail git bisect terms only-one &&
 	test_must_fail git bisect terms 1 2 &&
 	test_must_fail git bisect terms 2>actual &&
-	echo "no terms defined" >expected &&
+	echo "error: no terms defined" >expected &&
 	test_i18ncmp expected actual
 '
 
-- 
gitgitgadget


```

## Ramsay Jones, 2019-01-03 01:19

Subject: Re: [PATCH v17 0/7] git bisect: convert from shell to C
Message-ID: <d64492dc-4643-823b-c804-3c152c4d9090@ramsayjones.plus.com>
URL: https://gitlist.dev/e/d64492dc-4643-823b-c804-3c152c4d9090%40ramsayjones.plus.com
In-Reply-To: <pull.101.v17.git.gitgitgadget@gmail.com>

```


On 02/01/2019 15:38, Tanushree Tumane via GitGitGadget wrote:
[snip]
> base-commit: 7f4e64169352e03476b0ea64e7e2973669e491a2
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-101%2Ftanushree27%2Fgit-bisect_part2_fixup-v17
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-101/tanushree27/git-bisect_part2_fixup-v17
> Pull-Request: https://github.com/gitgitgadget/git/pull/101

I didn't look at the patches, only the range-diff below, and the
only thing I noticed was ...

> 
> Range-diff vs v16:
> 
>  1:  f1e89ba517 ! 1:  338ebdc97a bisect--helper: `bisect_reset` shell function in C
>      @@ -16,8 +16,9 @@
>       
>           Mentored-by: Lars Schneider <larsxschneider@gmail.com>
>           Mentored-by: Christian Couder <chriscool@tuxfamily.org>
>      +    Mentored by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
>           Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
>      -    Signed-off-by: Junio C Hamano <gitster@pobox.com>
>      +    Signed-off-by: Tanushree Tumane <tanushreetumane@gmail.com>
>       
>        diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
>        --- a/builtin/bisect--helper.c
>      @@ -53,8 +54,10 @@
>       +	struct strbuf branch = STRBUF_INIT;
>       +
>       +	if (!commit) {
>      -+		if (strbuf_read_file(&branch, git_path_bisect_start(), 0) < 1)
>      -+			return !printf(_("We are not bisecting.\n"));
>      ++		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;
>      @@ -69,11 +72,11 @@
>       +
>       +		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;
>      ++			return error(_("could not check out original"
>      ++				       " HEAD '%s'. Try 'git bisect"
>      ++				       "reset <commit>'."), branch.buf);

... this 'branch.buf' will refer to the empty 'slopbuf', since
the call to 'strbuf_release(&branch)' now precedes this call
to error().

ATB,
Ramsay Jones

```

## TANUSHREE TUMANE, 2019-01-07 09:15

Subject: Re: [PATCH v17 0/7] git bisect: convert from shell to C
Message-ID: <CAJ6Xce1A2RtBtd0FLouAUQRqaOVtTM4z28eYf45HMO6qK-w+fQ@mail.gmail.com>
URL: https://gitlist.dev/e/CAJ6Xce1A2RtBtd0FLouAUQRqaOVtTM4z28eYf45HMO6qK-w%2BfQ%40mail.gmail.com
In-Reply-To: <d64492dc-4643-823b-c804-3c152c4d9090@ramsayjones.plus.com>

```
Yes, you are right. I will revert this back.

On Fri, Jan 4, 2019 at 8:08 PM Ramsay Jones <ramsay@ramsayjones.plus.com> wrote:
>
>
>
> On 02/01/2019 15:38, Tanushree Tumane via GitGitGadget wrote:
> [snip]
> > base-commit: 7f4e64169352e03476b0ea64e7e2973669e491a2
> > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-101%2Ftanushree27%2Fgit-bisect_part2_fixup-v17
> > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-101/tanushree27/git-bisect_part2_fixup-v17
> > Pull-Request: https://github.com/gitgitgadget/git/pull/101
>
> I didn't look at the patches, only the range-diff below, and the
> only thing I noticed was ...
>
> >
> > Range-diff vs v16:
> >
> >  1:  f1e89ba517 ! 1:  338ebdc97a bisect--helper: `bisect_reset` shell function in C
> >      @@ -16,8 +16,9 @@
> >
> >           Mentored-by: Lars Schneider <larsxschneider@gmail.com>
> >           Mentored-by: Christian Couder <chriscool@tuxfamily.org>
> >      +    Mentored by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
> >           Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
> >      -    Signed-off-by: Junio C Hamano <gitster@pobox.com>
> >      +    Signed-off-by: Tanushree Tumane <tanushreetumane@gmail.com>
> >
> >        diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
> >        --- a/builtin/bisect--helper.c
> >      @@ -53,8 +54,10 @@
> >       +       struct strbuf branch = STRBUF_INIT;
> >       +
> >       +       if (!commit) {
> >      -+               if (strbuf_read_file(&branch, git_path_bisect_start(), 0) < 1)
> >      -+                       return !printf(_("We are not bisecting.\n"));
> >      ++               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;
> >      @@ -69,11 +72,11 @@
> >       +
> >       +               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;
> >      ++                       return error(_("could not check out original"
> >      ++                                      " HEAD '%s'. Try 'git bisect"
> >      ++                                      "reset <commit>'."), branch.buf);
>
> ... this 'branch.buf' will refer to the empty 'slopbuf', since
> the call to 'strbuf_release(&branch)' now precedes this call
> to error().
>
> ATB,
> Ramsay Jones
>

```