Re: [WIP PATCH 1/3] git bisect old/new

2015-06-04 Thread Junio C Hamano
Antoine Delaite antoine.dela...@ensimag.grenoble-inp.fr writes:

 From: Christian Couder chrisc...@tuxfamily.org

 When not looking for a regression during a bisect but for a fix or a
 change in another given property, it can be confusing to use 'good'
 and 'bad'.

 This patch introduce `git bisect new` and `git bisect old` as an
 alternative to 'bad' and good': the commits which have the most
 recent version of the property must be marked as `new` and the ones
 with the older version as `old`.

The phrase the most recent version of invites an unnecessary
confusion.  git bisect is a tool to find a single bit flip and if
the behaviour changed twice in the history from A to B to C, you
cannot look for the transition between A to B or B to C.  You need
to say I classify behaviour B and C to be both good (new) and I
know it used to have behaviour A which was bad (old).

You can mark the commits with the new property as new, and
the old property as old; good being new and bad being
old becomes a mere special case of this.  The new property
in the traditional bisection is commit has the bug we are
hunting, while old is commit lacks that bug.

 diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
 index 4cb52a7..12c7711 100644
 --- a/Documentation/git-bisect.txt
 +++ b/Documentation/git-bisect.txt
 ...
 @@ -104,6 +104,33 @@ 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.
  
 +
 +Alternative terms: bisect new and bisect old
 +
 +
 +If you are not at ease with the terms bad and good, perhaps
 +because you are looking for the commit that introduced a fix, you can
 +alternatively use new and old instead.
 +But note that you cannot mix bad and good with new and old.

There is a dq missing somewhere on this line.

 +
 +
 +git bisect new [rev]
 +
 +
 +Marks the commit as new, which means the version is fixed.

I am not sure , which means the version is fixed. is a good idea.

The whole point of introducing old vs new, the reason why we may
want to do this patch series, is to reduce confusion by removing the
value judgement associated with good vs bad.  That is what made
the bisect confusing when the old ones were bad and new ones were
good.  Saying fixed implicitly contrasts it with broken, and
introduces the value judgement on the states again.

Side note: yes, if the reader is careful, fixed needs to
have previous states to be broken, so in that sense,
fixed can be read about time-sequence and not about value
judgement.  But let's try to be understandable by even
casual readers.

We would instead want to emphasize that what we are judging is not
values but time sequence.  You would want to say It used to be X
but it no longer is X, without saying if X is a good thing or not,
e.g.

Marks the commit as new, e.g. the bug is no longer there,
if you are looking for a commit that fixed a bug, or the
feature that used to work is now broken at this point, if
you are looking for a commit that introduced a bug.

 +
 +git bisect old [rev...]
 +
 +
 +Marks the commit as old, which means the bug is present.

This is much worse than the previous one. Unless you force the
reader to _assume_ that you are hunting for a fix, bug is present
can mean bug is still present, or bug was introduced somewhere
along the line and now it is there.  And a casual mention of
perhaps because you are looking for a fix at the beginning of the
section is not strong enough hint that these examples are _only_
about hunting for bugfix.

I'd stop here for now, but a casual skimming told me that the same
issue exists throughout the remainder of the doc.

