[PATCH v3 3/6] bisect: simplify the addition of new bisect terms
We create a file BISECT_TERMS in the repository .git to be read during a
bisection. The fonctions to be changed if we add new terms are quite
few.
In git-bisect.sh :
check_and_set_terms
bisect_voc
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
---
bisect.c | 38 --
git-bisect.sh | 70 +++-
2 files changed, 98 insertions(+), 10 deletions(-)
diff --git a/bisect.c b/bisect.c
index cda30fa..2fc8a78 100644
--- a/bisect.c
+++ b/bisect.c
@@ -747,7 +747,10 @@ static void handle_bad_merge_base(void)
between %s and [%s].\n,
bad_hex, bad_hex, good_hex);
} else {
- die(BUG: terms %s/%s not managed, name_bad,
name_good);
+ fprintf(stderr, The merge base %s is %s.\n
+ This means the first commit marked %s is
+ between %s and [%s].\n,
+ bad_hex, name_bad, name_bad, bad_hex, good_hex);
}
exit(3);
}
@@ -902,6 +905,36 @@ static void show_diff_tree(const char *prefix, struct
commit *commit)
}
/*
+ * The terms used for this bisect session are stored in BISECT_TERMS.
+ * We read them and store them to adapt the messages accordingly.
+ * Default is bad/good.
+ */
+void read_bisect_terms(const char **read_bad, const char **read_good)
+{
+ struct strbuf str = STRBUF_INIT;
+ const char *filename = git_path(BISECT_TERMS);
+ FILE *fp = fopen(filename, r);
+
+ if (!fp) {
+ if (errno==2) {
+ *read_bad = bad;
+ *read_good = good;
+ return;
+ } else {
+ die(could not read file '%s': %s, filename,
+ strerror(errno));
+ }
+ } else {
+ strbuf_getline(str, fp, '\n');
+ *read_bad = strbuf_detach(str, NULL);
+ strbuf_getline(str, fp, '\n');
+ *read_good = strbuf_detach(str, NULL);
+ }
+ strbuf_release(str);
+ fclose(fp);
+}
+
+/*
* We use the convention that exiting with an exit code 10 means that
* the bisection process finished successfully.
* In this case the calling shell script should exit 0.
@@ -917,8 +950,7 @@ int bisect_next_all(const char *prefix, int no_checkout)
const unsigned char *bisect_rev;
char bisect_rev_hex[GIT_SHA1_HEXSZ + 1];
- name_bad=bad;
- name_good=good;
+ read_bisect_terms(name_bad, name_good);
if (read_bisect_refs())
die(reading bisect refs failed);
diff --git a/git-bisect.sh b/git-bisect.sh
index ce6412f..55b9ebd 100644
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -77,6 +77,9 @@ bisect_start() {
orig_args=$(git rev-parse --sq-quote $@)
bad_seen=0
eval=''
+ # revision_seen is true if a git bisect start
+ # has revision as arguments
+ revision_seen=0
if test z$(git rev-parse --is-bare-repository) != zfalse
then
mode=--no-checkout
@@ -101,6 +104,9 @@ bisect_start() {
die $(eval_gettext '\$arg' does not appear to
be a valid revision)
break
}
+
+ revision_seen=1
+
case $bad_seen in
0) state=$NAME_BAD ; bad_seen=1 ;;
*) state=$NAME_GOOD ;;
@@ -172,6 +178,11 @@ bisect_start() {
}
git rev-parse --sq-quote $@ $GIT_DIR/BISECT_NAMES
eval $eval true
+ if test $revision_seen -eq 1 test ! -s $GIT_DIR/BISECT_TERMS
+ then
+ echo $NAME_BAD $GIT_DIR/BISECT_TERMS
+ echo $NAME_GOOD $GIT_DIR/BISECT_TERMS
+ fi
echo git bisect start$orig_args $GIT_DIR/BISECT_LOG || exit
#
# Check if we can proceed to the next bisect state.
@@ -232,6 +243,7 @@ bisect_skip() {
bisect_state() {
bisect_autostart
state=$1
+ check_and_set_terms $state
case $#,$state in
0,*)
die $(gettext Please call 'bisect_state' with at least one
argument.) ;;
@@ -291,15 +303,17 @@ bisect_next_check() {
: bisect without $NAME_GOOD...
;;
*)
-
+ bad_syn=$(bisect_voc bad
[PATCH v3 2/6] bisect: replace hardcoded bad|good by variables
To add new tags like old/new and have keywords less confusing, the
first step is to avoid hardcoding the keywords.
The default mode is still bad/good.
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
---
bisect.c | 51 ++-
git-bisect.sh | 57 +++--
2 files changed, 65 insertions(+), 43 deletions(-)
mode change 100755 = 100644 git-bisect.sh
diff --git a/bisect.c b/bisect.c
index 5b8357d..cda30fa 100644
--- a/bisect.c
+++ b/bisect.c
@@ -21,6 +21,9 @@ static const char *argv_checkout[] = {checkout, -q, NULL,
--, NULL};
static const char *argv_show_branch[] = {show-branch, NULL, NULL};
static const char *argv_update_ref[] = {update-ref, --no-deref,
BISECT_HEAD, NULL, NULL};
+static const char *name_bad;
+static const char *name_good;
+
/* Remember to update object flag allocation in object.h */
#define COUNTED(1u16)
@@ -403,15 +406,21 @@ struct commit_list *find_bisection(struct commit_list
*list,
static int register_ref(const char *refname, const struct object_id *oid,
int flags, void *cb_data)
{
- if (!strcmp(refname, bad)) {
+ struct strbuf good_prefix = STRBUF_INIT;
+ strbuf_addstr(good_prefix, name_good);
+ strbuf_addstr(good_prefix, -);
+
+ if (!strcmp(refname, name_bad)) {
current_bad_oid = xmalloc(sizeof(*current_bad_oid));
oidcpy(current_bad_oid, oid);
- } else if (starts_with(refname, good-)) {
+ } else if (starts_with(refname, good_prefix.buf)) {
sha1_array_append(good_revs, oid-hash);
} else if (starts_with(refname, skip-)) {
sha1_array_append(skipped_revs, oid-hash);
}
+ strbuf_release(good_prefix);
+
return 0;
}
@@ -634,7 +643,7 @@ static void exit_if_skipped_commits(struct commit_list
*tried,
return;
printf(There are only 'skip'ped commits left to test.\n
- The first bad commit could be any of:\n);
+ The first %s commit could be any of:\n, name_bad);
print_commit_list(tried, %s\n, %s\n);
if (bad)
printf(%s\n, oid_to_hex(bad));
@@ -732,18 +741,21 @@ 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(name_bad, bad)) {
+ 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);
+ } else {
+ die(BUG: terms %s/%s not managed, name_bad,
name_good);
+ }
exit(3);
}
- fprintf(stderr, Some good revs are not ancestor of the bad rev.\n
+ fprintf(stderr, Some %s revs are not ancestor of the %s rev.\n
git bisect cannot work properly in this case.\n
- Maybe you mistook good and bad revs?\n);
+ Maybe you mistook %s and %s revs?\n,
+ name_good, name_bad, name_good, name_bad);
exit(1);
}
@@ -755,10 +767,10 @@ static void handle_skipped_merge_base(const unsigned char
*mb)
warning(the merge base between %s and [%s]
must be skipped.\n
- So we cannot be sure the first bad commit is
+ So we cannot be sure the first %s commit is
between %s and %s.\n
We continue anyway.,
- bad_hex, good_hex, mb_hex, bad_hex);
+ bad_hex, good_hex, name_bad, mb_hex, bad_hex);
free(good_hex);
}
@@ -839,7 +851,7 @@ static void check_good_are_ancestors_of_bad(const char
*prefix, int no_checkout)
int fd;
if (!current_bad_oid)
- die(a bad revision is needed);
+ die(a %s revision is needed, name_bad);
/* Check if file BISECT_ANCESTORS_OK exists. */
if (!stat(filename, st) S_ISREG(st.st_mode))
@@ -905,6 +917,8 @@ int bisect_next_all(const char *prefix, int
[PATCH v3 6/6] bisect: allows any terms set by user
Introduction of the git bisect terms function.
The user can set its 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
---
Documentation/git-bisect.txt | 19
git-bisect.sh| 68 ++---
t/t6030-bisect-porcelain.sh | 43 ++
3 files changed, 125 insertions(+), 5 deletions(-)
diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
index 3c3021a..ef0c03c 100644
--- a/Documentation/git-bisect.txt
+++ b/Documentation/git-bisect.txt
@@ -133,6 +133,25 @@ 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 term1 term2
+
+
+This command has to be used before a bisection has started.
+The term1 must be associated with the latest revisions and term2 with the
+ancestors of term1.
+
+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 term1
+term2'.
+
+
Bisect visualize
diff --git a/git-bisect.sh b/git-bisect.sh
index a11ca06..7da22b1 100644
--- 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 term1 term2
+ set up term1 and term2 as bisection terms.
git bisect skip [(rev|range)...]
mark rev... untestable revisions.
git bisect next
@@ -82,6 +84,14 @@ bisect_start() {
# revision_seen is true if a git bisect start
# has revision as arguments
revision_seen=0
+ # terms_defined is used to detect if the user
+ # defined his own terms with git bisect terms
+ terms_defined=0
+ if test -s $GIT_DIR/TERMS_DEFINED
+ then
+ terms_defined=1
+ get_terms
+ fi
if test z$(git rev-parse --is-bare-repository) != zfalse
then
mode=--no-checkout
@@ -180,10 +190,14 @@ bisect_start() {
}
git rev-parse --sq-quote $@ $GIT_DIR/BISECT_NAMES
eval $eval true
- if test $revision_seen -eq 1 test ! -s $GIT_DIR/BISECT_TERMS
+ if test $revision_seen -eq 1 test ! -s $GIT_DIR/BISECT_TERMS ||
test $terms_defined -eq 1
then
echo $NAME_BAD $GIT_DIR/BISECT_TERMS
- echo $NAME_GOOD $GIT_DIR/BISECT_TERMS
+ echo $NAME_GOOD $GIT_DIR/BISECT_TERMS
+ if test $terms_defined -eq 1
+ then
+ echo git bisect terms $NAME_BAD $NAME_GOOD
$GIT_DIR/BISECT_LOG || exit
+ fi
fi
echo git bisect start$orig_args $GIT_DIR/BISECT_LOG || exit
#
@@ -419,6 +433,7 @@ bisect_clean_state() {
rm -f $GIT_DIR/BISECT_NAMES
rm -f $GIT_DIR/BISECT_RUN
rm -f $GIT_DIR/BISECT_TERMS
+ rm -f $GIT_DIR/TERMS_DEFINED
# Cleanup head-name if it got left by an old version of git-bisect
rm -f $GIT_DIR/head-name
git update-ref -d --no-deref BISECT_HEAD
@@ -447,6 +462,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
@@ -531,7 +548,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.)
@@ -564,6 +582,44 @@ bisect_voc () {
esac
}
+bisect_terms () {
+ case $# in
+ 0)
+ if test -s $GIT_DIR/BISECT_TERMS
+ then
+ {
+ read term1
+ read term2
[PATCH v3 5/6] revision: fix rev-list --bisect in old/new mode
From: Louis Stuber stub...@ensimag.grenoble-inp.fr
Calling git rev-list --bisect when an old/new mode bisection was started
shows the help notice. This has been fixed by reading BISECT_TERMS in
revision.c to find the correct bisect refs path (which was always
refs/bisect/bad (or good) before and can be refs/bisect/new (old) now).
Signed-off-by: Louis Stuber stub...@ensimag.grenoble-inp.fr
Signed-off-by: Antoine Delaite antoine.dela...@ensimag.grenoble-inp.fr
---
revision.c | 16 ++--
1 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/revision.c b/revision.c
index 3ff8723..27750ac 100644
--- a/revision.c
+++ b/revision.c
@@ -21,6 +21,9 @@
volatile show_early_output_fn_t show_early_output;
+static const char *name_bad;
+static const char *name_good;
+
char *path_name(const struct name_path *path, const char *name)
{
const struct name_path *p;
@@ -2076,14 +2079,22 @@ void parse_revision_opt(struct rev_info *revs, struct
parse_opt_ctx_t *ctx,
ctx-argc -= n;
}
+extern void read_bisect_terms(const char **bad, const char **good);
+
static int for_each_bad_bisect_ref(const char *submodule, each_ref_fn fn, void
*cb_data)
{
- return for_each_ref_in_submodule(submodule, refs/bisect/bad, fn,
cb_data);
+ char bisect_refs_path[256];
+ strcpy(bisect_refs_path, refs/bisect/);
+ strcat(bisect_refs_path, name_bad);
+ return for_each_ref_in_submodule(submodule, bisect_refs_path, fn,
cb_data);
}
static int for_each_good_bisect_ref(const char *submodule, each_ref_fn fn,
void *cb_data)
{
- return for_each_ref_in_submodule(submodule, refs/bisect/good, fn,
cb_data);
+ char bisect_refs_path[256];
+ strcpy(bisect_refs_path, refs/bisect/);
+ strcat(bisect_refs_path, name_good);
+ return for_each_ref_in_submodule(submodule, bisect_refs_path, fn,
cb_data);
}
static int handle_revision_pseudo_opt(const char *submodule,
@@ -2112,6 +2123,7 @@ static int handle_revision_pseudo_opt(const char
*submodule,
handle_refs(submodule, revs, *flags,
for_each_branch_ref_submodule);
clear_ref_exclusion(revs-ref_excludes);
} else if (!strcmp(arg, --bisect)) {
+ read_bisect_terms(name_bad, name_good);
handle_refs(submodule, revs, *flags, for_each_bad_bisect_ref);
handle_refs(submodule, revs, *flags ^ (UNINTERESTING | BOTTOM),
for_each_good_bisect_ref);
revs-bisect = 1;
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe git in
[PATCH v3 4/6] bisect: add the terms old/new
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. Some commands are still not available for old/new: * git rev-list --bisect does not treat the revs/bisect/new and revs/bisect/old-SHA1 files. Old 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 New discussions: - http://thread.gmane.org/gmane.comp.version-control.git/271320 ( v2 1/7-4/7 ) - http://comments.gmane.org/gmane.comp.version-control.git/271343 ( v2 5/7-7/7 ) 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 --- Documentation/git-bisect.txt | 48 - bisect.c | 11 +++-- git-bisect.sh| 30 + t/t6030-bisect-porcelain.sh | 38 + 4 files changed, 112 insertions(+), 15 deletions(-) diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt index 4cb52a7..3c3021a 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,35 @@ 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, 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. +It is the equivalent of git bisect bad [rev]. + + +git bisect old [rev...] + + +Marks the commit as old, as the opposite of 'git bisect new'. +It is the equivalent of git bisect good [rev...]. + +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. + Bisect visualize @@ -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 + ++ +Let's consider the last commit has a given property, and that we are looking +for the commit which introduced this property. For each commit the bisection +guide 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. +At the end of the bisect session, the result will be the first new commit (e.g +the first one with the property). + SEE ALSO diff --git a/bisect.c b/bisect.c index 2fc8a78..7492fdc 100644 --- a/bisect.c +++ b/bisect.c @@ -746,6 +746,11 @@ static
[PATCH v3 1/6] bisect: correction of typo
Signed-off-by: Antoine Delaite antoine.dela...@ensimag.grenoble-inp.fr --- bisect.c|2 +- t/t6030-bisect-porcelain.sh |2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/bisect.c b/bisect.c index 03d5cd9..5b8357d 100644 --- a/bisect.c +++ b/bisect.c @@ -743,7 +743,7 @@ static void handle_bad_merge_base(void) fprintf(stderr, Some good revs are not ancestor of the bad rev.\n git bisect cannot work properly in this case.\n - Maybe you mistake good and bad revs?\n); + Maybe you mistook good and bad revs?\n); exit(1); } diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh index 06b4868..9e2c203 100755 --- a/t/t6030-bisect-porcelain.sh +++ b/t/t6030-bisect-porcelain.sh @@ -362,7 +362,7 @@ test_expect_success 'bisect starting with a detached HEAD' ' test_expect_success 'bisect errors out if bad and good are mistaken' ' git bisect reset test_must_fail git bisect start $HASH2 $HASH4 2 rev_list_error - grep mistake good and bad rev_list_error + grep mistook good and bad rev_list_error git bisect reset ' -- 1.7.1 -- To unsubscribe from this list: send the line unsubscribe git in
[PATCH v2 2/7] bisect: replace hardcoded bad|good by variables
Matthieu Moy matthieu@grenoble-inp.fr wrote: Being an acceptable ref name is a constraint you have to check (Junio already mentionned check-ref-format). I think quoting variables makes sense too I don't get how 'git check-ref-format' works exactly. It says it needs at least one slash in the ref name. So it would not be good for bisect terms. Then do we take the same format as branches ? ( i.e 'git check-ref-format --branch' ) Thanks. -- To unsubscribe from this list: send the line unsubscribe git in
[PATCH v2 7/7] bisect: allows any terms set by user
Matthieu Moy matthieu@grenoble-inp.fr writes: # terms_defined is 0 when the user did not define the terms explicitely # yet. This is the case when running 'git bisect start bad_rev good_rev' # before we see an explicit reference to a term. terms_defined=0 The thing is: 'git bisect reset git bisect new HEAD git bisect new does not exist. Did you mean git bisect start HEAD? No I meant new but it can be 'git bisect bad' aswell So ' git bisect reset git bisect bad answer yes to autostart ? ' I don't understand. The user didn't say either bad or good, so in both cases we haven't seen a term yet. Or I misunderstood what you meant by define a term. In the case I rewrited, we saw a 'bad' but terms_defined value in bisect_start (called by the autostart) is 0. 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
[PATCH v2 7/7] bisect: allows any terms set by user
Matthieu Moy matthieu@grenoble-inp.fr writes: I would say for the current bisection, i.e. $ git bisect start $ git bisect terms foo bar # Scope starts here ... The first 'bar' commit is deadbeef. # Scope ends here $ git bisect terms foo bar You need to start by git bisect start Do you want me to do it for you [Y/n]? I personnaly don't like this autostart. In the first version of this patch 7/7 'git bisect terms' has to be called before the start. I think it is better because we can then use 'git bisect start rev1 rev2 ...' The next 'bisect start rev1 rev2' would be in bad/good mode. But this have to be discuted, do the user have to type 'git bisect terms' each bisection if he wants to use special terms ? To me, yes. If we allow arbitrary terms, then they will most likely be specific to one bisect session (e.g. if I bisect present/absent once, it tells me nothing about what I'll want for my next bisection). Ok. 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
[PATCH v2 7/7] bisect: allows any terms set by user
Matthieu Moy matthieu@grenoble-inp.fr writes: # terms_defined is 0 when the user did not define the terms explicitely # yet. This is the case when running 'git bisect start bad_rev good_rev' # before we see an explicit reference to a term. terms_defined=0 The thing is: 'git bisect reset git bisect new HEAD (autostart ?) y' is strictly equivalent to 'git bisect reset git bisect start git bisect new HEAD' In both case terms_defined value would be 0 while we kinda know that a term has been used. It just that in the code it is not taken in account yet. I don't really mind doing the change but I'm just pointing out that it can be confusing. 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
[PATCH v2 7/7] bisect: allows any terms set by user
Matthieu Moy matthieu@grenoble-inp.fr writes: +if test -s $GIT_DIR/TERMS_DEFINED +then +terms_defined=1 +get_terms +rm -rf $GIT_DIR/TERMS_DEFINED I don't understand why you need to delete this file. I did not review thoroughly so there may be a reason, but you can help the reader with a comment here. I will just complete Louis' answer. We delete it with backward compatibility with old/new in mind (even if old/new is not merged yet). For instance, after a old/new mode, if you do a 'bisect start rev1 rev2' the mode would be bad/good ie the default mode. So if you defined your terms, we decided it would only be for the following bisection. The next 'bisect start rev1 rev2' would be in bad/good mode. But this have to be discuted, do the user have to type 'git bisect terms' each bisection if he wants to use special terms ? -- 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 v2 7/7] bisect: allows any terms set by user
This message's goal is to explained where am I on the bisect functions. -git bisect replay now work with any terms. -I took account of most of the last reviews (coding style, double sed ...) -'git bisect terms' without arguments now display the current terms -bisect_terms : if a bisection has already started, a more verbose message is displayed -I solved a merge conflict in bisect.c due to the update of master branch. To submit a v3 I would need answer about how we rebase the commit history and what do we do to simplify the life of the user with the terms (see my last mails). I was thinking of: -a config variable that would say :as long as I don't reset keep the terms of the previous bisection or we could decided that this is the default behaviour of bisect. -two config variables to choose default terms I may have less time in the next days but I would like to submit a v3. -- 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 v2 7/7] bisect: allows any terms set by user
Louis Stuber louis--alexandre.stu...@ensimag.grenoble-inp.fr writes: Matthieu Moy matthieu@grenoble-inp.fr writes: Modifying in PATCH 7 some code that you introduced in PATCH 3 is suspicious. Is there any reason you did not name the variable terms_defined in the first place? (i.e. squash this hunk and the other instance of start_bad_good into PATCH 3) (Whether this is a rethorical question is up to you ;-) ) In the previouses versions where we only want to introduce old/new, the terms can only be defined in bisect_start if the user typed start bad good. The name start_bad_good is not very explicit indeed, but isn't it more appropriate in this case than terms_defined ? I agree with Louis, but maybe a consistant commit history is more important. But if only the first patches (which implement old/new ) would come to be accepted the name of the variable would sounds strange. I don't understand why you need to delete this file. I did not review thoroughly so there may be a reason, but you can help the reader with a comment here. I think it's a mistake. I'd say we should put this test just before the bisect_clean_state || exit line, but that would deserve more attention indeed. The idea is to delete the file at the right moment because we don't want it to exist again when the user starts an other bisection, but also have an intelligent behaviour if the start command fails. Yes if the start commands fails, like if you gave wrong sha1, it would be nice not to have to type again 'git bisect terms ...' . There is no use to put it before clean_state because clean_state because clean_state will actually delete the file. So we can just let clean_state do this. -- 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 4/4] bisect: add the terms old/new
Hi, thanks for the review, (sorry if you received this twice) Junio C Hamano gits...@pobox.com writes: 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 Yes it would be nice and should not be hard to implement. But it was not the idea of how the code was made by our elders. For now we just rebased, corrected and finishing to implement functionalities. * 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. It will be corrected in the next patch. 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. It is still nice and necessary to have new/old as builtin tags but if we have time we will do that. The others point have been taken in account. -- 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 3/4] bisect: simplify the add of new bisect terms
Hi,
Thanks for the review !
(sorry if you received this twice)
Christian Couder christian.cou...@gmail.com wrote :
+ name_bad = bad;
+ name_good = good;
+ } else {
+ strbuf_getline(str, fp, '\n');
+ name_bad = strbuf_detach(str, NULL);
+ strbuf_getline(str, fp, '\n');
+ name_good = strbuf_detach(str, NULL);
+ }
I would have kept just
name_bad = bad;
name_good = good;
in this patch, and introduce BISECT_TERMS in a separate one.
Yeah I agree that it is more logical to have first a patch that does
on bisect.c the same thing as patch 2 does on git-bisect.sh.
For example the patch series could be for now:
1) bisect: typo fix
2) bisect: replace hardcoded bad|good with variables
3) git-bisect: replace hardcoded bad|good with variables
4) bisect: simplify adding new terms
5) bisect: add old/new terms
For now we will keep name_bad and name_good as variables.
About the patch series shouldn't I squash the commit 2) and 3) into 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 2/4] bisect: replace hardcoded bad|good by variables
Hi,
Thanks for the review,
(sorry if you received this twice)
Matthieu Moy matthieu@grenoble-inp.fr wrote:
+static const char *name_bad;
+static const char *name_good;
Same remark as PATCH 2.
After the discussion you had with Christian I think we will
keep name_bad/good for now.
-
- 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(name_bad, bad)) {
+ 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);
+ }
You need an else here. Maybe it comes later, but as a reviewer, I want
to check that you did not forget it now (because I don't trust myself to
remember that it must be added later).
Should I put an else {} with nothing in beetween?
+ name_bad = bad;
+ name_good = good;
+ } else {
+ strbuf_getline(str, fp, '\n');
+ name_bad = strbuf_detach(str, NULL);
+ strbuf_getline(str, fp, '\n');
+ name_good = strbuf_detach(str, NULL);
+ }
I would have kept just
name_bad = bad;
name_good = good;
in this patch, and introduce BISECT_TERMS in a separate one.
Should not I introduce BISECT_TERMS in bisect.c and git-bisect.sh
with the same commit? I did some rebase though and now name_bad and
name_good appears in the first commit, and read_bisect_terms in the
second.
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -77,6 +77,7 @@ bisect_start() {
orig_args=$(git rev-parse --sq-quote $@)
bad_seen=0
eval=''
+ start_bad_good=0
if test z$(git rev-parse --is-bare-repository) != zfalse
then
mode=--no-checkout
@@ -101,6 +102,9 @@ bisect_start() {
die $(eval_gettext '\$arg' does not appear to be a valid revision)
break
}
+
+ start_bad_good=1
+
Why do you need this variable? It seems to me that you are hardcoding
once more that terms can be either good/bad or old/new, which you
tried to eliminate from the previous round.
I answered to Junio on this too, it is because our function which create
the bisect_terms file is not called after
'git bisect start bad_rev good_rev'.
+ then
+ echo $NAME_BAD $GIT_DIR/BISECT_TERMS
+ echo $NAME_GOOD $GIT_DIR/BISECT_TERMS
+ fi
Why not do this unconditionnally? Whether terms are good/bad or old/new,
you can write them to BISECT_TERMS.
Because after a git bisect start we don't yet what terms are used.
This line is only for the case 'git bisect start bad_rev good_rev'.
+ fi
+ case $cmd in
+ bad|good)
+ if test ! -s $GIT_DIR/BISECT_TERMS
+ then
+ echo bad $GIT_DIR/BISECT_TERMS
+ echo good $GIT_DIR/BISECT_TERMS
+ fi
+ NAME_BAD=bad
+ NAME_GOOD=good ;;
+ esac ;;
+ esac
+}
+
+bisect_voc () {
+ case $1 in
+ bad) echo bad ;;
+ good) echo good ;;
+ esac
+}
It's weird to have these hardcoded bad/good when you already have
BISECT_TERMS in the same patch.
bisect_voc is used to display what commands the user can do, thus the
builtins tags. I did not find a way to not hardcode them.
The other points have been taken into account.
--
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 v2 3/7] bisect: simplify the addition of new bisect terms
We create a file BISECT_TERMS in the repository .git to be read during a
bisection. The fonctions to be changed if we add new terms are quite
few.
In git-bisect.sh :
check_and_set_terms
bisect_voc
In bisect.c :
handle_bad_merge_base
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
---
bisect.c | 33 +--
git-bisect.sh | 66 +++-
2 files changed, 90 insertions(+), 9 deletions(-)
diff --git a/bisect.c b/bisect.c
index eb2f555..827d2f3 100644
--- a/bisect.c
+++ b/bisect.c
@@ -745,7 +745,10 @@ static void handle_bad_merge_base(void)
between %s and [%s].\n,
bad_hex, bad_hex, good_hex);
} else {
- die(BUG: terms %s/%s not managed, name_bad,
name_good);
+ fprintf(stderr, The merge base %s is %s.\n
+ This means the first commit marked %s is
+ between %s and [%s].\n,
+ bad_hex, name_bad, name_bad, bad_hex, good_hex);
}
exit(3);
}
@@ -900,6 +903,31 @@ static void show_diff_tree(const char *prefix, struct
commit *commit)
}
/*
+ * The terms used for this bisect session are stored in
+ * BISECT_TERMS: it can be bad/good or new/old.
+ * We read them and store them to adapt the messages
+ * accordingly. Default is bad/good.
+ */
+void read_bisect_terms(void)
+{
+ struct strbuf str = STRBUF_INIT;
+ const char *filename = git_path(BISECT_TERMS);
+ FILE *fp = fopen(filename, r);
+
+ if (!fp) {
+ die(could not read file '%s': %s, filename,
+ strerror(errno));
+ } else {
+ strbuf_getline(str, fp, '\n');
+ name_bad = strbuf_detach(str, NULL);
+ strbuf_getline(str, fp, '\n');
+ name_good = strbuf_detach(str, NULL);
+ }
+ strbuf_release(str);
+ fclose(fp);
+}
+
+/*
* We use the convention that exiting with an exit code 10 means that
* the bisection process finished successfully.
* In this case the calling shell script should exit 0.
@@ -915,8 +943,7 @@ int bisect_next_all(const char *prefix, int no_checkout)
const unsigned char *bisect_rev;
char bisect_rev_hex[GIT_SHA1_HEXSZ + 1];
- name_bad=bad;
- name_good=good;
+ read_bisect_terms();
if (read_bisect_refs())
die(reading bisect refs failed);
diff --git a/git-bisect.sh b/git-bisect.sh
index ce6412f..d63b4b0 100644
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -77,6 +77,9 @@ bisect_start() {
orig_args=$(git rev-parse --sq-quote $@)
bad_seen=0
eval=''
+ # start_bad_good is used to detect if we did a
+ # 'git bisect start bad_rev good_rev'
+ start_bad_good=0
if test z$(git rev-parse --is-bare-repository) != zfalse
then
mode=--no-checkout
@@ -101,6 +104,9 @@ bisect_start() {
die $(eval_gettext '\$arg' does not appear to
be a valid revision)
break
}
+
+ start_bad_good=1
+
case $bad_seen in
0) state=$NAME_BAD ; bad_seen=1 ;;
*) state=$NAME_GOOD ;;
@@ -172,6 +178,11 @@ bisect_start() {
}
git rev-parse --sq-quote $@ $GIT_DIR/BISECT_NAMES
eval $eval true
+ if test $start_bad_good -eq 1 test ! -s $GIT_DIR/BISECT_TERMS
+ then
+ echo $NAME_BAD $GIT_DIR/BISECT_TERMS
+ echo $NAME_GOOD $GIT_DIR/BISECT_TERMS
+ fi
echo git bisect start$orig_args $GIT_DIR/BISECT_LOG || exit
#
# Check if we can proceed to the next bisect state.
@@ -232,6 +243,7 @@ bisect_skip() {
bisect_state() {
bisect_autostart
state=$1
+ check_and_set_terms $state
case $#,$state in
0,*)
die $(gettext Please call 'bisect_state' with at least one
argument.) ;;
@@ -291,15 +303,17 @@ bisect_next_check() {
: bisect without $NAME_GOOD...
;;
*)
-
+ bad_syn=$(bisect_voc bad)
+ good_syn=$(bisect_voc good)
if test -s $GIT_DIR/BISECT_START
then
- gettextln You need to give me at least one good
[PATCH v2 2/7] bisect: replace hardcoded bad|good by variables
To add new tags like old/new and have keywords less confusing, the
first step is to avoid hardcoding the keywords.
The default mode is still bad/good.
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
---
bisect.c | 49 -
git-bisect.sh | 57 +++--
2 files changed, 63 insertions(+), 43 deletions(-)
mode change 100755 = 100644 git-bisect.sh
diff --git a/bisect.c b/bisect.c
index de92953..eb2f555 100644
--- a/bisect.c
+++ b/bisect.c
@@ -21,6 +21,9 @@ static const char *argv_checkout[] = {checkout, -q, NULL,
--, NULL};
static const char *argv_show_branch[] = {show-branch, NULL, NULL};
static const char *argv_update_ref[] = {update-ref, --no-deref,
BISECT_HEAD, NULL, NULL};
+static const char *name_bad;
+static const char *name_good;
+
/* Remember to update object flag allocation in object.h */
#define COUNTED(1u16)
@@ -403,10 +406,14 @@ struct commit_list *find_bisection(struct commit_list
*list,
static int register_ref(const char *refname, const unsigned char *sha1,
int flags, void *cb_data)
{
- if (!strcmp(refname, bad)) {
+ char good_prefix[256];
+ strcpy(good_prefix, name_good);
+ strcat(good_prefix, -);
+
+ 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_prefix)) {
sha1_array_append(good_revs, sha1);
} else if (starts_with(refname, skip-)) {
sha1_array_append(skipped_revs, sha1);
@@ -634,7 +641,7 @@ static void exit_if_skipped_commits(struct commit_list
*tried,
return;
printf(There are only 'skip'ped commits left to test.\n
- The first bad commit could be any of:\n);
+ The first %s commit could be any of:\n, name_bad);
print_commit_list(tried, %s\n, %s\n);
if (bad)
printf(%s\n, oid_to_hex(bad));
@@ -732,18 +739,21 @@ 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(name_bad, bad)) {
+ 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);
+ } else {
+ die(BUG: terms %s/%s not managed, name_bad,
name_good);
+ }
exit(3);
}
- fprintf(stderr, Some good revs are not ancestor of the bad rev.\n
+ fprintf(stderr, Some %s revs are not ancestor of the %s rev.\n
git bisect cannot work properly in this case.\n
- Maybe you mistook good and bad revs?\n);
+ Maybe you mistook %s and %s revs?\n,
+ name_good, name_bad, name_good, name_bad);
exit(1);
}
@@ -755,10 +765,10 @@ static void handle_skipped_merge_base(const unsigned char
*mb)
warning(the merge base between %s and [%s]
must be skipped.\n
- So we cannot be sure the first bad commit is
+ So we cannot be sure the first %s commit is
between %s and %s.\n
We continue anyway.,
- bad_hex, good_hex, mb_hex, bad_hex);
+ bad_hex, good_hex, name_bad, mb_hex, bad_hex);
free(good_hex);
}
@@ -839,7 +849,7 @@ static void check_good_are_ancestors_of_bad(const char
*prefix, int no_checkout)
int fd;
if (!current_bad_oid)
- die(a bad revision is needed);
+ die(a %s revision is needed, name_bad);
/* Check if file BISECT_ANCESTORS_OK exists. */
if (!stat(filename, st) S_ISREG(st.st_mode))
@@ -905,6 +915,8 @@ int bisect_next_all(const char *prefix, int no_checkout)
const unsigned char *bisect_rev;
char bisect_rev_hex[GIT_SHA1_HEXSZ + 1];
+ name_bad=bad
[PATCH v2 4/7] bisect: add the terms old/new
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. * 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 --- Documentation/git-bisect.txt | 48 - bisect.c | 11 +++-- git-bisect.sh| 30 + t/t6030-bisect-porcelain.sh | 38 + 4 files changed, 112 insertions(+), 15 deletions(-) diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt index 4cb52a7..3c3021a 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,35 @@ 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, 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. +It is the equivalent of git bisect bad [rev]. + + +git bisect old [rev...] + + +Marks the commit as old, as the opposite of 'git bisect new'. +It is the equivalent of git bisect good [rev...]. + +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. + Bisect visualize @@ -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 + ++ +Let's consider the last commit has a given property, and that we are looking +for the commit which introduced this property. For each commit the bisection +guide 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. +At the end of the bisect session, the result will be the first new commit (e.g +the first one with the property). + SEE ALSO diff --git a/bisect.c
[PATCH v2 5/7] bisect: change read_bisect_terms parameters
From: Louis Stuber stub...@ensimag.grenoble-inp.fr
The function reads BISECT_TERMS and stores it at the adress given in
parameters (instead of global variables name_bad and name_good).
This allows to use the function outside bisect.c.
Signed-off-by: Antoine Delaite antoine.dela...@ensimag.grenoble-inp.fr
Signed-off-by: Louis Stuber stub...@ensimag.grenoble-inp.fr
---
bisect.c | 15 +++
1 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/bisect.c b/bisect.c
index eaa85b6..7afd335 100644
--- a/bisect.c
+++ b/bisect.c
@@ -908,12 +908,11 @@ static void show_diff_tree(const char *prefix, struct
commit *commit)
}
/*
- * The terms used for this bisect session are stored in
- * BISECT_TERMS: it can be bad/good or new/old.
- * We read them and store them to adapt the messages
- * accordingly. Default is bad/good.
+ * The terms used for this bisect session are stored in BISECT_TERMS.
+ * We read them and store them to adapt the messages accordingly.
+ * Default is bad/good.
*/
-void read_bisect_terms(void)
+void read_bisect_terms(const char **read_bad, const char **read_good)
{
struct strbuf str = STRBUF_INIT;
const char *filename = git_path(BISECT_TERMS);
@@ -924,9 +923,9 @@ void read_bisect_terms(void)
strerror(errno));
} else {
strbuf_getline(str, fp, '\n');
- name_bad = strbuf_detach(str, NULL);
+ *read_bad = strbuf_detach(str, NULL);
strbuf_getline(str, fp, '\n');
- name_good = strbuf_detach(str, NULL);
+ *read_good = strbuf_detach(str, NULL);
}
strbuf_release(str);
fclose(fp);
@@ -948,7 +947,7 @@ int bisect_next_all(const char *prefix, int no_checkout)
const unsigned char *bisect_rev;
char bisect_rev_hex[GIT_SHA1_HEXSZ + 1];
- read_bisect_terms();
+ read_bisect_terms(name_bad, name_good);
if (read_bisect_refs())
die(reading bisect refs failed);
--
1.7.1
--
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 v2 7/7] bisect: allows any terms set by user
Introduction of the git bisect terms function.
The user can set its own terms.
List of known commands not available :
`git bisect replay`
`git bisect terms term1 term2
then
git bisect start bad_rev good_rev`
Signed-off-by: Antoine Delaite antoine.dela...@ensimag.grenoble-inp.fr
Signed-off-by: Louis Stuber stub...@ensimag.grenoble-inp.fr
---
Documentation/git-bisect.txt | 19 ++
git-bisect.sh| 44 ++---
2 files changed, 55 insertions(+), 8 deletions(-)
diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
index 3c3021a..ef0c03c 100644
--- a/Documentation/git-bisect.txt
+++ b/Documentation/git-bisect.txt
@@ -133,6 +133,25 @@ 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 term1 term2
+
+
+This command has to be used before a bisection has started.
+The term1 must be associated with the latest revisions and term2 with the
+ancestors of term1.
+
+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 term1
+term2'.
+
+
Bisect visualize
diff --git a/git-bisect.sh b/git-bisect.sh
index c012f5d..22d65b1 100644
--- 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 term1 term2
+ set up term1 and term2 as bisection terms.
git bisect skip [(rev|range)...]
mark rev... untestable revisions.
git bisect next
@@ -79,9 +81,16 @@ bisect_start() {
orig_args=$(git rev-parse --sq-quote $@)
bad_seen=0
eval=''
- # start_bad_good is used to detect if we did a
- # 'git bisect start bad_rev good_rev'
- start_bad_good=0
+ # terms_defined is used to detect if we did a
+ # 'git bisect start bad_rev good_rev' or if the user
+ # defined his own terms with git bisect terms
+ terms_defined=0
+ if test -s $GIT_DIR/TERMS_DEFINED
+ then
+ terms_defined=1
+ get_terms
+ rm -rf $GIT_DIR/TERMS_DEFINED
+ fi
if test z$(git rev-parse --is-bare-repository) != zfalse
then
mode=--no-checkout
@@ -107,7 +116,7 @@ bisect_start() {
break
}
- start_bad_good=1
+ terms_defined=1
case $bad_seen in
0) state=$NAME_BAD ; bad_seen=1 ;;
@@ -180,7 +189,7 @@ bisect_start() {
}
git rev-parse --sq-quote $@ $GIT_DIR/BISECT_NAMES
eval $eval true
- if test $start_bad_good -eq 1 test ! -s $GIT_DIR/BISECT_TERMS
+ if test $terms_defined -eq 1 test ! -s $GIT_DIR/BISECT_TERMS
then
echo $NAME_BAD $GIT_DIR/BISECT_TERMS
echo $NAME_GOOD $GIT_DIR/BISECT_TERMS
@@ -419,6 +428,7 @@ bisect_clean_state() {
rm -f $GIT_DIR/BISECT_NAMES
rm -f $GIT_DIR/BISECT_RUN
rm -f $GIT_DIR/BISECT_TERMS
+ rm -f $GIT_DIR/TERMS_DEFINED
# Cleanup head-name if it got left by an old version of git-bisect
rm -f $GIT_DIR/head-name
git update-ref -d --no-deref BISECT_HEAD
@@ -529,7 +539,8 @@ get_terms () {
check_and_set_terms () {
cmd=$1
case $cmd in
- bad|good|new|old)
+ skip) ;;
+ *)
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.)
@@ -562,6 +573,21 @@ bisect_voc () {
esac
}
+bisect_terms () {
+ test $# -eq 2 ||
+ die You need to give me at least two arguments
+
+ if ! test -s $GIT_DIR/BISECT_START
+ then
+ echo $1 $GIT_DIR/BISECT_TERMS
+ echo $2 $GIT_DIR/BISECT_TERMS
+ echo 1 $GIT_DIR/TERMS_DEFINED
+ else
+ die A bisection has already started, please use \
+ 'git bisect reset' to restart and change the terms
+ fi
[PATCH v2 6/7] revision: fix rev-list --bisect in old/new mode
From: Louis Stuber stub...@ensimag.grenoble-inp.fr
Calling git rev-list --bisect when an old/new mode bisection was started
shows the help notice. This has been fixed by reading BISECT_TERMS in
revision.c to find the correct bisect refs path (which was always
refs/bisect/bad (or good) before and can be refs/bisect/new (old) now).
Signed-off-by: Louis Stuber stub...@ensimag.grenoble-inp.fr
Signed-off-by: Antoine Delaite antoine.dela...@ensimag.grenoble-inp.fr
---
revision.c | 16 ++--
1 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/revision.c b/revision.c
index 7ddbaa0..a332270 100644
--- a/revision.c
+++ b/revision.c
@@ -21,6 +21,9 @@
volatile show_early_output_fn_t show_early_output;
+static const char *name_bad;
+static const char *name_good;
+
char *path_name(const struct name_path *path, const char *name)
{
const struct name_path *p;
@@ -2073,14 +2076,22 @@ void parse_revision_opt(struct rev_info *revs, struct
parse_opt_ctx_t *ctx,
ctx-argc -= n;
}
+extern void read_bisect_terms(const char **bad, const char **good);
+
static int for_each_bad_bisect_ref(const char *submodule, each_ref_fn fn, void
*cb_data)
{
- return for_each_ref_in_submodule(submodule, refs/bisect/bad, fn,
cb_data);
+ char bisect_refs_path[256];
+ strcpy(bisect_refs_path, refs/bisect/);
+ strcat(bisect_refs_path, name_bad);
+ return for_each_ref_in_submodule(submodule, bisect_refs_path, fn,
cb_data);
}
static int for_each_good_bisect_ref(const char *submodule, each_ref_fn fn,
void *cb_data)
{
- return for_each_ref_in_submodule(submodule, refs/bisect/good, fn,
cb_data);
+ char bisect_refs_path[256];
+ strcpy(bisect_refs_path, refs/bisect/);
+ strcat(bisect_refs_path, name_good);
+ return for_each_ref_in_submodule(submodule, bisect_refs_path, fn,
cb_data);
}
static int handle_revision_pseudo_opt(const char *submodule,
@@ -2109,6 +2120,7 @@ static int handle_revision_pseudo_opt(const char
*submodule,
handle_refs(submodule, revs, *flags,
for_each_branch_ref_submodule);
clear_ref_exclusion(revs-ref_excludes);
} else if (!strcmp(arg, --bisect)) {
+ read_bisect_terms(name_bad, name_good);
handle_refs(submodule, revs, *flags, for_each_bad_bisect_ref);
handle_refs(submodule, revs, *flags ^ (UNINTERESTING | BOTTOM),
for_each_good_bisect_ref);
revs-bisect = 1;
--
1.7.1
--
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 3/4] bisect: simplify the add of new bisect terms
Hi,
Thanks for the review,
Junio C Hamano gits...@pobox.com writes:
/*
+ * The terms used for this bisect session are stocked in
+ * BISECT_TERMS: it can be bad/good or new/old.
+ * We read them and stock them to adapt the messages
+ * accordingly. Default is bad/good.
+ */
s/stock/store/ perhaps? I think the idea is not to have this file
in the default case so that absence of it would mean you would be
looking for a transition from (older) good to (more recent) bad.
Yes it is nice but a bisect_terms file is a very useful tool for
verifications at a little cost. For instance, if the user type this:
git bisect start
git bisect bad HEAD
git bisect old HEAD~10
We created the bisect_terms file after the bad then the error is
directly detected when the user type git bisect old.
And I think having we should limit the impact of this good/bad
default case as we would prefer old/new.
+void read_bisect_terms(void)
+{
+struct strbuf str = STRBUF_INIT;
+const char *filename = git_path(BISECT_TERMS);
+FILE *fp = fopen(filename, r);
+
+if (!fp) {
We might want to see why fopen() failed here. If it is because the
file did not exist, great. But otherwise?
Should we display a specific message and cancel the last command?
diff --git a/git-bisect.sh b/git-bisect.sh
index 1f16aaf..529bb43 100644
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -77,6 +77,7 @@ bisect_start() {
orig_args=$(git rev-parse --sq-quote $@)
bad_seen=0
eval=''
+start_bad_good=0
if test z$(git rev-parse --is-bare-repository) != zfalse
then
mode=--no-checkout
@@ -101,6 +102,9 @@ bisect_start() {
die $(eval_gettext '\$arg' does not
appear to be a valid revision)
break
}
+
+start_bad_good=1
+
It is unclear what this variable means, or what it means to have
zero or one as its value.
This has been done by our elders and we kept because it was
useful. We are however trying to remove it. It is in case of
a 'git bisect start bad_rev good_rev', our function that creates
the bisect_terms is not called. Thus we need to do it manually
in the code.
+get_terms () {
+if test -s $GIT_DIR/BISECT_TERMS
+then
+NAME_BAD=$(sed -n 1p $GIT_DIR/BISECT_TERMS)
+NAME_GOOD=$(sed -n 2p $GIT_DIR/BISECT_TERMS)
It is sad that we need to open the file twice. Can't we do
something using read perhaps?
The cost of it is quite low and we see directly what we meant. We didn't
found a pretty way to read two lines with read.
Don't we want to make sure these two names are sensible? We do not
want an empty-string, for example. I suspect you do not want to
take anything that check-ref-format does not like.
Same comment applies to the C code.
Yes, for now only bad/good/old/new are allowed. But in the future
it will be necessary.
+bisect_voc () {
+case $1 in
+bad) echo bad ;;
+good) echo good ;;
+esac
+}
What is voc?
What if $1 is neither bad/good?
Did you mean to translate 'bad' to $NAME_BAD and 'good' to $NAME_GOOD?
voc stands for vocabulary.
This fonction is mainly used after to display all the builtins possibility. It
is only called internally and if the argument is bad it returns the synonyms of
bad (bad|new in the next patch).
--
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 4/4] bisect: add the terms old/new
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. * 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 --- Documentation/git-bisect.txt | 48 ++-- bisect.c | 15 ++ git-bisect.sh| 32 +++-- t/t6030-bisect-porcelain.sh | 38 +++ 4 files changed, 116 insertions(+), 17 deletions(-) diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt index 4cb52a7..441a4bd 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,35 @@ 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, 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. +It is the equivalent of git bisect bad [rev]. + + +git bisect old [rev...] + + +Marks the commit as old, as the opposite of 'git bisect new'. +It is the equivalent of git bisect good [rev...]. + +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. + Bisect visualize @@ -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. For each commit the bisection guide 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. +At the end of the bisect session, the result will be the first new commit (e.g +the first one with the property). + SEE ALSO diff --git a/bisect.c b/bisect.c
[PATCH 2/4] bisect: replace hardcoded bad|good by variables
To add new tags like old/new and have keywords less confusing, the
first step is to avoid hardcoding the keywords.
The default mode is still bad/good.
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
---
git-bisect.sh | 47 ++-
1 file changed, 26 insertions(+), 21 deletions(-)
mode change 100755 = 100644 git-bisect.sh
diff --git a/git-bisect.sh b/git-bisect.sh
old mode 100755
new mode 100644
index ae3fec2..1f16aaf
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -32,6 +32,8 @@ OPTIONS_SPEC=
_x40='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]'
_x40=$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40
+NAME_BAD=bad
+NAME_GOOD=good
bisect_head()
{
@@ -184,9 +186,12 @@ bisect_write() {
rev=$2
nolog=$3
case $state in
- bad)tag=$state ;;
- good|skip) tag=$state-$rev ;;
- *) die $(eval_gettext Bad bisect_write argument:
\$state) ;;
+ $NAME_BAD)
+ tag=$state ;;
+ $NAME_GOOD|skip)
+ tag=$state-$rev ;;
+ *)
+ die $(eval_gettext Bad bisect_write argument:
\$state) ;;
esac
git update-ref refs/bisect/$tag $rev || exit
echo # $state: $(git show-branch $rev) $GIT_DIR/BISECT_LOG
@@ -230,12 +235,12 @@ bisect_state() {
case $#,$state in
0,*)
die $(gettext Please call 'bisect_state' with at least one
argument.) ;;
- 1,bad|1,good|1,skip)
+ 1,$NAME_BAD|1,$NAME_GOOD|1,skip)
rev=$(git rev-parse --verify $(bisect_head)) ||
die $(gettext Bad rev input: $(bisect_head))
bisect_write $state $rev
check_expected_revs $rev ;;
- 2,bad|*,good|*,skip)
+ 2,$NAME_BAD|*,$NAME_GOOD|*,skip)
shift
hash_list=''
for rev in $@
@@ -249,8 +254,8 @@ bisect_state() {
bisect_write $state $rev
done
check_expected_revs $hash_list ;;
- *,bad)
- die $(gettext 'git bisect bad' can take only one argument.)
;;
+ *,$NAME_BAD)
+ die $(gettext 'git bisect $NAME_BAD' can take only one
argument.) ;;
*)
usage ;;
esac
@@ -259,21 +264,21 @@ bisect_state() {
bisect_next_check() {
missing_good= missing_bad=
- git show-ref -q --verify refs/bisect/bad || missing_bad=t
- test -n $(git for-each-ref refs/bisect/good-*) || missing_good=t
+ git show-ref -q --verify refs/bisect/$NAME_BAD || missing_bad=t
+ test -n $(git for-each-ref refs/bisect/$NAME_GOOD-*) ||
missing_good=t
case $missing_good,$missing_bad,$1 in
,,*)
- : have both good and bad - ok
+ : have both $NAME_GOOD and $NAME_BAD - ok
;;
*,)
# do not have both but not asked to fail - just report.
false
;;
- t,,good)
+ t,,$NAME_GOOD)
# have bad but not good. we could bisect although
# this is less optimum.
- gettextln Warning: bisecting only with a bad commit. 2
+ gettextln Warning: bisecting only with a $NAME_BAD commit. 2
if test -t 0
then
# TRANSLATORS: Make sure to include [Y] and [n] in your
@@ -283,7 +288,7 @@ bisect_next_check() {
read yesno
case $yesno in [Nn]*) exit 1 ;; esac
fi
- : bisect without good...
+ : bisect without $NAME_GOOD...
;;
*)
@@ -307,7 +312,7 @@ bisect_auto_next() {
bisect_next() {
case $# in 0) ;; *) usage ;; esac
bisect_autostart
- bisect_next_check good
+ bisect_next_check $NAME_GOOD
# Perform all bisection computation, display and checkout
git bisect--helper --next-all $(test -f $GIT_DIR/BISECT_HEAD echo
--no-checkout)
@@ -316,15 +321,15 @@ bisect_next() {
# Check if we should exit because bisection is finished
if test $res -eq 10
then
- bad_rev=$(git show-ref --hash --verify refs/bisect/bad)
+ bad_rev=$(git show-ref --hash --verify refs/bisect/$NAME_BAD)
bad_commit=$(git show-branch $bad_rev)
echo # first bad commit: $bad_commit $GIT_DIR/BISECT_LOG
[PATCH 1/4] bisect : correction of typo
--- bisect.c| 2 +- t/t6030-bisect-porcelain.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/bisect.c b/bisect.c index 10f5e57..de92953 100644 --- a/bisect.c +++ b/bisect.c @@ -743,7 +743,7 @@ static void handle_bad_merge_base(void) fprintf(stderr, Some good revs are not ancestor of the bad rev.\n git bisect cannot work properly in this case.\n - Maybe you mistake good and bad revs?\n); + Maybe you mistook good and bad revs?\n); exit(1); } diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh index 06b4868..9e2c203 100755 --- a/t/t6030-bisect-porcelain.sh +++ b/t/t6030-bisect-porcelain.sh @@ -362,7 +362,7 @@ test_expect_success 'bisect starting with a detached HEAD' ' test_expect_success 'bisect errors out if bad and good are mistaken' ' git bisect reset test_must_fail git bisect start $HASH2 $HASH4 2 rev_list_error - grep mistake good and bad rev_list_error + grep mistook good and bad rev_list_error git bisect reset ' -- 2.4.1.414.ge7a9de3.dirty -- 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 3/4] bisect: simplify the add of new bisect terms
We create a file BISECT_TERMS in the repository .git to be read during a
bisection. The fonctions to be changed if we add new terms are quite
few.
In git-bisect.sh :
check_and_set_terms
bisect_voc
In bisect.c :
handle_bad_merge_base
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
---
bisect.c | 65 ---
git-bisect.sh | 58
2 files changed, 103 insertions(+), 20 deletions(-)
diff --git a/bisect.c b/bisect.c
index de92953..3b7df85 100644
--- a/bisect.c
+++ b/bisect.c
@@ -21,6 +21,9 @@ static const char *argv_checkout[] = {checkout, -q, NULL,
--, NULL};
static const char *argv_show_branch[] = {show-branch, NULL, NULL};
static const char *argv_update_ref[] = {update-ref, --no-deref,
BISECT_HEAD, NULL, NULL};
+static const char *name_bad;
+static const char *name_good;
+
/* Remember to update object flag allocation in object.h */
#define COUNTED(1u16)
@@ -403,7 +406,7 @@ struct commit_list *find_bisection(struct commit_list *list,
static int register_ref(const char *refname, const unsigned char *sha1,
int flags, void *cb_data)
{
- if (!strcmp(refname, bad)) {
+ 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-)) {
@@ -634,7 +637,7 @@ static void exit_if_skipped_commits(struct commit_list
*tried,
return;
printf(There are only 'skip'ped commits left to test.\n
- The first bad commit could be any of:\n);
+ The first %s commit could be any of:\n, name_bad);
print_commit_list(tried, %s\n, %s\n);
if (bad)
printf(%s\n, oid_to_hex(bad));
@@ -732,18 +735,19 @@ 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(name_bad, bad)) {
+ 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);
+ }
exit(3);
}
- fprintf(stderr, Some good revs are not ancestor of the bad rev.\n
+ fprintf(stderr, Some %s revs are not ancestor of the %s rev.\n
git bisect cannot work properly in this case.\n
- Maybe you mistook good and bad revs?\n);
+ Maybe you mistook %s and %s revs?\n,
+ name_good, name_bad, name_good, name_bad);
exit(1);
}
@@ -755,10 +759,10 @@ static void handle_skipped_merge_base(const unsigned char
*mb)
warning(the merge base between %s and [%s]
must be skipped.\n
- So we cannot be sure the first bad commit is
+ So we cannot be sure the first %s commit is
between %s and %s.\n
We continue anyway.,
- bad_hex, good_hex, mb_hex, bad_hex);
+ bad_hex, good_hex, name_bad, mb_hex, bad_hex);
free(good_hex);
}
@@ -839,7 +843,7 @@ static void check_good_are_ancestors_of_bad(const char
*prefix, int no_checkout)
int fd;
if (!current_bad_oid)
- die(a bad revision is needed);
+ die(a %s revision is needed, name_bad);
/* Check if file BISECT_ANCESTORS_OK exists. */
if (!stat(filename, st) S_ISREG(st.st_mode))
@@ -890,6 +894,31 @@ static void show_diff_tree(const char *prefix, struct
commit *commit)
}
/*
+ * The terms used for this bisect session are stocked in
+ * BISECT_TERMS: it can be bad/good or new/old.
+ * We read them and stock them to adapt the messages
+ * accordingly. Default is bad/good.
+ */
+void read_bisect_terms(void)
+{
+ struct strbuf str = STRBUF_INIT;
+ const char *filename = git_path(BISECT_TERMS);
+ FILE *fp = fopen(filename, r);
+
+ if (!fp) {
+ name_bad = bad;
+ name_good
[WIP PATCH 1/3] git bisect old/new
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
[PATCH 3/3] bisect: fix indentation
From: Christian Couder chrisc...@tuxfamily.org
Signed-off-by: Antoine Delaite antoine.dela...@ensimag.grenoble-inp.fr
---
bisect.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/bisect.c b/bisect.c
index 68417bb..87a5f6d 100644
--- a/bisect.c
+++ b/bisect.c
@@ -915,10 +915,10 @@ void read_bisect_terms(void)
name_bad = bad;
name_good = good;
} else {
- strbuf_getline(str, fp, '\n');
- name_bad = strbuf_detach(str, NULL);
- strbuf_getline(str, fp, '\n');
- name_good = strbuf_detach(str, NULL);
+ strbuf_getline(str, fp, '\n');
+ name_bad = strbuf_detach(str, NULL);
+ strbuf_getline(str, fp, '\n');
+ name_good = strbuf_detach(str, NULL);
}
strbuf_release(str);
fclose(fp);
--
2.4.1.173.gd47f443.dirty
--
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 2/3] bisect: use name_(bad|good) instead of bisect_(bad|good)
From: Christian Couder chrisc...@tuxfamily.org
Signed-off-by: Antoine Delaite antoine.dela...@ensimag.grenoble-inp.fr
---
bisect.c | 35 +--
1 file changed, 17 insertions(+), 18 deletions(-)
diff --git a/bisect.c b/bisect.c
index d6c19fd..68417bb 100644
--- a/bisect.c
+++ b/bisect.c
@@ -21,8 +21,8 @@ static const char *argv_checkout[] = {checkout, -q, NULL,
--, NULL};
static const char *argv_show_branch[] = {show-branch, NULL, NULL};
static const char *argv_update_ref[] = {update-ref, --no-deref,
BISECT_HEAD, NULL, NULL};
-static const char *bisect_bad;
-static const char *bisect_good;
+static const char *name_bad;
+static const char *name_good;
/* Remember to update object flag allocation in object.h */
#define COUNTED(1u16)
@@ -406,7 +406,7 @@ struct commit_list *find_bisection(struct commit_list *list,
static int register_ref(const char *refname, const unsigned char *sha1,
int flags, void *cb_data)
{
- if (!strcmp(refname, bisect_bad)) {
+ 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-) ||
@@ -638,7 +638,7 @@ static void exit_if_skipped_commits(struct commit_list
*tried,
return;
printf(There are only 'skip'ped commits left to test.\n
- The first %s commit could be any of:\n, bisect_bad);
+ The first %s commit could be any of:\n, name_bad);
print_commit_list(tried, %s\n, %s\n);
if (bad)
printf(%s\n, oid_to_hex(bad));
@@ -736,7 +736,7 @@ 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, ' ');
- if (!strcmp(bisect_bad,bad)) {
+ if (!strcmp(name_bad,bad)) {
fprintf(stderr, The merge base %s is bad.\n
This means the bug has been fixed
between %s and [%s].\n,
@@ -753,8 +753,7 @@ static void handle_bad_merge_base(void)
fprintf(stderr, Some %s revs are not ancestor of the %s rev.\n
git bisect cannot work properly in this case.\n
Maybe you mistook %s and %s revs?\n,
- bisect_good, bisect_bad, bisect_good,
- bisect_bad);
+ name_good, name_bad, name_good, name_bad);
exit(1);
}
@@ -769,7 +768,7 @@ static void handle_skipped_merge_base(const unsigned char
*mb)
So we cannot be sure the first %s commit is
between %s and %s.\n
We continue anyway.,
- bad_hex, good_hex, bisect_bad, mb_hex, bad_hex);
+ bad_hex, good_hex, name_bad, mb_hex, bad_hex);
free(good_hex);
}
@@ -850,7 +849,7 @@ static void check_good_are_ancestors_of_bad(const char
*prefix, int no_checkout)
int fd;
if (!current_bad_oid)
- die(a %s revision is needed, bisect_bad);
+ die(a %s revision is needed, name_bad);
/* Check if file BISECT_ANCESTORS_OK exists. */
if (!stat(filename, st) S_ISREG(st.st_mode))
@@ -913,13 +912,13 @@ void read_bisect_terms(void)
FILE *fp = fopen(filename, r);
if (!fp) {
- bisect_bad = bad;
- bisect_good = good;
+ name_bad = bad;
+ name_good = good;
} else {
strbuf_getline(str, fp, '\n');
- bisect_bad = strbuf_detach(str, NULL);
+ name_bad = strbuf_detach(str, NULL);
strbuf_getline(str, fp, '\n');
- bisect_good = strbuf_detach(str, NULL);
+ name_good = strbuf_detach(str, NULL);
}
strbuf_release(str);
fclose(fp);
@@ -965,8 +964,8 @@ int bisect_next_all(const char *prefix, int no_checkout)
printf(%s was both %s and %s\n,
oid_to_hex(current_bad_oid),
- bisect_good,
- bisect_bad);
+ name_good,
+ name_bad);
exit(1);
}
@@ -982,7 +981,7 @@ int bisect_next_all(const char *prefix, int no_checkout)
if (!hashcmp(bisect_rev, current_bad_oid-hash)) {
exit_if_skipped_commits(tried, current_bad_oid);
printf(%s is the first %s commit\n, bisect_rev_hex,
- bisect_bad);
+ name_bad);
show_diff_tree(prefix, revs.commits-item);
/* This means the bisection process succeeded. */
exit(10);
@@ -994,8 +993,8 @@ int bisect_next_all(const char *prefix, int no_checkout)
(roughly %d step%s)\n, nr, (nr == 1 ? : s),
steps, (steps == 1 ? : s));
- free
GIT BISECT old/new (fix/unfixed)
Hi, git bisect old/new is an alternative to good/bad, which can be confusing to use in some situations. A work on it was done a few years ago, it is partially working but there are a lot of issues. We based on Christian Couder's github: https://github.com/chriscool/git/commits/boldnew2 We're currently working on it again. Older discussions lead to this state: 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. * thus, git bisect run cmd is not available for new/old. * git bisect visualize seem to work partially: the tags are displayed correctly but the tree is not limited to the bisect section. We did some testing with git bisect old/new and found that it was quite working, returning the commit that introduces the fix. We noticed an other error once the bisection is finished : fatal: 'refs/bisect/bad' - not a valid ref We did not understand why git bisect run is supposed not to work with old/new. The only thing is that the script has to return 0 if the version is old, which seems to work like in good/bad mode. This should be modified if the user wants to use the same script to find a fix or a regression (without changing the return value each time). Antoine D -- 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
