[PATCH] bisect reset: Allow resetting to any commit, not just a branch

‘git bisect reset’ could already checkout an arbitrary commit if you
were on a detached HEAD before starting the bisection.  This lets you
specify an arbitrary commit to ‘git bisect reset <commit>’.

This also provides a way to clean the bisection state without moving
HEAD: ‘git bisect reset HEAD’.

Signed-off-by: Anders Kaseorg <redacted>
---
 git-bisect.sh |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/git-bisect.sh b/git-bisect.sh
index 6f6f039..d319b9f 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh

@@ -311,8 +311,7 @@ bisect_reset() {
 	}
 	case "$#" in
 	0) branch=$(cat "$GIT_DIR/BISECT_START") ;;
-	1) git show-ref --verify --quiet -- "refs/heads/$1" ||
-	       die "$1 does not seem to be a valid branch"
+	1) git rev-parse --verify "$1^{commit}" || exit
 	   branch="$1" ;;
 	*)
 	    usage ;;

-- 
1.6.5

Anders Kaseorg [off-list ref] writes:

Thanks.

The "one parameter" case dates back to the original bisect implementation
in commit 8cc6a08 (Making it easier to find which change introduced a bug,
2005-07-30), and the only reason of existence for that case was that the
code looked like this back then:

    bisect_reset() {
           case "$#" in
           0) branch=master ;;
           1) test -f "$GIT_DIR/refs/heads/$1" || {
                  echo >&2 "$1 does not seem to be a valid branch"
                  exit 1
              }
              branch="$1" ;;
            *)
              usage ;;
           esac
    ...

An important difference to notice, compared to a more recent version, is
that we did not remember (nor use) the original branch, and without an
argument we always switched to 'master'.  Back then, the user who started
bisecting a side branch needed to remember the name of the branch before
starting the bisection, and needed to give that to the reset subcommand.

Because we remember where we came from these days, I do not think it makes
much sense to even keep this "one parameter" case, let alone extending
this interface to allow switching to an arbitrary commit.

I even think that the support for an explicit branch name in the reset
subcommand should eventually be deprecated, perhaps unless it matches what
is stored in BISECT_START.

The documentation, does not even talk about what the optional <branch>
argument is good for, even though it lists the optional <branch> in the
synopsis section.

Having said all that, four years and two months are looooooong time in git
timescale, and I am discounting, without any evidence to judge either way,
the possibility that people may have learned during that time to (ab)use
this as a (very useful?) shortcut to finish the current bisection and
switch to some entirely different branch.

I offhand do not see a good rationale for such a shortcut to finish bisect
and switch to another branch (IOW, I understand "it is shorter to type",
but I do not see "it is often done and very useful"), but I am open to be
enlightened by a workflow where such a shortcut is useful.

On Mon, 12 Oct 2009, Junio C Hamano wrote:

