bisect--helper: double-check run command on exit code 126 and 127

When a run command cannot be executed or found, shells return exit code
126 or 127, respectively.  Valid run commands are allowed to return
these codes as well to indicate bad revisions, though, for historical
reasons.  This means typos can cause bogus bisect runs that go over the
full distance and end up reporting invalid results.

The best solution would be to reserve exit codes 126 and 127, like
71b0251cdd (Bisect run: "skip" current commit if script exit code is
125., 2007-10-26) did for 125, and abort bisect run when we get them.
That might be inconvenient for those who relied on the documentation
stating that 126 and 127 can be used for bad revisions, though.

The workaround used by this patch is to run the command on a known-good
revision and abort if we still get the same error code.  This adds one
step to runs with scripts that use exit codes 126 and 127, but keeps
them supported, with one exception: It won't work with commands that
cannot recognize the (manually marked) known-good revision as such.

Run commands that use low exit codes are unaffected.  Typos are reported
after executing the missing command twice and three checkouts (the first
step, the known good revision and back to the revision of the first
step).

Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

1 files changed, 63 insertions, 0 deletions

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index e529665d9f..50783a586c 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -1089,6 +1089,44 @@ static int bisect_visualize(struct bisect_terms *terms, const char **argv, int a
 	return res;
 }
 
+static int get_first_good(const char *refname, const struct object_id *oid,
+			  int flag, void *cb_data)
+{
+	oidcpy(cb_data, oid);
+	return 1;
+}
+
+static int verify_good(const struct bisect_terms *terms,
+		       const char **quoted_argv)
+{
+	int rc;
+	enum bisect_error res;
+	struct object_id good_rev;
+	struct object_id current_rev;
+	char *good_glob = xstrfmt("%s-*", terms->term_good);
+	int no_checkout = ref_exists("BISECT_HEAD");
+
+	for_each_glob_ref_in(get_first_good, good_glob, "refs/bisect/",
+			     &good_rev);
+	free(good_glob);
+
+	if (read_ref(no_checkout ? "BISECT_HEAD" : "HEAD", &current_rev))
+		return -1;
+
+	res = bisect_checkout(&good_rev, no_checkout);
+	if (res != BISECT_OK)
+		return -1;
+
+	printf(_("running %s\n"), quoted_argv[0]);
+	rc = run_command_v_opt(quoted_argv, RUN_USING_SHELL);
+
+	res = bisect_checkout(&current_rev, no_checkout);
+	if (res != BISECT_OK)
+		return -1;
+
+	return rc;
+}
+
 static int bisect_run(struct bisect_terms *terms, const char **argv, int argc)
 {
 	int res = BISECT_OK;
@@ -1096,6 +1134,7 @@ static int bisect_run(struct bisect_terms *terms, const char **argv, int argc)
 	struct strvec run_args = STRVEC_INIT;
 	const char *new_state;
 	int temporary_stdout_fd, saved_stdout;
+	int is_first_run = 1;
 
 	if (bisect_next_check(terms, NULL))
 		return BISECT_FAILED;
@@ -1113,6 +1152,30 @@ static int bisect_run(struct bisect_terms *terms, const char **argv, int argc)
 		printf(_("running %s\n"), command.buf);
 		res = run_command_v_opt(run_args.v, RUN_USING_SHELL);
 
+		/*
+		 * Exit code 126 and 127 can either come from the shell
+		 * if it was unable to execute or even find the script,
+		 * or from the script itself.  Check with a known-good
+		 * revision to avoid trashing the bisect run due to a
+		 * missing or non-executable script.
+		 */
+		if (is_first_run && (res == 126 || res == 127)) {
+			int rc = verify_good(terms, run_args.v);
+			is_first_run = 0;
+			if (rc < 0) {
+				error(_("unable to verify '%s' on good"
+					" revision"), command.buf);
+				res = BISECT_FAILED;
+				break;
+			}
+			if (rc == res) {
+				error(_("bogus exit code %d for good revision"),
+				      rc);
+				res = BISECT_FAILED;
+				break;
+			}
+		}
+
 		if (res < 0 || 128 <= res) {
 			error(_("bisect run failed: exit code %d from"
 				" '%s' is < 0 or >= 128"), res, command.buf);