Re* [PATCH] Fix git-bisect when show-branch is configured to run with pager

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 28, 2021, 17:07 UTC
Message-ID: <xmqq4kcebc5l.fsf_-_@gitster.g>
In-Reply-To: <xmqq8s1qbdcr.fsf@gitster.g>

I'll fix the missing 'a' in the log message, but the "res" simplification is probably better done as a separate patch on top.

How does this look?

Thanks.

---- >8 ------- >8 ------- >8 ------- >8 ------- >8 ------- >8 ----
Subject: [PATCH] bisect: simplify return code from bisect_checkout()

The function was designed to return only BISECT_OK (0) or BISECT_FAILED (-1) and no other values, but there were two issues:

 - The comment misspelled BISECT_FAILED as BISECT_FAILURE, even
   though the logic it described (i.e. any non-zero return should be
   reported as a single BISECT_FAILED) was correct.

 - It took the return value from run_command_v_opt(), and assumed it
   was either -1 or 1 upon error, which is not the case; it can relay
   errors from wait_or_whine(), which can report exit status of the
   child process.

Translate any error return from run_command_v_opt() to BISECT_FAILED, and simplify the resulting code by losing the 'res' variable that is no longer needed.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 bisect.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/bisect.c b/bisect.c
index 2b8b6546e9..888949fba6 100644
--- a/bisect.c
+++ b/bisect.c
@@ -727,7 +727,6 @@ static int is_expected_rev(const struct object_id *oid)
 static enum bisect_error bisect_checkout(const struct object_id *bisect_rev, int no_checkout)
 {
 	char bisect_rev_hex[GIT_MAX_HEXSZ + 1];
-	enum bisect_error res = BISECT_OK;
 	struct commit *commit;
 	struct pretty_print_context pp = {0};
 	struct strbuf commit_msg = STRBUF_INIT;
@@ -740,14 +739,13 @@ static enum bisect_error bisect_checkout(const struct object_id *bisect_rev, int
 		update_ref(NULL, "BISECT_HEAD", bisect_rev, NULL, 0,
 			   UPDATE_REFS_DIE_ON_ERR);
 	} else {
-		res = run_command_v_opt(argv_checkout, RUN_GIT_CMD);
-		if (res)
+		if (run_command_v_opt(argv_checkout, RUN_GIT_CMD))
 			/*
 			 * Errors in `run_command()` itself, signaled by res < 0,
 			 * and errors in the child process, signaled by res > 0
-			 * can both be treated as regular BISECT_FAILURE (-1).
+			 * can both be treated as regular BISECT_FAILED (-1).
 			 */
-			return -abs(res);
+			return BISECT_FAILED;
 	}
 
 	commit = lookup_commit_reference(the_repository, bisect_rev);
@@ -755,7 +753,7 @@ static enum bisect_error bisect_checkout(const struct object_id *bisect_rev, int
 	fputs(commit_msg.buf, stdout);
 	strbuf_release(&commit_msg);
 
-	return -abs(res);
+	return BISECT_OK;
 }
 
 static struct commit *get_commit_reference(struct repository *r,

-- 
2.32.0-561-g6177dfa0d2