All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] bisect reset: Allow resetting to any commit, not just a branch
@ 2009-10-12 16:38 Anders Kaseorg
  2009-10-12 21:06 ` Junio C Hamano
  0 siblings, 1 reply; 18+ messages in thread
From: Anders Kaseorg @ 2009-10-12 16:38 UTC (permalink / raw)
  To: gister; +Cc: git

‘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 <andersk@mit.edu>
---
 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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH] bisect reset: Allow resetting to any commit, not just a branch
  2009-10-12 16:38 [PATCH] bisect reset: Allow resetting to any commit, not just a branch Anders Kaseorg
@ 2009-10-12 21:06 ` Junio C Hamano
  2009-10-12 21:31   ` Anders Kaseorg
  2009-10-13  6:39   ` [PATCH] " Johannes Sixt
  0 siblings, 2 replies; 18+ messages in thread
From: Junio C Hamano @ 2009-10-12 21:06 UTC (permalink / raw)
  To: Anders Kaseorg; +Cc: git

Anders Kaseorg <andersk@MIT.EDU> writes:

> ‘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 <andersk@mit.edu>
> ---
>  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 ;;

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.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] bisect reset: Allow resetting to any commit, not just a branch
  2009-10-12 21:06 ` Junio C Hamano
@ 2009-10-12 21:31   ` Anders Kaseorg
  2009-10-12 23:41     ` Junio C Hamano
  2009-10-13  6:39   ` [PATCH] " Johannes Sixt
  1 sibling, 1 reply; 18+ messages in thread
From: Anders Kaseorg @ 2009-10-12 21:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] bisect reset: Allow resetting to any commit, not just a branch
  2009-10-12 21:31   ` Anders Kaseorg
@ 2009-10-12 23:41     ` Junio C Hamano
  2009-10-13  6:45       ` Johannes Sixt
                         ` (2 more replies)
  0 siblings, 3 replies; 18+ messages in thread
From: Junio C Hamano @ 2009-10-12 23:41 UTC (permalink / raw)
  To: Anders Kaseorg; +Cc: git

Anders Kaseorg <andersk@MIT.EDU> 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.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] bisect reset: Allow resetting to any commit, not just a   branch
  2009-10-12 21:06 ` Junio C Hamano
  2009-10-12 21:31   ` Anders Kaseorg
@ 2009-10-13  6:39   ` Johannes Sixt
  2009-10-13  7:01     ` Junio C Hamano
  1 sibling, 1 reply; 18+ messages in thread
From: Johannes Sixt @ 2009-10-13  6:39 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Anders Kaseorg, git

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] bisect reset: Allow resetting to any commit, not just a   branch
  2009-10-12 23:41     ` Junio C Hamano
@ 2009-10-13  6:45       ` Johannes Sixt
  2009-10-13  7:21         ` Junio C Hamano
  2009-10-13  7:03       ` Anders Kaseorg
  2009-10-13 15:22       ` [PATCH v2] " Anders Kaseorg
  2 siblings, 1 reply; 18+ messages in thread
From: Johannes Sixt @ 2009-10-13  6:45 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Anders Kaseorg, git

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] bisect reset: Allow resetting to any commit, not just a  branch
  2009-10-13  6:39   ` [PATCH] " Johannes Sixt
@ 2009-10-13  7:01     ` Junio C Hamano
  2009-10-13  7:45       ` Johannes Sixt
  0 siblings, 1 reply; 18+ messages in thread
From: Junio C Hamano @ 2009-10-13  7:01 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Anders Kaseorg, git

Johannes Sixt <j.sixt@viscovery.net> writes:

> 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.

Are you sure about what you are saying?

Half of the time, the commit you test in your "git bisect" section would
be a "good" one, and immediately after you tell it "bisect good", it tells
you that some _other_ commit you marked "bad" is the first bad commit.  In
such a case, you won't be on the commit that the bisect has found.

So I _do_ agree that you would always want to stare at the commit that is
the first bad one, leaving the bisection session at the detached HEAD
state bisection session ends at is _totally_ different from what you want.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] bisect reset: Allow resetting to any commit, not just a branch
  2009-10-12 23:41     ` Junio C Hamano
  2009-10-13  6:45       ` Johannes Sixt
@ 2009-10-13  7:03       ` Anders Kaseorg
  2009-10-13 15:22       ` [PATCH v2] " Anders Kaseorg
  2 siblings, 0 replies; 18+ messages in thread
