Re: [PATCH v9 5/5] bisect: allow any terms set by user

2015-06-26 Thread Matthieu Moy
Junio C Hamano gits...@pobox.com writes:

 Matthieu Moy matthieu@imag.fr writes:

 +
 +git bisect terms term-new term-old
 +

 The mnemonic for git bisect start bad good is Bad comes before
 Good (B = 0x42, G = 0x47) and this is same for new/old, New comes
 before Old (N = 0x4e, O = 0x4f).  git bisect terms new old follows
 the same pattern, which is good.  Easy to remember.

 +This command has to be used before a bisection has started. term-old
 +must be associated with the latest revisions and term-new with the
 +ancestors of term-old.

 Whoa?  This gets new and old mixed up, doesn't it?

Right. I think it was already the case before, but using term1/term2 in
the text made it hard to spot. The new wording is clearer, and makes it
easier to find bugs in the explanations ;-).

 +Only the first bisection following the `git bisect terms` will use the
 +terms. If you mistyped one of the terms you can do again `git bisect
 +terms term-old term-new`.

 This is also the other way around, no?

Indeed.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v9 5/5] bisect: allow any terms set by user

2015-06-26 Thread Matthieu Moy
Junio C Hamano gits...@pobox.com writes:

 The second sentence may want to be something like

   If you mistyped one of the terms, you can do another git
   bisect terms term-new term-old to correct them, but
   that is possible only before you start the bisection.

Applied, thanks.

I currently have this in addition to v9 in my branch. I'll resend later
(https://github.com/moy/git/tree/bisect-terms is up to date).

diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
index e783f87..7609cd6 100644
--- a/Documentation/git-bisect.txt
+++ b/Documentation/git-bisect.txt
@@ -143,19 +143,19 @@ set your own terms.
 git bisect terms term-new term-old
 
 
-This command has to be used before a bisection has started. term-old
-must be associated with the latest revisions and term-new with the
-ancestors of term-old. For example, if something was buggy in the
+This command has to be used before a bisection has started. term-new
+must be associated with the latest revisions and term-old with some
+ancestors of term-new. For example, if something was buggy in the
 old part of the history, you know somewhere the bug was fixed, and you
 want to find the exact commit that fixed it, you may want to say `git
-bisect terms fixed broken`; this way, you would mark a commit that
+bisect terms broken fixed`; this way, you would mark a commit that
 still has the bug with `broken`, and a newer one after the fix with
 `fixed`.
 
-Only the first bisection following the `git bisect terms` will use the
+Only the bisection following the `git bisect terms` will use the
 terms. If you mistyped one of the terms you can do again `git bisect
-terms term-old term-new`.
-
+terms term-new term-old`, but that is possible only before you
+start the bisection.
 
 Bisect visualize
 
diff --git a/git-bisect.sh b/git-bisect.sh
index 8fee712..07c64d9 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -549,6 +549,20 @@ get_terms () {
fi
 }
 
+write_terms () {
+   NAME_BAD=$1
+   NAME_GOOD=$2
+   check_term_format $NAME_BAD
+   check_term_format $NAME_GOOD
+   printf '%s\n%s\n' $NAME_BAD $NAME_GOOD $GIT_DIR/BISECT_TERMS
+}
+
+check_term_format () {
+   term=$1
+   git check-ref-format refs/bisect/$term ||
+   die $(eval_gettext '\$term' is not a valid term)
+}
+
 check_and_set_terms () {
cmd=$1
case $cmd in
@@ -579,8 +593,8 @@ check_and_set_terms () {
 
 bisect_voc () {
case $1 in
-   bad) echo bad|old ;;
-   good) echo good|new ;;
+   bad) echo bad|new ;;
+   good) echo good|old ;;
esac
 }
 
@@ -611,20 +625,6 @@ Otherwise, to start a new bisection with new terms, please 
use
esac
 }
 
