[PATCH] git-bisect.txt: clarify that reset finishes bisect

[PATCH] git-bisect.txt: clarify that reset finishes bisect

[Michael J Gruber]

"reset" can be easily misunderstood as resetting a bisect session to its
start without finishing it. Clarify that it actually finishes the bisect
session.

Reported-by: Andreas Mohr <andi@lisas.de>
Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
 Documentation/git-bisect.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
index b4831bb..d50bd89 100644
--- a/Documentation/git-bisect.txt
+++ b/Documentation/git-bisect.txt
@@ -83,7 +83,7 @@ Bisect reset
 ~~~~~~~~~~~~
 
 After a bisect session, to clean up the bisection state and return to
-the original HEAD, issue the following command:
+the original HEAD (i.e., to finish bisect), issue the following command:
 
 ------------------------------------------------
 $ git bisect reset
-- 
1.8.1.2.752.g32d147e

Re: [PATCH] git-bisect.txt: clarify that reset finishes bisect

[Junio C Hamano]

Michael J Gruber <git@drmicha.warpmail.net> writes:

> "reset" can be easily misunderstood as resetting a bisect session to its
> start without finishing it. Clarify that it actually finishes the bisect
> session.
>
> Reported-by: Andreas Mohr <andi@lisas.de>
> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
> ---
>  Documentation/git-bisect.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
> index b4831bb..d50bd89 100644
> --- a/Documentation/git-bisect.txt
> +++ b/Documentation/git-bisect.txt
> @@ -83,7 +83,7 @@ Bisect reset
>  ~~~~~~~~~~~~
>  
>  After a bisect session, to clean up the bisection state and return to
> -the original HEAD, issue the following command:
> +the original HEAD (i.e., to finish bisect), issue the following command:

Makes sense.

Re: [PATCH] git-bisect.txt: clarify that reset finishes bisect

[Andreas Mohr]

Hi,

On Sat, Feb 09, 2013 at 01:53:04PM -0800, Junio C Hamano wrote:
> Michael J Gruber <git@drmicha.warpmail.net> writes:
> >  After a bisect session, to clean up the bisection state and return to
> > -the original HEAD, issue the following command:
> > +the original HEAD (i.e., to finish bisect), issue the following command:
> 
> Makes sense.

Doesn't ;)

[aww, very sorry for this blunt reply]

The main point of my mail was to stretch the (whether actually intended)
*perceived* start <-> stop symmetry
(which a *user* quite likely would end up searching for in the document,
and fail to find any "stop" keyword, thus not getting to the relevant
reset parameter section in time).
Quite likely I failed to properly word things to make that obvious,
in my quickly carved mail.


To clarify intent behind these docs, I would thus propose to have
the sentence improved to something quite similar to

the original HEAD (i.e., to finish - "stop" a started - bisect),
issue the following command:


Thanks a ton for your immediate handling of my wishlist item!

Andreas Mohr

Re: [PATCH] git-bisect.txt: clarify that reset finishes bisect

[Junio C Hamano]

Andreas Mohr <andi@lisas.de> writes:

> On Sat, Feb 09, 2013 at 01:53:04PM -0800, Junio C Hamano wrote:
>> Michael J Gruber <git@drmicha.warpmail.net> writes:
>> >  After a bisect session, to clean up the bisection state and return to
>> > -the original HEAD, issue the following command:
>> > +the original HEAD (i.e., to finish bisect), issue the following command:
>> 
>> Makes sense.
>
> Doesn't ;)
>
> [aww, very sorry for this blunt reply]
>
> The main point of my mail was to stretch the (whether actually intended)
> *perceived* start <-> stop symmetry

Actually, in that sense, I do no think finish is exactly a good
wording.  The majority of use case would be to finish up after you
found the sole culprit, so in that sense "finish" is not too bad,
but in general, when you "reset", there is not necessarily any
symmmetry with "start".  We should definitely not be giving you an
illusion that there is one by using "stop" [*1*].

It is more like "abort".

I may be done with bisection after running the bisection to the very
end, of I may have realized that the problem is not bisectable due
to many reasons (e.g. the sympotom may be intermittent, or it has
already become apparent that there are more than one cause of the
observed breakage) way before we found "the first bad commit".

And "reset" cleans the bisectoin state and returns to the original
HEAD, regardless of the reason why you are cleaning up.  If we were
to add any explanation to the sentence, I think "finish" makes a lot
more sense than "stop".


[Footnote]

*1* another reason to avoid "stop" is that it could mean "I stop
here for now, to later come back and start digging again from
there", which is not "reset" is about at all.

Re: [PATCH] git-bisect.txt: clarify that reset finishes bisect

[Andreas Mohr]

On Sat, Feb 09, 2013 at 03:35:35PM -0800, Junio C Hamano wrote:
> Andreas Mohr <andi@lisas.de> writes:
> > The main point of my mail was to stretch the (whether actually intended)
> > *perceived* start <-> stop symmetry
> 
> Actually, in that sense, I do no think finish is exactly a good
> wording.  The majority of use case would be to finish up after you
> found the sole culprit, so in that sense "finish" is not too bad,
> but in general, when you "reset", there is not necessarily any
> symmmetry with "start".  We should definitely not be giving you an
> illusion that there is one by using "stop" [*1*].