From: Anders Kaseorg @ 2009-10-13  7:03 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] bisect reset: Allow resetting to any commit, not just a  branch
  2009-10-13  6:45       ` Johannes Sixt
@ 2009-10-13  7:21         ` Junio C Hamano
  0 siblings, 0 replies; 18+ messages in thread
From: Junio C Hamano @ 2009-10-13  7:21 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Junio C Hamano, Anders Kaseorg, git

Johannes Sixt <j.sixt@viscovery.net> writes:

> 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'.

Unlike your other message, now, I can see _this_ one making sense very
much.

It is a very good explanation as to why BISECT_START (whose sole purpose
is to go back there) is not a very useful concept.  What you wrote deserve
to go to the "bisect reset" documentation to explain what the optional
<branch> argument (perhaps we would make it a <commit> with Anders's
patch) is good for and how it is intended to be used.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] bisect reset: Allow resetting to any commit, not just a   branch
  2009-10-13  7:01     ` Junio C Hamano
@ 2009-10-13  7:45       ` Johannes Sixt
  2009-10-13  8:33         ` Junio C Hamano
  0 siblings, 1 reply; 18+ messages in thread
From: Johannes Sixt @ 2009-10-13  7:45 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Anders Kaseorg, git

Junio C Hamano schrieb:
> Half of the time, the commit you test in your "git bisect" section would
> be a "good" one, and immediately after you tell it "bisect good", it tells
> you that some _other_ commit you marked "bad" is the first bad commit.  In
> such a case, you won't be on the commit that the bisect has found.

Oh, yes, very true; but it is very close. But the commit that git bisect
reset warps me to is perhaps 1000 steps in history away. I certainly do
not want to go there, ever, because I want to go back near the bad commit
right away. (Think of fewer files changed means less build time.) If git
bisect reset would check out the bad commit, this would be *very* convenient.

-- Hannes

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] bisect reset: Allow resetting to any commit, not just a  branch
  2009-10-13  7:45       ` Johannes Sixt
@ 2009-10-13  8:33         ` Junio C Hamano
  2009-10-13 14:29           ` Anders Kaseorg
  0 siblings, 1 reply; 18+ messages in thread
From: Junio C Hamano @ 2009-10-13  8:33 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Anders Kaseorg, git

Johannes Sixt <j.sixt@viscovery.net> writes:

> Junio C Hamano schrieb:
>> Half of the time, the commit you test in your "git bisect" section would
>> be a "good" one, and immediately after you tell it "bisect good", it tells
>> you that some _other_ commit you marked "bad" is the first bad commit.  In
>> such a case, you won't be on the commit that the bisect has found.
>
> Oh, yes, very true; but it is very close. But the commit that git bisect
> reset warps me to is perhaps 1000 steps in history away. I certainly do
> not want to go there, ever, because I want to go back near the bad commit
> right away. (Think of fewer files changed means less build time.) If git
> bisect reset would check out the bad commit, this would be *very* convenient.

I agree that "git bisect reset-and-detach-at-the-first-bad-one" would make
a lot of sense.

In my workflow, after I chased a bug in frotz, I often do

    $ git name-rev $the_bad_one_that_was_found
    
to learn what was the first tagged release that has the bug, and create a
topic from there:

    $ git checkout -b jc/maint-X.Y.Z-fix-frotz $the_bad_one_that_was_found

so that the fix I'd build on the commit can be merged initially to 'pu',
then 'next', then 'maint-X.Y.Z' and upwards to 'master', but all of that
is done after "git bisect reset" to switch back to the 'master' branch.
It is cumbersome to have to type (actually, I use the cut buffer in screen)
the commit object name of the first bad one twice.

It certainly sounds attractive if we can do:

    $ git bisect reset-and-detach-at-the-first-bad-one
    $ git name-rev HEAD
    $ git checkout -b jc/maint-X.Y.Z-fix-frotz
    $ hack hack hack
    $ git commit

But at that point we are not talking about switching to arbitrary commit
anymore.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] bisect reset: Allow resetting to any commit, not just a   branch
  2009-10-13  8:33         ` Junio C Hamano
@ 2009-10-13 14:29           ` Anders Kaseorg
  0 siblings, 0 replies; 18+ messages in thread
From: Anders Kaseorg @ 2009-10-13 14:29 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Sixt, git

On Tue, 13 Oct 2009, Junio C Hamano wrote:
> I agree that "git bisect reset-and-detach-at-the-first-bad-one" would make
> a lot of sense.