Thanks.
--
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: [WIP PATCH 1/3] git bisect old/new

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

 @@ -732,18 +736,25 @@ static void handle_bad_merge_base(void)
   if (is_expected_rev(current_bad_oid)) {
   char *bad_hex = oid_to_hex(current_bad_oid);
   char *good_hex = join_sha1_array_hex(good_revs, ' ');
 -
 - fprintf(stderr, The merge base %s is bad.\n
 - This means the bug has been fixed 
 - between %s and [%s].\n,
 - bad_hex, bad_hex, good_hex);
 -
 + if (!strcmp(bisect_bad,bad)) {

space after comma.

 @@ -954,6 +994,9 @@ int bisect_next_all(const char *prefix, int no_checkout)
  (roughly %d step%s)\n, nr, (nr == 1 ?  : s),
  steps, (steps == 1 ?  : s));
  
 + free((char*)bisect_bad);
 + free((char*)bisect_good);

Already noted last time I think: these must not be freed as they are
pointing to static strings.

-- 
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: [WIP PATCH 1/3] git bisect old/new

2015-06-04 Thread Christian Couder
On Thu, Jun 4, 2015 at 9:59 AM, Antoine Delaite
antoine.dela...@ensimag.grenoble-inp.fr wrote:
 From: Christian Couder chrisc...@tuxfamily.org

 When not looking for a regression during a bisect but for a fix or a
 change in another given property, it can be confusing to use 'good'
 and 'bad'.

 This patch introduce `git bisect new` and `git bisect old` as an
 alternative to 'bad' and good': the commits which have the most
 recent version of the property must be marked as `new` and the ones
 with the older version as `old`.

 The output will be the first commit after the change in the property.
 During a new/old bisect session you cannot use bad/good commands and
 vice-versa.

 `git bisect replay` works fine for old/new bisect sessions.

 Some commands are still not available for old/new:

  * git bisect start [new [old...]] is not possible: the
commits will be treated as bad and good.
  * git rev-list --bisect does not treat the revs/bisect/new and
revs/bisect/old-SHA1 files.
  * git bisect visualize seem to work partially: the tags are
displayed correctly but the tree is not limited to the bisect
section.

 Related discussions:

 - http://thread.gmane.org/gmane.comp.version-control.git/86063
 introduced bisect fix unfixed to find fix.
 - http://thread.gmane.org/gmane.comp.version-control.git/182398
 discussion around bisect yes/no or old/new.
 - http://thread.gmane.org/gmane.comp.version-control.git/199758
 last discussion and reviews

 Signed-off-by: Antoine Delaite antoine.dela...@ensimag.grenoble-inp.fr
 Signed-off-by: Louis Stuber stub...@ensimag.grenoble-inp.fr
 Signed-off-by: Valentin Duperray valentin.duper...@ensimag.imag.fr
 Signed-off-by: Franck Jonas franck.jo...@ensimag.imag.fr
 Signed-off-by: Lucien Kong lucien.k...@ensimag.imag.fr
 Signed-off-by: Thomas Nguy thomas.n...@ensimag.imag.fr
 Signed-off-by: Huynh Khoi Nguyen Nguyen 
 huynh-khoi-nguyen.ngu...@ensimag.imag.fr
 Signed-off-by: Matthieu Moy matthieu@grenoble-inp.fr
 ---
 We took account of most of the easy reviews that were discuted two years 
 ago.
 We hope we didn't missed any.
 http://thread.gmane.org/gmane.comp.version-control.git/199758

 We corrected various issues that were not reported:
 -one that caused a fatal ... not a valid ref at the end of the bisection.
 -the autostart was causing issues, the following lines were working :
 git bisect new HEAD
 git bisect bad HEAD
 git bisect good aGoodCommit

 The hard review which we were thinking on was the issue of the maintaining
 of old/new and allow easy support of new tags like yes/no in the future.
 I tried to remove the maximum of bad/good and old/new which were hard wrote in
 the code but I'm not completly satisfied. This patch is clearly a v1.

Thanks for working on this. Here are some suggestions that I should
probably have told you before, but didn't:

- Take ownership of all the patches.
- Patch 3/3 renames some variables in bisect.c, do the same thing in
git-bisect.sh for consistency.
- Squash all the patches together.
- Try to find a way to break down the resulting patch into a logical
patch series which adds tests at each logical step. This might be
difficult. You might want to add features to git bisect--helper first
for example, then test those features, then add features to git bisect
and then test those features.

Best,
Christian.

 We're currently working on:

 * rebasing the history of the patch
 * git rev-list --bisect does not treat the revs/bisect/new and
 revs/bisect/old-SHA1 files.
 * git bisect visualize seem to work partially: the tags are displayed
 correctly but the tree is not limited to the bisect section.
 * adding tests about old/new

 Some other problems that might also be considerred later :
 * change/add valid terms (e.g unfixed/fixed instead of old/new)
 *
 * git bisect start [new [old...]] is not possible: the commits
 will be treated as bad and 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


[WIP PATCH 1/3] git bisect old/new

2015-06-04 Thread Antoine Delaite
From: Christian Couder chrisc...@tuxfamily.org

When not looking for a regression during a bisect but for a fix or a
change in another given property, it can be confusing to use 'good'
and 'bad'.

This patch introduce `git bisect new` and `git bisect old` as an
alternative to 'bad' and good': the commits which have the most
recent version of the property must be marked as `new` and the ones
with the older version as `old`.

The output will be the first commit after the change in the property.
During a new/old bisect session you cannot use bad/good commands and
vice-versa.

`git bisect replay` works fine for old/new bisect sessions.

Some commands are still not available for old/new:

 * git bisect start [new [old...]] is not possible: the
   commits will be treated as bad and good.
 * git rev-list --bisect does not treat the revs/bisect/new and
   revs/bisect/old-SHA1 files.
 * git bisect visualize seem to work partially: the tags are
   displayed correctly but the tree is not limited to the bisect
   section.

Related discussions:

- http://thread.gmane.org/gmane.comp.version-control.git/86063
introduced bisect fix unfixed to find fix.
- http://thread.gmane.org/gmane.comp.version-control.git/182398
discussion around bisect yes/no or old/new.
- http://thread.gmane.org/gmane.comp.version-control.git/199758
last discussion and reviews

Signed-off-by: Antoine Delaite antoine.dela...@ensimag.grenoble-inp.fr
Signed-off-by: Louis Stuber stub...@ensimag.grenoble-inp.fr
Signed-off-by: Valentin Duperray valentin.duper...@ensimag.imag.fr
Signed-off-by: Franck Jonas franck.jo...@ensimag.imag.fr
Signed-off-by: Lucien Kong lucien.k...@ensimag.imag.fr
Signed-off-by: Thomas Nguy thomas.n...@ensimag.imag.fr
Signed-off-by: Huynh Khoi Nguyen Nguyen 
huynh-khoi-nguyen.ngu...@ensimag.imag.fr
Signed-off-by: Matthieu Moy matthieu@grenoble-inp.fr
---
We took account of most of the easy reviews that were discuted two years ago.
We hope we didn't missed any.
http://thread.gmane.org/gmane.comp.version-control.git/199758

We corrected various issues that were not reported: 
-one that caused a fatal ... not a valid ref at the end of the bisection.
-the autostart was causing issues, the following lines were working :
git bisect new HEAD
git bisect bad HEAD
git bisect good aGoodCommit

The hard review which we were thinking on was the issue of the maintaining
of old/new and allow easy support of new tags like yes/no in the future.
I tried to remove the maximum of bad/good and old/new which were hard wrote in
the code but I'm not completly satisfied. This patch is clearly a v1.

We're currently working on:

* rebasing the history of the patch
* git rev-list --bisect does not treat the revs/bisect/new and
revs/bisect/old-SHA1 files.
* git bisect visualize seem to work partially: the tags are displayed
correctly but the tree is not limited to the bisect section.
* adding tests about old/new

Some other problems that might also be considerred later :
* change/add valid terms (e.g unfixed/fixed instead of old/new)
*
* git bisect start [new [old...]] is not possible: the commits
will be treated as bad and good.


 Documentation/git-bisect.txt |  43 +-
 bisect.c |  83 ---
 git-bisect.sh| 132 ---
 t/t6030-bisect-porcelain.sh  |  40 +
 4 files changed, 243 insertions(+), 55 deletions(-)
 mode change 100755 = 100644 git-bisect.sh

diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
index 4cb52a7..12c7711 100644
--- a/Documentation/git-bisect.txt
+++ b/Documentation/git-bisect.txt
@@ -18,8 +18,8 @@ on the subcommand:
 
  git bisect help
  git bisect start [--no-checkout] [bad [good...]] [--] [paths...]
- git bisect bad [rev]
- git bisect good [rev...]
+ git bisect (bad|new) [rev]
+ git bisect (good|old) [rev...]
  git bisect skip [(rev|range)...]
  git bisect reset [commit]
  git bisect visualize
@@ -104,6 +104,33 @@ 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.
 
+
+Alternative terms: bisect new and bisect old
+
+
+If you are not at ease with the terms bad and good, perhaps
+because you are looking for the commit that introduced a fix, you can
+alternatively use new and old instead.
+But note that you cannot mix bad and good with new and old.
+
+
+git bisect new [rev]
+
+
+Marks the commit as new, which means the version is fixed.
+It is the equivalent of git bisect bad [rev].
+
+
+git bisect old [rev...]
+
+
+Marks the