I offhand do not see a good rationale for such a shortcut to finish 
bisect and switch to another branch (IOW, I understand "it is shorter to 
type", but I do not see "it is often done and very useful"), but I am 
open to be enlightened by a workflow where such a shortcut is useful.

I agree that ‘git bisect reset <branch>’ is a confusing shortcut.  It only 
really made sense before Git supported detached HEADs, and you needed to 
be on a branch all the time.  I think that lifting the arbitrary 
restriction to branch names makes it less confusing, but if you want to 
remove the argument altogether, that would eliminate the confusion 
entirely.

I had in mind only one case where ‘git bisect reset <commit>’ would be 
useful.  I often don’t even remember what commit I was on before I started 
a bisect, much less believe that I want to immediately switch back to it.  
I would prefer to be able to clean the bisection state without checking 
out another commit at all, because that takes forever and invalidates my 
compiled tree.  This is what ‘git bisect reset HEAD’ would do if it 
worked.

Perhaps it makes sense to add a command that just clears the bisection 
state.  ‘git bisect stop’?

Anders

Anders Kaseorg [off-list ref] writes:

I had in mind only one case where ‘git bisect reset <commit>’ would be 
useful.  I often don’t even remember what commit I was on before I started 
a bisect, much less believe that I want to immediately switch back to it.  
I would prefer to be able to clean the bisection state without checking 
out another commit at all, because that takes forever and invalidates my 
compiled tree.  This is what ‘git bisect reset HEAD’ would do if it 
worked.

I am not sure what "removing bisect states" really buys you.

If having bisect states somehow interferes what you need to do in order to
decide which commit you want to switch to, it may make sense to do 'git
bisect reset HEAD' or 'git bisect stop', before starting whatever you need
to do to make that decision.

But I do not know how it hurts to still have bisect states around, in
order to find where you want to go next.  Could you elaborate?

But your explanation "I often don't even remember" makes sense to me.

I would understand it, if not agreeing that I also am often in that
situation myself", if somebody does not even care which commit he was on
before starting the bisection, but knows (or is willing to decide at that
point) which branch (or even a specific commit, while still being
detached) he wants to switch to.  And it would make sense to avoid an
extra checkout that snaps back to the pre-bisection commit before
switching to the new state he has chosen.

So in that sense, I would agree with your original patch more than I would
agree with what you suggested as an alternative (i.e. "git bisect stop"
which is what "git bisect reset HEAD" would do if we do not verify the
argument is the name of an existing branch) in your response.

I am inclined to ask you to come up with a paragraph in the documentation
to discuss how the optional <branch> (now it will be <commit>) parameter
to the reset subcommand is meant to be used and re-submit the original
patch, perhaps with an updated commit log message.  "Allow resetting to
any" said only what the patch does, without saying why such a mode of
operation was even a good thing to begin with.

Thanks.

Junio C Hamano schrieb:

I even think that the support for an explicit branch name in the reset
subcommand should eventually be deprecated, perhaps unless it matches what
is stored in BISECT_START.

Goodness, NO!

The documentation, does not even talk about what the optional <branch>
argument is good for, even though it lists the optional <branch> in the
synopsis section.

If I had know about this feature (yes, FEATURE), I would have used it like
this in the past:

  $ git branch tmp
  $ git bisect reset tmp
  $ git branch -d tmp

With the patch proposed by Anders this shortens to:

  $ git bisect reset HEAD

Having said all that, four years and two months are looooooong time in git
timescale, and I am discounting, without any evidence to judge either way,
the possibility that people may have learned during that time to (ab)use
this as a (very useful?) shortcut to finish the current bisection and
switch to some entirely different branch.

In all the bisect runs that I have done in my live, the ONLY way I wanted
'bisect reset' to act was to NOT change the commit it currently was on.
The fact that it switched back to the commit or branch that the bisect was
started on, was always a major inconvenience.

So, I have no problem if you want to deprecate the branch parameter, if at
the same time bisect reset no longer switches to some other commit. ;)

I offhand do not see a good rationale for such a shortcut to finish bisect
and switch to another branch (IOW, I understand "it is shorter to type",
but I do not see "it is often done and very useful"), but I am open to be
enlightened by a workflow where such a shortcut is useful.

In my workflow, after I've found the bad commit, I always want bisect to
stay at the commit that it found. I don't want it to warp me somewhere
else; I want to make the decision where to go next myself.

-- Hannes

Junio C Hamano schrieb:

I would understand it, if not agreeing that I also am often in that
situation myself", if somebody does not even care which commit he was on
before starting the bisection, but knows (or is willing to decide at that
point) which branch (or even a specific commit, while still being
detached) he wants to switch to.  And it would make sense to avoid an
extra checkout that snaps back to the pre-bisection commit before
switching to the new state he has chosen.

The situation that I'm faced quite frequently is that after I find a
regression, I cannot tell which released version did not have the
breakage. Hence, the first thing I have to do is to find a good commit.
Therefore, I jump around in ancient history until I find a good commit.
Then I start bisect. I certainly do NOT want to be warped back to this
ancient commit by 'bisect reset'.

-- Hannes

On Mon, 12 Oct 2009, Junio C Hamano wrote:

But I do not know how it hurts to still have bisect states around, in
order to find where you want to go next.  Could you elaborate?

