Merge branch 'dd/git-bisect-builtin'

`git bisect` becomes a builtin.

* dd/git-bisect-builtin:
  bisect; remove unused "git-bisect.sh" and ".gitignore" entry
  Turn `git bisect` into a full built-in
  bisect--helper: log: allow arbitrary number of arguments
  bisect--helper: handle states directly
  bisect--helper: emit usage for "git bisect"
  bisect test: test exit codes on bad usage
  bisect--helper: identify as bisect when report error
  bisect-run: verify_good: account for non-negative exit status
  bisect run: keep some of the post-v2.30.0 output
  bisect: fix output regressions in v2.30.0
  bisect: refactor bisect_run() to match CodingGuidelines
  bisect tests: test for v2.30.0 "bisect run" regressions

.gitignore

@@ -21,7 +21,6 @@
 /git-archimport
 /git-archive
 /git-bisect
-/git-bisect--helper
 /git-blame
 /git-branch
 /git-bugreport

Makefile

@@ -691,7 +691,6 @@ THIRD_PARTY_SOURCES =
 # interactive shell sessions without exporting it.
 unexport CDPATH
 
-SCRIPT_SH += git-bisect.sh
 SCRIPT_SH += git-difftool--helper.sh
 SCRIPT_SH += git-filter-branch.sh
 SCRIPT_SH += git-merge-octopus.sh
@@ -1202,7 +1201,7 @@ BUILTIN_OBJS += builtin/am.o
 BUILTIN_OBJS += builtin/annotate.o
 BUILTIN_OBJS += builtin/apply.o
 BUILTIN_OBJS += builtin/archive.o
-BUILTIN_OBJS += builtin/bisect--helper.o
+BUILTIN_OBJS += builtin/bisect.o
 BUILTIN_OBJS += builtin/blame.o
 BUILTIN_OBJS += builtin/branch.o
 BUILTIN_OBJS += builtin/bugreport.o

builtin.h

@@ -116,7 +116,7 @@ int cmd_am(int argc, const char **argv, const char *prefix);
 int cmd_annotate(int argc, const char **argv, const char *prefix);
 int cmd_apply(int argc, const char **argv, const char *prefix);
 int cmd_archive(int argc, const char **argv, const char *prefix);
-int cmd_bisect__helper(int argc, const char **argv, const char *prefix);
+int cmd_bisect(int argc, const char **argv, const char *prefix);
 int cmd_blame(int argc, const char **argv, const char *prefix);
 int cmd_branch(int argc, const char **argv, const char *prefix);
 int cmd_bugreport(int argc, const char **argv, const char *prefix);

builtin/bisect.c

@@ -20,18 +20,40 @@ static GIT_PATH_FUNC(git_path_bisect_names, "BISECT_NAMES")
 static GIT_PATH_FUNC(git_path_bisect_first_parent, "BISECT_FIRST_PARENT")
 static GIT_PATH_FUNC(git_path_bisect_run, "BISECT_RUN")
 
