git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Christian Couder <christian.couder@gmail.com>
Cc: git <git@vger.kernel.org>,
	Oded S via GitGitGadget <gitgitgadget@gmail.com>,
	Oded S <oded@istraresearch.com>
Subject: Re* [PATCH] Fix git-bisect when show-branch is configured to run with pager
Date: Wed, 28 Jul 2021 10:07:02 -0700	[thread overview]
Message-ID: <xmqq4kcebc5l.fsf_-_@gitster.g> (raw)
In-Reply-To: <xmqq8s1qbdcr.fsf@gitster.g> (Junio C. Hamano's message of "Wed, 28 Jul 2021 09:41:08 -0700")

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


  reply	other threads:[~2021-07-28 17:07 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-26 15:00 [PATCH] Fix git-bisect when show-branch is configured to run with pager Oded S via GitGitGadget
2021-07-26 18:13 ` Christian Couder
2021-07-26 18:39 ` Junio C Hamano
2021-07-27 18:22   ` Junio C Hamano
2021-07-28  6:37     ` Christian Couder
2021-07-28 16:41       ` Junio C Hamano
2021-07-28 17:07         ` Junio C Hamano [this message]
2021-07-29  2:34           ` Re* " Christian Couder
2021-07-27  8:12 ` [PATCH v2] bisect: disable pager while invoking show-branch Oded S via GitGitGadget
2021-07-27 18:29   ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqq4kcebc5l.fsf_-_@gitster.g \
    --to=gitster@pobox.com \
    --cc=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=oded@istraresearch.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).