Mostly little irritations.  Extra bisect/* refs show up in gitk.  If you 
use __git_ps1 in your prompt (from git-completion.bash), it adds 
|BISECTING to your prompt.

Also, I just noticed that if you start a new bisection without ever 
cleaning up the old one, the next ‘git bisect reset’ will bring you back 
to HEAD before the old bisection instead of HEAD before the new one, which 
is not what you would expect if you forgot that the old bisection ever 
happened.

I am inclined to ask you to come up with a paragraph in the 
documentation to discuss how the optional <branch> (now it will be 
<commit>) parameter to the reset subcommand is meant to be used and 
re-submit the original patch, perhaps with an updated commit log 
message.

I note that the ‘git checkout’ documentation mentions <branch> and not 
<commit>, perhaps to emphasize that HEAD will become attached to the 
branch if you specify a branch name.  Do you think it makes sense for 
these to be documented differently?

Anders

‘git bisect reset’ accepts an optional argument specifying a branch to
check out after cleaning up the bisection state.  This lets you
specify an arbitrary commit.

In particular, this provides a way to clean the bisection state
without moving HEAD: ‘git bisect reset HEAD’.  This may be useful if
you are not interested in the state before you began a bisect,
especially if checking out the old commit would be expensive and
invalidate most of your compiled tree.

Clarify the ‘git bisect reset’ documentation to explain this optional
argument, which was previously mentioned only in the usage message.

Signed-off-by: Anders Kaseorg <redacted>
---
 Documentation/git-bisect.txt |   23 +++++++++++++++++------
 git-bisect.sh                |    7 +++----
 2 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
index 63e7a42..d2ffae0 100644
--- a/Documentation/git-bisect.txt
+++ b/Documentation/git-bisect.txt

@@ -20,7 +20,7 @@ on the subcommand:
  git bisect bad [<rev>]
  git bisect good [<rev>...]
  git bisect skip [(<rev>|<range>)...]
- git bisect reset [<branch>]
+ git bisect reset [<commit>]
  git bisect visualize
  git bisect replay <logfile>
  git bisect log

@@ -81,16 +81,27 @@ will have been left with the first bad kernel revision in "refs/bisect/bad".
 Bisect reset
 ~~~~~~~~~~~~
 
-To return to the original head after a bisect session, issue the
-following command:
+After a bisect session, to clean up the bisection state and return to
+the original HEAD, issue the following command:
 
 ------------------------------------------------
 $ git bisect reset
 ------------------------------------------------
 
-This resets the tree to the original branch instead of being on the
-bisection commit ("git bisect start" will also do that, as it resets
-the bisection state).
+By default, this will return your tree to the commit that was checked
+out before `git bisect start`.  (A new `git bisect start` will also do
+that, as it cleans up the old bisection state.)
+
+With an optional argument, you can return to a different commit
+instead:
+
+------------------------------------------------
+$ git bisect reset <commit>
+------------------------------------------------
+
+For example, `git bisect reset HEAD` will leave you on the current
+bisection commit and avoid switching commits at all, while `git bisect
+reset bisect/bad` will check out the first bad revision.
 
 Bisect visualize
 ~~~~~~~~~~~~~~~~

diff --git a/git-bisect.sh b/git-bisect.sh
index 6f6f039..0c56c26 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh

@@ -13,8 +13,8 @@ git bisect skip [(<rev>|<range>)...]
         mark <rev>... untestable revisions.
 git bisect next
         find next bisection to test and check it out.
-git bisect reset [<branch>]
-        finish bisection search and go back to branch.
+git bisect reset [<commit>]
+        finish bisection search and go back to commit.
 git bisect visualize
         show bisect status in gitk.
 git bisect replay <logfile>

@@ -311,8 +311,7 @@ bisect_reset() {
 	}
 	case "$#" in
 	0) branch=$(cat "$GIT_DIR/BISECT_START") ;;
-	1) git show-ref --verify --quiet -- "refs/heads/$1" ||
-	       die "$1 does not seem to be a valid branch"
+	1) git rev-parse --verify "$1^{commit}" || exit
 	   branch="$1" ;;
 	*)
 	    usage ;;

-- 
1.6.5

On Tuesday 13 October 2009, Anders Kaseorg wrote:

I agree with the purpose of the patch but I think something like the 
following would be better:

@@ -311,8 +311,8 @@ bisect_reset() {
        }
        case "$#" in
        0) branch=$(cat "$GIT_DIR/BISECT_START") ;;
-       1) git show-ref --verify --quiet -- "refs/heads/$1" ||
-              die "$1 does not seem to be a valid branch"
+       1) git rev-parse --quiet --verify "$1^{commit}" > /dev/null ||
+               die "'$1' does not seem to point to a valid commit"
           branch="$1" ;;
        *)
            usage ;;

It would give a better error message when "git rev-parse" fails instead of:

fatal: Needed a single revision

and it would not print the SHA1 from "$1^{commit}" when "git rev-parse" 
succeeds.

Best regards,
Christian.

On Tue, 13 Oct 2009, Christian Couder wrote:

+       1) git rev-parse --quiet --verify "$1^{commit}" > /dev/null ||
+               die "'$1' does not seem to point to a valid commit"

It would give a better error message when "git rev-parse" fails instead of:

fatal: Needed a single revision

and it would not print the SHA1 from "$1^{commit}" when "git rev-parse" 
succeeds.

Oh, oops, I somehow lost the > /dev/null in my version.

But as for the ‘git rev-parse’ error being confusing, why don’t we fix 
‘git rev-parse’ instead?

Anders

On Tuesday 13 October 2009, Anders Kaseorg wrote:

On Tue, 13 Oct 2009, Christian Couder wrote:

quoted

+       1) git rev-parse --quiet --verify "$1^{commit}" > /dev/null ||
+               die "'$1' does not seem to point to a valid commit"

It would give a better error message when "git rev-parse" fails instead
of:

fatal: Needed a single revision

and it would not print the SHA1 from "$1^{commit}" when "git rev-parse"
succeeds.

Oh, oops, I somehow lost the > /dev/null in my version.

But as for the ‘git rev-parse’ error being confusing, why don’t we fix
‘git rev-parse’ instead?

It's a plumbing command, and the output of plumbing commands is not supposed 
to change a lot as some scripts may rely on it.

And anyway that would be in another patch.

Best regards,
Christian.

‘git bisect reset’ accepts an optional argument specifying a branch to
check out after cleaning up the bisection state.  This lets you
specify an arbitrary commit.

In particular, this provides a way to clean the bisection state
without moving HEAD: ‘git bisect reset HEAD’.  This may be useful if
you are not interested in the state before you began a bisect,
especially if checking out the old commit would be expensive and
invalidate most of your compiled tree.

Clarify the ‘git bisect reset’ documentation to explain this optional
argument, which was previously mentioned only in the usage message.

Signed-off-by: Anders Kaseorg <redacted>
---
 Documentation/git-bisect.txt |   23 +++++++++++++++++------
 git-bisect.sh                |    8 ++++----
 2 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
index 63e7a42..d2ffae0 100644
--- a/Documentation/git-bisect.txt
+++ b/Documentation/git-bisect.txt

@@ -20,7 +20,7 @@ on the subcommand:
  git bisect bad [<rev>]
  git bisect good [<rev>...]
  git bisect skip [(<rev>|<range>)...]
- git bisect reset [<branch>]
+ git bisect reset [<commit>]
  git bisect visualize
  git bisect replay <logfile>
  git bisect log

@@ -81,16 +81,27 @@ will have been left with the first bad kernel revision in "refs/bisect/bad".
 Bisect reset
 ~~~~~~~~~~~~
 
-To return to the original head after a bisect session, issue the
-following command:
+After a bisect session, to clean up the bisection state and return to
+the original HEAD, issue the following command:
 
 ------------------------------------------------
 $ git bisect reset
 ------------------------------------------------
 
-This resets the tree to the original branch instead of being on the
-bisection commit ("git bisect start" will also do that, as it resets
-the bisection state).
+By default, this will return your tree to the commit that was checked
+out before `git bisect start`.  (A new `git bisect start` will also do
+that, as it cleans up the old bisection state.)
+
+With an optional argument, you can return to a different commit
+instead:
+
+------------------------------------------------
+$ git bisect reset <commit>
+------------------------------------------------
+
+For example, `git bisect reset HEAD` will leave you on the current
+bisection commit and avoid switching commits at all, while `git bisect
+reset bisect/bad` will check out the first bad revision.
 
 Bisect visualize
 ~~~~~~~~~~~~~~~~

diff --git a/git-bisect.sh b/git-bisect.sh
index 6f6f039..8b3c585 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh

@@ -13,8 +13,8 @@ git bisect skip [(<rev>|<range>)...]
         mark <rev>... untestable revisions.
 git bisect next
         find next bisection to test and check it out.
-git bisect reset [<branch>]
-        finish bisection search and go back to branch.
+git bisect reset [<commit>]
+        finish bisection search and go back to commit.
 git bisect visualize
         show bisect status in gitk.
 git bisect replay <logfile>

@@ -311,8 +311,8 @@ bisect_reset() {
 	}
 	case "$#" in
 	0) branch=$(cat "$GIT_DIR/BISECT_START") ;;
-	1) git show-ref --verify --quiet -- "refs/heads/$1" ||
-	       die "$1 does not seem to be a valid branch"
+	1) git rev-parse --quiet --verify "$1^{commit}" > /dev/null ||
+	       die "'$1' is not a valid commit"
 	   branch="$1" ;;
 	*)
 	    usage ;;

-- 
1.6.5

Thanks; will replace your v2 that was queued in 'pu'.