Under my patch, that’s ‘git bisect reset bisect/bad’.  Similar arguments 
may be made as to whether that should or shouldn’t be a separate command 
(although it’s less clear what to call it, in this case).

Anders

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2] bisect reset: Allow resetting to any commit, not just a branch
  2009-10-12 23:41     ` Junio C Hamano
  2009-10-13  6:45       ` Johannes Sixt
  2009-10-13  7:03       ` Anders Kaseorg
@ 2009-10-13 15:22       ` Anders Kaseorg
  2009-10-13 20:06         ` Christian Couder
  2 siblings, 1 reply; 18+ messages in thread
From: Anders Kaseorg @ 2009-10-13 15:22 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

‘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 <andersk@mit.edu>
---
 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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH v2] bisect reset: Allow resetting to any commit, not just a branch
  2009-10-13 15:22       ` [PATCH v2] " Anders Kaseorg
@ 2009-10-13 20:06         ` Christian Couder
  2009-10-13 20:09           ` Anders Kaseorg
  2009-10-13 21:02           ` [PATCH v3] " Anders Kaseorg
  0 siblings, 2 replies; 18+ messages in thread
From: Christian Couder @ 2009-10-13 20:06 UTC (permalink / raw)
  To: Anders Kaseorg; +Cc: Junio C Hamano, git

On Tuesday 13 October 2009, Anders Kaseorg wrote:
> @@ -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 ;;

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.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2] bisect reset: Allow resetting to any commit, not just a branch
  2009-10-13 20:06         ` Christian Couder
@ 2009-10-13 20:09           ` Anders Kaseorg
  2009-10-13 20:30             ` Christian Couder
  2009-10-13 21:02           ` [PATCH v3] " Anders Kaseorg
  1 sibling, 1 reply; 18+ messages in thread
From: Anders Kaseorg @ 2009-10-13 20:09 UTC (permalink / raw)
  To: Christian Couder; +Cc: Junio C Hamano, git

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2] bisect reset: Allow resetting to any commit, not just a branch
  2009-10-13 20:09           ` Anders Kaseorg
@ 2009-10-13 20:30             ` Christian Couder
  0 siblings, 0 replies; 18+ messages in thread
From: Christian Couder @ 2009-10-13 20:30 UTC (permalink / raw)
  To: Anders Kaseorg; +Cc: Junio C Hamano, git

On Tuesday 13 October 2009, Anders Kaseorg wrote:
> 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?

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.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v3] bisect reset: Allow resetting to any commit, not just a branch
  2009-10-13 20:06         ` Christian Couder
  2009-10-13 20:09           ` Anders Kaseorg
@ 2009-10-13 21:02           ` Anders Kaseorg
  2009-10-14  9:13             ` Junio C Hamano
  1 sibling, 1 reply; 18+ messages in thread
From: Anders Kaseorg @ 2009-10-13 21:02 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Christian Couder, git

‘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 <andersk@mit.edu>
---
 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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH v3] bisect reset: Allow resetting to any commit, not just a branch
  2009-10-13 21:02           ` [PATCH v3] " Anders Kaseorg
@ 2009-10-14  9:13             ` Junio C Hamano
  0 siblings, 0 replies; 18+ messages in thread
From: Junio C Hamano @ 2009-10-14  9:13 UTC (permalink / raw)
  To: Anders Kaseorg; +Cc: Christian Couder, git

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2009-10-14  9:14 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-10-12 16:38 [PATCH] bisect reset: Allow resetting to any commit, not just a branch Anders Kaseorg
2009-10-12 21:06 ` Junio C Hamano
2009-10-12 21:31   ` Anders Kaseorg
2009-10-12 23:41     ` Junio C Hamano
2009-10-13  6:45       ` Johannes Sixt
2009-10-13  7:21         ` Junio C Hamano
2009-10-13  7:03       ` Anders Kaseorg
2009-10-13 15:22       ` [PATCH v2] " Anders Kaseorg
2009-10-13 20:06         ` Christian Couder
2009-10-13 20:09           ` Anders Kaseorg
2009-10-13 20:30             ` Christian Couder
2009-10-13 21:02           ` [PATCH v3] " Anders Kaseorg
2009-10-14  9:13             ` Junio C Hamano
2009-10-13  6:39   ` [PATCH] " Johannes Sixt
2009-10-13  7:01     ` Junio C Hamano
2009-10-13  7:45       ` Johannes Sixt
2009-10-13  8:33         ` Junio C Hamano
2009-10-13 14:29           ` Anders Kaseorg

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.