Yeah, of course, but the whole point was that the *user* will *directly*
and *immediately* mentally go from a bisect "start" parameter to "stop",
and end up failing to find it.
Thus the "stop" *KEYWORD* *has* to be mentioned in the man page
(even if only to then expressly state that reset is not quite about "stop"iing
things).

> *1* another reason to avoid "stop" is that it could mean "I stop
> here for now, to later come back and start digging again from
> there", which is not "reset" is about at all.

Such reasons for not listing "stop" might even be stronger
than the reason of
users being required to successfully look up the symmetric "stop" keyword.
If that was decided to be the case, then "stop" would indeed have to be
omitted in the man page - to the detriments of usability.

Thanks,

Andreas Mohr

Re: [PATCH] git-bisect.txt: clarify that reset finishes bisect

[Jonathan Nieder]

Hi,

Michael J Gruber wrote:

> "reset" can be easily misunderstood as resetting a bisect session to its
> start without finishing it. Clarify that it actually finishes the bisect
> session.

FWIW,
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

Addressing Andreas's original concern about the discoverability of
'git bisect reset' would presumably require doing two more things:

 1. adding an example of the normal bisection workflow to the EXAMPLES
    section

 2. training users to look to the EXAMPLES section

That is, something like the below.  But I'm not happy with it, because
it just runs over the same material as the current Description
section.  Maybe the current tutorial material could be moved to
examples and replaced with something terser that fleshes out the
descriptions in "git bisect -h" output.  What do you think?

diff --git i/Documentation/git-bisect.txt w/Documentation/git-bisect.txt
index e4f46bc1..b89abd78 100644
--- i/Documentation/git-bisect.txt
+++ w/Documentation/git-bisect.txt
@@ -356,6 +356,54 @@ $ git bisect run sh -c "make || exit 125; ~/check_test_case.sh"
 This shows that you can do without a run script if you write the test
 on a single line.
 
+* Bisect to find which patch caused a boot failure:
++
+Install a recent kernel:
++
+------------
+$ cd ~/src/linux
+$ git checkout origin/master
+$ make deb-pkg # or binrpm-pkg, or tar-pkg
+$ dpkg -i ../<name of package> # as root (or rpm -i, or tar -C / -xf)
+$ reboot # as root
+------------
++
+Hopefully it fails to boot, so tell git so and begin bisection:
++
+------------
+$ cd ~/src/linux
+$ git bisect start HEAD v3.2 # assuming 3.2 works fine
+-------------
++
+A candidate revision to test is automatically checked out.
+Test it:
++
+-------------
+$ make deb-pkg # or binrpm-pkg, or tar-pkg
+$ dpkg -i ../<name of package> # as root (or rpm -i, or tar -C / -xf)
+$ reboot # as root
+-------------
++
+Record the result:
++
+-------------
+$ cd ~/src/linux
+$ git bisect good # if it booted correctly
+$ git bisect bad # if it failed to boot
+$ git bisect skip # if some other bug made it hard to test
+-------------
++
+Repeat until bored or git prints the "first bad commit".  When
+done:
++
+-------------
+$ git bisect log >log # let others pick up where you left off
+$ git bisect reset HEAD # exit the bisecting state
+-------------
++
+At any step, you can run `git bisect visualize` to watch the
+regression range narrowing.
+
 * Locate a good region of the object graph in a damaged repository
 +
 ------------

Re: [PATCH] git-bisect.txt: clarify that reset finishes bisect

[Michael J Gruber]

Jonathan Nieder venit, vidit, dixit 10.02.2013 02:49:
> Hi,
> 
> Michael J Gruber wrote:
> 
>> "reset" can be easily misunderstood as resetting a bisect session to its
>> start without finishing it. Clarify that it actually finishes the bisect
>> session.
> 
> FWIW,
> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
> 
> Addressing Andreas's original concern about the discoverability of
> 'git bisect reset' would presumably require doing two more things:
> 
>  1. adding an example of the normal bisection workflow to the EXAMPLES
>     section
> 
>  2. training users to look to the EXAMPLES section
> 
> That is, something like the below.  But I'm not happy with it, because
> it just runs over the same material as the current Description
> section.  Maybe the current tutorial material could be moved to
> examples and replaced with something terser that fleshes out the
> descriptions in "git bisect -h" output.  What do you think?
> 
> diff --git i/Documentation/git-bisect.txt w/Documentation/git-bisect.txt
> index e4f46bc1..b89abd78 100644
> --- i/Documentation/git-bisect.txt
> +++ w/Documentation/git-bisect.txt
> @@ -356,6 +356,54 @@ $ git bisect run sh -c "make || exit 125; ~/check_test_case.sh"
>  This shows that you can do without a run script if you write the test
>  on a single line.
>  
> +* Bisect to find which patch caused a boot failure:
> ++
> +Install a recent kernel:
> ++
> +------------
> +$ cd ~/src/linux
> +$ git checkout origin/master
> +$ make deb-pkg # or binrpm-pkg, or tar-pkg
> +$ dpkg -i ../<name of package> # as root (or rpm -i, or tar -C / -xf)
> +$ reboot # as root
> +------------
> ++
> +Hopefully it fails to boot, so tell git so and begin bisection:
> ++
> +------------
> +$ cd ~/src/linux
> +$ git bisect start HEAD v3.2 # assuming 3.2 works fine
> +-------------
> ++
> +A candidate revision to test is automatically checked out.
> +Test it:
> ++
> +-------------
> +$ make deb-pkg # or binrpm-pkg, or tar-pkg
> +$ dpkg -i ../<name of package> # as root (or rpm -i, or tar -C / -xf)
> +$ reboot # as root
> +-------------
> ++
> +Record the result:
> ++
> +-------------
> +$ cd ~/src/linux
> +$ git bisect good # if it booted correctly
> +$ git bisect bad # if it failed to boot
> +$ git bisect skip # if some other bug made it hard to test
> +-------------
> ++
> +Repeat until bored or git prints the "first bad commit".  When
> +done:
> ++
> +-------------
> +$ git bisect log >log # let others pick up where you left off
> +$ git bisect reset HEAD # exit the bisecting state
> +-------------
> ++
> +At any step, you can run `git bisect visualize` to watch the
> +regression range narrowing.
> +
>  * Locate a good region of the object graph in a damaged repository
>  +
>  ------------
> 

