@matheusbsbs / matheus-bagliano / commits / 48af1fdee3

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>

René Scharfe committed Jan 18, 2022 at 13:46 UTC 48af1fdee374e205c9774c163fbd4683d1110d11

4 files changed +71 -3

bisect.c

+2 -1

@@ -724,7 +724,8 @@ static int is_expected_rev(const struct object_id *oid)

724 return res;

725 }

726

727 -static enum bisect_error bisect_checkout(const struct object_id *bisect_rev, int no_checkout)

727 +enum bisect_error bisect_checkout(const struct object_id *bisect_rev,

728 + int no_checkout)

729 {

730 char bisect_rev_hex[GIT_MAX_HEXSZ + 1];

731 struct commit *commit;

bisect.h

@@ -3,6 +3,7 @@

4 struct commit_list;

5 struct repository;

6 +struct object_id;

8 /*

9 * Find bisection. If something is found, `reaches` will be the number of

@@ -69,4 +70,7 @@ void read_bisect_terms(const char **bad, const char **good);

71 int bisect_clean_state(void);

73 +enum bisect_error bisect_checkout(const struct object_id *bisect_rev,

74 + int no_checkout);

75 +

76 #endif

builtin/bisect--helper.c

+63

@@ -1089,6 +1089,44 @@ static int bisect_visualize(struct bisect_terms *terms, const char **argv, int a

1089 return res;

1090 }

1091

1092 +static int get_first_good(const char *refname, const struct object_id *oid,

1093 + int flag, void *cb_data)

1094 +{

1095 + oidcpy(cb_data, oid);

1096 + return 1;

1097 +}

1098 +

1099 +static int verify_good(const struct bisect_terms *terms,

1100 + const char **quoted_argv)

1101 +{

1102 + int rc;

1103 + enum bisect_error res;

1104 + struct object_id good_rev;

1105 + struct object_id current_rev;

1106 + char *good_glob = xstrfmt("%s-*", terms->term_good);

1107 + int no_checkout = ref_exists("BISECT_HEAD");

1108 +

1109 + for_each_glob_ref_in(get_first_good, good_glob, "refs/bisect/",

1110 + &good_rev);

1111 + free(good_glob);

1112 +

1113 + if (read_ref(no_checkout ? "BISECT_HEAD" : "HEAD", &current_rev))

1114 + return -1;

1115 +

1116 + res = bisect_checkout(&good_rev, no_checkout);

1117 + if (res != BISECT_OK)

1118 + return -1;

1119 +

1120 + printf(_("running %s\n"), quoted_argv[0]);

1121 + rc = run_command_v_opt(quoted_argv, RUN_USING_SHELL);

1122 +

1123 + res = bisect_checkout(&current_rev, no_checkout);

1124 + if (res != BISECT_OK)

1125 + return -1;

1126 +

1127 + return rc;

1128 +}

1129 +

1130 static int bisect_run(struct bisect_terms *terms, const char **argv, int argc)

1131 {

1132 int res = BISECT_OK;

@@ -1096,6 +1134,7 @@ static int bisect_run(struct bisect_terms *terms, const char **argv, int argc)

1134 struct strvec run_args = STRVEC_INIT;

1135 const char *new_state;

1136 int temporary_stdout_fd, saved_stdout;

1137 + int is_first_run = 1;

1138

1139 if (bisect_next_check(terms, NULL))

1140 return BISECT_FAILED;

@@ -1113,6 +1152,30 @@ static int bisect_run(struct bisect_terms *terms, const char **argv, int argc)

1152 printf(_("running %s\n"), command.buf);

1153 res = run_command_v_opt(run_args.v, RUN_USING_SHELL);

1154

1155 + /*

1156 + * Exit code 126 and 127 can either come from the shell

1157 + * if it was unable to execute or even find the script,

1158 + * or from the script itself. Check with a known-good

1159 + * revision to avoid trashing the bisect run due to a

1160 + * missing or non-executable script.

1161 + */

1162 + if (is_first_run && (res == 126 || res == 127)) {

1163 + int rc = verify_good(terms, run_args.v);

1164 + is_first_run = 0;

1165 + if (rc < 0) {

1166 + error(_("unable to verify '%s' on good"

1167 + " revision"), command.buf);

1168 + res = BISECT_FAILED;

1169 + break;

1170 + }

1171 + if (rc == res) {

1172 + error(_("bogus exit code %d for good revision"),

1173 + rc);

1174 + res = BISECT_FAILED;

1175 + break;

1176 + }

1177 + }

1178 +

1179 if (res < 0 || 128 <= res) {

1180 error(_("bisect run failed: exit code %d from"

1181 " '%s' is < 0 or >= 128"), res, command.buf);

t/t6030-bisect-porcelain.sh

+2 -2

@@ -290,7 +290,7 @@ test_expect_success 'bisect run accepts exit code 126 as bad' '

290 grep "$HASH3 is the first bad commit" my_bisect_log.txt

291 '

292

293 -test_expect_failure POSIXPERM 'bisect run fails with non-executable test script' '

293 +test_expect_success POSIXPERM 'bisect run fails with non-executable test script' '

294 test_when_finished "git bisect reset" &&

295 >not-executable.sh &&

296 chmod -x not-executable.sh &&

@@ -313,7 +313,7 @@ test_expect_success 'bisect run accepts exit code 127 as bad' '

313 grep "$HASH3 is the first bad commit" my_bisect_log.txt

314 '

315

316 -test_expect_failure 'bisect run fails with missing test script' '

316 +test_expect_success 'bisect run fails with missing test script' '

317 test_when_finished "git bisect reset" &&

318 rm -f does-not-exist.sh &&

319 git bisect start &&