All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.