[BTW, sorry for failing to set --in-reply-to in the patch e-mail. Need
to get that automated somehow.]

I did not use "stop" for the exact reasons that Junio mentioned. Just
throw in a third alternative: "quit" may convey that it's not possible
to ressume, without sounding as "exceptional" as "abort" does. After
all, it's the normal end to a bisect session much unlike "--abort" for
rebase, for example.

As for the example, we have an example section, and we could simply
throw in "git reset" lines there. I would even amend my patch with that;
the great git-bisect.txt refactoring I'd definitely leave to someone else.

Michael

[PATCHv2] git-bisect.txt: clarify that reset quits bisect

[Michael J Gruber]

"reset" can be easily misunderstood as resetting a bisect session to its
start without finishing it. Clarify that it actually quits the bisect
session.

Reported-by: Andreas Mohr <andi@lisas.de>
Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
 Documentation/git-bisect.txt | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
index b4831bb..f986c5c 100644
--- a/Documentation/git-bisect.txt
+++ b/Documentation/git-bisect.txt
@@ -83,7 +83,7 @@ Bisect reset
 ~~~~~~~~~~~~
 
 After a bisect session, to clean up the bisection state and return to
-the original HEAD, issue the following command:
+the original HEAD (i.e., to quit bisecting), issue the following command:
 
 ------------------------------------------------
 $ git bisect reset
@@ -284,6 +284,7 @@ EXAMPLES
 ------------
 $ git bisect start HEAD v1.2 --      # HEAD is bad, v1.2 is good
 $ git bisect run make                # "make" builds the app
+$ git bisect reset                   # quit the bisect session
 ------------
 
 * Automatically bisect a test failure between origin and HEAD:
@@ -291,6 +292,7 @@ $ git bisect run make                # "make" builds the app
 ------------
 $ git bisect start HEAD origin --    # HEAD is bad, origin is good
 $ git bisect run make test           # "make test" builds and tests
+$ git bisect reset                   # quit the bisect session
 ------------
 
 * Automatically bisect a broken test case:
@@ -302,6 +304,7 @@ make || exit 125                     # this skips broken builds
 ~/check_test_case.sh                 # does the test case pass?
 $ git bisect start HEAD HEAD~10 --   # culprit is among the last 10
 $ git bisect run ~/test.sh
+$ git bisect reset                   # quit the bisect session
 ------------
 +
 Here we use a "test.sh" custom script. In this script, if "make"
@@ -351,6 +354,7 @@ use `git cherry-pick` instead of `git merge`.)
 ------------
 $ git bisect start HEAD HEAD~10 --   # culprit is among the last 10
 $ git bisect run sh -c "make || exit 125; ~/check_test_case.sh"
+$ git bisect reset                   # quit the bisect session
 ------------
 +
 This shows that you can do without a run script if you write the test
@@ -368,6 +372,7 @@ $ git bisect run sh -c '
 	rm -f tmp.$$
 	test $rc = 0'
 
+$ git bisect reset                   # quit the bisect session
 ------------
 +
 In this case, when 'git bisect run' finishes, bisect/bad will refer to a commit that
-- 
1.8.1.2.752.g32d147e

Re: [PATCHv2] git-bisect.txt: clarify that reset quits bisect

[Jonathan Nieder]

Michael J Gruber wrote:

> --- a/Documentation/git-bisect.txt
> +++ b/Documentation/git-bisect.txt
[...]
> @@ -284,6 +284,7 @@ EXAMPLES
>  ------------
>  $ git bisect start HEAD v1.2 --      # HEAD is bad, v1.2 is good
>  $ git bisect run make                # "make" builds the app
> +$ git bisect reset                   # quit the bisect session

I had forgotten that "git bisect run" ends in a bisecting state.
Good call.

Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

Thanks,
Jonathan