bisect: add --force flag to force checkout

From: Kevin Wang <[email protected]>

Adds a --force/-f flag to git bisect good/bad and git bisect run to force a checkout. Currently, if the repository state adds any local changes the user must manually reset the repository state before moving to the next bisection step. This can happen with package lock files or log output data, for example. With this change, a developer can run git bisect run --force to automatically reset the repository state after each evaluation. The flag is also supported as git bisect (good|bad) --force as well.

---
bisect: add --force flag to force checkout

Adds a --force/-f flag to git bisect good/bad and git bisect run to force a checkout. Currently, if the repository state adds any local changes the user must manually reset the repository state before moving to the next bisection step. This can happen with package lock files or log output data, for example. With this change, a developer can run git bisect run --force to automatically reset the repository state after each evaluation. The flag is also supported as git bisect (good|bad) --force as well.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1641%2Fkevmo314%2Fbisect-force-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1641/kevmo314/bisect-force-v1
Pull-Request: https://github.com/git/git/pull/1641

 Documentation/git-bisect.txt |  13 ++++-
 bisect.c                     |  17 ++++--
 bisect.h                     |   4 +-
 builtin/bisect.c             |  50 +++++++++++-----
 t/t6030-bisect-porcelain.sh  | 109 +++++++++++++++++++++++++++++++++++
 5 files changed, 167 insertions(+), 26 deletions(-)

index aa02e462244..57357221718 100644
@@ -19,14 +19,14 @@ on the subcommand:
  git bisect start [--term-(new|bad)=<term-new> --term-(old|good)=<term-old>]
 		  [--no-checkout] [--first-parent] [<bad> [<good>...]] [--] [<paths>...]
  git bisect (bad|new|<term-new>) [<rev>]
- git bisect (good|old|<term-old>) [<rev>...]
+ git bisect (good|old|<term-old>) [-f] [<rev>...]
  git bisect terms [--term-good | --term-bad]
  git bisect skip [(<rev>|<range>)...]
  git bisect reset [<commit>]
  git bisect (visualize|view)
  git bisect replay <logfile>
  git bisect log
- git bisect run <cmd> [<arg>...]
+ git bisect run [-f] <cmd> [<arg>...]
  git bisect help
 
 This command uses a binary search algorithm to find which commit in

@@ -381,6 +381,15 @@ ignored.
 This option is particularly useful in avoiding false positives when a merged
 branch contained broken or non-buildable commits, but the merge itself was OK.
 
+-f::
+--force::
++
+Throw away any local changes and untracked files before moving to the next
+bisection step.
++
+This option may be useful if the repository state changes when testing a
+revision.
+
 EXAMPLES
 --------
 

index 985b96ed132..72ce09f2015 100644
@@ -713,7 +713,7 @@ static int is_expected_rev(const struct object_id *oid)
 }
 
 enum bisect_error bisect_checkout(const struct object_id *bisect_rev,
-				  int no_checkout)
+				  int no_checkout, int force_checkout)
 {
 	struct commit *commit;
 	struct pretty_print_context pp = {0};

@@ -728,8 +728,13 @@ enum bisect_error bisect_checkout(const struct object_id *bisect_rev,
 		struct child_process cmd = CHILD_PROCESS_INIT;
 
 		cmd.git_cmd = 1;
-		strvec_pushl(&cmd.args, "checkout", "-q",
-			     oid_to_hex(bisect_rev), "--", NULL);
+		if (force_checkout) {
+			strvec_pushl(&cmd.args, "checkout", "-f", "-q",
+					oid_to_hex(bisect_rev), "--", NULL);
+		} else {
+			strvec_pushl(&cmd.args, "checkout", "-q",
+					oid_to_hex(bisect_rev), "--", NULL);
+		}
 		if (run_command(&cmd))
 			/*
 			 * Errors in `run_command()` itself, signaled by res < 0,

@@ -850,7 +855,7 @@ static enum bisect_error check_merge_bases(int rev_nr, struct commit **rev, int
 			handle_skipped_merge_base(mb);
 		} else {
 			printf(_("Bisecting: a merge base must be tested\n"));
-			res = bisect_checkout(mb, no_checkout);
+			res = bisect_checkout(mb, no_checkout, 0);
 			if (!res)
 				/* indicate early success */
 				res = BISECT_INTERNAL_SUCCESS_MERGE_BASE;

@@ -1002,7 +1007,7 @@ void read_bisect_terms(const char **read_bad, const 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, int force_checkout)
 {
 	struct strvec rev_argv = STRVEC_INIT;
 	struct rev_info revs = REV_INFO_INIT;

@@ -1104,7 +1109,7 @@ enum bisect_error bisect_next_all(struct repository *r, const char *prefix)
 	/* Clean up objects used, as they will be reused. */
 	repo_clear_commit_marks(r, ALL_REV_FLAGS);
 
-	res = bisect_checkout(bisect_rev, no_checkout);
+	res = bisect_checkout(bisect_rev, no_checkout, force_checkout);
 cleanup:
 	release_revisions(&revs);
 	strvec_clear(&rev_argv);

index ee3fd65f3bd..6f972365faa 100644
@@ -71,7 +71,7 @@ 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, int force_checkout);
 
 int estimate_bisect_steps(int all);
 

@@ -80,6 +80,6 @@ void read_bisect_terms(const char **bad, const char **good);
 int bisect_clean_state(void);
 
 enum bisect_error bisect_checkout(const struct object_id *bisect_rev,
-				  int no_checkout);
+				  int no_checkout, int force_checkout);
 
 #endif

base-commit: a26002b62827b89a19b1084bd75d9371d565d03c

gitgitgadget

The usual way to compose a log message is to

- Give an observation on how the current system work in the present tense (so no need to say "Currently X is Y", just "X is Y"), and discuss what you perceive as a problem in it.

- Propose a solution (optional---often, problem description trivially leads to an obvious solution in reader's minds).

- Give commands to the make codebase "like so".

in this order.

To those who have been intimately following the discussion, it often is understandable without both, but we are not writing for those who review the patches. We are writing for future readers who are not aware of the review discussion we have on list, so we should give something to prepare them by setting the stage and stating the objective first, before going into how the patch solved it.

Having said all that.

I highly doubt that this patch is a good idea. If your "bisect run" script needs to update something in the working tree before it runs some test, the script is in a much better place than Git, which is unaware of what your run script is doing, to prepare the working tree into pristine state. The best Git would be able to do would be to "reset --hard", but that will lose local modifications that are deliberately there and has nothing to do with what your run script did.

Adding some description to the documentation of "bisect run" and teaching readers a common trick of structuring their run script better might be a more productive approach, I would have to say. For example, when I bisect some old code, I may have to apply a temporary patch to some of the sources to get them compile with more recent compilers (I usually do this with a cherry-picking of a local fixup). So my "bisect run" script might go like so:

    #!/bin/sh
    # bisect run

    git apply local-fixup || exit 125
    make test
    status=$?
    make distclean
    git apply -R local-fixup || exit 125
    exit $status

That is, I'd apply some local fix-up to the working tree files before running tests, and once done, I revert the local fix-up and exit from the run script with the exit status of the test I wanted to perform.

This way, I can keep other local changes (things like changes to documentation files that I am working on, which has nothing to do with the problem I am bisecting but I know they do not interfere) without wiping them away with a sledgehammer "reset --hard". Your "bisect good/bad -f" sounds like the sledgehammer approach to me.

Thanks.