-write_terms () {
-   NAME_BAD=$1
-   NAME_GOOD=$2
-   check_term_format $NAME_BAD
-   check_term_format $NAME_GOOD
-   printf '%s\n%s\n' $NAME_BAD $NAME_GOOD $GIT_DIR/BISECT_TERMS
-}
-
-check_term_format () {
-   term=$1
-   git check-ref-format refs/bisect/$term ||
-   die $(eval_gettext '\$term' is not a valid term)
-}
-
 case $# in
 0)
usage ;;

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v9 5/5] bisect: allow any terms set by user

2015-06-26 Thread Junio C Hamano
Matthieu Moy matthieu@grenoble-inp.fr writes:

 This is currently how it's implemented. You need to say

 $ git bisect terms foo bar
 $ git bisect start

Ahh, it means everything including the description (i.e. you start
bisection) is consistent and perfectly fine.

I misread the patch, it seems.  Sorry for the noise.

Thanks.

 And indeed git bisect terms foo bar errors out. I think the reason it
 is this way is to allow

 $ git bisect terms foo bar
 $ git bisect start sha1 sha1

 But actually, we can allow git bisect terms until BISECT_TERMS is
 created, which is your intuition and makes more sense IMHO. I'll try to
 do that.

I am not sure if it is a good idea, though.  Matching the intuition
of those who (think they) know how it is implemented is much less
important than providing a behaviour that is simple to explain to
casual users.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v9 5/5] bisect: allow any terms set by user

2015-06-26 Thread Matthieu Moy
Junio C Hamano gits...@pobox.com writes:

 And worse yet, majority of users may read git bisect start is
 where you start bisection, but bisect start (either called
 directly, or via bisect_autostart by the first git bisect good)
 is where you set up the machinery, so doing bisect terms before
 doing bisect start does not make much sense.

This is currently how it's implemented. You need to say

$ git bisect terms foo bar
$ git bisect start

And indeed git bisect terms foo bar errors out. I think the reason it
is this way is to allow

$ git bisect terms foo bar
$ git bisect start sha1 sha1

But actually, we can allow git bisect terms until BISECT_TERMS is
created, which is your intuition and makes more sense IMHO. I'll try to
do that.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v9 5/5] bisect: allow any terms set by user

2015-06-26 Thread Junio C Hamano
Matthieu Moy matthieu@grenoble-inp.fr writes:

 Junio C Hamano gits...@pobox.com writes:

 The second sentence may want to be something like

  If you mistyped one of the terms, you can do another git
  bisect terms term-new term-old to correct them, but
  that is possible only before you start the bisection.

 Applied, thanks.

I didn't say it out loud while writing the above, but this (and we
have other places that use the same phrase in the doc) mentions that
you have some point in time where you start the bisection, without
having a clear definition of where that bisection starts, and that
bothers me.  You can do X until you do Y, when it is not clear
what Y exactly happens, is not very helpful.

We who know how bisection works internally know that the point of no
return is when we commit to the two terms and write one of the good
or bad bisect refs.  At that point, technically we haven't done a
bisection yet (git bisect good maint would bisect_autostart, but
without the other end, does not have a graph to bisect yet to find a
midpoint).

