git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v15 13/27] bisect--helper: `bisect_start` shell function partially in C

From
Pranit Bauva <pranit.bauva@gmail.com>
Date
Oct 14, 2016, 14:14 UTC
Message-ID
<01020157c38b1ad3-ea75ed97-2514-427e-8e57-9f10efd4e6e9-000000@eu-west-1.amazonses.com>
In-Reply-To
<01020157c38b19e0-81123fa5-5d9d-4f64-8f1b-ff336e83ebe4-000000@eu-west-1.amazonses.com>

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

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

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

Mentored-by: Lars Schneider <larsxschneider@gmail.com>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
---
 builtin/bisect--helper.c | 217 ++++++++++++++++++++++++++++++++++++++++++++++-
 git-bisect.sh            | 133 +----------------------------
 2 files changed, 217 insertions(+), 133 deletions(-)
diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 6a5878c..1d3e17f 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_log, "BISECT_LOG")
 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_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> <TERM_GOOD> <TERM_BAD>"),
 	N_("git bisect--helper --bisect-next-check [<term>] <TERM_GOOD> <TERM_BAD"),
 	N_("git bisect--helper --bisect-terms [--term-good | --term-old | --term-bad | --term-new]"),
+	N_("git bisect--helper --bisect start [--term-{old,good}=<term> --term-{new,bad}=<term>]"
+					      "[--no-checkout] [<bad> [<good>...]] [--] [<paths>...]"),
 	NULL
 };
 
@@ -403,6 +408,205 @@ static int bisect_terms(struct bisect_terms *terms, const char **argv, int argc)
 	return 0;
 }
 
