threads / patch / 32870

patchgit-bisect.txt: clarify that reset finishes bisect

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

## tl;dr

9 messages between Feb 9, 2013 and Feb 11, 2013. Diffs are folded; open one to read it.

replies: 8people: 4as markdown or json

Michael J Gruber· Feb 9, 2013, 19:47 UTC · lore

"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(-)
Show changes to Documentation/git-bisect.txt +1 −1
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
Junio C Hamano· Feb 9, 2013, 21:53 UTC · re: Michael J Gruber · lore

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

Michael J Gruber <git@drmicha.warpmail.net> writes:
Show 20 quoted lines
> "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.
Andreas Mohr· Feb 9, 2013, 23:24 UTC · re: Junio C Hamano · lore

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

Hi,
On Sat, Feb 09, 2013 at 01:53:04PM -0800, Junio C Hamano wrote:
Show 6 quoted lines
> 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
Junio C Hamano· Feb 9, 2013, 23:35 UTC · re: Andreas Mohr · lore

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

Andreas Mohr <andi@lisas.de> writes:
Show 14 quoted lines
> 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.

Andreas Mohr· Feb 10, 2013, 00:23 UTC · re: Junio C Hamano · lore

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

On Sat, Feb 09, 2013 at 03:35:35PM -0800, Junio C Hamano wrote:
Show 10 quoted lines
> 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
Jonathan Nieder· Feb 10, 2013, 01:49 UTC · re: Michael J Gruber · lore

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

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?

Show changes to diff +48 −0
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
 +
 ------------
Michael J Gruber· Feb 11, 2013, 08:04 UTC · re: Jonathan Nieder · lore

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

Jonathan Nieder venit, vidit, dixit 10.02.2013 02:49:
Show 85 quoted lines
> 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
Michael J Gruber· Feb 11, 2013, 08:35 UTC · re: Michael J Gruber · lore

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

"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(-)
Show changes to Documentation/git-bisect.txt +6 −1
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
Jonathan Nieder· Feb 11, 2013, 08:39 UTC · re: Michael J Gruber · lore

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

Michael J Gruber wrote:
> --- a/Documentation/git-bisect.txt
> +++ b/Documentation/git-bisect.txt
[...]
Show 5 quoted lines
> @@ -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

← back to recent threads