Re: [PATCH 4/4] bisect: add the terms old/new
From: Junio C Hamano <hidden>
Date: 2016-06-15 23:05:13
Antoine Delaite [off-list ref] writes:
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 a certain
property must be marked as `new` and the ones which do not 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.Just throwing a suggestion. You could perhaps add a new verb to be used before starting to do anything, e.g. $ git bisect terms new old (where the default mode is "git bisect terms bad good") to set up the "terms", and then after that $ git bisect start $new $old1 $old2... would be treated as you would naturally expect, no?
* git rev-list --bisect does not treat the revs/bisect/new and
revs/bisect/old-SHA1 files.That shouldn't be hard to add, I would imagine, either by git rev-list --bisect=new,old or teaching "git rev-list --bisect" to read the "terms" itself, as a follow-up patch.
* git bisect visualize seem to work partially: the tags are
displayed correctly but the tree is not limited to the bisect
section.This is directly related to the lack of "git rev-list --bisect" support, I would imagine. If you make the command read "terms", I suspect that it would solve itself.
quoted hunk
@@ -379,6 +408,21 @@ In this case, when 'git bisect run' finishes, bisect/bad will refer to a commit has at least one parent whose reachable graph is fully traversable in the sense required by 'git pack objects'. +* Look for a fix instead of a regression in the code ++ +------------ +$ git bisect start +$ git bisect new HEAD # current commit is marked as new +$ git bisect old HEAD~10 # the tenth commit from now is marked as old +------------ ++ +If the last commit has a given property, and we are looking for the commit +which introduced this property.
Hmph, that is not a complete sentence and I cannot understand what it is trying to say.
+For each commit the bisection guide us to we
s/guide us to we/guides us to, we/;
+will test if the property is present, if it is we will mark the commit as new +with 'git bisect new', otherwise we will mark it as old.
It would be easier to read as separate sentences, i.e. s/is present, if it is/is present. If it is/;
quoted hunk
diff --git a/bisect.c b/bisect.c index 3b7df85..511b905 100644 --- a/bisect.c +++ b/bisect.c@@ -409,7 +409,8 @@ static int register_ref(const char *refname, const unsigned char *sha1, if (!strcmp(refname, name_bad)) { current_bad_oid = xmalloc(sizeof(*current_bad_oid)); hashcpy(current_bad_oid->hash, sha1); - } else if (starts_with(refname, "good-")) { + } else if (starts_with(refname, "good-") || + starts_with(refname, "old-")) { sha1_array_append(&good_revs, sha1); } else if (starts_with(refname, "skip-")) { sha1_array_append(&skipped_revs, sha1);@@ -741,6 +742,12 @@ static void handle_bad_merge_base(void) "between %s and [%s].\n", bad_hex, bad_hex, good_hex); } + else if (!strcmp(name_bad, "new")) { + fprintf(stderr, "The merge base %s is new.\n" + "The property has changed " + "between %s and [%s].\n", + bad_hex, bad_hex, good_hex); + }
After reading the previous patches in the series, I expected that this new code would know to read "terms" and does not limit us only to "new" and "old". Somewhat disappointing.
quoted hunk
@@ -439,7 +441,7 @@ bisect_replay () { start) cmd="bisect_start $rev" eval "$cmd" ;; - good|bad|skip) + good|bad|skip|old|new)
Not NAME_GOOD / NAME_BAD? To support replay, you may want to consider the "bisect terms" approach and when the bisection is using non-standard terms record that as the first entry to the bisect log.