[PATCH v6 0/2] bisect: add --reset-when-found to leave when done

["Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com>]

Add a --reset-when-found option to git bisect that resets the bisect session
when culprit is found.

Changes in v6:

 * Reuse the existing bad bisect ref instead of propagating the culprit OID
   through bisect_next_all().
 * Remove the redundant reset_when_found_arg_seen flag and use
   RESET_WHEN_FOUND_NONE to detect whether the option was given.

Changes in v5:

 * Move automatic reset handling to cmd_bisect() after subcommand resources
   are closed.
 * Propagate the first-bad commit OID from bisect_next_all() and remove
   defer_reset plumbing.
 * Separate checkout from state cleanup in bisect_reset(), and use bool for
   its quiet flag.

Changes in v4:

 * Simplify translation calls.
 * Avoid git subshell calls in tests, that can bury errors.

Changes in v3:

 * Rename --auto-reset to --reset-when-found, including internal names.
 * Defer git bisect run cleanup until captured output is printed and
   BISECT_RUN is closed. Drop the open-descriptor preparatory change,
   retaining the existing filename-based output handling.

Changes in v2:

 * Add option --auto-reset[=<where>] with option to go to final commit as
   well as original.
 * Refactored tests.

Harald Nordgren (2):
  bisect: let bisect_reset() optionally check out quietly
  bisect: add --reset-when-found to leave when done

 Documentation/git-bisect.adoc |  14 ++-
 bisect.c                      |   2 +
 builtin/bisect.c              | 162 +++++++++++++++++++++++++++++++---
 t/t6030-bisect-porcelain.sh   | 121 +++++++++++++++++++++++++
 4 files changed, 285 insertions(+), 14 deletions(-)


base-commit: a97fcc37c2bc6340a8d7ce78dedf227aac4e9aa7
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2335%2FHaraldNordgren%2Fbisect-auto-reset-v6
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2335/HaraldNordgren/bisect-auto-reset-v6
Pull-Request: https://github.com/git/git/pull/2335

Range-diff vs v5:

 1:  b6ab87fd29 = 1:  b6ab87fd29 bisect: let bisect_reset() optionally check out quietly
 2:  ec362f3b82 ! 2:  97a4da5537 bisect: add --reset-when-found to leave when done
     @@ Commit message
          Persist the selected target in a BISECT_RESET_WHEN_FOUND state file
          and perform the reset quietly.
      
     -    Propagate the internal first-bad result and its commit to
     -    cmd_bisect(), which performs the reset after the subcommand has
     -    returned. For "git bisect run", this means BISECT_RUN has been printed
     -    and closed before cleanup, which also works on systems that cannot
     -    unlink an open file.
     +    Let the internal first-bad result propagate to cmd_bisect(), which
     +    performs the reset using the existing bad bisect ref after the
     +    subcommand has returned. For "git bisect run", this means BISECT_RUN
     +    has been printed and closed before cleanup, which also works on systems
     +    that cannot unlink an open file.
      
          Reject this option together with "--no-checkout", since that mode must
          not check out either target.
     @@ bisect.c: static GIT_PATH_FUNC(git_path_bisect_start, "BISECT_START")
       
       static void read_bisect_paths(struct strvec *array)
       {
     -@@ bisect.c: void read_bisect_terms(char **read_bad, char **read_good)
     -  * the end of bisect_helper::cmd_bisect__helper() helps bypassing
     -  * all the code related to finding a commit to test.
     -  */
     --enum bisect_error bisect_next_all(struct repository *r, const char *prefix)
     -+enum bisect_error bisect_next_all(struct repository *r, const char *prefix,
     -+				  struct object_id *first_bad)
     - {
     - 	struct strvec rev_argv = STRVEC_INIT;
     - 	struct rev_info revs = REV_INFO_INIT;
     -@@ bisect.c: enum bisect_error bisect_next_all(struct repository *r, const char *prefix)
     - 		res = error_if_skipped_commits(tried, current_bad_oid);
     - 		if (res)
     - 			goto cleanup;
     -+		oidcpy(first_bad, bisect_rev);
     - 		printf("%s is the first '%s' commit\n", oid_to_hex(bisect_rev),
     - 			term_bad);
     - 
      @@ bisect.c: int bisect_clean_state(void)
       	unlink_or_warn(git_path_bisect_run());
       	unlink_or_warn(git_path_bisect_terms());
     @@ bisect.c: int bisect_clean_state(void)
       	 * Cleanup BISECT_START last to support the --no-checkout option
       	 * introduced in the commit 4796e823a.
      
     - ## bisect.h ##
     -@@ bisect.h: struct bisect_state {
     - 	unsigned int nr_bad;
     - };
     - 
     --enum bisect_error bisect_next_all(struct repository *r, const char *prefix);
     -+enum bisect_error bisect_next_all(struct repository *r, const char *prefix,
     -+				  struct object_id *first_bad);
     - 
     - int estimate_bisect_steps(int all);
     - 
     -
       ## builtin/bisect.c ##
      @@ builtin/bisect.c: static GIT_PATH_FUNC(git_path_bisect_start, "BISECT_START")
       static GIT_PATH_FUNC(git_path_bisect_log, "BISECT_LOG")
     @@ builtin/bisect.c: static const char * const git_bisect_usage[] = {
       struct add_bisect_ref_data {
       	struct rev_info *revs;
       	unsigned int object_flags;
     -@@ builtin/bisect.c: struct bisect_terms {
     - 	char *term_bad;
     - };
     - 
     -+static struct object_id first_bad_oid;
     -+
     - static void free_terms(struct bisect_terms *terms)
     - {
     - 	FREE_AND_NULL(terms->term_good);
      @@ builtin/bisect.c: static int bisect_reset(const char *commit, bool quiet)
       	}
       
     @@ builtin/bisect.c: static int bisect_reset(const char *commit, bool quiet)
      +
      +static int bisect_reset_when_found(enum reset_when_found_mode mode)
      +{
     -+	char first_bad_hex[GIT_MAX_HEXSZ + 1];
     -+	const char *commit = NULL;
     ++	struct bisect_terms terms = { 0 };
     ++	char *commit = NULL;
      +	int res;
      +
     -+	if (mode == RESET_WHEN_FOUND_TO_FOUND)
     -+		commit = oid_to_hex_r(first_bad_hex, &first_bad_oid);
     -+	else if (mode == RESET_WHEN_FOUND_NONE)
     ++	if (mode == RESET_WHEN_FOUND_TO_FOUND) {
     ++		read_bisect_terms(&terms.term_bad, &terms.term_good);
     ++		commit = xstrfmt("refs/bisect/%s", terms.term_bad);
     ++	} else if (mode == RESET_WHEN_FOUND_NONE) {
      +		BUG("automatic reset requested without a reset mode");
     ++	}
      +
      +	res = bisect_reset(commit, true);
      +	if (!res)
      +		res = bisect_clean_state();
      +
     ++	free(commit);
     ++	free_terms(&terms);
      +	return res;
       }
       
     @@ builtin/bisect.c: static int bisect_successful(struct bisect_terms *terms)
       {
       	enum bisect_error res;
       
     -@@ builtin/bisect.c: static enum bisect_error bisect_next(struct bisect_terms *terms, const char *pre
     - 		return BISECT_FAILED;
     - 
     - 	/* Perform all bisection computation */
     --	res = bisect_next_all(the_repository, prefix);
     -+	res = bisect_next_all(the_repository, prefix, &first_bad_oid);
     - 
     - 	if (res == BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND) {
     - 		res = bisect_successful(terms);
      @@ builtin/bisect.c: static enum bisect_error bisect_next(struct bisect_terms *terms, const char *pre
       	return res;
       }
     @@ builtin/bisect.c: static int bisect_run(struct bisect_terms *terms, int argc, co
       	int temporary_stdout_fd, saved_stdout;
       	int is_first_run = 1;
      +	enum reset_when_found_mode reset_when_found = RESET_WHEN_FOUND_NONE;
     -+	bool reset_when_found_arg_seen = false;
       
       	if (bisect_next_check(terms, NULL))
       		return BISECT_FAILED;
       
      +	if (argc && !strcmp(argv[0], "--reset-when-found")) {
      +		reset_when_found = RESET_WHEN_FOUND_TO_ORIGINAL;
     -+		reset_when_found_arg_seen = true;
      +	} else if (argc && skip_prefix(argv[0], "--reset-when-found=",
      +				    &reset_when_found_arg)) {
      +		if (parse_reset_when_found(reset_when_found_arg,
      +					   &reset_when_found))
      +			return BISECT_FAILED;
     -+		reset_when_found_arg_seen = true;
      +	}
      +
      +	if (reset_when_found != RESET_WHEN_FOUND_NONE &&
     @@ builtin/bisect.c: static int bisect_run(struct bisect_terms *terms, int argc, co
      +		return error(_("options '%s' and '%s' cannot be used together"),
      +			     "--reset-when-found", "--no-checkout");
      +
     -+	if (reset_when_found_arg_seen) {
     ++	if (reset_when_found != RESET_WHEN_FOUND_NONE) {
      +		write_file(git_path_bisect_reset_when_found(), "%s\n",
      +			   reset_when_found_mode_name(reset_when_found));
      +		argc--;

-- 
gitgitgadget