+static int bisect_start(struct bisect_terms *terms, int no_checkout,
+			const char **argv, int argc)
+{
+	int i, has_double_dash = 0, must_write_terms = 0, bad_seen = 0;
+	int flags, pathspec_pos, 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 strbuf orig_args = STRBUF_INIT;
+	const char *head;
+	unsigned char sha1[20];
+	FILE *fp = NULL;
+	struct object_id oid;
+
+	if (is_bare_repository())
+		no_checkout = 1;
+
+	for (i = 0; i < argc; i++) {
+		if (!strcmp(argv[i], "--")) {
+			has_double_dash = 1;
+			break;
+		}
+	}
+
+	for (i = 0; i < argc; i++) {
+		const char *commit_id = xstrfmt("%s^{commit}", argv[i]);
+		const char *arg = argv[i];
+		if (!strcmp(argv[i], "--")) {
+			has_double_dash = 1;
+			break;
+		} else if (!strcmp(arg, "--no-checkout")) {
+			no_checkout = 1;
+		} else if (!strcmp(arg, "--term-good") ||
+			 !strcmp(arg, "--term-old")) {
+			must_write_terms = 1;
+			terms->term_good = xstrdup(argv[++i]);
+		} else if (skip_prefix(arg, "--term-good=", &arg)) {
+			must_write_terms = 1;
+			terms->term_good = xstrdup(arg);
+		} else if (skip_prefix(arg, "--term-old=", &arg)) {
+			must_write_terms = 1;
+			terms->term_good = xstrdup(arg);
+		} else if (!strcmp(arg, "--term-bad") ||
+			 !strcmp(arg, "--term-new")) {
+			must_write_terms = 1;
+			terms->term_bad = xstrdup(argv[++i]);
+		} else if (skip_prefix(arg, "--term-bad=", &arg)) {
+			must_write_terms = 1;
+			terms->term_bad = xstrdup(arg);
+		} else if (skip_prefix(arg, "--term-new=", &arg)) {
+			must_write_terms = 1;
+			terms->term_good = xstrdup(arg);
+		} else if (starts_with(arg, "--") &&
+			 !one_of(arg, "--term-good", "--term-bad", NULL)) {
+			die(_("unrecognised option: '%s'"), arg);
+		} else if (get_oid(commit_id, &oid) && has_double_dash) {
+			die(_("'%s' does not appear to be a valid revision"), argv[i]);
+		} else {
+			string_list_append(&revs, oid_to_hex(&oid));
+		}
+	}
+	pathspec_pos = i;
+
+	/*
+	 * The user ran "git bisect start <sha1> <sha1>", hence did not
+	 * explicitly specify the terms, but we are already starting to
+	 * set references named with the default terms, and won't be able
+	 * to change afterwards.
+	 */
+	must_write_terms |= !!revs.nr;
+	for (i = 0; i < revs.nr; i++) {
+		if (bad_seen) {
+			string_list_append(&states, terms->term_good);
+		} else {
+			bad_seen = 1;
+			string_list_append(&states, terms->term_bad);
+		}
+	}
+
+	/*
+	 * Verify HEAD
+	 */
+	head = resolve_ref_unsafe("HEAD", 0, sha1, &flags);
+	if (!head)
+		if (get_sha1("HEAD", sha1))
+			die(_("Bad HEAD - I need a HEAD"));
+
+	if (!is_empty_or_missing_file(git_path_bisect_start())) {
+		/* Reset to the rev from where we started */
+		strbuf_read_file(&start_head, git_path_bisect_start(), 0);
+		strbuf_trim(&start_head);
+		if (!no_checkout) {
+			struct argv_array argv = ARGV_ARRAY_INIT;
+			argv_array_pushl(&argv, "checkout", start_head.buf,
+					 "--", NULL);
+			if (run_command_v_opt(argv.argv, RUN_GIT_CMD)) {
+				error(_("checking out '%s' failed. Try 'git "
+					"bisect start <valid-branch>'."),
+				      start_head.buf);
+				retval = -1;
+				goto finish;
+			}
+		}
+	} else {
+		if (!get_sha1(head, sha1) &&
+		    !starts_with(head, "refs/heads/")) {
+			strbuf_reset(&start_head);
+			strbuf_addstr(&start_head, sha1_to_hex(sha1));
+		} else if (!get_sha1(head, sha1) &&
+			   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()))
+				die(_("won't bisect on cg-seek'ed tree"));
+			strbuf_addstr(&start_head, head);
+		} else {
+			die(_("Bad HEAD - strange symbolic ref"));
+		}
+	}
+
+	/*
+	 * Get rid of any old bisect state.
+	 */
+	if (bisect_clean_state()) {
+		return -1;
+	}
+	/*
+	 * In case of mistaken revs or checkout error, or signals received,
+	 * "bisect_auto_next" below may exit or misbehave.
+	 * We have to trap this to be able to clean up using
+	 * "bisect_clean_state".
+	 */
+
+	/*
+	 * Write new start state
+	 */
+	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.hash, NULL, 0,
+			       UPDATE_REFS_MSG_ON_ERR)) {
+			retval = -1;
+			goto finish;
+		}
+	}
+
+	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)) {
+			retval = -1;
+			goto finish;
+		}
+	}
+
+	if (must_write_terms)
+		if (write_terms(terms->term_bad, terms->term_good)) {
+			retval = -1;
+			goto finish;
+		}
+
+	fp = fopen(git_path_bisect_log(), "a");
+	if (!fp)
+		return -1;
+
+	if (fprintf(fp, "git bisect start") < 1) {
+		retval = -1;
+		goto finish;
+	}
+
+	sq_quote_argv(&orig_args, argv, 0);
+	if (fprintf(fp, "%s", orig_args.buf) < 0) {
+		retval = -1;
+		goto finish;
+	}
+	if (fprintf(fp, "\n") < 1) {
+		retval = -1;
+		goto finish;
+	}
+	goto finish;
+finish:
+	if (fp)
+		fclose(fp);
+	string_list_clear(&revs, 0);
+	string_list_clear(&states, 0);
+	strbuf_release(&start_head);
+	strbuf_release(&bisect_names);
+	strbuf_release(&orig_args);
+	return retval;
+}
+
 int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 {
 	enum {
@@ -414,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;
 	struct option options[] = {
@@ -436,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_END()
@@ -443,7 +650,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);
@@ -497,6 +705,11 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 			die(_("--bisect-terms requires 0 or 1 argument"));
 		res = bisect_terms(&terms, argv, argc);
 		break;
+	case BISECT_START:
+		terms.term_good = "good";
+		terms.term_bad = "bad";
+		res = bisect_start(&terms, no_checkout, argv, argc);
+		break;
 	default:
 		die("BUG: unknown subcommand '%d'", cmdmode);
 	}
diff --git a/git-bisect.sh b/git-bisect.sh
index d6c8b5a..f0896b3 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.
@@ -197,24 +82,10 @@ bisect_start() {
 	#
 	trap 'git bisect--helper --bisect-clean-state' 0
 	trap 'exit 255' 1 2 3 15
-
-	#
-	# Write new start state.
-	#
-	echo "$start_head" >"$GIT_DIR/BISECT_START" && {
-		test "z$mode" != "z--no-checkout" ||
-		git update-ref --no-deref BISECT_HEAD "$start_head"
-	} &&
-	git rev-parse --sq-quote "$@" >"$GIT_DIR/BISECT_NAMES" &&
-	eval "$eval true" &&
-	if test $must_write_terms -eq 1
-	then
-		git bisect--helper --write-terms "$TERM_BAD" "$TERM_GOOD"
-	fi &&
-	echo "git bisect start$orig_args" >>"$GIT_DIR/BISECT_LOG" || exit
 	#
 	# Check if we can proceed to the next bisect state.
 	#
+	get_terms
 	bisect_auto_next
 
 	trap '-' 0

--
https://github.com/git/git/pull/287
Previous: Pranit BauvaNext: Stephan Beyer
Message 30 of 82 in “bisect--helper: use OPT_CMDMODE instead of OPT_BOOL”
  1. 01/27 bisect--helper: use OPT_CMDMODE instead of OPT_BOOLPranit Bauva, Oct 14, 2016
  2. 06/27 wrapper: move is_empty_file() and rename it as is_empty_or_missing_file()Pranit Bauva, Oct 14, 2016
  3. 05/27 t6030: explicitly test for bisection cleanupPranit Bauva, Oct 14, 2016
  4. 22/27 bisect--helper: `bisect_log` shell function in CPranit Bauva, Oct 14, 2016
  5. Stephan BeyerNov 17, 2016
  6. Pranit BauvaDec 6, 2016
  7. 10/27 bisect--helper: `check_and_set_terms` shell function in CPranit Bauva, Oct 14, 2016
  8. Stephan BeyerNov 17, 2016
  9. Pranit BauvaDec 6, 2016
  10. 23/27 bisect--helper: `bisect_replay` shell function in CPranit Bauva, Oct 14, 2016
  11. Stephan BeyerNov 22, 2016
  12. Pranit BauvaDec 6, 2016
  13. Stephan BeyerDec 6, 2016
  14. Stephan BeyerDec 6, 2016
  15. Christian CouderDec 7, 2016
  16. 04/27 bisect--helper: `bisect_clean_state` shell function in CPranit Bauva, Oct 14, 2016
  17. Stephan BeyerNov 15, 2016
  18. Junio C HamanoNov 15, 2016
  19. Stephan BeyerNov 15, 2016
  20. Pranit BauvaNov 16, 2016
  21. 09/27 bisect--helper: `bisect_write` shell function in CPranit Bauva, Oct 14, 2016
  22. Stephan BeyerNov 17, 2016
  23. Pranit BauvaDec 6, 2016
  24. 20/27 bisect--helper: retire `--check-expected-revs` subcommandPranit Bauva, Oct 14, 2016
  25. 12/27 bisect--helper: `get_terms` & `bisect_terms` shell function in CPranit Bauva, Oct 14, 2016
  26. Stephan BeyerNov 17, 2016
  27. Pranit BauvaDec 6, 2016
  28. Stephan BeyerDec 6, 2016
  29. Pranit BauvaDec 7, 2016
  30. 13/27 bisect--helper: `bisect_start` shell function partially in CPranit Bauva, Oct 14, 2016
  31. Stephan BeyerNov 15, 2016
  32. Pranit BauvaNov 16, 2016
  33. Stephan BeyerNov 20, 2016
  34. Stephan BeyerNov 20, 2016
  35. 18/27 bisect--helper: `bisect_autostart` shell function in CPranit Bauva, Oct 14, 2016
  36. Stephan BeyerNov 20, 2016
  37. Pranit BauvaDec 6, 2016
  38. 21/27 bisect--helper: retire `--write-terms` subcommandPranit Bauva, Oct 14, 2016
  39. 19/27 bisect--helper: `bisect_state` & `bisect_head` shell function in CPranit Bauva, Oct 14, 2016
  40. Stephan BeyerNov 22, 2016
  41. Pranit BauvaDec 6, 2016
  42. Stephan BeyerDec 6, 2016
  43. Pranit BauvaDec 8, 2016
  44. 14/27 t6030: no cleanup with bad merge basePranit Bauva, Oct 14, 2016
  45. Junio C HamanoOct 14, 2016
  46. Pranit BauvaOct 15, 2016
  47. 15/27 bisect--helper: `bisect_next` and `bisect_auto_next` shell function in CPranit Bauva, Oct 14, 2016
  48. Stephan BeyerNov 20, 2016
  49. Pranit BauvaDec 31, 2016
  50. Stephan BeyerNov 21, 2016
  51. Pranit BauvaDec 31, 2016
  52. Stephan BeyerJan 1, 2017
  53. Pranit BauvaJan 1, 2017
  54. 07/27 bisect--helper: `bisect_reset` shell function in CPranit Bauva, Oct 14, 2016
  55. Stephan BeyerNov 16, 2016
  56. Pranit BauvaNov 17, 2016
  57. 17/27 bisect--helper: retire `--next-all` subcommandPranit Bauva, Oct 14, 2016
  58. 24/27 bisect--helper: retire `--bisect-write` subcommandPranit Bauva, Oct 14, 2016
  59. 03/27 bisect--helper: `write_terms` shell function in CPranit Bauva, Oct 14, 2016
  60. 02/27 bisect: rewrite `check_term_format` shell function in CPranit Bauva, Oct 14, 2016
  61. Stephan BeyerNov 14, 2016
  62. Pranit BauvaNov 15, 2016
  63. 26/27 bisect--helper: retire `--bisect-auto-next` subcommandPranit Bauva, Oct 14, 2016
  64. 16/27 bisect--helper: retire `--bisect-clean-state` subcommandPranit Bauva, Oct 14, 2016
  65. 08/27 bisect--helper: `is_expected_rev` & `check_expected_revs` shell function in CPranit Bauva, Oct 14, 2016
  66. Stephan BeyerNov 16, 2016
  67. Pranit BauvaDec 6, 2016
  68. Pranit BauvaDec 16, 2016
  69. Stephan BeyerDec 17, 2016
  70. Pranit BauvaDec 16, 2016
  71. Stephan BeyerDec 17, 2016
  72. 25/27 bisect--helper: retire `--bisect-autostart` subcommandPranit Bauva, Oct 14, 2016
  73. Pranit BauvaOct 14, 2016
  74. 11/27 bisect--helper: `bisect_next_check` & bisect_voc shell function in CPranit Bauva, Oct 14, 2016
  75. Stephan BeyerNov 17, 2016
  76. Pranit BauvaDec 6, 2016
  77. 27/27 bisect--helper: remove the dequote in bisect_start()Pranit Bauva, Oct 14, 2016
  78. Junio C HamanoOct 27, 2016
  79. Christian CouderOct 27, 2016
  80. Matthieu MoyOct 28, 2016
  81. Stephan BeyerNov 15, 2016
  82. Junio C HamanoNov 16, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.