And worse yet, majority of users may read git bisect start is
where you start bisection, but bisect start (either called
directly, or via bisect_autostart by the first git bisect good)
is where you set up the machinery, so doing bisect terms before
doing bisect start does not make much sense.



 I currently have this in addition to v9 in my branch. I'll resend later
 (https://github.com/moy/git/tree/bisect-terms is up to date).

 diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
 index e783f87..7609cd6 100644
 --- a/Documentation/git-bisect.txt
 +++ b/Documentation/git-bisect.txt
 @@ -143,19 +143,19 @@ set your own terms.
  git bisect terms term-new term-old
  
  
 -This command has to be used before a bisection has started. term-old
 -must be associated with the latest revisions and term-new with the
 -ancestors of term-old. For example, if something was buggy in the
 +This command has to be used before a bisection has started. term-new
 +must be associated with the latest revisions and term-old with some
 +ancestors of term-new. For example, if something was buggy in the
  old part of the history, you know somewhere the bug was fixed, and you
  want to find the exact commit that fixed it, you may want to say `git
 -bisect terms fixed broken`; this way, you would mark a commit that
 +bisect terms broken fixed`; this way, you would mark a commit that

The example talks about a bug we used to have that was corrected, so
broken is old and fixed is new.  The earlier parts of this hunk
are correct but the last line should not be changed, no?

There unfortunately are two reasons why we cannot flip the order to
git bisect terms old new:

 * git bisect start $bad $good established the
   convention to list bad before good (and 'B'ad comes before
   'G'ood, so does 'N'ew before 'O'ld).

 * git bisect start $good $bad, even if we could use a time
   machine, would not be a good syntax, as you give zero or more
   good ones and one and only one bad one.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v9 5/5] bisect: allow any terms set by user

2015-06-25 Thread Matthieu Moy
From: Antoine Delaite antoine.dela...@ensimag.grenoble-inp.fr

Introduction of the git bisect terms command. The user can set his own
terms. It will work exactly like before. The terms must be set before the
start.

Signed-off-by: Antoine Delaite antoine.dela...@ensimag.grenoble-inp.fr
Signed-off-by: Louis Stuber stub...@ensimag.grenoble-inp.fr
Signed-off-by: Matthieu Moy matthieu@imag.fr
---
 Documentation/git-bisect.txt | 24 +
 git-bisect.sh| 80 ++--
 t/t6030-bisect-porcelain.sh  | 43 
 3 files changed, 137 insertions(+), 10 deletions(-)

diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
index 3c3021a..e783f87 100644
--- a/Documentation/git-bisect.txt
+++ b/Documentation/git-bisect.txt
@@ -133,6 +133,30 @@ You must run `git bisect start` without commits as 
argument and run
 `git bisect new rev`/`git bisect old rev...` after to add the
 commits.
 
+Alternative terms: use your own terms
+~
+
+If the builtins terms bad/good and new/old do not satisfy you, you can
+set your own terms.
+
+
+git bisect terms term-new term-old
+
+
+This command has to be used before a bisection has started. term-old
+must be associated with the latest revisions and term-new with the
+ancestors of term-old. For example, if something was buggy in the
+old part of the history, you know somewhere the bug was fixed, and you
+want to find the exact commit that fixed it, you may want to say `git
+bisect terms fixed broken`; this way, you would mark a commit that
+still has the bug with `broken`, and a newer one after the fix with
+`fixed`.
+
+Only the first bisection following the `git bisect terms` will use the
+terms. If you mistyped one of the terms you can do again `git bisect
+terms term-old term-new`.
+
+
 Bisect visualize
 
 
diff --git a/git-bisect.sh b/git-bisect.sh
index 569898b..8fee712 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -1,6 +1,6 @@
 #!/bin/sh
 
-USAGE='[help|start|bad|good|new|old|skip|next|reset|visualize|replay|log|run]'
+USAGE='[help|start|bad|good|new|old|terms|skip|next|reset|visualize|replay|log|run]'
 LONG_USAGE='git bisect help
print this long help message.
 git bisect start [--no-checkout] [bad [good...]] [--] [pathspec...]
@@ -11,6 +11,8 @@ git bisect (bad|new) [rev]
 git bisect (good|old) [rev...]
mark rev... known-good revisions/
revisions before change in a given property.
+git bisect terms term-new term-old
+   set up term-new and term-old as terms (default: bad, good)
 git bisect skip [(rev|range)...]
mark rev... untestable revisions.
 git bisect next
@@ -80,6 +82,16 @@ bisect_start() {
bad_seen=0
eval=''
must_write_terms=0
+   must_log_terms=0
+   if test -s $GIT_DIR/BISECT_TERMS
+   then
+   # We're going to restart from a clean state and the
+   # file will be deleted. Record the old state in
+   # variables and restore it below.
+   must_write_terms=1
+   must_log_terms=1
+   get_terms
+   fi
if test z$(git rev-parse --is-bare-repository) != zfalse
then
mode=--no-checkout
@@ -183,10 +195,14 @@ bisect_start() {
} 
git rev-parse --sq-quote $@ $GIT_DIR/BISECT_NAMES 
eval $eval true 
-   if test $must_write_terms -eq 1  test ! -s $GIT_DIR/BISECT_TERMS
+   if test $must_write_terms -eq 1
then
-   echo $NAME_BAD $GIT_DIR/BISECT_TERMS 
-   echo $NAME_GOOD $GIT_DIR/BISECT_TERMS
+   write_terms $NAME_BAD $NAME_GOOD 
+   if test $must_log_terms -eq 1
+   then
+   echo git bisect terms $NAME_BAD $NAME_GOOD \
+   $GIT_DIR/BISECT_LOG
+   fi
fi 
echo git bisect start$orig_args $GIT_DIR/BISECT_LOG || exit
#
@@ -450,6 +466,8 @@ bisect_replay () {
eval $cmd ;;
$NAME_GOOD|$NAME_BAD|skip)
bisect_write $command $rev ;;
+   terms)
+   bisect_terms $rev ;;
*)
die $(gettext ?? what are you talking about?) ;;
esac
@@ -534,7 +552,8 @@ get_terms () {
 check_and_set_terms () {
cmd=$1
case $cmd in
-   bad|good|new|old)
+   skip|start|terms) ;;
+   *)
if test -s $GIT_DIR/BISECT_TERMS  test $cmd != 
$NAME_BAD  test $cmd != $NAME_GOOD
then
die $(eval_gettext Invalid command: you're currently 
in a \$NAME_BAD/\$NAME_GOOD bisect.)
@@ -543,16 +562,14 @@ check_and_set_terms () {
bad|good)
if ! test -s $GIT_DIR/BISECT_TERMS
 

Re: [PATCH v9 5/5] bisect: allow any terms set by user

2015-06-25 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes:

 +Only the first bisection following the `git bisect terms` will use the
 +terms. If you mistyped one of the terms you can do again `git bisect
 +terms term-old term-new`.

This paragraph may need further polishing.

The first sentence makes it sound as if this is a valid sequence,
but I do not think that is what you meant:

$ git bisect start master maint
$ git bisect bad
$ git bisect terms new old
$ git bisect old
$ git bisect bad

I think what you wanted to say was that git bisect terms is in
effect during the single bisect session, and after you are done with
git bisect reset, the next bisect session, unless you use git
bisect terms, will use bad and good, as that is the default
pair of terms.

The second sentence may want to be something like

If you mistyped one of the terms, you can do another git
bisect terms term-new term-old to correct them, but
that is possible only before you start the bisection.

Otherwise, you invite this

$ git bisect start master maint
$ git bisect terms new olf
$ git bisect olf
$ git bisect new
$ git bisect old
... error message that says you can give either new and olf
$ git bisect terms new old
$ git bisect old

which may not work well.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v9 5/5] bisect: allow any terms set by user

2015-06-25 Thread Junio C Hamano
Matthieu Moy matthieu@imag.fr writes:

 +
 +git bisect terms term-new term-old
 +

The mnemonic for git bisect start bad good is Bad comes before
Good (B = 0x42, G = 0x47) and this is same for new/old, New comes
before Old (N = 0x4e, O = 0x4f).  git bisect terms new old follows
the same pattern, which is good.  Easy to remember.

 +This command has to be used before a bisection has started. term-old
 +must be associated with the latest revisions and term-new with the
 +ancestors of term-old.

Whoa?  This gets new and old mixed up, doesn't it?

 For example, if something was buggy in the
 +old part of the history, you know somewhere the bug was fixed, and you
 +want to find the exact commit that fixed it, you may want to say `git
 +bisect terms fixed broken`; this way, you would mark a commit that
 +still has the bug with `broken`, and a newer one after the fix with
 +`fixed`.

So, it used to be broken, it got fixed recently, so broken is old,
fixed is new, bad/new and then good/old mnemonic says you give
fixed broken to bisect terms.  OK.

 +Only the first bisection following the `git bisect terms` will use the
 +terms. If you mistyped one of the terms you can do again `git bisect
 +terms term-old term-new`.

This is also the other way around, no?

 +git bisect terms term-new term-old
 + set up term-new and term-old as terms (default: bad, good)

Good.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html