git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: Chris Down <chris@chrisdown.name>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>,
	Johannes Schindelin <johannes.schindelin@gmx.de>,
	Christian Couder <chriscool@tuxfamily.org>,
	Taylor Blau <me@ttaylorr.com>,
	kernel-team@fb.com
Subject: [PATCH v3 1/2] bisect: output state before we are ready to compute bisection
Date: Wed, 11 May 2022 19:00:09 +0100	[thread overview]
Message-ID: <2cd916580524d0d303b252687e29a064a3aaa387.1652291688.git.chris@chrisdown.name> (raw)
In-Reply-To: <cover.1652291688.git.chris@chrisdown.name>

Commit 73c6de06aff8 ("bisect: don't use invalid oid as rev when
starting") changes the behaviour of `git bisect` to consider invalid
oids as pathspecs again, as in the old shell implementation.

While that behaviour may be desirable, it can also cause confusion. For
example, while bisecting in a particular repo I encountered this:

    $ git bisect start d93ff48803f0 v6.3
    $

...which led to me sitting for a few moments, wondering why there's no
printout stating the first rev to check.

It turns out that the tag was actually "6.3", not "v6.3", and thus the
bisect was still silently started with only a bad rev, because
d93ff48803f0 was a valid oid and "v6.3" was silently considered to be a
pathspec.

While this behaviour may be desirable, it can be confusing, especially
with different repo conventions either using or not using "v" before
release names, or when a branch name or tag is simply misspelled on the
command line.

In order to avoid situations like this, make it more clear what we're
waiting for:

    $ git bisect start d93ff48803f0 v6.3
    status: waiting for good commit(s), bad commit known

We already have good output once the bisect process has begun in
earnest, so we don't need to do anything more there.

Signed-off-by: Chris Down <chris@chrisdown.name>
---
 bisect.h                    |  9 +++++++
 builtin/bisect--helper.c    | 53 ++++++++++++++++++++++++++++---------
 t/t6030-bisect-porcelain.sh | 18 +++++++++++++
 3 files changed, 67 insertions(+), 13 deletions(-)

diff --git a/bisect.h b/bisect.h
index 1015aeb8ea..ee3fd65f3b 100644
--- a/bisect.h
+++ b/bisect.h
@@ -62,6 +62,15 @@ enum bisect_error {
 	BISECT_INTERNAL_SUCCESS_MERGE_BASE = -11
 };
 
+/*
+ * Stores how many good/bad commits we have stored for a bisect. nr_bad can
+ * only be 0 or 1.
+ */
+struct bisect_state {
+	unsigned int nr_good;
+	unsigned int nr_bad;
+};
+
 enum bisect_error bisect_next_all(struct repository *r, const char *prefix);
 
 int estimate_bisect_steps(int all);
diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 8b2b259ff0..fa8024a864 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -329,12 +329,12 @@ static int check_and_set_terms(struct bisect_terms *terms, const char *cmd)
 	return 0;
 }
 
-static int mark_good(const char *refname, const struct object_id *oid,
-		     int flag, void *cb_data)
+static int inc_nr(const char *refname, const struct object_id *oid,
+		  int flag, void *cb_data)
 {
-	int *m_good = (int *)cb_data;
-	*m_good = 0;
-	return 1;
+	unsigned int *nr = (unsigned int *)cb_data;
+	(*nr)++;
+	return 0;
 }
 
 static const char need_bad_and_good_revision_warning[] =
@@ -384,23 +384,48 @@ static int decide_next(const struct bisect_terms *terms,
 			     vocab_good, vocab_bad, vocab_good, vocab_bad);
 }
 