-static const char * const git_bisect_helper_usage[] = {
+#define BUILTIN_GIT_BISECT_START_USAGE \
-	N_("git bisect--helper --bisect-reset [<commit>]"),
+	N_("git bisect start [--term-{new,bad}=<term> --term-{old,good}=<term>]" \
-	"git bisect--helper --bisect-terms [--term-good | --term-old | --term-bad | --term-new]",
+	   "    [--no-checkout] [--first-parent] [<bad> [<good>...]] [--]" \
-	N_("git bisect--helper --bisect-start [--term-{new,bad}=<term> --term-{old,good}=<term>]"
+	   "    [<pathspec>...]")
-					    " [--no-checkout] [--first-parent] [<bad> [<good>...]] [--] [<paths>...]"),
+#define BUILTIN_GIT_BISECT_STATE_USAGE \
-	"git bisect--helper --bisect-next",
+	N_("git bisect (good|bad) [<rev>...]")
-	N_("git bisect--helper --bisect-state (bad|new) [<rev>]"),
+#define BUILTIN_GIT_BISECT_TERMS_USAGE \
-	N_("git bisect--helper --bisect-state (good|old) [<rev>...]"),
+	"git bisect terms [--term-good | --term-bad]"
-	N_("git bisect--helper --bisect-replay <filename>"),
+#define BUILTIN_GIT_BISECT_SKIP_USAGE \
-	N_("git bisect--helper --bisect-skip [(<rev>|<range>)...]"),
+	N_("git bisect skip [(<rev>|<range>)...]")
-	"git bisect--helper --bisect-visualize",
+#define BUILTIN_GIT_BISECT_NEXT_USAGE \
-	N_("git bisect--helper --bisect-run <cmd>..."),
+	"git bisect next"
+#define BUILTIN_GIT_BISECT_RESET_USAGE \
+	N_("git bisect reset [<commit>]")
+#define BUILTIN_GIT_BISECT_VISUALIZE_USAGE \
+	"git bisect visualize"
+#define BUILTIN_GIT_BISECT_REPLAY_USAGE \
+	N_("git bisect replay <logfile>")
+#define BUILTIN_GIT_BISECT_LOG_USAGE \
+	"git bisect log"
+#define BUILTIN_GIT_BISECT_RUN_USAGE \
+	N_("git bisect run <cmd>...")
+
+static const char * const git_bisect_usage[] = {
+	BUILTIN_GIT_BISECT_START_USAGE,
+	BUILTIN_GIT_BISECT_STATE_USAGE,
+	BUILTIN_GIT_BISECT_TERMS_USAGE,
+	BUILTIN_GIT_BISECT_SKIP_USAGE,
+	BUILTIN_GIT_BISECT_NEXT_USAGE,
+	BUILTIN_GIT_BISECT_RESET_USAGE,
+	BUILTIN_GIT_BISECT_VISUALIZE_USAGE,
+	BUILTIN_GIT_BISECT_REPLAY_USAGE,
+	BUILTIN_GIT_BISECT_LOG_USAGE,
+	BUILTIN_GIT_BISECT_RUN_USAGE,
 	NULL
 };
 
@@ -1191,13 +1213,13 @@ static int bisect_run(struct bisect_terms *terms, const char **argv, int argc)
 	if (bisect_next_check(terms, NULL))
 		return BISECT_FAILED;
 
-	if (argc)
+	if (!argc) {
-		sq_quote_argv(&command, argv);
-	else {
 		error(_("bisect run failed: no command provided."));
 		return BISECT_FAILED;
 	}
 
+	sq_quote_argv(&command, argv);
+	strbuf_ltrim(&command);
 	while (1) {
 		res = do_bisect_run(command.buf);
 
@@ -1211,8 +1233,8 @@ static int bisect_run(struct bisect_terms *terms, const char **argv, int argc)
 		if (is_first_run && (res == 126 || res == 127)) {
 			int rc = verify_good(terms, command.buf);
 			is_first_run = 0;
-			if (rc < 0) {
+			if (rc < 0 || 128 <= rc) {
-				error(_("unable to verify '%s' on good"
+				error(_("unable to verify %s on good"
 					" revision"), command.buf);
 				res = BISECT_FAILED;
 				break;
@@ -1227,7 +1249,7 @@ static int bisect_run(struct bisect_terms *terms, const char **argv, int argc)
 
 		if (res < 0 || 128 <= res) {
 			error(_("bisect run failed: exit code %d from"
-				" '%s' is < 0 or >= 128"), res, command.buf);
+				" %s is < 0 or >= 128"), res, command.buf);
 			break;
 		}
 
@@ -1261,14 +1283,14 @@ static int bisect_run(struct bisect_terms *terms, const char **argv, int argc)
 		if (res == BISECT_ONLY_SKIPPED_LEFT)
 			error(_("bisect run cannot continue any more"));
 		else if (res == BISECT_INTERNAL_SUCCESS_MERGE_BASE) {
-			printf(_("bisect run success"));
+			puts(_("bisect run success"));
 			res = BISECT_OK;
 		} else if (res == BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND) {
-			printf(_("bisect found first bad commit"));
+			puts(_("bisect found first bad commit"));
 			res = BISECT_OK;
 		} else if (res) {
-			error(_("bisect run failed: 'git bisect--helper --bisect-state"
+			error(_("bisect run failed: 'bisect-state %s'"
-			" %s' exited with error code %d"), new_state, res);
+				" exited with error code %d"), new_state, res);
 		} else {
 			continue;
 		}
@@ -1282,7 +1304,8 @@ static int bisect_run(struct bisect_terms *terms, const char **argv, int argc)
 static int cmd_bisect__reset(int argc, const char **argv, const char *prefix UNUSED)
 {
 	if (argc > 1)
-		return error(_("--bisect-reset requires either no argument or a commit"));
+		return error(_("'%s' requires either no argument or a commit"),
+			     "git bisect reset");
 	return bisect_reset(argc ? argv[0] : NULL);
 }
 
@@ -1292,7 +1315,8 @@ static int cmd_bisect__terms(int argc, const char **argv, const char *prefix UNU
 	struct bisect_terms terms = { 0 };
 
 	if (argc > 1)
-		return error(_("--bisect-terms requires 0 or 1 argument"));
+		return error(_("'%s' requires 0 or 1 argument"),
+			     "git bisect terms");
 	res = bisect_terms(&terms, argc == 1 ? argv[0] : NULL);
 	free_terms(&terms);
 	return res;
@@ -1315,29 +1339,16 @@ static int cmd_bisect__next(int argc, const char **argv UNUSED, const char *pref
 	struct bisect_terms terms = { 0 };
 
 	if (argc)
-		return error(_("--bisect-next requires 0 arguments"));
+		return error(_("'%s' requires 0 arguments"),
+			     "git bisect next");
 	get_terms(&terms);
 	res = bisect_next(&terms, prefix);
 	free_terms(&terms);
 	return res;
 }
 
-static int cmd_bisect__state(int argc, const char **argv, const char *prefix UNUSED)
+static int cmd_bisect__log(int argc UNUSED, const char **argv UNUSED, const char *prefix UNUSED)
 {
-	int res;
-	struct bisect_terms terms = { 0 };
-
-	set_terms(&terms, "bad", "good");
-	get_terms(&terms);
-	res = bisect_state(&terms, argv, argc);
-	free_terms(&terms);
-	return res;
-}
-
-static int cmd_bisect__log(int argc, const char **argv UNUSED, const char *prefix UNUSED)
-{
-	if (argc)
-		return error(_("--bisect-log requires 0 arguments"));
 	return bisect_log();
 }
 
@@ -1383,14 +1394,14 @@ static int cmd_bisect__run(int argc, const char **argv, const char *prefix UNUSE
 	struct bisect_terms terms = { 0 };
 
 	if (!argc)
-		return error(_("bisect run failed: no command provided."));
+		return error(_("'%s' failed: no command provided."), "git bisect run");
 	get_terms(&terms);
 	res = bisect_run(&terms, argv, argc);
 	free_terms(&terms);
 	return res;
 }
 
-int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
+int cmd_bisect(int argc, const char **argv, const char *prefix)
 {
 	int res = 0;
 	parse_opt_subcommand_fn *fn = NULL;
@@ -1399,7 +1410,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 		OPT_SUBCOMMAND("terms", &fn, cmd_bisect__terms),
 		OPT_SUBCOMMAND("start", &fn, cmd_bisect__start),
 		OPT_SUBCOMMAND("next", &fn, cmd_bisect__next),
-		OPT_SUBCOMMAND("state", &fn, cmd_bisect__state),
 		OPT_SUBCOMMAND("log", &fn, cmd_bisect__log),
 		OPT_SUBCOMMAND("replay", &fn, cmd_bisect__replay),
 		OPT_SUBCOMMAND("skip", &fn, cmd_bisect__skip),
@@ -1408,15 +1418,27 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 		OPT_SUBCOMMAND("run", &fn, cmd_bisect__run),
 		OPT_END()
 	};
-	argc = parse_options(argc, argv, prefix, options,
+	argc = parse_options(argc, argv, prefix, options, git_bisect_usage,
-			     git_bisect_helper_usage, 0);
+			     PARSE_OPT_SUBCOMMAND_OPTIONAL);
 
-	if (!fn)
+	if (!fn) {
-		usage_with_options(git_bisect_helper_usage, options);
+		struct bisect_terms terms = { 0 };
-	argc--;
-	argv++;
 
-	res = fn(argc, argv, prefix);
+		if (!argc)
+			usage_msg_opt(_("need a command"), git_bisect_usage, options);
+
+		set_terms(&terms, "bad", "good");
+		get_terms(&terms);
+		if (check_and_set_terms(&terms, argv[0]))
+			usage_msg_optf(_("unknown command: '%s'"), git_bisect_usage,
+				       options, argv[0]);
+		res = bisect_state(&terms, argv, argc);
+		free_terms(&terms);
+	} else {
+		argc--;
+		argv++;
+		res = fn(argc, argv, prefix);
+	}
 
 	/*
 	 * Handle early success

git-bisect.sh

@@ -1,67 +0,0 @@
-#!/bin/sh
-
-USAGE='[help|start|bad|good|new|old|terms|skip|next|reset|visualize|view|replay|log|run]'
-LONG_USAGE='git bisect help
-	print this long help message.
-git bisect start [--term-{new,bad}=<term> --term-{old,good}=<term>]
-		 [--no-checkout] [--first-parent] [<bad> [<good>...]] [--] [<pathspec>...]
-	reset bisect state and start bisection.
-git bisect (bad|new) [<rev>]
-	mark <rev> a known-bad revision/
-		a revision after change in a given property.
-git bisect (good|old) [<rev>...]
-	mark <rev>... known-good revisions/
-		revisions before change in a given property.
-git bisect terms [--term-good | --term-bad]
-	show the terms used for old and new commits (default: bad, good)
-git bisect skip [(<rev>|<range>)...]
-	mark <rev>... untestable revisions.
-git bisect next
-	find next bisection to test and check it out.
-git bisect reset [<commit>]
-	finish bisection search and go back to commit.
-git bisect (visualize|view)
-	show bisect status in gitk.
-git bisect replay <logfile>
-	replay bisection log.
-git bisect log
-	show bisect log.
-git bisect run <cmd>...
-	use <cmd>... to automatically bisect.
-
-Please use "git help bisect" to get the full man page.'
-
-OPTIONS_SPEC=
-. git-sh-setup
-
-TERM_BAD=bad
-TERM_GOOD=good
-
-get_terms () {
-	if test -s "$GIT_DIR/BISECT_TERMS"
-	then
-		{
-		read TERM_BAD
-		read TERM_GOOD
-		} <"$GIT_DIR/BISECT_TERMS"
-	fi
-}
-
-case "$#" in
-0)
-	usage ;;
-*)
-	cmd="$1"
-	get_terms
-	shift
-	case "$cmd" in
-	help)
-		git bisect -h ;;
-	bad|good|new|old|"$TERM_BAD"|"$TERM_GOOD")
-		git bisect--helper state "$cmd" "$@" ;;
-	log)
-		git bisect--helper log || exit ;;
-	*)
-		git bisect--helper "$cmd" "$@" ;;
-	esac
-esac

git.c

@@ -492,7 +492,7 @@ static struct cmd_struct commands[] = {
 	{ "annotate", cmd_annotate, RUN_SETUP },
 	{ "apply", cmd_apply, RUN_SETUP_GENTLY },
 	{ "archive", cmd_archive, RUN_SETUP_GENTLY },
-	{ "bisect--helper", cmd_bisect__helper, RUN_SETUP },
+	{ "bisect", cmd_bisect, RUN_SETUP },
 	{ "blame", cmd_blame, RUN_SETUP },
 	{ "branch", cmd_branch, RUN_SETUP | DELAY_PAGER_CONFIG },
 	{ "bugreport", cmd_bugreport, RUN_SETUP_GENTLY },

t/t6030-bisect-porcelain.sh

@@ -34,6 +34,36 @@ HASH2=
 HASH3=
 HASH4=
 
+test_bisect_usage () {
+	local code="$1" &&
+	shift &&
+	cat >expect &&
+	test_expect_code $code "$@" >out 2>actual &&
+	test_must_be_empty out &&
+	test_cmp expect actual
+}
+
+test_expect_success 'bisect usage' "
+	test_bisect_usage 1 git bisect reset extra1 extra2 <<-\EOF &&
+	error: 'git bisect reset' requires either no argument or a commit
+	EOF
+	test_bisect_usage 1 git bisect terms extra1 extra2 <<-\EOF &&
+	error: 'git bisect terms' requires 0 or 1 argument
+	EOF
+	test_bisect_usage 1 git bisect next extra1 <<-\EOF &&
+	error: 'git bisect next' requires 0 arguments
+	EOF
+	test_bisect_usage 1 git bisect log extra1 <<-\EOF &&
+	error: We are not bisecting.
+	EOF
+	test_bisect_usage 1 git bisect replay <<-\EOF &&
+	error: no logfile given
+	EOF
+	test_bisect_usage 1 git bisect run <<-\EOF
+	error: 'git bisect run' failed: no command provided.
+	EOF
+"
+
 test_expect_success 'set up basic repo with 1 file (hello) and 4 commits' '
      add_line_into_file "1: Hello World" hello &&
      HASH1=$(git rev-parse --verify HEAD) &&
@@ -252,6 +282,124 @@ test_expect_success 'bisect skip: with commit both bad and skipped' '
 	grep $HASH4 my_bisect_log.txt
 '
 
+test_bisect_run_args () {
+	test_when_finished "rm -f run.sh actual" &&
+	>actual &&
+	cat >expect.args &&
+	cat <&6 >expect.out &&
+	cat <&7 >expect.err &&
+	write_script run.sh <<-\EOF &&
+	while test $# != 0
+	do
+		echo "<$1>" &&
+		shift
+	done >actual.args
+	EOF
+
+	test_when_finished "git bisect reset" &&
+	git bisect start &&
+	git bisect good $HASH1 &&
+	git bisect bad $HASH4 &&
+	git bisect run ./run.sh $@ >actual.out.raw 2>actual.err &&
+	# Prune just the log output
+	sed -n \
+		-e '/^Author:/d' \
+		-e '/^Date:/d' \
+		-e '/^$/d' \
+		-e '/^commit /d' \
+		-e '/^ /d' \
+		-e 'p' \
+		<actual.out.raw >actual.out &&
+	test_cmp expect.out actual.out &&
+	test_cmp expect.err actual.err &&
+	test_cmp expect.args actual.args
+}
+
+test_expect_success 'git bisect run: args, stdout and stderr with no arguments' "
+	test_bisect_run_args <<-'EOF_ARGS' 6<<-EOF_OUT 7<<-'EOF_ERR'
+	EOF_ARGS
+	running './run.sh'
+	$HASH4 is the first bad commit
+	bisect found first bad commit
+	EOF_OUT
+	EOF_ERR
+"
+
+test_expect_success 'git bisect run: args, stdout and stderr: "--" argument' "
+	test_bisect_run_args -- <<-'EOF_ARGS' 6<<-EOF_OUT 7<<-'EOF_ERR'
+	<-->
+	EOF_ARGS
+	running './run.sh' '--'
+	$HASH4 is the first bad commit
+	bisect found first bad commit
+	EOF_OUT
+	EOF_ERR
+"
+
+test_expect_success 'git bisect run: args, stdout and stderr: "--log foo --no-log bar" arguments' "
+	test_bisect_run_args --log foo --no-log bar <<-'EOF_ARGS' 6<<-EOF_OUT 7<<-'EOF_ERR'
+	<--log>
+	<foo>
+	<--no-log>
+	<bar>
+	EOF_ARGS
+	running './run.sh' '--log' 'foo' '--no-log' 'bar'
+	$HASH4 is the first bad commit
+	bisect found first bad commit
+	EOF_OUT
+	EOF_ERR
+"
+
+test_expect_success 'git bisect run: args, stdout and stderr: "--bisect-start" argument' "
+	test_bisect_run_args --bisect-start <<-'EOF_ARGS' 6<<-EOF_OUT 7<<-'EOF_ERR'
+	<--bisect-start>
+	EOF_ARGS
+	running './run.sh' '--bisect-start'
+	$HASH4 is the first bad commit
+	bisect found first bad commit
+	EOF_OUT
+	EOF_ERR
+"
+
+test_expect_success 'git bisect run: negative exit code' "
+	write_script fail.sh <<-'EOF' &&
+	exit 255
+	EOF
+	cat <<-'EOF' >expect &&
+	bisect run failed: exit code -1 from './fail.sh' is < 0 or >= 128
+	EOF
+	test_when_finished 'git bisect reset' &&
+	git bisect start &&
+	git bisect good $HASH1 &&
+	git bisect bad $HASH4 &&
+	! git bisect run ./fail.sh 2>err &&
+	sed -En 's/.*(bisect.*code) (-?[0-9]+) (from.*)/\1 -1 \3/p' err >actual &&
+	test_cmp expect actual
+"
+
+test_expect_success 'git bisect run: unable to verify on good' "
+	write_script fail.sh <<-'EOF' &&
+	head=\$(git rev-parse --verify HEAD)
+	good=\$(git rev-parse --verify $HASH1)
+	if test "\$head" = "\$good"
+	then
+		exit 255
+	else
+		exit 127
+	fi
+	EOF
+	cat <<-'EOF' >expect &&
+	unable to verify './fail.sh' on good revision
+	EOF
+	test_when_finished 'git bisect reset' &&
+	git bisect start &&
+	git bisect good $HASH1 &&
+	git bisect bad $HASH4 &&
+	! git bisect run ./fail.sh 2>err &&
+	sed -n 's/.*\(unable to verify.*\)/\1/p' err >actual &&
+	test_cmp expect actual
+"
+
 # We want to automatically find the commit that
 # added "Another" into hello.
 test_expect_success '"git bisect run" simple case' '