-static int bisect_next_check(const struct bisect_terms *terms,
-			     const char *current_term)
+static void bisect_status(struct bisect_state *state,
+			  const struct bisect_terms *terms)
 {
-	int missing_good = 1, missing_bad = 1;
 	char *bad_ref = xstrfmt("refs/bisect/%s", terms->term_bad);
 	char *good_glob = xstrfmt("%s-*", terms->term_good);
 
 	if (ref_exists(bad_ref))
-		missing_bad = 0;
+		state->nr_bad = 1;
 
-	for_each_glob_ref_in(mark_good, good_glob, "refs/bisect/",
-			     (void *) &missing_good);
+	for_each_glob_ref_in(inc_nr, good_glob, "refs/bisect/",
+			     (void *) &state->nr_good);
 
 	free(good_glob);
 	free(bad_ref);
+}
+
+static void bisect_print_status(const struct bisect_terms *terms)
+{
+	struct bisect_state state = { 0 };
+
+	bisect_status(&state, terms);
+
+	/* If we had both, we'd already be started, and shouldn't get here. */
+	if (state.nr_good && state.nr_bad)
+		return;
 
-	return decide_next(terms, current_term, missing_good, missing_bad);
+	if (!state.nr_good && !state.nr_bad)
+		printf(_("status: waiting for both good and bad commits\n"));
+	else if (state.nr_good)
+		printf(Q_("status: waiting for bad commit, %d good commit known\n",
+			  "status: waiting for bad commit, %d good commits known\n",
+			  state.nr_good), state.nr_good);
+	else
+		printf(_("status: waiting for good commit(s), bad commit known\n"));
+}
+
+static int bisect_next_check(const struct bisect_terms *terms,
+			     const char *current_term)
+{
+	struct bisect_state state = { 0 };
+	bisect_status(&state, terms);
+	return decide_next(terms, current_term, !state.nr_good, !state.nr_bad);
 }
 
 static int get_terms(struct bisect_terms *terms)
@@ -606,8 +631,10 @@ static enum bisect_error bisect_next(struct bisect_terms *terms, const char *pre
 
 static enum bisect_error bisect_auto_next(struct bisect_terms *terms, const char *prefix)
 {
-	if (bisect_next_check(terms, NULL))
+	if (bisect_next_check(terms, NULL)) {
+		bisect_print_status(terms);
 		return BISECT_OK;
+	}
 
 	return bisect_next(terms, prefix);
 }
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index 5382e5d216..686f6d5c7f 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -1025,4 +1025,22 @@ test_expect_success 'bisect visualize with a filename with dash and space' '
 	git bisect visualize -p -- "-hello 2"
 '
 
+test_expect_success 'bisect state output with multiple good commits' '
+	git bisect reset &&
+	git bisect start >output &&
+	grep "waiting for both good and bad commits" output &&
+	git bisect good "$HASH1" >output &&
+	grep "waiting for bad commit, 1 good commit known" output &&
+	git bisect good "$HASH2" >output &&
+	grep "waiting for bad commit, 2 good commits known" output
+'
+
+test_expect_success 'bisect state output with bad commit' '
+	git bisect reset &&
+	git bisect start >output &&
+	grep "waiting for both good and bad commits" output &&
+	git bisect bad "$HASH4" >output &&
+	grep -F "waiting for good commit(s), bad commit known" output
+'
+
 test_done
-- 
2.36.0


  reply	other threads:[~2022-05-11 18:01 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-11 18:00 [PATCH v3 0/2] bisect: status improvements when bisect is not fully fleshed out Chris Down
2022-05-11 18:00 ` Chris Down [this message]
2022-05-11 18:00 ` [PATCH v3 2/2] bisect: output bisect setup status in bisect log Chris Down
2022-05-11 19:41 ` [PATCH v3 0/2] bisect: status improvements when bisect is not fully fleshed out 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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=2cd916580524d0d303b252687e29a064a3aaa387.1652291688.git.chris@chrisdown.name \
    --to=chris@chrisdown.name \
    --cc=chriscool@tuxfamily.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=johannes.schindelin@gmx.de \
    --cc=kernel-team@fb.com \
    --cc=me@ttaylorr.com \
    --subject='Re: [PATCH v3 1/2] bisect: output state before we are ready to compute bisection' \
    /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

Code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).