* [PATCH v7 1/5] bisect: correction of typo
2015-06-23 12:54 ` [PATCH v7 0/5] git bisect old/new Matthieu Moy
@ 2015-06-23 12:54 ` Matthieu Moy
2015-06-23 12:54 ` [PATCH v7 2/5] bisect: replace hardcoded "bad|good" by variables Matthieu Moy
` (6 subsequent siblings)
7 siblings, 0 replies; 48+ messages in thread
From: Matthieu Moy @ 2015-06-23 12:54 UTC (permalink / raw)
To: gitster
Cc: git, antoine.delaite, louis--alexandre.stuber, chriscool,
thomasxnguy, valentinduperray, Matthieu Moy
From: Antoine Delaite <antoine.delaite@ensimag.grenoble-inp.fr>
Signed-off-by: Antoine Delaite <antoine.delaite@ensimag.grenoble-inp.fr>
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.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
'
--
2.4.4.414.ge37915c
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH v7 2/5] bisect: replace hardcoded "bad|good" by variables
2015-06-23 12:54 ` [PATCH v7 0/5] git bisect old/new Matthieu Moy
2015-06-23 12:54 ` [PATCH v7 1/5] bisect: correction of typo Matthieu Moy
@ 2015-06-23 12:54 ` Matthieu Moy
2015-06-23 12:54 ` [PATCH v7 3/5] bisect: simplify the addition of new bisect terms Matthieu Moy
` (5 subsequent siblings)
7 siblings, 0 replies; 48+ messages in thread
From: Matthieu Moy @ 2015-06-23 12:54 UTC (permalink / raw)
To: gitster
Cc: git, antoine.delaite, louis--alexandre.stuber, chriscool,
thomasxnguy, valentinduperray, Louis Stuber, Valentin Duperray,
Franck Jonas, Lucien Kong, Thomas Nguy, Huynh Khoi Nguyen Nguyen,
Matthieu Moy, Matthieu Moy
From: Antoine Delaite <antoine.delaite@ensimag.grenoble-inp.fr>
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.delaite@ensimag.grenoble-inp.fr>
Signed-off-by: Louis Stuber <stuberl@ensimag.grenoble-inp.fr>
Signed-off-by: Valentin Duperray <Valentin.Duperray@ensimag.imag.fr>
Signed-off-by: Franck Jonas <Franck.Jonas@ensimag.imag.fr>
Signed-off-by: Lucien Kong <Lucien.Kong@ensimag.imag.fr>
Signed-off-by: Thomas Nguy <Thomas.Nguy@ensimag.imag.fr>
Signed-off-by: Huynh Khoi Nguyen Nguyen <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>
Signed-off-by: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.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..2d3dbdc 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 (1u<<16)
@@ -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 no_checkout)
const unsigned char *bisect_rev;
char bisect_rev_hex[GIT_SHA1_HEXSZ + 1];
+ name_bad = "bad";
+ name_good = "good";
if (read_bisect_refs())
die("reading bisect refs failed");
@@ -926,8 +940,10 @@ int bisect_next_all(const char *prefix, int no_checkout)
*/
exit_if_skipped_commits(tried, NULL);
- printf("%s was both good and bad\n",
- oid_to_hex(current_bad_oid));
+ printf("%s was both %s and %s\n",
+ oid_to_hex(current_bad_oid),
+ name_good,
+ name_bad);
exit(1);
}
@@ -942,7 +958,8 @@ 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 bad commit\n", bisect_rev_hex);
+ printf("%s is the first %s commit\n", bisect_rev_hex,
+ name_bad);
show_diff_tree(prefix, revs.commits->item);
/* This means the bisection process succeeded. */
exit(10);
diff --git a/git-bisect.sh b/git-bisect.sh
old mode 100755
new mode 100644
index ae3fec2..ce6412f
--- 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()
{
@@ -100,8 +102,8 @@ bisect_start() {
break
}
case $bad_seen in
- 0) state='bad' ; bad_seen=1 ;;
- *) state='good' ;;
+ 0) state=$NAME_BAD ; bad_seen=1 ;;
+ *) state=$NAME_GOOD ;;
esac
eval="$eval bisect_write '$state' '$rev' 'nolog' &&"
shift
@@ -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 "$(eval_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
+ eval_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,18 +321,18 @@ 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"
+ echo "# first $NAME_BAD commit: $bad_commit" >>"$GIT_DIR/BISECT_LOG"
exit 0
elif test $res -eq 2
then
echo "# only skipped commits left to test" >>"$GIT_DIR/BISECT_LOG"
- good_revs=$(git for-each-ref --format="%(objectname)" "refs/bisect/good-*")
- for skipped in $(git rev-list refs/bisect/bad --not $good_revs)
+ good_revs=$(git for-each-ref --format="%(objectname)" "refs/bisect/$NAME_GOOD-*")
+ for skipped in $(git rev-list refs/bisect/$NAME_BAD --not $good_revs)
do
skipped_commit=$(git show-branch $skipped)
- echo "# possible first bad commit: $skipped_commit" >>"$GIT_DIR/BISECT_LOG"
+ echo "# possible first $NAME_BAD commit: $skipped_commit" >>"$GIT_DIR/BISECT_LOG"
done
exit $res
fi
@@ -421,7 +426,7 @@ bisect_replay () {
start)
cmd="bisect_start $rev"
eval "$cmd" ;;
- good|bad|skip)
+ $NAME_GOOD|$NAME_BAD|skip)
bisect_write "$command" "$rev" ;;
*)
die "$(gettext "?? what are you talking about?")" ;;
@@ -455,9 +460,9 @@ exit code \$res from '\$command' is < 0 or >= 128" >&2
state='skip'
elif [ $res -gt 0 ]
then
- state='bad'
+ state="$NAME_BAD"
else
- state='good'
+ state="$NAME_GOOD"
fi
# We have to use a subshell because "bisect_state" can exit.
@@ -466,7 +471,7 @@ exit code \$res from '\$command' is < 0 or >= 128" >&2
cat "$GIT_DIR/BISECT_RUN"
- if sane_grep "first bad commit could be any of" "$GIT_DIR/BISECT_RUN" \
+ if sane_grep "first $NAME_BAD commit could be any of" "$GIT_DIR/BISECT_RUN" \
>/dev/null
then
gettextln "bisect run cannot continue any more" >&2
@@ -480,7 +485,7 @@ exit code \$res from '\$command' is < 0 or >= 128" >&2
exit $res
fi
- if sane_grep "is the first bad commit" "$GIT_DIR/BISECT_RUN" >/dev/null
+ if sane_grep "is the first $NAME_BAD commit" "$GIT_DIR/BISECT_RUN" >/dev/null
then
gettextln "bisect run success"
exit 0;
--
2.4.4.414.ge37915c
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH v7 3/5] bisect: simplify the addition of new bisect terms
2015-06-23 12:54 ` [PATCH v7 0/5] git bisect old/new Matthieu Moy
2015-06-23 12:54 ` [PATCH v7 1/5] bisect: correction of typo Matthieu Moy
2015-06-23 12:54 ` [PATCH v7 2/5] bisect: replace hardcoded "bad|good" by variables Matthieu Moy
@ 2015-06-23 12:54 ` Matthieu Moy
2015-06-23 17:49 ` Eric Sunshine
2015-06-23 12:54 ` [PATCH v7 4/5] bisect: add the terms old/new Matthieu Moy
` (4 subsequent siblings)
7 siblings, 1 reply; 48+ messages in thread
From: Matthieu Moy @ 2015-06-23 12:54 UTC (permalink / raw)
To: gitster
Cc: git, antoine.delaite, louis--alexandre.stuber, chriscool,
thomasxnguy, valentinduperray, Louis Stuber, Valentin Duperray,
Franck Jonas, Lucien Kong, Thomas Nguy, Huynh Khoi Nguyen Nguyen,
Matthieu Moy, Matthieu Moy
From: Antoine Delaite <antoine.delaite@ensimag.grenoble-inp.fr>
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
Co-authored-by: Louis Stuber <stuberl@ensimag.grenoble-inp.fr>
Tweaked-by: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
Signed-off-by: Antoine Delaite <antoine.delaite@ensimag.grenoble-inp.fr>
Signed-off-by: Louis Stuber <stuberl@ensimag.grenoble-inp.fr>
Signed-off-by: Valentin Duperray <Valentin.Duperray@ensimag.imag.fr>
Signed-off-by: Franck Jonas <Franck.Jonas@ensimag.imag.fr>
Signed-off-by: Lucien Kong <Lucien.Kong@ensimag.imag.fr>
Signed-off-by: Thomas Nguy <Thomas.Nguy@ensimag.imag.fr>
Signed-off-by: Huynh Khoi Nguyen Nguyen <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>
Signed-off-by: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
bisect.c | 38 +++++++++++++++++++++++++++++---
git-bisect.sh | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++++------
revision.c | 26 ++++++++++++++++++++--
3 files changed, 122 insertions(+), 12 deletions(-)
diff --git a/bisect.c b/bisect.c
index 2d3dbdc..08be634 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 == ENOENT) {
+ *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..7bb18db 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)
+ good_syn=$(bisect_voc good)
if test -s "$GIT_DIR/BISECT_START"
then
- gettextln "You need to give me at least one good and one bad revision.
-(You can use \"git bisect bad\" and \"git bisect good\" for that.)" >&2
+
+ eval_gettextln "You need to give me at least one \$bad_syn and one \$good_syn revision.
+(You can use \"git bisect \$bad_syn\" and \"git bisect \$good_syn\" for that.)" >&2
else
- gettextln "You need to start by \"git bisect start\".
-You then need to give me at least one good and one bad revision.
-(You can use \"git bisect bad\" and \"git bisect good\" for that.)" >&2
+ eval_gettextln "You need to start by \"git bisect start\".
+You then need to give me at least one \$good_syn and one \$bad_syn revision.
+(You can use \"git bisect \$bad_syn\" and \"git bisect \$good_syn\" for that.)" >&2
fi
exit 1 ;;
esac
@@ -402,6 +416,7 @@ bisect_clean_state() {
rm -f "$GIT_DIR/BISECT_LOG" &&
rm -f "$GIT_DIR/BISECT_NAMES" &&
rm -f "$GIT_DIR/BISECT_RUN" &&
+ rm -f "$GIT_DIR/BISECT_TERMS" &&
# 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 &&
@@ -422,11 +437,13 @@ bisect_replay () {
rev="$command"
command="$bisect"
fi
+ get_terms
+ check_and_set_terms "$command"
case "$command" in
start)
cmd="bisect_start $rev"
eval "$cmd" ;;
- $NAME_GOOD|$NAME_BAD|skip)
+ "$NAME_GOOD"|"$NAME_BAD"|skip)
bisect_write "$command" "$rev" ;;
*)
die "$(gettext "?? what are you talking about?")" ;;
@@ -499,11 +516,50 @@ bisect_log () {
cat "$GIT_DIR/BISECT_LOG"
}
+get_terms () {
+ if test -s "$GIT_DIR/BISECT_TERMS"
+ then
+ {
+ read NAME_BAD
+ read NAME_GOOD
+ } <"$GIT_DIR/BISECT_TERMS"
+ fi
+}
+
+check_and_set_terms () {
+ cmd="$1"
+ case "$cmd" in
+ bad|good)
+ 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.")"
+ 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
+}
+
case "$#" in
0)
usage ;;
*)
cmd="$1"
+ get_terms
shift
case "$cmd" in
help)
diff --git a/revision.c b/revision.c
index 3ff8723..f22923f 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,32 @@ 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);
+ struct strbuf bisect_refs_buf = STRBUF_INIT;
+ const char *bisect_refs_str;
+ int status;
+ strbuf_addstr(&bisect_refs_buf, "refs/bisect/");
+ strbuf_addstr(&bisect_refs_buf, name_bad);
+ bisect_refs_str = strbuf_detach(&bisect_refs_buf, NULL);
+ status = for_each_ref_in_submodule(submodule, bisect_refs_str, fn, cb_data);
+ free((char *)bisect_refs_str);
+ return status;
}
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);
+ struct strbuf bisect_refs_buf = STRBUF_INIT;
+ const char *bisect_refs_str;
+ int status;
+ strbuf_addstr(&bisect_refs_buf, "refs/bisect/");
+ strbuf_addstr(&bisect_refs_buf, name_bad);
+ bisect_refs_str = strbuf_detach(&bisect_refs_buf, NULL);
+ status = for_each_ref_in_submodule(submodule, bisect_refs_str, fn, cb_data);
+ free((char *)bisect_refs_str);
+ return status;
}
static int handle_revision_pseudo_opt(const char *submodule,
@@ -2112,6 +2133,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;
--
2.4.4.414.ge37915c
^ permalink raw reply related [flat|nested] 48+ messages in thread* Re: [PATCH v7 3/5] bisect: simplify the addition of new bisect terms
2015-06-23 12:54 ` [PATCH v7 3/5] bisect: simplify the addition of new bisect terms Matthieu Moy
@ 2015-06-23 17:49 ` Eric Sunshine
2015-06-23 18:18 ` Matthieu Moy
0 siblings, 1 reply; 48+ messages in thread
From: Eric Sunshine @ 2015-06-23 17:49 UTC (permalink / raw)
To: Matthieu Moy
Cc: Junio C Hamano, Git List, Antoine Delaite,
Louis-Alexandre Stuber, Christian Couder, Thomas Nguy,
Valentin Duperray, Louis Stuber, Valentin Duperray, Franck Jonas,
Lucien Kong, Thomas Nguy, Huynh Khoi Nguyen Nguyen, Matthieu Moy
On Tue, Jun 23, 2015 at 8:54 AM, Matthieu Moy <Matthieu.Moy@imag.fr> wrote:
> diff --git a/revision.c b/revision.c
> index 3ff8723..f22923f 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -2076,14 +2079,32 @@ 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);
> + struct strbuf bisect_refs_buf = STRBUF_INIT;
> + const char *bisect_refs_str;
> + int status;
> + strbuf_addstr(&bisect_refs_buf, "refs/bisect/");
> + strbuf_addstr(&bisect_refs_buf, name_bad);
A single strbuf_addf() rather than two strbuf_addstr()s?
> + bisect_refs_str = strbuf_detach(&bisect_refs_buf, NULL);
> + status = for_each_ref_in_submodule(submodule, bisect_refs_str, fn, cb_data);
> + free((char *)bisect_refs_str);
Why the above rather than the simpler?
strbuf_addstr(&bisect_refs, ...);
status = for_each_ref_in_submodule(submodule, bisect_refs.buf, fn, cb_data);
strbuf_release(&bisect_refs);
What am I missing?
> + return status;
> }
>
> 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);
> + struct strbuf bisect_refs_buf = STRBUF_INIT;
> + const char *bisect_refs_str;
> + int status;
> + strbuf_addstr(&bisect_refs_buf, "refs/bisect/");
> + strbuf_addstr(&bisect_refs_buf, name_bad);
> + bisect_refs_str = strbuf_detach(&bisect_refs_buf, NULL);
> + status = for_each_ref_in_submodule(submodule, bisect_refs_str, fn, cb_data);
> + free((char *)bisect_refs_str);
Ditto.
> + return status;
> }
^ permalink raw reply [flat|nested] 48+ messages in thread* Re: [PATCH v7 3/5] bisect: simplify the addition of new bisect terms
2015-06-23 17:49 ` Eric Sunshine
@ 2015-06-23 18:18 ` Matthieu Moy
0 siblings, 0 replies; 48+ messages in thread
From: Matthieu Moy @ 2015-06-23 18:18 UTC (permalink / raw)
To: Eric Sunshine
Cc: Junio C Hamano, Git List, Antoine Delaite,
Louis-Alexandre Stuber, Christian Couder, Thomas Nguy,
Valentin Duperray, Louis Stuber, Valentin Duperray, Franck Jonas,
Lucien Kong, Thomas Nguy, Huynh Khoi Nguyen Nguyen
Eric Sunshine <sunshine@sunshineco.com> writes:
> On Tue, Jun 23, 2015 at 8:54 AM, Matthieu Moy <Matthieu.Moy@imag.fr> wrote:
>> + strbuf_addstr(&bisect_refs_buf, "refs/bisect/");
>> + strbuf_addstr(&bisect_refs_buf, name_bad);
>
> A single strbuf_addf() rather than two strbuf_addstr()s?
>> + bisect_refs_str = strbuf_detach(&bisect_refs_buf, NULL);
>> + status = for_each_ref_in_submodule(submodule, bisect_refs_str, fn, cb_data);
>> + free((char *)bisect_refs_str);
>
> Why the above rather than the simpler?
>
> strbuf_addstr(&bisect_refs, ...);
> status = for_each_ref_in_submodule(submodule, bisect_refs.buf, fn, cb_data);
> strbuf_release(&bisect_refs);
>
> What am I missing?
Indeed, your version is much better.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v7 4/5] bisect: add the terms old/new
2015-06-23 12:54 ` [PATCH v7 0/5] git bisect old/new Matthieu Moy
` (2 preceding siblings ...)
2015-06-23 12:54 ` [PATCH v7 3/5] bisect: simplify the addition of new bisect terms Matthieu Moy
@ 2015-06-23 12:54 ` Matthieu Moy
2015-06-23 19:27 ` Remi Galan Alfonso
2015-06-23 12:54 ` [PATCH v7 5/5] bisect: allows any terms set by user Matthieu Moy
` (3 subsequent siblings)
7 siblings, 1 reply; 48+ messages in thread
From: Matthieu Moy @ 2015-06-23 12:54 UTC (permalink / raw)
To: gitster
Cc: git, antoine.delaite, louis--alexandre.stuber, chriscool,
thomasxnguy, valentinduperray, Louis Stuber, Valentin Duperray,
Franck Jonas, Lucien Kong, Thomas Nguy, Huynh Khoi Nguyen Nguyen,
Matthieu Moy, Matthieu Moy
From: Antoine Delaite <antoine.delaite@ensimag.grenoble-inp.fr>
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.delaite@ensimag.grenoble-inp.fr>
Signed-off-by: Louis Stuber <stuberl@ensimag.grenoble-inp.fr>
Signed-off-by: Valentin Duperray <Valentin.Duperray@ensimag.imag.fr>
Signed-off-by: Franck Jonas <Franck.Jonas@ensimag.imag.fr>
Signed-off-by: Lucien Kong <Lucien.Kong@ensimag.imag.fr>
Signed-off-by: Thomas Nguy <Thomas.Nguy@ensimag.imag.fr>
Signed-off-by: Huynh Khoi Nguyen Nguyen <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>
Signed-off-by: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.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 08be634..ab09650 100644
--- a/bisect.c
+++ b/bisect.c
@@ -746,6 +746,11 @@ static void handle_bad_merge_base(void)
"This means the bug has been fixed "
"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);
} else {
fprintf(stderr, "The merge base %s is %s.\n"
"This means the first commit marked %s is "
@@ -778,11 +783,11 @@ static void handle_skipped_merge_base(const unsigned char *mb)
}
/*
- * "check_merge_bases" checks that merge bases are not "bad".
+ * "check_merge_bases" checks that merge bases are not "bad" (or "new").
*
- * - If one is "bad", it means the user assumed something wrong
+ * - If one is "bad" (or "new"), it means the user assumed something wrong
* and we must exit with a non 0 error code.
- * - If one is "good", that's good, we have nothing to do.
+ * - If one is "good" (or "old"), that's good, we have nothing to do.
* - If one is "skipped", we can't know but we should warn.
* - If we don't know, we should check it out and ask the user to test.
*/
diff --git a/git-bisect.sh b/git-bisect.sh
index 7bb18db..73763a2 100644
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -1,14 +1,16 @@
#!/bin/sh
-USAGE='[help|start|bad|good|skip|next|reset|visualize|replay|log|run]'
+USAGE='[help|start|bad|good|new|old|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>...]
reset bisect state and start bisection.
-git bisect bad [<rev>]
- mark <rev> a known-bad revision.
-git bisect good [<rev>...]
- mark <rev>... known-good revisions.
+git bisect (bad|new) [<rev>]
+ mark <rev> a known-bad revision/
+ a revision after change in a given property.
+git bisect (good|old) [<rev>...]
+ mark <rev>... known-good revisions/
+ revisions before change in a given property.
git bisect skip [(<rev>|<range>)...]
mark <rev>... untestable revisions.
git bisect next
@@ -288,7 +290,7 @@ bisect_next_check() {
false
;;
t,,"$NAME_GOOD")
- # have bad but not good. we could bisect although
+ # have bad (or new) but not good (or old). we could bisect although
# this is less optimum.
eval_gettextln "Warning: bisecting only with a \$NAME_BAD commit." >&2
if test -t 0
@@ -529,7 +531,7 @@ get_terms () {
check_and_set_terms () {
cmd="$1"
case "$cmd" in
- bad|good)
+ bad|good|new|old)
if test -s "$GIT_DIR/BISECT_TERMS" && test "$cmd" != "$NAME_BAD" && test "$cmd" != "$NAME_GOOD"
then
die "$(eval_gettext "Invalid command: you're currently in a \$NAME_BAD/\$NAME_GOOD bisect.")"
@@ -543,14 +545,22 @@ check_and_set_terms () {
fi
NAME_BAD="bad"
NAME_GOOD="good" ;;
+ new|old)
+ if ! test -s "$GIT_DIR/BISECT_TERMS"
+ then
+ echo "new" >"$GIT_DIR/BISECT_TERMS" &&
+ echo "old" >>"$GIT_DIR/BISECT_TERMS"
+ fi
+ NAME_BAD="new"
+ NAME_GOOD="old" ;;
esac ;;
esac
}
bisect_voc () {
case "$1" in
- bad) echo "bad" ;;
- good) echo "good" ;;
+ bad) echo "bad|old" ;;
+ good) echo "good|new" ;;
esac
}
@@ -566,7 +576,7 @@ case "$#" in
git bisect -h ;;
start)
bisect_start "$@" ;;
- bad|good)
+ bad|good|new|old)
bisect_state "$cmd" "$@" ;;
skip)
bisect_skip "$@" ;;
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index 9e2c203..983c503 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -759,4 +759,42 @@ test_expect_success '"git bisect bad HEAD" behaves as "git bisect bad"' '
git bisect reset
'
+test_expect_success 'bisect starts with only one new' '
+ git bisect reset &&
+ git bisect start &&
+ git bisect new $HASH4 &&
+ git bisect next
+'
+
+test_expect_success 'bisect does not start with only one old' '
+ git bisect reset &&
+ git bisect start &&
+ git bisect old $HASH1 &&
+ test_must_fail git bisect next
+'
+
+test_expect_success 'bisect start with one new and old' '
+ git bisect reset &&
+ git bisect start &&
+ git bisect old $HASH1 &&
+ git bisect new $HASH4 &&
+ git bisect new &&
+ git bisect new >bisect_result &&
+ grep "$HASH2 is the first new commit" bisect_result &&
+ git bisect log >log_to_replay.txt &&
+ git bisect reset
+'
+
+test_expect_success 'bisect replay with old and new' '
+ git bisect replay log_to_replay.txt >bisect_result &&
+ grep "$HASH2 is the first new commit" bisect_result &&
+ git bisect reset
+'
+
+test_expect_success 'bisect cannot mix old/new and good/bad' '
+ git bisect start &&
+ git bisect bad $HASH4 &&
+ test_must_fail git bisect old $HASH1
+'
+
test_done
--
2.4.4.414.ge37915c
^ permalink raw reply related [flat|nested] 48+ messages in thread* Re: [PATCH v7 4/5] bisect: add the terms old/new
2015-06-23 12:54 ` [PATCH v7 4/5] bisect: add the terms old/new Matthieu Moy
@ 2015-06-23 19:27 ` Remi Galan Alfonso
2015-06-23 20:26 ` Matthieu Moy
0 siblings, 1 reply; 48+ messages in thread
From: Remi Galan Alfonso @ 2015-06-23 19:27 UTC (permalink / raw)
To: Matthieu Moy
Cc: gitster, git, antoine delaite, louis--alexandre stuber,
chriscool, thomasxnguy, valentinduperray, Louis Stuber,
Valentin Duperray, Franck Jonas, Lucien Kong, Thomas Nguy,
Huynh Khoi Nguyen Nguyen, Matthieu Moy
Matthieu Moy <Matthieu.Moy@imag.fr> writes:
> Signed-off-by: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
Sounds like you went all out with this patch.
Rémi
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v7 4/5] bisect: add the terms old/new
2015-06-23 19:27 ` Remi Galan Alfonso
@ 2015-06-23 20:26 ` Matthieu Moy
0 siblings, 0 replies; 48+ messages in thread
From: Matthieu Moy @ 2015-06-23 20:26 UTC (permalink / raw)
To: Remi Galan Alfonso
Cc: gitster, git, antoine delaite, louis--alexandre stuber,
chriscool, thomasxnguy, valentinduperray, Louis Stuber,
Valentin Duperray, Franck Jonas, Lucien Kong, Thomas Nguy,
Huynh Khoi Nguyen Nguyen
Remi Galan Alfonso <remi.galan-alfonso@ensimag.grenoble-inp.fr> writes:
> Matthieu Moy <Matthieu.Moy@imag.fr> writes:
>> Signed-off-by: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
>> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
>
> Sounds like you went all out with this patch.
Ah, the first line was in the original patch, and the second is added by
send-email -s ... Both me and myself agree that one of them can be
removed indeed ;-).
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v7 5/5] bisect: allows any terms set by user
2015-06-23 12:54 ` [PATCH v7 0/5] git bisect old/new Matthieu Moy
` (3 preceding siblings ...)
2015-06-23 12:54 ` [PATCH v7 4/5] bisect: add the terms old/new Matthieu Moy
@ 2015-06-23 12:54 ` Matthieu Moy
2015-06-23 18:48 ` Junio C Hamano
2015-06-23 19:04 ` [PATCH v7 0/5] git bisect old/new Junio C Hamano
` (2 subsequent siblings)
7 siblings, 1 reply; 48+ messages in thread
From: Matthieu Moy @ 2015-06-23 12:54 UTC (permalink / raw)
To: gitster
Cc: git, antoine.delaite, louis--alexandre.stuber, chriscool,
thomasxnguy, valentinduperray, Louis Stuber, Matthieu Moy
From: Antoine Delaite <antoine.delaite@ensimag.grenoble-inp.fr>
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.delaite@ensimag.grenoble-inp.fr>
Signed-off-by: Louis Stuber <stuberl@ensimag.grenoble-inp.fr>
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.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 73763a2..8ef2b94 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
+ }<"$GIT_DIR/BISECT_TERMS"
+ gettextln "Your current terms are $term1 and $term2."
+ else
+ die "$(gettext "No terms defined.")"
+ fi ;;
+ 2)
+ check_term_format refs/bisect/"$1"
+ check_term_format refs/bisect/"$2"
+ 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"
+ echo "git bisect terms $NAME_BAD $NAME_GOOD" >>"$GIT_DIR/BISECT_LOG" || exit
+ else
+ die "$(gettext "A bisection has already started, and you can't change terms in the middle of it.
+Use 'git bisect terms' to see the current terms.
+Otherwise, to start a new bisection with new terms, please use 'git bisect reset' and set the terms before the start")"
+ fi ;;
+ *)
+ usage ;;
+ esac
+}
+
+check_term_format () {
+ arg="$1"
+ git check-ref-format $arg ||
+ die "$(eval_gettext "'\$arg' is not a valid term")"
+}
+
case "$#" in
0)
usage ;;
@@ -576,7 +632,7 @@ case "$#" in
git bisect -h ;;
start)
bisect_start "$@" ;;
- bad|good|new|old)
+ bad|good|new|old|"$NAME_BAD"|"$NAME_GOOD")
bisect_state "$cmd" "$@" ;;
skip)
bisect_skip "$@" ;;
@@ -593,6 +649,8 @@ case "$#" in
bisect_log ;;
run)
bisect_run "$@" ;;
+ terms)
+ bisect_terms "$@" ;;
*)
usage ;;
esac
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index 983c503..289dbb0 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -797,4 +797,47 @@ test_expect_success 'bisect cannot mix old/new and good/bad' '
test_must_fail git bisect old $HASH1
'
+test_expect_success 'bisect start with one term1 and term2' '
+ git bisect reset &&
+ git bisect terms term1 term2 &&
+ git bisect start &&
+ git bisect term2 $HASH1 &&
+ git bisect term1 $HASH4 &&
+ git bisect term1 &&
+ git bisect term1 >bisect_result &&
+ grep "$HASH2 is the first term1 commit" bisect_result &&
+ git bisect log >log_to_replay.txt &&
+ git bisect reset
+'
+
+test_expect_success 'bisect replay with term1 and term2' '
+ git bisect replay log_to_replay.txt >bisect_result &&
+ grep "$HASH2 is the first term1 commit" bisect_result &&
+ git bisect reset
+'
+
+test_expect_success 'bisect start term1 term2' '
+ git bisect reset &&
+ git bisect terms term1 term2 &&
+ git bisect start $HASH4 $HASH1 &&
+ git bisect term1 &&
+ git bisect term1 >bisect_result &&
+ grep "$HASH2 is the first term1 commit" bisect_result &&
+ git bisect log >log_to_replay.txt &&
+ git bisect reset
+'
+
+test_expect_success 'bisect cannot mix terms' '
+ git bisect reset &&
+ git bisect terms a b &&
+ git bisect terms term1 term2 &&
+ git bisect start $HASH4 $HASH1 &&
+ test_must_fail git bisect a &&
+ test_must_fail git bisect b &&
+ test_must_fail git bisect bad &&
+ test_must_fail git bisect good &&
+ test_must_fail git bisect new &&
+ test_must_fail git bisect old
+'
+
test_done
--
2.4.4.414.ge37915c
^ permalink raw reply related [flat|nested] 48+ messages in thread* Re: [PATCH v7 0/5] git bisect old/new
2015-06-23 12:54 ` [PATCH v7 0/5] git bisect old/new Matthieu Moy
` (4 preceding siblings ...)
2015-06-23 12:54 ` [PATCH v7 5/5] bisect: allows any terms set by user Matthieu Moy
@ 2015-06-23 19:04 ` Junio C Hamano
2015-06-23 20:16 ` Matthieu Moy
2015-06-24 15:17 ` [PATCH v8 0/5] Bisect terms Matthieu Moy
2015-06-25 18:50 ` [PATCH v9 0/5] bisect terms Matthieu Moy
7 siblings, 1 reply; 48+ messages in thread
From: Junio C Hamano @ 2015-06-23 19:04 UTC (permalink / raw)
To: Matthieu Moy
Cc: git, antoine.delaite, louis--alexandre.stuber, chriscool,
thomasxnguy, valentinduperray
Matthieu Moy <Matthieu.Moy@imag.fr> writes:
> I fixed a few minor issues in v6. Patch between my version and v6 is
> below. I also pushed my branch here:
>
> https://github.com/moy/git/tree/bisect-terms
It is somewhat confusing to see v3 yesterday and then this v7 next
day. How did I miss v4 thru v6?
> Not visible in the patch below: I squashed PATCH 5 into PATCH 3 to
> avoid the pattern "break stuff and then repair it".
Good.
> The first two patches seem ready.
Yeah, the first one is obviously fine ;-), and I agree the second
one looks more or less OK.
Regarding the second and third one, the messages they give when the
user marked one tip of a side branch as old and the other new gave
me a hiccup while reading them, though.
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 {
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);
The "bad" side is inherited from the original and not your fault,
but it was already very hard to understand. The other side is not
just unreadable, but I think is incorrect and confusing to say
"first commit marked %(name_bad)s"; you know there are history
segments whose oldest ends (i.e. merge base that is bad) are marked
as 'bad', and the other ends are marked as 'good', and you haven't
marked any of the commits in between yet. So there is no "first
commit marked" either as bad or good there between these endpoints
(yet).
Also I was somewhat puzzled and disappointed to still see
name_{bad,good} not name_{new,old} used as variable names even in
the endgame patch, though. Is that intended?
> PATCH 4 (add old/new) is still buggy. When starting a bisection with
>
> git bisect start $old $new
>
> the command "git bisect visualize" does not work (it shows no commit).
>
> I consider PATCH 5 as WIP, I think it would need a lot of polishing
> and testing to be mergeable. I think a reasonable objective for now it
> to get old/new working in the user-interface, and drop PATCH 5 from
> the series when it gets merged. The existance of PATCH 5 is a very
> good thing even if it doesn't get merged:
>
> * The fact that it's possible to do it on top of the series shows that
> we make the code more generic. I think it's important that
> regardless of features, the code moves in the right direction.
>
> * The patch can be taken over later by someone else.
Yeah, if I may rephrase to make sure we are on the same page, in
order for 5/5 to be done sanely, 1-4/5 must be giving a good
foundation to build on. I agree with that, I agree that including a
polished 5/5 would be a good thing, and then I further agree that
1-4/5 could be delivered before 5/5 is ready.
Thanks.
^ permalink raw reply [flat|nested] 48+ messages in thread* Re: [PATCH v7 0/5] git bisect old/new
2015-06-23 19:04 ` [PATCH v7 0/5] git bisect old/new Junio C Hamano
@ 2015-06-23 20:16 ` Matthieu Moy
2015-06-23 20:34 ` Junio C Hamano
0 siblings, 1 reply; 48+ messages in thread
From: Matthieu Moy @ 2015-06-23 20:16 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, antoine.delaite, louis--alexandre.stuber, chriscool,
thomasxnguy, valentinduperray
Junio C Hamano <gitster@pobox.com> writes:
> Matthieu Moy <Matthieu.Moy@imag.fr> writes:
>
>> I fixed a few minor issues in v6. Patch between my version and v6 is
>> below. I also pushed my branch here:
>>
>> https://github.com/moy/git/tree/bisect-terms
>
> It is somewhat confusing to see v3 yesterday and then this v7 next
> day. How did I miss v4 thru v6?
Oops, I pattern-matched the wrong part when reading [PATCH v3 6/6].
Indeed, this should have been numberred v4, I should have s/v6/v3/ in my
sentence above.
> Regarding the second and third one, the messages they give when the
> user marked one tip of a side branch as old and the other new gave
> me a hiccup while reading them, though.
>
> 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 {
> 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);
>
> The "bad" side is inherited from the original and not your fault,
> but it was already very hard to understand. The other side is not
> just unreadable, but I think is incorrect and confusing to say
> "first commit marked %(name_bad)s"; you know there are history
> segments whose oldest ends (i.e. merge base that is bad) are marked
> as 'bad', and the other ends are marked as 'good', and you haven't
> marked any of the commits in between yet. So there is no "first
> commit marked" either as bad or good there between these endpoints
> (yet).
The situation is (the bisection started with bad=branch1 and
good=branch2):
---- base (name_bad) ------ branch1 (name_bad)
\
`----------------- branch2 (name_good)
So, the first commit having property name_good is between base and
branch2. So, the message seems wrong in two ways:
* As you say, the wording "marked as" seem to imply that we did mark the
commit, while we actually did not explore base..branch2
I'd write "the first '%s' commit" (the important is to remove
"marked").
* The message uses 'name_bad' where it should use 'name_good'. Indeed,
the original message talks about "bug fixed", i.e. the first _good_
commit is in base..branch2.
Is this what you meant?
(Sorry, I need drawings and bullet lists to think properly ;-) ).
Actually, I think it would make sense to include my drawing above in a
comment in the code.
> Also I was somewhat puzzled and disappointed to still see
> name_{bad,good} not name_{new,old} used as variable names even in
> the endgame patch, though. Is that intended?
I still think that name_old/name_new would have been much better names
if we were to write bisect from scratch, but I got convinced by
Christian that name_bad/name_good made sense too. Even if we adopted
old/new in these variables, we would still have e.g. a variable
"bad_seen" in the code. So, moving the codebase to adopt the old/new
convention internally is more than just chosing the name of variables to
avoid hardcoded "good"/"bad" string litterals. Moving the brain of
developpers to adopt the old/new convention is even another debate.
I don't know if this is the reason why Antoine did not change it, but
that's why I did not insist further and did not do the change myself.
> Yeah, if I may rephrase to make sure we are on the same page, in
> order for 5/5 to be done sanely, 1-4/5 must be giving a good
> foundation to build on. I agree with that, I agree that including a
> polished 5/5 would be a good thing, and then I further agree that
> 1-4/5 could be delivered before 5/5 is ready.
Yes.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply [flat|nested] 48+ messages in thread* Re: [PATCH v7 0/5] git bisect old/new
2015-06-23 20:16 ` Matthieu Moy
@ 2015-06-23 20:34 ` Junio C Hamano
0 siblings, 0 replies; 48+ messages in thread
From: Junio C Hamano @ 2015-06-23 20:34 UTC (permalink / raw)
To: Matthieu Moy
Cc: git, antoine.delaite, louis--alexandre.stuber, chriscool,
thomasxnguy, valentinduperray
Matthieu Moy <Matthieu.Moy@Grenoble-INP.fr> writes:
>> It is somewhat confusing to see v3 yesterday and then this v7 next
>> day. How did I miss v4 thru v6?
>
> Oops, I pattern-matched the wrong part when reading [PATCH v3 6/6].
> Indeed, this should have been numberred v4, I should have s/v6/v3/ in my
> sentence above.
OK.
> So, the first commit having property name_good is between base and
> branch2. So, the message seems wrong in two ways:
Yeah, I agree with you on both points.
> Actually, I think it would make sense to include my drawing above
> in a comment in the code.
Sounds like a good idea.
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v8 0/5] Bisect terms
2015-06-23 12:54 ` [PATCH v7 0/5] git bisect old/new Matthieu Moy
` (5 preceding siblings ...)
2015-06-23 19:04 ` [PATCH v7 0/5] git bisect old/new Junio C Hamano
@ 2015-06-24 15:17 ` Matthieu Moy
2015-06-24 15:17 ` [PATCH v8 1/5] bisect: correction of typo Matthieu Moy
` (6 more replies)
2015-06-25 18:50 ` [PATCH v9 0/5] bisect terms Matthieu Moy
7 siblings, 7 replies; 48+ messages in thread
From: Matthieu Moy @ 2015-06-24 15:17 UTC (permalink / raw)
To: gitster
Cc: git, antoine.delaite, louis--alexandre.stuber, chriscool,
thomasxnguy, valentinduperray, Matthieu Moy
This is a minor iteration over v7 to take into account Junio and
Eric's comments, AND fix an important typo that I introduced in the
strbuf code conversion (I used name_good instead of name_bad). This
fixes the "git bisect visualize" bug I found earlier. I played a bit
with the result and didn't find any bug.
Except for the last patch, it seems at least close to mergeable.
diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
index ef0c03c..a37336e 100644
--- a/Documentation/git-bisect.txt
+++ b/Documentation/git-bisect.txt
@@ -134,7 +134,7 @@ You must run `git bisect start` without commits as argument and run
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.
diff --git a/revision.c b/revision.c
index f22923f..24ce842 100644
--- a/revision.c
+++ b/revision.c
@@ -2083,27 +2083,21 @@ 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)
{
- struct strbuf bisect_refs_buf = STRBUF_INIT;
- const char *bisect_refs_str;
+ struct strbuf bisect_refs = STRBUF_INIT;
int status;
- strbuf_addstr(&bisect_refs_buf, "refs/bisect/");
- strbuf_addstr(&bisect_refs_buf, name_bad);
- bisect_refs_str = strbuf_detach(&bisect_refs_buf, NULL);
- status = for_each_ref_in_submodule(submodule, bisect_refs_str, fn, cb_data);
- free((char *)bisect_refs_str);
+ strbuf_addf(&bisect_refs, "refs/bisect/%s", name_bad);
+ status = for_each_ref_in_submodule(submodule, bisect_refs.buf, fn, cb_data);
+ strbuf_release(&bisect_refs);
return status;
}
static int for_each_good_bisect_ref(const char *submodule, each_ref_fn fn, void *cb_data)
{
- struct strbuf bisect_refs_buf = STRBUF_INIT;
- const char *bisect_refs_str;
+ struct strbuf bisect_refs = STRBUF_INIT;
int status;
- strbuf_addstr(&bisect_refs_buf, "refs/bisect/");
- strbuf_addstr(&bisect_refs_buf, name_bad);
- bisect_refs_str = strbuf_detach(&bisect_refs_buf, NULL);
- status = for_each_ref_in_submodule(submodule, bisect_refs_str, fn, cb_data);
- free((char *)bisect_refs_str);
+ strbuf_addf(&bisect_refs, "refs/bisect/%s", name_good);
+ status = for_each_ref_in_submodule(submodule, bisect_refs.buf, fn, cb_data);
+ strbuf_release(&bisect_refs);
return status;
}
Antoine Delaite (5):
bisect: correction of typo
bisect: replace hardcoded "bad|good" by variables
bisect: simplify the addition of new bisect terms
bisect: add the terms old/new
bisect: allow any terms set by user
Documentation/git-bisect.txt | 67 +++++++++++++-
bisect.c | 94 +++++++++++++++-----
git-bisect.sh | 207 +++++++++++++++++++++++++++++++++++--------
revision.c | 20 ++++-
t/t6030-bisect-porcelain.sh | 83 ++++++++++++++++-
5 files changed, 407 insertions(+), 64 deletions(-)
mode change 100755 => 100644 git-bisect.sh
--
2.4.4.414.g59d82e6
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH v8 1/5] bisect: correction of typo
2015-06-24 15:17 ` [PATCH v8 0/5] Bisect terms Matthieu Moy
@ 2015-06-24 15:17 ` Matthieu Moy
2015-06-24 15:17 ` [PATCH v8 2/5] bisect: replace hardcoded "bad|good" by variables Matthieu Moy
` (5 subsequent siblings)
6 siblings, 0 replies; 48+ messages in thread
From: Matthieu Moy @ 2015-06-24 15:17 UTC (permalink / raw)
To: gitster
Cc: git, antoine.delaite, louis--alexandre.stuber, chriscool,
thomasxnguy, valentinduperray, Matthieu Moy
From: Antoine Delaite <antoine.delaite@ensimag.grenoble-inp.fr>
Signed-off-by: Antoine Delaite <antoine.delaite@ensimag.grenoble-inp.fr>
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.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
'
--
2.4.4.414.g59d82e6
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH v8 2/5] bisect: replace hardcoded "bad|good" by variables
2015-06-24 15:17 ` [PATCH v8 0/5] Bisect terms Matthieu Moy
2015-06-24 15:17 ` [PATCH v8 1/5] bisect: correction of typo Matthieu Moy
@ 2015-06-24 15:17 ` Matthieu Moy
2015-06-24 15:17 ` [PATCH v8 3/5] bisect: simplify the addition of new bisect terms Matthieu Moy
` (4 subsequent siblings)
6 siblings, 0 replies; 48+ messages in thread
From: Matthieu Moy @ 2015-06-24 15:17 UTC (permalink / raw)
To: gitster
Cc: git, antoine.delaite, louis--alexandre.stuber, chriscool,
thomasxnguy, valentinduperray, Louis Stuber, Valentin Duperray,
Franck Jonas, Lucien Kong, Thomas Nguy, Huynh Khoi Nguyen Nguyen,
Matthieu Moy
From: Antoine Delaite <antoine.delaite@ensimag.grenoble-inp.fr>
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.delaite@ensimag.grenoble-inp.fr>
Signed-off-by: Louis Stuber <stuberl@ensimag.grenoble-inp.fr>
Signed-off-by: Valentin Duperray <Valentin.Duperray@ensimag.imag.fr>
Signed-off-by: Franck Jonas <Franck.Jonas@ensimag.imag.fr>
Signed-off-by: Lucien Kong <Lucien.Kong@ensimag.imag.fr>
Signed-off-by: Thomas Nguy <Thomas.Nguy@ensimag.imag.fr>
Signed-off-by: Huynh Khoi Nguyen Nguyen <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.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..2d3dbdc 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 (1u<<16)
@@ -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 no_checkout)
const unsigned char *bisect_rev;
char bisect_rev_hex[GIT_SHA1_HEXSZ + 1];
+ name_bad = "bad";
+ name_good = "good";
if (read_bisect_refs())
die("reading bisect refs failed");
@@ -926,8 +940,10 @@ int bisect_next_all(const char *prefix, int no_checkout)
*/
exit_if_skipped_commits(tried, NULL);
- printf("%s was both good and bad\n",
- oid_to_hex(current_bad_oid));
+ printf("%s was both %s and %s\n",
+ oid_to_hex(current_bad_oid),
+ name_good,
+ name_bad);
exit(1);
}
@@ -942,7 +958,8 @@ 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 bad commit\n", bisect_rev_hex);
+ printf("%s is the first %s commit\n", bisect_rev_hex,
+ name_bad);
show_diff_tree(prefix, revs.commits->item);
/* This means the bisection process succeeded. */
exit(10);
diff --git a/git-bisect.sh b/git-bisect.sh
old mode 100755
new mode 100644
index ae3fec2..ce6412f
--- 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()
{
@@ -100,8 +102,8 @@ bisect_start() {
break
}
case $bad_seen in
- 0) state='bad' ; bad_seen=1 ;;
- *) state='good' ;;
+ 0) state=$NAME_BAD ; bad_seen=1 ;;
+ *) state=$NAME_GOOD ;;
esac
eval="$eval bisect_write '$state' '$rev' 'nolog' &&"
shift
@@ -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 "$(eval_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
+ eval_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,18 +321,18 @@ 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"
+ echo "# first $NAME_BAD commit: $bad_commit" >>"$GIT_DIR/BISECT_LOG"
exit 0
elif test $res -eq 2
then
echo "# only skipped commits left to test" >>"$GIT_DIR/BISECT_LOG"
- good_revs=$(git for-each-ref --format="%(objectname)" "refs/bisect/good-*")
- for skipped in $(git rev-list refs/bisect/bad --not $good_revs)
+ good_revs=$(git for-each-ref --format="%(objectname)" "refs/bisect/$NAME_GOOD-*")
+ for skipped in $(git rev-list refs/bisect/$NAME_BAD --not $good_revs)
do
skipped_commit=$(git show-branch $skipped)
- echo "# possible first bad commit: $skipped_commit" >>"$GIT_DIR/BISECT_LOG"
+ echo "# possible first $NAME_BAD commit: $skipped_commit" >>"$GIT_DIR/BISECT_LOG"
done
exit $res
fi
@@ -421,7 +426,7 @@ bisect_replay () {
start)
cmd="bisect_start $rev"
eval "$cmd" ;;
- good|bad|skip)
+ $NAME_GOOD|$NAME_BAD|skip)
bisect_write "$command" "$rev" ;;
*)
die "$(gettext "?? what are you talking about?")" ;;
@@ -455,9 +460,9 @@ exit code \$res from '\$command' is < 0 or >= 128" >&2
state='skip'
elif [ $res -gt 0 ]
then
- state='bad'
+ state="$NAME_BAD"
else
- state='good'
+ state="$NAME_GOOD"
fi
# We have to use a subshell because "bisect_state" can exit.
@@ -466,7 +471,7 @@ exit code \$res from '\$command' is < 0 or >= 128" >&2
cat "$GIT_DIR/BISECT_RUN"
- if sane_grep "first bad commit could be any of" "$GIT_DIR/BISECT_RUN" \
+ if sane_grep "first $NAME_BAD commit could be any of" "$GIT_DIR/BISECT_RUN" \
>/dev/null
then
gettextln "bisect run cannot continue any more" >&2
@@ -480,7 +485,7 @@ exit code \$res from '\$command' is < 0 or >= 128" >&2
exit $res
fi
- if sane_grep "is the first bad commit" "$GIT_DIR/BISECT_RUN" >/dev/null
+ if sane_grep "is the first $NAME_BAD commit" "$GIT_DIR/BISECT_RUN" >/dev/null
then
gettextln "bisect run success"
exit 0;
--
2.4.4.414.g59d82e6
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH v8 3/5] bisect: simplify the addition of new bisect terms
2015-06-24 15:17 ` [PATCH v8 0/5] Bisect terms Matthieu Moy
2015-06-24 15:17 ` [PATCH v8 1/5] bisect: correction of typo Matthieu Moy
2015-06-24 15:17 ` [PATCH v8 2/5] bisect: replace hardcoded "bad|good" by variables Matthieu Moy
@ 2015-06-24 15:17 ` Matthieu Moy
2015-06-24 17:29 ` Junio C Hamano
2015-06-24 15:17 ` [PATCH v8 4/5] bisect: add the terms old/new Matthieu Moy
` (3 subsequent siblings)
6 siblings, 1 reply; 48+ messages in thread
From: Matthieu Moy @ 2015-06-24 15:17 UTC (permalink / raw)
To: gitster
Cc: git, antoine.delaite, louis--alexandre.stuber, chriscool,
thomasxnguy, valentinduperray, Louis Stuber, Valentin Duperray,
Franck Jonas, Lucien Kong, Thomas Nguy, Huynh Khoi Nguyen Nguyen,
Matthieu Moy
From: Antoine Delaite <antoine.delaite@ensimag.grenoble-inp.fr>
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
Co-authored-by: Louis Stuber <stuberl@ensimag.grenoble-inp.fr>
Tweaked-by: Matthieu Moy <Matthieu.Moy@imag.fr>
Signed-off-by: Antoine Delaite <antoine.delaite@ensimag.grenoble-inp.fr>
Signed-off-by: Louis Stuber <stuberl@ensimag.grenoble-inp.fr>
Signed-off-by: Valentin Duperray <Valentin.Duperray@ensimag.imag.fr>
Signed-off-by: Franck Jonas <Franck.Jonas@ensimag.imag.fr>
Signed-off-by: Lucien Kong <Lucien.Kong@ensimag.imag.fr>
Signed-off-by: Thomas Nguy <Thomas.Nguy@ensimag.imag.fr>
Signed-off-by: Huynh Khoi Nguyen Nguyen <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
bisect.c | 38 +++++++++++++++++++++++++++++---
git-bisect.sh | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++++------
revision.c | 20 +++++++++++++++--
3 files changed, 116 insertions(+), 12 deletions(-)
diff --git a/bisect.c b/bisect.c
index 2d3dbdc..08be634 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 == ENOENT) {
+ *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..7bb18db 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)
+ good_syn=$(bisect_voc good)
if test -s "$GIT_DIR/BISECT_START"
then
- gettextln "You need to give me at least one good and one bad revision.
-(You can use \"git bisect bad\" and \"git bisect good\" for that.)" >&2
+
+ eval_gettextln "You need to give me at least one \$bad_syn and one \$good_syn revision.
+(You can use \"git bisect \$bad_syn\" and \"git bisect \$good_syn\" for that.)" >&2
else
- gettextln "You need to start by \"git bisect start\".
-You then need to give me at least one good and one bad revision.
-(You can use \"git bisect bad\" and \"git bisect good\" for that.)" >&2
+ eval_gettextln "You need to start by \"git bisect start\".
+You then need to give me at least one \$good_syn and one \$bad_syn revision.
+(You can use \"git bisect \$bad_syn\" and \"git bisect \$good_syn\" for that.)" >&2
fi
exit 1 ;;
esac
@@ -402,6 +416,7 @@ bisect_clean_state() {
rm -f "$GIT_DIR/BISECT_LOG" &&
rm -f "$GIT_DIR/BISECT_NAMES" &&
rm -f "$GIT_DIR/BISECT_RUN" &&
+ rm -f "$GIT_DIR/BISECT_TERMS" &&
# 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 &&
@@ -422,11 +437,13 @@ bisect_replay () {
rev="$command"
command="$bisect"
fi
+ get_terms
+ check_and_set_terms "$command"
case "$command" in
start)
cmd="bisect_start $rev"
eval "$cmd" ;;
- $NAME_GOOD|$NAME_BAD|skip)
+ "$NAME_GOOD"|"$NAME_BAD"|skip)
bisect_write "$command" "$rev" ;;
*)
die "$(gettext "?? what are you talking about?")" ;;
@@ -499,11 +516,50 @@ bisect_log () {
cat "$GIT_DIR/BISECT_LOG"
}
+get_terms () {
+ if test -s "$GIT_DIR/BISECT_TERMS"
+ then
+ {
+ read NAME_BAD
+ read NAME_GOOD
+ } <"$GIT_DIR/BISECT_TERMS"
+ fi
+}
+
+check_and_set_terms () {
+ cmd="$1"
+ case "$cmd" in
+ bad|good)
+ 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.")"
+ 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
+}
+
case "$#" in
0)
usage ;;
*)
cmd="$1"
+ get_terms
shift
case "$cmd" in
help)
diff --git a/revision.c b/revision.c
index 3ff8723..24ce842 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,26 @@ 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);
+ struct strbuf bisect_refs = STRBUF_INIT;
+ int status;
+ strbuf_addf(&bisect_refs, "refs/bisect/%s", name_bad);
+ status = for_each_ref_in_submodule(submodule, bisect_refs.buf, fn, cb_data);
+ strbuf_release(&bisect_refs);
+ return status;
}
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);
+ struct strbuf bisect_refs = STRBUF_INIT;
+ int status;
+ strbuf_addf(&bisect_refs, "refs/bisect/%s", name_good);
+ status = for_each_ref_in_submodule(submodule, bisect_refs.buf, fn, cb_data);
+ strbuf_release(&bisect_refs);
+ return status;
}
static int handle_revision_pseudo_opt(const char *submodule,
@@ -2112,6 +2127,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;
--
2.4.4.414.g59d82e6
^ permalink raw reply related [flat|nested] 48+ messages in thread* Re: [PATCH v8 3/5] bisect: simplify the addition of new bisect terms
2015-06-24 15:17 ` [PATCH v8 3/5] bisect: simplify the addition of new bisect terms Matthieu Moy
@ 2015-06-24 17:29 ` Junio C Hamano
2015-06-24 21:26 ` Matthieu Moy
0 siblings, 1 reply; 48+ messages in thread
From: Junio C Hamano @ 2015-06-24 17:29 UTC (permalink / raw)
To: Matthieu Moy
Cc: git, antoine.delaite, louis--alexandre.stuber, chriscool,
thomasxnguy, valentinduperray, Louis Stuber, Valentin Duperray,
Franck Jonas, Lucien Kong, Thomas Nguy, Huynh Khoi Nguyen Nguyen
Matthieu Moy <Matthieu.Moy@imag.fr> writes:
> diff --git a/bisect.c b/bisect.c
> index 2d3dbdc..08be634 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);
> }
Before the pre-context of this hunk is
if (!strcmp(name_bad, "bad")) {
fprintf(stderr, "The merge base %s is bad.\n"
"This means the bug has been fixed "
So, after 5/5, the user could do
git bisect terms bad worse
and get utterly confused. I think
- if (!strcmp(name_bad, "bad")) {
+ if (!strcmp(name_bad, "bad") && !strcmp(name_good, "good") {
needs to be a part of this step.
^ permalink raw reply [flat|nested] 48+ messages in thread* Re: [PATCH v8 3/5] bisect: simplify the addition of new bisect terms
2015-06-24 17:29 ` Junio C Hamano
@ 2015-06-24 21:26 ` Matthieu Moy
0 siblings, 0 replies; 48+ messages in thread
From: Matthieu Moy @ 2015-06-24 21:26 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, antoine.delaite, louis--alexandre.stuber, chriscool,
thomasxnguy, valentinduperray, Louis Stuber, Valentin Duperray,
Franck Jonas, Lucien Kong, Thomas Nguy, Huynh Khoi Nguyen Nguyen
Junio C Hamano <gitster@pobox.com> writes:
> Matthieu Moy <Matthieu.Moy@imag.fr> writes:
>
>> diff --git a/bisect.c b/bisect.c
>> index 2d3dbdc..08be634 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);
Indeed, I forgot to apply the previous remark. Fixed.
> - if (!strcmp(name_bad, "bad")) {
> + if (!strcmp(name_bad, "bad") && !strcmp(name_good, "good") {
Indeed. Applied.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v8 4/5] bisect: add the terms old/new
2015-06-24 15:17 ` [PATCH v8 0/5] Bisect terms Matthieu Moy
` (2 preceding siblings ...)
2015-06-24 15:17 ` [PATCH v8 3/5] bisect: simplify the addition of new bisect terms Matthieu Moy
@ 2015-06-24 15:17 ` Matthieu Moy
2015-06-24 15:17 ` [PATCH v8 5/5] bisect: allow any terms set by user Matthieu Moy
` (2 subsequent siblings)
6 siblings, 0 replies; 48+ messages in thread
From: Matthieu Moy @ 2015-06-24 15:17 UTC (permalink / raw)
To: gitster
Cc: git, antoine.delaite, louis--alexandre.stuber, chriscool,
thomasxnguy, valentinduperray, Louis Stuber, Valentin Duperray,
Franck Jonas, Lucien Kong, Thomas Nguy, Huynh Khoi Nguyen Nguyen,
Matthieu Moy
From: Antoine Delaite <antoine.delaite@ensimag.grenoble-inp.fr>
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.delaite@ensimag.grenoble-inp.fr>
Signed-off-by: Louis Stuber <stuberl@ensimag.grenoble-inp.fr>
Signed-off-by: Valentin Duperray <Valentin.Duperray@ensimag.imag.fr>
Signed-off-by: Franck Jonas <Franck.Jonas@ensimag.imag.fr>
Signed-off-by: Lucien Kong <Lucien.Kong@ensimag.imag.fr>
Signed-off-by: Thomas Nguy <Thomas.Nguy@ensimag.imag.fr>
Signed-off-by: Huynh Khoi Nguyen Nguyen <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.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 08be634..ab09650 100644
--- a/bisect.c
+++ b/bisect.c
@@ -746,6 +746,11 @@ static void handle_bad_merge_base(void)
"This means the bug has been fixed "
"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);
} else {
fprintf(stderr, "The merge base %s is %s.\n"
"This means the first commit marked %s is "
@@ -778,11 +783,11 @@ static void handle_skipped_merge_base(const unsigned char *mb)
}
/*
- * "check_merge_bases" checks that merge bases are not "bad".
+ * "check_merge_bases" checks that merge bases are not "bad" (or "new").
*
- * - If one is "bad", it means the user assumed something wrong
+ * - If one is "bad" (or "new"), it means the user assumed something wrong
* and we must exit with a non 0 error code.
- * - If one is "good", that's good, we have nothing to do.
+ * - If one is "good" (or "old"), that's good, we have nothing to do.
* - If one is "skipped", we can't know but we should warn.
* - If we don't know, we should check it out and ask the user to test.
*/
diff --git a/git-bisect.sh b/git-bisect.sh
index 7bb18db..73763a2 100644
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -1,14 +1,16 @@
#!/bin/sh
-USAGE='[help|start|bad|good|skip|next|reset|visualize|replay|log|run]'
+USAGE='[help|start|bad|good|new|old|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>...]
reset bisect state and start bisection.
-git bisect bad [<rev>]
- mark <rev> a known-bad revision.
-git bisect good [<rev>...]
- mark <rev>... known-good revisions.
+git bisect (bad|new) [<rev>]
+ mark <rev> a known-bad revision/
+ a revision after change in a given property.
+git bisect (good|old) [<rev>...]
+ mark <rev>... known-good revisions/
+ revisions before change in a given property.
git bisect skip [(<rev>|<range>)...]
mark <rev>... untestable revisions.
git bisect next
@@ -288,7 +290,7 @@ bisect_next_check() {
false
;;
t,,"$NAME_GOOD")
- # have bad but not good. we could bisect although
+ # have bad (or new) but not good (or old). we could bisect although
# this is less optimum.
eval_gettextln "Warning: bisecting only with a \$NAME_BAD commit." >&2
if test -t 0
@@ -529,7 +531,7 @@ get_terms () {
check_and_set_terms () {
cmd="$1"
case "$cmd" in
- bad|good)
+ bad|good|new|old)
if test -s "$GIT_DIR/BISECT_TERMS" && test "$cmd" != "$NAME_BAD" && test "$cmd" != "$NAME_GOOD"
then
die "$(eval_gettext "Invalid command: you're currently in a \$NAME_BAD/\$NAME_GOOD bisect.")"
@@ -543,14 +545,22 @@ check_and_set_terms () {
fi
NAME_BAD="bad"
NAME_GOOD="good" ;;
+ new|old)
+ if ! test -s "$GIT_DIR/BISECT_TERMS"
+ then
+ echo "new" >"$GIT_DIR/BISECT_TERMS" &&
+ echo "old" >>"$GIT_DIR/BISECT_TERMS"
+ fi
+ NAME_BAD="new"
+ NAME_GOOD="old" ;;
esac ;;
esac
}
bisect_voc () {
case "$1" in
- bad) echo "bad" ;;
- good) echo "good" ;;
+ bad) echo "bad|old" ;;
+ good) echo "good|new" ;;
esac
}
@@ -566,7 +576,7 @@ case "$#" in
git bisect -h ;;
start)
bisect_start "$@" ;;
- bad|good)
+ bad|good|new|old)
bisect_state "$cmd" "$@" ;;
skip)
bisect_skip "$@" ;;
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index 9e2c203..983c503 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -759,4 +759,42 @@ test_expect_success '"git bisect bad HEAD" behaves as "git bisect bad"' '
git bisect reset
'
+test_expect_success 'bisect starts with only one new' '
+ git bisect reset &&
+ git bisect start &&
+ git bisect new $HASH4 &&
+ git bisect next
+'
+
+test_expect_success 'bisect does not start with only one old' '
+ git bisect reset &&
+ git bisect start &&
+ git bisect old $HASH1 &&
+ test_must_fail git bisect next
+'
+
+test_expect_success 'bisect start with one new and old' '
+ git bisect reset &&
+ git bisect start &&
+ git bisect old $HASH1 &&
+ git bisect new $HASH4 &&
+ git bisect new &&
+ git bisect new >bisect_result &&
+ grep "$HASH2 is the first new commit" bisect_result &&
+ git bisect log >log_to_replay.txt &&
+ git bisect reset
+'
+
+test_expect_success 'bisect replay with old and new' '
+ git bisect replay log_to_replay.txt >bisect_result &&
+ grep "$HASH2 is the first new commit" bisect_result &&
+ git bisect reset
+'
+
+test_expect_success 'bisect cannot mix old/new and good/bad' '
+ git bisect start &&
+ git bisect bad $HASH4 &&
+ test_must_fail git bisect old $HASH1
+'
+
test_done
--
2.4.4.414.g59d82e6
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH v8 5/5] bisect: allow any terms set by user
2015-06-24 15:17 ` [PATCH v8 0/5] Bisect terms Matthieu Moy
` (3 preceding siblings ...)
2015-06-24 15:17 ` [PATCH v8 4/5] bisect: add the terms old/new Matthieu Moy
@ 2015-06-24 15:17 ` Matthieu Moy
2015-06-24 17:46 ` Junio C Hamano
2015-06-24 17:27 ` [PATCH v8 0/5] Bisect terms Junio C Hamano
2015-06-24 19:41 ` Junio C Hamano
6 siblings, 1 reply; 48+ messages in thread
From: Matthieu Moy @ 2015-06-24 15:17 UTC (permalink / raw)
To: gitster
Cc: git, antoine.delaite, louis--alexandre.stuber, chriscool,
thomasxnguy, valentinduperray, Louis Stuber, Matthieu Moy
From: Antoine Delaite <antoine.delaite@ensimag.grenoble-inp.fr>
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.delaite@ensimag.grenoble-inp.fr>
Signed-off-by: Louis Stuber <stuberl@ensimag.grenoble-inp.fr>
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.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..a37336e 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 73763a2..8ef2b94 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
+ }<"$GIT_DIR/BISECT_TERMS"
+ gettextln "Your current terms are $term1 and $term2."
+ else
+ die "$(gettext "No terms defined.")"
+ fi ;;
+ 2)
+ check_term_format refs/bisect/"$1"
+ check_term_format refs/bisect/"$2"
+ 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"
+ echo "git bisect terms $NAME_BAD $NAME_GOOD" >>"$GIT_DIR/BISECT_LOG" || exit
+ else
+ die "$(gettext "A bisection has already started, and you can't change terms in the middle of it.
+Use 'git bisect terms' to see the current terms.
+Otherwise, to start a new bisection with new terms, please use 'git bisect reset' and set the terms before the start")"
+ fi ;;
+ *)
+ usage ;;
+ esac
+}
+
+check_term_format () {
+ arg="$1"
+ git check-ref-format $arg ||
+ die "$(eval_gettext "'\$arg' is not a valid term")"
+}
+
case "$#" in
0)
usage ;;
@@ -576,7 +632,7 @@ case "$#" in
git bisect -h ;;
start)
bisect_start "$@" ;;
- bad|good|new|old)
+ bad|good|new|old|"$NAME_BAD"|"$NAME_GOOD")
bisect_state "$cmd" "$@" ;;
skip)
bisect_skip "$@" ;;
@@ -593,6 +649,8 @@ case "$#" in
bisect_log ;;
run)
bisect_run "$@" ;;
+ terms)
+ bisect_terms "$@" ;;
*)
usage ;;
esac
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index 983c503..289dbb0 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -797,4 +797,47 @@ test_expect_success 'bisect cannot mix old/new and good/bad' '
test_must_fail git bisect old $HASH1
'
+test_expect_success 'bisect start with one term1 and term2' '
+ git bisect reset &&
+ git bisect terms term1 term2 &&
+ git bisect start &&
+ git bisect term2 $HASH1 &&
+ git bisect term1 $HASH4 &&
+ git bisect term1 &&
+ git bisect term1 >bisect_result &&
+ grep "$HASH2 is the first term1 commit" bisect_result &&
+ git bisect log >log_to_replay.txt &&
+ git bisect reset
+'
+
+test_expect_success 'bisect replay with term1 and term2' '
+ git bisect replay log_to_replay.txt >bisect_result &&
+ grep "$HASH2 is the first term1 commit" bisect_result &&
+ git bisect reset
+'
+
+test_expect_success 'bisect start term1 term2' '
+ git bisect reset &&
+ git bisect terms term1 term2 &&
+ git bisect start $HASH4 $HASH1 &&
+ git bisect term1 &&
+ git bisect term1 >bisect_result &&
+ grep "$HASH2 is the first term1 commit" bisect_result &&
+ git bisect log >log_to_replay.txt &&
+ git bisect reset
+'
+
+test_expect_success 'bisect cannot mix terms' '
+ git bisect reset &&
+ git bisect terms a b &&
+ git bisect terms term1 term2 &&
+ git bisect start $HASH4 $HASH1 &&
+ test_must_fail git bisect a &&
+ test_must_fail git bisect b &&
+ test_must_fail git bisect bad &&
+ test_must_fail git bisect good &&
+ test_must_fail git bisect new &&
+ test_must_fail git bisect old
+'
+
test_done
--
2.4.4.414.g59d82e6
^ permalink raw reply related [flat|nested] 48+ messages in thread* Re: [PATCH v8 5/5] bisect: allow any terms set by user
2015-06-24 15:17 ` [PATCH v8 5/5] bisect: allow any terms set by user Matthieu Moy
@ 2015-06-24 17:46 ` Junio C Hamano
2015-06-24 21:23 ` Matthieu Moy
0 siblings, 1 reply; 48+ messages in thread
From: Junio C Hamano @ 2015-06-24 17:46 UTC (permalink / raw)
To: Matthieu Moy
Cc: git, antoine.delaite, louis--alexandre.stuber, chriscool,
thomasxnguy, valentinduperray, Louis Stuber
Matthieu Moy <Matthieu.Moy@imag.fr> writes:
> +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.
While this is not incorrect per-se, it would be more helpful to tell
the readers that they are hunting for a commit that changes the state
they would call "term2" into "term1" somewhere. e.g.
-ancestors of term1.
+ancestors of term1. For example, if something was buggy in
+the old part of the history, you know somewhere the bug was
+fixed, and you want to find the exact commit that fixed it,
+you may want to say `git bisect terms fixed broken`; this
+way, you would mark a commit that still has the bug with
+`broken`, and a newer one after the fix with `fixed`.
or something?
> -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.
This will not help those who cannot remember which one between these
two they want:
git bisect terms new old
git bisect terms old new
I am wondering (together with the documentation patch) if it would
be better to be more explicit, instead of term[12], like this:
git bisect terms new old
or even
git bisect terms bad good
assuming that the only reason they use 'terms' is because they are
sufficiently familiar with 'git bisect' and (intellectually) know
that 'bad' is more recent and 'good' is what it used to be, but have
trouble remembering which one is which during a hunt for a fix.
> +bisect_terms () {
> + case "$#" in
> + 0)
> + if test -s "$GIT_DIR/BISECT_TERMS"
> + then
> + {
> + read term1
> + read term2
> + }<"$GIT_DIR/BISECT_TERMS"
> + gettextln "Your current terms are $term1 and $term2."
The same comment on this part. Instead of "git bisect terms" that
just says "You are using $term1 and $term2", the users would benefit
if it said "You are using $term1 for newer state and $term2 for
older state" [*1*].
Thanks.
[Footnote]
*1* It is funny that I had to rewrite this "if it said..." a few
times to make sure I got newer and older right, even though I had
the relevant pieces (and only releavant pieces) of information for
the doc and help text in a single patch form while composing this
response. I suspect that an end user without such material would be
a lot more confused than I was.
^ permalink raw reply [flat|nested] 48+ messages in thread* Re: [PATCH v8 5/5] bisect: allow any terms set by user
2015-06-24 17:46 ` Junio C Hamano
@ 2015-06-24 21:23 ` Matthieu Moy
0 siblings, 0 replies; 48+ messages in thread
From: Matthieu Moy @ 2015-06-24 21:23 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, antoine.delaite, louis--alexandre.stuber, chriscool,
thomasxnguy, valentinduperray, Louis Stuber
Junio C Hamano <gitster@pobox.com> writes:
> they would call "term2" into "term1" somewhere. e.g.
>
> -ancestors of term1.
> +ancestors of term1. For example, if something was buggy in
> +the old part of the history, you know somewhere the bug was
> +fixed, and you want to find the exact commit that fixed it,
> +you may want to say `git bisect terms fixed broken`; this
> +way, you would mark a commit that still has the bug with
> +`broken`, and a newer one after the fix with `fixed`.
>
> or something?
Yes.
> I am wondering (together with the documentation patch) if it would
> be better to be more explicit, instead of term[12], like this:
>
> git bisect terms new old
Yes. I eliminated all instance of term1 and term2 in the doc of the
patch, and replaced with <term-old> and <term-new>.
>> +bisect_terms () {
>> + case "$#" in
>> + 0)
>> + if test -s "$GIT_DIR/BISECT_TERMS"
>> + then
>> + {
>> + read term1
>> + read term2
>> + }<"$GIT_DIR/BISECT_TERMS"
>> + gettextln "Your current terms are $term1 and $term2."
>
> The same comment on this part. Instead of "git bisect terms" that
> just says "You are using $term1 and $term2", the users would benefit
> if it said "You are using $term1 for newer state and $term2 for
> older state" [*1*].
Done. It's up to date on
https://github.com/moy/git/tree/bisect-terms
Will resend.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v8 0/5] Bisect terms
2015-06-24 15:17 ` [PATCH v8 0/5] Bisect terms Matthieu Moy
` (4 preceding siblings ...)
2015-06-24 15:17 ` [PATCH v8 5/5] bisect: allow any terms set by user Matthieu Moy
@ 2015-06-24 17:27 ` Junio C Hamano
2015-06-24 19:41 ` Junio C Hamano
6 siblings, 0 replies; 48+ messages in thread
From: Junio C Hamano @ 2015-06-24 17:27 UTC (permalink / raw)
To: Matthieu Moy
Cc: git, antoine.delaite, louis--alexandre.stuber, chriscool,
thomasxnguy, valentinduperray
Matthieu Moy <Matthieu.Moy@imag.fr> writes:
> This is a minor iteration over v7 to take into account Junio and
> Eric's comments, AND fix an important typo that I introduced in the
> strbuf code conversion (I used name_good instead of name_bad). This
> fixes the "git bisect visualize" bug I found earlier. I played a bit
> with the result and didn't find any bug.
It seems that [3/5] essentially is the same from the previous round,
though. I'll have more comments on it In-Reply-To that message.
Thanks.
^ permalink raw reply [flat|nested] 48+ messages in thread* Re: [PATCH v8 0/5] Bisect terms
2015-06-24 15:17 ` [PATCH v8 0/5] Bisect terms Matthieu Moy
` (5 preceding siblings ...)
2015-06-24 17:27 ` [PATCH v8 0/5] Bisect terms Junio C Hamano
@ 2015-06-24 19:41 ` Junio C Hamano
6 siblings, 0 replies; 48+ messages in thread
From: Junio C Hamano @ 2015-06-24 19:41 UTC (permalink / raw)
To: Matthieu Moy
Cc: git, antoine.delaite, louis--alexandre.stuber, chriscool,
thomasxnguy, valentinduperray
Matthieu Moy <Matthieu.Moy@imag.fr> writes:
> Documentation/git-bisect.txt | 67 +++++++++++++-
> bisect.c | 94 +++++++++++++++-----
> git-bisect.sh | 207 +++++++++++++++++++++++++++++++++++--------
> revision.c | 20 ++++-
> t/t6030-bisect-porcelain.sh | 83 ++++++++++++++++-
> 5 files changed, 407 insertions(+), 64 deletions(-)
> mode change 100755 => 100644 git-bisect.sh
How come nobody noticed this last line so far, during the 7 rounds
of reviews?
I'll locally fix it up so no need to resend.
Thanks.
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v9 0/5] bisect terms
2015-06-23 12:54 ` [PATCH v7 0/5] git bisect old/new Matthieu Moy
` (6 preceding siblings ...)
2015-06-24 15:17 ` [PATCH v8 0/5] Bisect terms Matthieu Moy
@ 2015-06-25 18:50 ` Matthieu Moy
2015-06-25 18:50 ` [PATCH v9 1/5] bisect: correction of typo Matthieu Moy
` (4 more replies)
7 siblings, 5 replies; 48+ messages in thread
From: Matthieu Moy @ 2015-06-25 18:50 UTC (permalink / raw)
To: gitster
Cc: git, antoine.delaite, louis--alexandre.stuber, chriscool,
thomasxnguy, valentinduperray, Matthieu Moy
This version takes into account Junio's remark on the previous series,
and I did a much more thourough review of the whole, which led to a
few fixes (one forgotten strcmp(..., "bad") in addition to the other
noted by Junio), some style fixes, and some simplifications (the file
TERMS_DEFINED of PATCH 5 is gone, it was redundant with BISECT_TERMS
in all cases I could think of).
Hopefully, patches 1-4 are actually ready to be merged.
I'm hesitant about patch 5: I actually ended up spending some time
reviewing it and simplifying it, and I'm tempted to consider it as
finished. But it probably lacks more tests and review.
diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
index a37336e..e783f87 100644
--- a/Documentation/git-bisect.txt
+++ b/Documentation/git-bisect.txt
@@ -140,16 +140,21 @@ If the builtins terms bad/good and new/old do not satisfy you, you can
set your own terms.
------------------------------------------------
-git bisect terms term1 term2
+git bisect terms <term-new> <term-old>
------------------------------------------------
-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'.
+This command has to be used before a bisection has started. <term-old>
+must be associated with the latest revisions and <term-new> with the
+ancestors of <term-old>. For example, if something was buggy in the
+old part of the history, you know somewhere the bug was fixed, and you
+want to find the exact commit that fixed it, you may want to say `git
+bisect terms fixed broken`; this way, you would mark a commit that
+still has the bug with `broken`, and a newer one after the fix with
+`fixed`.
+
+Only the first bisection following the `git bisect terms` will use the
+terms. If you mistyped one of the terms you can do again `git bisect
+terms <term-old> <term-new>`.
Bisect visualize
diff --git a/bisect.c b/bisect.c
index ab09650..d447b65 100644
--- a/bisect.c
+++ b/bisect.c
@@ -741,21 +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, ' ');
- if (!strcmp(name_bad, "bad")) {
+ if (!strcmp(name_bad, "bad") && !strcmp(name_good, "good")) {
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 if (!strcmp(name_bad, "new")) {
+ } else if (!strcmp(name_bad, "new") && !strcmp(name_good, "old")) {
fprintf(stderr, "The merge base %s is new.\n"
"The property has changed "
"between %s and [%s].\n",
bad_hex, bad_hex, good_hex);
} else {
fprintf(stderr, "The merge base %s is %s.\n"
- "This means the first commit marked %s is "
+ "This means the first '%s' commit is "
"between %s and [%s].\n",
- bad_hex, name_bad, name_bad, bad_hex, good_hex);
+ bad_hex, name_bad, name_good, bad_hex, good_hex);
}
exit(3);
}
diff --git a/git-bisect.sh b/git-bisect.sh
old mode 100644
new mode 100755
index 8ef2b94..8fee712
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -11,8 +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 terms <term-new> <term-old>
+ set up <term-new> and <term-old> as terms (default: bad, good)
git bisect skip [(<rev>|<range>)...]
mark <rev>... untestable revisions.
git bisect next
@@ -81,15 +81,15 @@ 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
- # 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"
+ must_write_terms=0
+ must_log_terms=0
+ if test -s "$GIT_DIR/BISECT_TERMS"
then
- terms_defined=1
+ # We're going to restart from a clean state and the
+ # file will be deleted. Record the old state in
+ # variables and restore it below.
+ must_write_terms=1
+ must_log_terms=1
get_terms
fi
if test "z$(git rev-parse --is-bare-repository)" != zfalse
@@ -117,7 +117,12 @@ bisect_start() {
break
}
- revision_seen=1
+ # The user ran "git bisect start <sha1>
+ # <sha1>", hence did not explicitly specify
+ # the terms, but we are already starting to
+ # set references named with the default terms,
+ # and won't be able to change afterwards.
+ must_write_terms=1
case $bad_seen in
0) state=$NAME_BAD ; bad_seen=1 ;;
@@ -190,13 +195,13 @@ 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" || test $terms_defined -eq 1
+ if test $must_write_terms -eq 1
then
- echo "$NAME_BAD" >"$GIT_DIR/BISECT_TERMS" &&
- echo "$NAME_GOOD" >>"$GIT_DIR/BISECT_TERMS" &&
- if test $terms_defined -eq 1
+ write_terms "$NAME_BAD" "$NAME_GOOD" &&
+ if test $must_log_terms -eq 1
then
- echo "git bisect terms $NAME_BAD $NAME_GOOD" >>"$GIT_DIR/BISECT_LOG" || exit
+ echo "git bisect terms $NAME_BAD $NAME_GOOD" \
+ >>"$GIT_DIR/BISECT_LOG"
fi
fi &&
echo "git bisect start$orig_args" >>"$GIT_DIR/BISECT_LOG" || exit
@@ -433,7 +438,6 @@ 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 &&
@@ -558,16 +562,14 @@ check_and_set_terms () {
bad|good)
if ! test -s "$GIT_DIR/BISECT_TERMS"
then
- echo "bad" >"$GIT_DIR/BISECT_TERMS" &&
- echo "good" >>"$GIT_DIR/BISECT_TERMS"
+ write_terms bad good
fi
NAME_BAD="bad"
NAME_GOOD="good" ;;
new|old)
if ! test -s "$GIT_DIR/BISECT_TERMS"
then
- echo "new" >"$GIT_DIR/BISECT_TERMS" &&
- echo "old" >>"$GIT_DIR/BISECT_TERMS"
+ write_terms new old
fi
NAME_BAD="new"
NAME_GOOD="old" ;;
@@ -587,37 +589,40 @@ bisect_terms () {
0)
if test -s "$GIT_DIR/BISECT_TERMS"
then
- {
- read term1
- read term2
- }<"$GIT_DIR/BISECT_TERMS"
- gettextln "Your current terms are $term1 and $term2."
+ get_terms
+ gettextln "Your current terms are $NAME_GOOD for the old state
+and $NAME_BAD for the new state."
else
die "$(gettext "No terms defined.")"
fi ;;
2)
- check_term_format refs/bisect/"$1"
- check_term_format refs/bisect/"$2"
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"
+ write_terms "$1" "$2"
echo "git bisect terms $NAME_BAD $NAME_GOOD" >>"$GIT_DIR/BISECT_LOG" || exit
else
die "$(gettext "A bisection has already started, and you can't change terms in the middle of it.
Use 'git bisect terms' to see the current terms.
-Otherwise, to start a new bisection with new terms, please use 'git bisect reset' and set the terms before the start")"
+Otherwise, to start a new bisection with new terms, please use
+'git bisect reset' and set the terms before the start")"
fi ;;
*)
usage ;;
esac
}
+write_terms () {
+ NAME_BAD=$1
+ NAME_GOOD=$2
+ check_term_format "$NAME_BAD"
+ check_term_format "$NAME_GOOD"
+ printf '%s\n%s\n' "$NAME_BAD" "$NAME_GOOD" >"$GIT_DIR/BISECT_TERMS"
+}
+
check_term_format () {
- arg="$1"
- git check-ref-format $arg ||
- die "$(eval_gettext "'\$arg' is not a valid term")"
+ term=$1
+ git check-ref-format refs/bisect/"$term" ||
+ die "$(eval_gettext "'\$term' is not a valid term")"
}
case "$#" in
Antoine Delaite (5):
bisect: correction of typo
bisect: replace hardcoded "bad|good" by variables
bisect: simplify the addition of new bisect terms
bisect: add the terms old/new
bisect: allow any terms set by user
Documentation/git-bisect.txt | 72 ++++++++++++++-
bisect.c | 94 +++++++++++++++----
git-bisect.sh | 212 +++++++++++++++++++++++++++++++++++--------
revision.c | 20 +++-
t/t6030-bisect-porcelain.sh | 83 ++++++++++++++++-
5 files changed, 417 insertions(+), 64 deletions(-)
--
2.4.4.414.g318df7a.dirty
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH v9 1/5] bisect: correction of typo
2015-06-25 18:50 ` [PATCH v9 0/5] bisect terms Matthieu Moy
@ 2015-06-25 18:50 ` Matthieu Moy
2015-06-25 18:50 ` [PATCH v9 2/5] bisect: replace hardcoded "bad|good" by variables Matthieu Moy
` (3 subsequent siblings)
4 siblings, 0 replies; 48+ messages in thread
From: Matthieu Moy @ 2015-06-25 18:50 UTC (permalink / raw)
To: gitster
Cc: git, antoine.delaite, louis--alexandre.stuber, chriscool,
thomasxnguy, valentinduperray, Matthieu Moy
From: Antoine Delaite <antoine.delaite@ensimag.grenoble-inp.fr>
Signed-off-by: Antoine Delaite <antoine.delaite@ensimag.grenoble-inp.fr>
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.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
'
--
2.4.4.414.g318df7a.dirty
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH v9 2/5] bisect: replace hardcoded "bad|good" by variables
2015-06-25 18:50 ` [PATCH v9 0/5] bisect terms Matthieu Moy
2015-06-25 18:50 ` [PATCH v9 1/5] bisect: correction of typo Matthieu Moy
@ 2015-06-25 18:50 ` Matthieu Moy
2015-06-25 18:50 ` [PATCH v9 3/5] bisect: simplify the addition of new bisect terms Matthieu Moy
` (2 subsequent siblings)
4 siblings, 0 replies; 48+ messages in thread
From: Matthieu Moy @ 2015-06-25 18:50 UTC (permalink / raw)
To: gitster
Cc: git, antoine.delaite, louis--alexandre.stuber, chriscool,
thomasxnguy, valentinduperray, Louis Stuber, Valentin Duperray,
Franck Jonas, Lucien Kong, Thomas Nguy, Huynh Khoi Nguyen Nguyen,
Matthieu Moy
From: Antoine Delaite <antoine.delaite@ensimag.grenoble-inp.fr>
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.delaite@ensimag.grenoble-inp.fr>
Signed-off-by: Louis Stuber <stuberl@ensimag.grenoble-inp.fr>
Signed-off-by: Valentin Duperray <Valentin.Duperray@ensimag.imag.fr>
Signed-off-by: Franck Jonas <Franck.Jonas@ensimag.imag.fr>
Signed-off-by: Lucien Kong <Lucien.Kong@ensimag.imag.fr>
Signed-off-by: Thomas Nguy <Thomas.Nguy@ensimag.imag.fr>
Signed-off-by: Huynh Khoi Nguyen Nguyen <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
bisect.c | 54 +++++++++++++++++++++++++++++++++++++-----------------
git-bisect.sh | 57 +++++++++++++++++++++++++++++++--------------------------
2 files changed, 68 insertions(+), 43 deletions(-)
diff --git a/bisect.c b/bisect.c
index 5b8357d..9368f6c 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 (1u<<16)
@@ -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,24 @@ 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") && !strcmp(name_good, "good")) {
+ 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 {
+ fprintf(stderr, "The merge base %s is %s.\n"
+ "This means the first '%s' commit is "
+ "between %s and [%s].\n",
+ bad_hex, name_bad, name_good, 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 +770,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 +854,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 +920,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";
+ name_good = "good";
if (read_bisect_refs())
die("reading bisect refs failed");
@@ -926,8 +943,10 @@ int bisect_next_all(const char *prefix, int no_checkout)
*/
exit_if_skipped_commits(tried, NULL);
- printf("%s was both good and bad\n",
- oid_to_hex(current_bad_oid));
+ printf("%s was both %s and %s\n",
+ oid_to_hex(current_bad_oid),
+ name_good,
+ name_bad);
exit(1);
}
@@ -942,7 +961,8 @@ 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 bad commit\n", bisect_rev_hex);
+ printf("%s is the first %s commit\n", bisect_rev_hex,
+ name_bad);
show_diff_tree(prefix, revs.commits->item);
/* This means the bisection process succeeded. */
exit(10);
diff --git a/git-bisect.sh b/git-bisect.sh
index ae3fec2..ce6412f 100755
--- 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()
{
@@ -100,8 +102,8 @@ bisect_start() {
break
}
case $bad_seen in
- 0) state='bad' ; bad_seen=1 ;;
- *) state='good' ;;
+ 0) state=$NAME_BAD ; bad_seen=1 ;;
+ *) state=$NAME_GOOD ;;
esac
eval="$eval bisect_write '$state' '$rev' 'nolog' &&"
shift
@@ -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 "$(eval_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
+ eval_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,18 +321,18 @@ 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"
+ echo "# first $NAME_BAD commit: $bad_commit" >>"$GIT_DIR/BISECT_LOG"
exit 0
elif test $res -eq 2
then
echo "# only skipped commits left to test" >>"$GIT_DIR/BISECT_LOG"
- good_revs=$(git for-each-ref --format="%(objectname)" "refs/bisect/good-*")
- for skipped in $(git rev-list refs/bisect/bad --not $good_revs)
+ good_revs=$(git for-each-ref --format="%(objectname)" "refs/bisect/$NAME_GOOD-*")
+ for skipped in $(git rev-list refs/bisect/$NAME_BAD --not $good_revs)
do
skipped_commit=$(git show-branch $skipped)
- echo "# possible first bad commit: $skipped_commit" >>"$GIT_DIR/BISECT_LOG"
+ echo "# possible first $NAME_BAD commit: $skipped_commit" >>"$GIT_DIR/BISECT_LOG"
done
exit $res
fi
@@ -421,7 +426,7 @@ bisect_replay () {
start)
cmd="bisect_start $rev"
eval "$cmd" ;;
- good|bad|skip)
+ $NAME_GOOD|$NAME_BAD|skip)
bisect_write "$command" "$rev" ;;
*)
die "$(gettext "?? what are you talking about?")" ;;
@@ -455,9 +460,9 @@ exit code \$res from '\$command' is < 0 or >= 128" >&2
state='skip'
elif [ $res -gt 0 ]
then
- state='bad'
+ state="$NAME_BAD"
else
- state='good'
+ state="$NAME_GOOD"
fi
# We have to use a subshell because "bisect_state" can exit.
@@ -466,7 +471,7 @@ exit code \$res from '\$command' is < 0 or >= 128" >&2
cat "$GIT_DIR/BISECT_RUN"
- if sane_grep "first bad commit could be any of" "$GIT_DIR/BISECT_RUN" \
+ if sane_grep "first $NAME_BAD commit could be any of" "$GIT_DIR/BISECT_RUN" \
>/dev/null
then
gettextln "bisect run cannot continue any more" >&2
@@ -480,7 +485,7 @@ exit code \$res from '\$command' is < 0 or >= 128" >&2
exit $res
fi
- if sane_grep "is the first bad commit" "$GIT_DIR/BISECT_RUN" >/dev/null
+ if sane_grep "is the first $NAME_BAD commit" "$GIT_DIR/BISECT_RUN" >/dev/null
then
gettextln "bisect run success"
exit 0;
--
2.4.4.414.g318df7a.dirty
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH v9 3/5] bisect: simplify the addition of new bisect terms
2015-06-25 18:50 ` [PATCH v9 0/5] bisect terms Matthieu Moy
2015-06-25 18:50 ` [PATCH v9 1/5] bisect: correction of typo Matthieu Moy
2015-06-25 18:50 ` [PATCH v9 2/5] bisect: replace hardcoded "bad|good" by variables Matthieu Moy
@ 2015-06-25 18:50 ` Matthieu Moy
2015-06-25 18:50 ` [PATCH v9 4/5] bisect: add the terms old/new Matthieu Moy
2015-06-25 18:50 ` [PATCH v9 5/5] bisect: allow any terms set by user Matthieu Moy
4 siblings, 0 replies; 48+ messages in thread
From: Matthieu Moy @ 2015-06-25 18:50 UTC (permalink / raw)
To: gitster
Cc: git, antoine.delaite, louis--alexandre.stuber, chriscool,
thomasxnguy, valentinduperray, Louis Stuber, Valentin Duperray,
Franck Jonas, Lucien Kong, Thomas Nguy, Huynh Khoi Nguyen Nguyen,
Matthieu Moy
From: Antoine Delaite <antoine.delaite@ensimag.grenoble-inp.fr>
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
Co-authored-by: Louis Stuber <stuberl@ensimag.grenoble-inp.fr>
Tweaked-by: Matthieu Moy <Matthieu.Moy@imag.fr>
Signed-off-by: Antoine Delaite <antoine.delaite@ensimag.grenoble-inp.fr>
Signed-off-by: Louis Stuber <stuberl@ensimag.grenoble-inp.fr>
Signed-off-by: Valentin Duperray <Valentin.Duperray@ensimag.imag.fr>
Signed-off-by: Franck Jonas <Franck.Jonas@ensimag.imag.fr>
Signed-off-by: Lucien Kong <Lucien.Kong@ensimag.imag.fr>
Signed-off-by: Thomas Nguy <Thomas.Nguy@ensimag.imag.fr>
Signed-off-by: Huynh Khoi Nguyen Nguyen <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
bisect.c | 33 ++++++++++++++++++++++++++--
git-bisect.sh | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++++------
revision.c | 20 +++++++++++++++--
3 files changed, 112 insertions(+), 11 deletions(-)
diff --git a/bisect.c b/bisect.c
index 9368f6c..c5f96eb 100644
--- a/bisect.c
+++ b/bisect.c
@@ -905,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 == ENOENT) {
+ *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.
@@ -920,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..7bb18db 100755
--- 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)
+ good_syn=$(bisect_voc good)
if test -s "$GIT_DIR/BISECT_START"
then
- gettextln "You need to give me at least one good and one bad revision.
-(You can use \"git bisect bad\" and \"git bisect good\" for that.)" >&2
+
+ eval_gettextln "You need to give me at least one \$bad_syn and one \$good_syn revision.
+(You can use \"git bisect \$bad_syn\" and \"git bisect \$good_syn\" for that.)" >&2
else
- gettextln "You need to start by \"git bisect start\".
-You then need to give me at least one good and one bad revision.
-(You can use \"git bisect bad\" and \"git bisect good\" for that.)" >&2
+ eval_gettextln "You need to start by \"git bisect start\".
+You then need to give me at least one \$good_syn and one \$bad_syn revision.
+(You can use \"git bisect \$bad_syn\" and \"git bisect \$good_syn\" for that.)" >&2
fi
exit 1 ;;
esac
@@ -402,6 +416,7 @@ bisect_clean_state() {
rm -f "$GIT_DIR/BISECT_LOG" &&
rm -f "$GIT_DIR/BISECT_NAMES" &&
rm -f "$GIT_DIR/BISECT_RUN" &&
+ rm -f "$GIT_DIR/BISECT_TERMS" &&
# 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 &&
@@ -422,11 +437,13 @@ bisect_replay () {
rev="$command"
command="$bisect"
fi
+ get_terms
+ check_and_set_terms "$command"
case "$command" in
start)
cmd="bisect_start $rev"
eval "$cmd" ;;
- $NAME_GOOD|$NAME_BAD|skip)
+ "$NAME_GOOD"|"$NAME_BAD"|skip)
bisect_write "$command" "$rev" ;;
*)
die "$(gettext "?? what are you talking about?")" ;;
@@ -499,11 +516,50 @@ bisect_log () {
cat "$GIT_DIR/BISECT_LOG"
}
+get_terms () {
+ if test -s "$GIT_DIR/BISECT_TERMS"
+ then
+ {
+ read NAME_BAD
+ read NAME_GOOD
+ } <"$GIT_DIR/BISECT_TERMS"
+ fi
+}
+
+check_and_set_terms () {
+ cmd="$1"
+ case "$cmd" in
+ bad|good)
+ 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.")"
+ 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
+}
+
case "$#" in
0)
usage ;;
*)
cmd="$1"
+ get_terms
shift
case "$cmd" in
help)
diff --git a/revision.c b/revision.c
index 3ff8723..24ce842 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,26 @@ 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);
+ struct strbuf bisect_refs = STRBUF_INIT;
+ int status;
+ strbuf_addf(&bisect_refs, "refs/bisect/%s", name_bad);
+ status = for_each_ref_in_submodule(submodule, bisect_refs.buf, fn, cb_data);
+ strbuf_release(&bisect_refs);
+ return status;
}
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);
+ struct strbuf bisect_refs = STRBUF_INIT;
+ int status;
+ strbuf_addf(&bisect_refs, "refs/bisect/%s", name_good);
+ status = for_each_ref_in_submodule(submodule, bisect_refs.buf, fn, cb_data);
+ strbuf_release(&bisect_refs);
+ return status;
}
static int handle_revision_pseudo_opt(const char *submodule,
@@ -2112,6 +2127,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;
--
2.4.4.414.g318df7a.dirty
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH v9 4/5] bisect: add the terms old/new
2015-06-25 18:50 ` [PATCH v9 0/5] bisect terms Matthieu Moy
` (2 preceding siblings ...)
2015-06-25 18:50 ` [PATCH v9 3/5] bisect: simplify the addition of new bisect terms Matthieu Moy
@ 2015-06-25 18:50 ` Matthieu Moy
2015-06-26 4:11 ` Christian Couder
2015-06-25 18:50 ` [PATCH v9 5/5] bisect: allow any terms set by user Matthieu Moy
4 siblings, 1 reply; 48+ messages in thread
From: Matthieu Moy @ 2015-06-25 18:50 UTC (permalink / raw)
To: gitster
Cc: git, antoine.delaite, louis--alexandre.stuber, chriscool,
thomasxnguy, valentinduperray, Louis Stuber, Valentin Duperray,
Franck Jonas, Lucien Kong, Thomas Nguy, Huynh Khoi Nguyen Nguyen,
Matthieu Moy
From: Antoine Delaite <antoine.delaite@ensimag.grenoble-inp.fr>
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.delaite@ensimag.grenoble-inp.fr>
Signed-off-by: Louis Stuber <stuberl@ensimag.grenoble-inp.fr>
Signed-off-by: Valentin Duperray <Valentin.Duperray@ensimag.imag.fr>
Signed-off-by: Franck Jonas <Franck.Jonas@ensimag.imag.fr>
Signed-off-by: Lucien Kong <Lucien.Kong@ensimag.imag.fr>
Signed-off-by: Thomas Nguy <Thomas.Nguy@ensimag.imag.fr>
Signed-off-by: Huynh Khoi Nguyen Nguyen <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
Documentation/git-bisect.txt | 48 ++++++++++++++++++++++++++++++++++++++++++--
bisect.c | 11 +++++++---
git-bisect.sh | 43 +++++++++++++++++++++++++--------------
t/t6030-bisect-porcelain.sh | 38 +++++++++++++++++++++++++++++++++++
4 files changed, 120 insertions(+), 20 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 c5f96eb..d447b65 100644
--- a/bisect.c
+++ b/bisect.c
@@ -746,6 +746,11 @@ static void handle_bad_merge_base(void)
"This means the bug has been fixed "
"between %s and [%s].\n",
bad_hex, bad_hex, good_hex);
+ } else if (!strcmp(name_bad, "new") && !strcmp(name_good, "old")) {
+ fprintf(stderr, "The merge base %s is new.\n"
+ "The property has changed "
+ "between %s and [%s].\n",
+ bad_hex, bad_hex, good_hex);
} else {
fprintf(stderr, "The merge base %s is %s.\n"
"This means the first '%s' commit is "
@@ -778,11 +783,11 @@ static void handle_skipped_merge_base(const unsigned char *mb)
}
/*
- * "check_merge_bases" checks that merge bases are not "bad".
+ * "check_merge_bases" checks that merge bases are not "bad" (or "new").
*
- * - If one is "bad", it means the user assumed something wrong
+ * - If one is "bad" (or "new"), it means the user assumed something wrong
* and we must exit with a non 0 error code.
- * - If one is "good", that's good, we have nothing to do.
+ * - If one is "good" (or "old"), that's good, we have nothing to do.
* - If one is "skipped", we can't know but we should warn.
* - If we don't know, we should check it out and ask the user to test.
*/
diff --git a/git-bisect.sh b/git-bisect.sh
index 7bb18db..569898b 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -1,14 +1,16 @@
#!/bin/sh
-USAGE='[help|start|bad|good|skip|next|reset|visualize|replay|log|run]'
+USAGE='[help|start|bad|good|new|old|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>...]
reset bisect state and start bisection.
-git bisect bad [<rev>]
- mark <rev> a known-bad revision.
-git bisect good [<rev>...]
- mark <rev>... known-good revisions.
+git bisect (bad|new) [<rev>]
+ mark <rev> a known-bad revision/
+ a revision after change in a given property.
+git bisect (good|old) [<rev>...]
+ mark <rev>... known-good revisions/
+ revisions before change in a given property.
git bisect skip [(<rev>|<range>)...]
mark <rev>... untestable revisions.
git bisect next
@@ -77,9 +79,7 @@ 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
+ must_write_terms=0
if test "z$(git rev-parse --is-bare-repository)" != zfalse
then
mode=--no-checkout
@@ -105,7 +105,12 @@ bisect_start() {
break
}
- revision_seen=1
+ # The user ran "git bisect start <sha1>
+ # <sha1>", hence did not explicitly specify
+ # the terms, but we are already starting to
+ # set references named with the default terms,
+ # and won't be able to change afterwards.
+ must_write_terms=1
case $bad_seen in
0) state=$NAME_BAD ; bad_seen=1 ;;
@@ -178,7 +183,7 @@ 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 $must_write_terms -eq 1 && test ! -s "$GIT_DIR/BISECT_TERMS"
then
echo "$NAME_BAD" >"$GIT_DIR/BISECT_TERMS" &&
echo "$NAME_GOOD" >>"$GIT_DIR/BISECT_TERMS"
@@ -288,7 +293,7 @@ bisect_next_check() {
false
;;
t,,"$NAME_GOOD")
- # have bad but not good. we could bisect although
+ # have bad (or new) but not good (or old). we could bisect although
# this is less optimum.
eval_gettextln "Warning: bisecting only with a \$NAME_BAD commit." >&2
if test -t 0
@@ -529,7 +534,7 @@ get_terms () {
check_and_set_terms () {
cmd="$1"
case "$cmd" in
- bad|good)
+ bad|good|new|old)
if test -s "$GIT_DIR/BISECT_TERMS" && test "$cmd" != "$NAME_BAD" && test "$cmd" != "$NAME_GOOD"
then
die "$(eval_gettext "Invalid command: you're currently in a \$NAME_BAD/\$NAME_GOOD bisect.")"
@@ -543,14 +548,22 @@ check_and_set_terms () {
fi
NAME_BAD="bad"
NAME_GOOD="good" ;;
+ new|old)
+ if ! test -s "$GIT_DIR/BISECT_TERMS"
+ then
+ echo "new" >"$GIT_DIR/BISECT_TERMS" &&
+ echo "old" >>"$GIT_DIR/BISECT_TERMS"
+ fi
+ NAME_BAD="new"
+ NAME_GOOD="old" ;;
esac ;;
esac
}
bisect_voc () {
case "$1" in
- bad) echo "bad" ;;
- good) echo "good" ;;
+ bad) echo "bad|old" ;;
+ good) echo "good|new" ;;
esac
}
@@ -566,7 +579,7 @@ case "$#" in
git bisect -h ;;
start)
bisect_start "$@" ;;
- bad|good)
+ bad|good|new|old)
bisect_state "$cmd" "$@" ;;
skip)
bisect_skip "$@" ;;
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index 9e2c203..983c503 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -759,4 +759,42 @@ test_expect_success '"git bisect bad HEAD" behaves as "git bisect bad"' '
git bisect reset
'
+test_expect_success 'bisect starts with only one new' '
+ git bisect reset &&
+ git bisect start &&
+ git bisect new $HASH4 &&
+ git bisect next
+'
+
+test_expect_success 'bisect does not start with only one old' '
+ git bisect reset &&
+ git bisect start &&
+ git bisect old $HASH1 &&
+ test_must_fail git bisect next
+'
+
+test_expect_success 'bisect start with one new and old' '
+ git bisect reset &&
+ git bisect start &&
+ git bisect old $HASH1 &&
+ git bisect new $HASH4 &&
+ git bisect new &&
+ git bisect new >bisect_result &&
+ grep "$HASH2 is the first new commit" bisect_result &&
+ git bisect log >log_to_replay.txt &&
+ git bisect reset
+'
+
+test_expect_success 'bisect replay with old and new' '
+ git bisect replay log_to_replay.txt >bisect_result &&
+ grep "$HASH2 is the first new commit" bisect_result &&
+ git bisect reset
+'
+
+test_expect_success 'bisect cannot mix old/new and good/bad' '
+ git bisect start &&
+ git bisect bad $HASH4 &&
+ test_must_fail git bisect old $HASH1
+'
+
test_done
--
2.4.4.414.g318df7a.dirty
^ permalink raw reply related [flat|nested] 48+ messages in thread* Re: [PATCH v9 4/5] bisect: add the terms old/new
2015-06-25 18:50 ` [PATCH v9 4/5] bisect: add the terms old/new Matthieu Moy
@ 2015-06-26 4:11 ` Christian Couder
2015-06-26 7:00 ` Matthieu Moy
0 siblings, 1 reply; 48+ messages in thread
From: Christian Couder @ 2015-06-26 4:11 UTC (permalink / raw)
To: Matthieu Moy
Cc: Junio C Hamano, git, Antoine Delaite, louis--alexandre stuber,
Christian Couder, Thomas Nguy, Valentin Duperray, Louis Stuber,
Valentin Duperray, Franck Jonas, Lucien Kong, Thomas Nguy,
Huynh Khoi Nguyen Nguyen
On Thu, Jun 25, 2015 at 8:50 PM, Matthieu Moy <Matthieu.Moy@imag.fr> wrote:
[...]
> @@ -178,7 +183,7 @@ 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 $must_write_terms -eq 1 && test ! -s "$GIT_DIR/BISECT_TERMS"
> then
> echo "$NAME_BAD" >"$GIT_DIR/BISECT_TERMS" &&
> echo "$NAME_GOOD" >>"$GIT_DIR/BISECT_TERMS"
You are writing BISECT_TERMS here...
> @@ -543,14 +548,22 @@ check_and_set_terms () {
> fi
> NAME_BAD="bad"
> NAME_GOOD="good" ;;
> + new|old)
> + if ! test -s "$GIT_DIR/BISECT_TERMS"
> + then
> + echo "new" >"$GIT_DIR/BISECT_TERMS" &&
> + echo "old" >>"$GIT_DIR/BISECT_TERMS"
> + fi
> + NAME_BAD="new"
> + NAME_GOOD="old" ;;
...and here nearly in the same way.
So perhaps you could use a function like:
write_bisect_terms() {
if test ! -s "$GIT_DIR/BISECT_TERMS"
then
echo "$NAME_BAD" >"$GIT_DIR/BISECT_TERMS" &&
echo "$NAME_GOOD" >>"$GIT_DIR/BISECT_TERMS"
fi
}
^ permalink raw reply [flat|nested] 48+ messages in thread* Re: [PATCH v9 4/5] bisect: add the terms old/new
2015-06-26 4:11 ` Christian Couder
@ 2015-06-26 7:00 ` Matthieu Moy
0 siblings, 0 replies; 48+ messages in thread
From: Matthieu Moy @ 2015-06-26 7:00 UTC (permalink / raw)
To: Christian Couder
Cc: Junio C Hamano, git, Antoine Delaite, louis--alexandre stuber,
Christian Couder, Thomas Nguy, Valentin Duperray, Louis Stuber
Christian Couder <christian.couder@gmail.com> writes:
> So perhaps you could use a function like:
>
> write_bisect_terms() {
> if test ! -s "$GIT_DIR/BISECT_TERMS"
> then
> echo "$NAME_BAD" >"$GIT_DIR/BISECT_TERMS" &&
> echo "$NAME_GOOD" >>"$GIT_DIR/BISECT_TERMS"
> fi
> }
I already had it in the last patch, but reworked the code to introduce
it earlier, and never >"$GIT_DIR/BISECT_TERMS" outside this function.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v9 5/5] bisect: allow any terms set by user
2015-06-25 18:50 ` [PATCH v9 0/5] bisect terms Matthieu Moy
` (3 preceding siblings ...)
2015-06-25 18:50 ` [PATCH v9 4/5] bisect: add the terms old/new Matthieu Moy
@ 2015-06-25 18:50 ` Matthieu Moy
2015-06-25 21:41 ` Junio C Hamano
4 siblings, 1 reply; 48+ messages in thread
From: Matthieu Moy @ 2015-06-25 18:50 UTC (permalink / raw)
To: gitster
Cc: git, antoine.delaite, louis--alexandre.stuber, chriscool,
thomasxnguy, valentinduperray, Louis Stuber, Matthieu Moy
From: Antoine Delaite <antoine.delaite@ensimag.grenoble-inp.fr>
Introduction of the git bisect terms command. The user can set his own
terms. It will work exactly like before. The terms must be set before the
start.
Signed-off-by: Antoine Delaite <antoine.delaite@ensimag.grenoble-inp.fr>
Signed-off-by: Louis Stuber <stuberl@ensimag.grenoble-inp.fr>
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
Documentation/git-bisect.txt | 24 +++++++++++++
git-bisect.sh | 80 ++++++++++++++++++++++++++++++++++++++------
t/t6030-bisect-porcelain.sh | 43 ++++++++++++++++++++++++
3 files changed, 137 insertions(+), 10 deletions(-)
diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
index 3c3021a..e783f87 100644
--- a/Documentation/git-bisect.txt
+++ b/Documentation/git-bisect.txt
@@ -133,6 +133,30 @@ You must run `git bisect start` without commits as argument and run
`git bisect new <rev>`/`git bisect old <rev>...` after to add the
commits.
+Alternative terms: use your own terms
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+If the builtins terms bad/good and new/old do not satisfy you, you can
+set your own terms.
+
+------------------------------------------------
+git bisect terms <term-new> <term-old>
+------------------------------------------------
+
+This command has to be used before a bisection has started. <term-old>
+must be associated with the latest revisions and <term-new> with the
+ancestors of <term-old>. For example, if something was buggy in the
+old part of the history, you know somewhere the bug was fixed, and you
+want to find the exact commit that fixed it, you may want to say `git
+bisect terms fixed broken`; this way, you would mark a commit that
+still has the bug with `broken`, and a newer one after the fix with
+`fixed`.
+
+Only the first bisection following the `git bisect terms` will use the
+terms. If you mistyped one of the terms you can do again `git bisect
+terms <term-old> <term-new>`.
+
+
Bisect visualize
~~~~~~~~~~~~~~~~
diff --git a/git-bisect.sh b/git-bisect.sh
index 569898b..8fee712 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -1,6 +1,6 @@
#!/bin/sh
-USAGE='[help|start|bad|good|new|old|skip|next|reset|visualize|replay|log|run]'
+USAGE='[help|start|bad|good|new|old|terms|skip|next|reset|visualize|replay|log|run]'
LONG_USAGE='git bisect help
print this long help message.
git bisect start [--no-checkout] [<bad> [<good>...]] [--] [<pathspec>...]
@@ -11,6 +11,8 @@ git bisect (bad|new) [<rev>]
git bisect (good|old) [<rev>...]
mark <rev>... known-good revisions/
revisions before change in a given property.
+git bisect terms <term-new> <term-old>
+ set up <term-new> and <term-old> as terms (default: bad, good)
git bisect skip [(<rev>|<range>)...]
mark <rev>... untestable revisions.
git bisect next
@@ -80,6 +82,16 @@ bisect_start() {
bad_seen=0
eval=''
must_write_terms=0
+ must_log_terms=0
+ if test -s "$GIT_DIR/BISECT_TERMS"
+ then
+ # We're going to restart from a clean state and the
+ # file will be deleted. Record the old state in
+ # variables and restore it below.
+ must_write_terms=1
+ must_log_terms=1
+ get_terms
+ fi
if test "z$(git rev-parse --is-bare-repository)" != zfalse
then
mode=--no-checkout
@@ -183,10 +195,14 @@ bisect_start() {
} &&
git rev-parse --sq-quote "$@" >"$GIT_DIR/BISECT_NAMES" &&
eval "$eval true" &&
- if test $must_write_terms -eq 1 && test ! -s "$GIT_DIR/BISECT_TERMS"
+ if test $must_write_terms -eq 1
then
- echo "$NAME_BAD" >"$GIT_DIR/BISECT_TERMS" &&
- echo "$NAME_GOOD" >>"$GIT_DIR/BISECT_TERMS"
+ write_terms "$NAME_BAD" "$NAME_GOOD" &&
+ if test $must_log_terms -eq 1
+ then
+ echo "git bisect terms $NAME_BAD $NAME_GOOD" \
+ >>"$GIT_DIR/BISECT_LOG"
+ fi
fi &&
echo "git bisect start$orig_args" >>"$GIT_DIR/BISECT_LOG" || exit
#
@@ -450,6 +466,8 @@ bisect_replay () {
eval "$cmd" ;;
"$NAME_GOOD"|"$NAME_BAD"|skip)
bisect_write "$command" "$rev" ;;
+ terms)
+ bisect_terms $rev ;;
*)
die "$(gettext "?? what are you talking about?")" ;;
esac
@@ -534,7 +552,8 @@ get_terms () {
check_and_set_terms () {
cmd="$1"
case "$cmd" in
- bad|good|new|old)
+ skip|start|terms) ;;
+ *)
if test -s "$GIT_DIR/BISECT_TERMS" && test "$cmd" != "$NAME_BAD" && test "$cmd" != "$NAME_GOOD"
then
die "$(eval_gettext "Invalid command: you're currently in a \$NAME_BAD/\$NAME_GOOD bisect.")"
@@ -543,16 +562,14 @@ check_and_set_terms () {
bad|good)
if ! test -s "$GIT_DIR/BISECT_TERMS"
then
- echo "bad" >"$GIT_DIR/BISECT_TERMS" &&
- echo "good" >>"$GIT_DIR/BISECT_TERMS"
+ write_terms bad good
fi
NAME_BAD="bad"
NAME_GOOD="good" ;;
new|old)
if ! test -s "$GIT_DIR/BISECT_TERMS"
then
- echo "new" >"$GIT_DIR/BISECT_TERMS" &&
- echo "old" >>"$GIT_DIR/BISECT_TERMS"
+ write_terms new old
fi
NAME_BAD="new"
NAME_GOOD="old" ;;
@@ -567,6 +584,47 @@ bisect_voc () {
esac
}
+bisect_terms () {
+ case "$#" in
+ 0)
+ if test -s "$GIT_DIR/BISECT_TERMS"
+ then
+ get_terms
+ gettextln "Your current terms are $NAME_GOOD for the old state
+and $NAME_BAD for the new state."
+ else
+ die "$(gettext "No terms defined.")"
+ fi ;;
+ 2)
+ if ! test -s "$GIT_DIR/BISECT_START"
+ then
+ write_terms "$1" "$2"
+ echo "git bisect terms $NAME_BAD $NAME_GOOD" >>"$GIT_DIR/BISECT_LOG" || exit
+ else
+ die "$(gettext "A bisection has already started, and you can't change terms in the middle of it.
+Use 'git bisect terms' to see the current terms.
+Otherwise, to start a new bisection with new terms, please use
+'git bisect reset' and set the terms before the start")"
+ fi ;;
+ *)
+ usage ;;
+ esac
+}
+
+write_terms () {
+ NAME_BAD=$1
+ NAME_GOOD=$2
+ check_term_format "$NAME_BAD"
+ check_term_format "$NAME_GOOD"
+ printf '%s\n%s\n' "$NAME_BAD" "$NAME_GOOD" >"$GIT_DIR/BISECT_TERMS"
+}
+
+check_term_format () {
+ term=$1
+ git check-ref-format refs/bisect/"$term" ||
+ die "$(eval_gettext "'\$term' is not a valid term")"
+}
+
case "$#" in
0)
usage ;;
@@ -579,7 +637,7 @@ case "$#" in
git bisect -h ;;
start)
bisect_start "$@" ;;
- bad|good|new|old)
+ bad|good|new|old|"$NAME_BAD"|"$NAME_GOOD")
bisect_state "$cmd" "$@" ;;
skip)
bisect_skip "$@" ;;
@@ -596,6 +654,8 @@ case "$#" in
bisect_log ;;
run)
bisect_run "$@" ;;
+ terms)
+ bisect_terms "$@" ;;
*)
usage ;;
esac
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index 983c503..289dbb0 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -797,4 +797,47 @@ test_expect_success 'bisect cannot mix old/new and good/bad' '
test_must_fail git bisect old $HASH1
'
+test_expect_success 'bisect start with one term1 and term2' '
+ git bisect reset &&
+ git bisect terms term1 term2 &&
+ git bisect start &&
+ git bisect term2 $HASH1 &&
+ git bisect term1 $HASH4 &&
+ git bisect term1 &&
+ git bisect term1 >bisect_result &&
+ grep "$HASH2 is the first term1 commit" bisect_result &&
+ git bisect log >log_to_replay.txt &&
+ git bisect reset
+'
+
+test_expect_success 'bisect replay with term1 and term2' '
+ git bisect replay log_to_replay.txt >bisect_result &&
+ grep "$HASH2 is the first term1 commit" bisect_result &&
+ git bisect reset
+'
+
+test_expect_success 'bisect start term1 term2' '
+ git bisect reset &&
+ git bisect terms term1 term2 &&
+ git bisect start $HASH4 $HASH1 &&
+ git bisect term1 &&
+ git bisect term1 >bisect_result &&
+ grep "$HASH2 is the first term1 commit" bisect_result &&
+ git bisect log >log_to_replay.txt &&
+ git bisect reset
+'
+
+test_expect_success 'bisect cannot mix terms' '
+ git bisect reset &&
+ git bisect terms a b &&
+ git bisect terms term1 term2 &&
+ git bisect start $HASH4 $HASH1 &&
+ test_must_fail git bisect a &&
+ test_must_fail git bisect b &&
+ test_must_fail git bisect bad &&
+ test_must_fail git bisect good &&
+ test_must_fail git bisect new &&
+ test_must_fail git bisect old
+'
+
test_done
--
2.4.4.414.g318df7a.dirty
^ permalink raw reply related [flat|nested] 48+ messages in thread* Re: [PATCH v9 5/5] bisect: allow any terms set by user
2015-06-25 18:50 ` [PATCH v9 5/5] bisect: allow any terms set by user Matthieu Moy
@ 2015-06-25 21:41 ` Junio C Hamano
2015-06-25 22:10 ` Junio C Hamano
2015-06-26 6:59 ` Matthieu Moy
0 siblings, 2 replies; 48+ messages in thread
From: Junio C Hamano @ 2015-06-25 21:41 UTC (permalink / raw)
To: Matthieu Moy
Cc: git, antoine.delaite, louis--alexandre.stuber, chriscool,
thomasxnguy, valentinduperray, Louis Stuber
Matthieu Moy <Matthieu.Moy@imag.fr> writes:
> +------------------------------------------------
> +git bisect terms <term-new> <term-old>
> +------------------------------------------------
The mnemonic for "git bisect start bad good" is Bad comes before
Good (B = 0x42, G = 0x47) and this is same for "new/old", New comes
before Old (N = 0x4e, O = 0x4f). "git bisect terms new old" follows
the same pattern, which is good. Easy to remember.
> +This command has to be used before a bisection has started. <term-old>
> +must be associated with the latest revisions and <term-new> with the
> +ancestors of <term-old>.
Whoa? This gets new and old mixed up, doesn't it?
> For example, if something was buggy in the
> +old part of the history, you know somewhere the bug was fixed, and you
> +want to find the exact commit that fixed it, you may want to say `git
> +bisect terms fixed broken`; this way, you would mark a commit that
> +still has the bug with `broken`, and a newer one after the fix with
> +`fixed`.
So, it used to be broken, it got fixed recently, so broken is old,
fixed is new, "bad/new and then good/old" mnemonic says you give
"fixed broken" to "bisect terms". OK.
> +Only the first bisection following the `git bisect terms` will use the
> +terms. If you mistyped one of the terms you can do again `git bisect
> +terms <term-old> <term-new>`.
This is also the other way around, no?
> +git bisect terms <term-new> <term-old>
> + set up <term-new> and <term-old> as terms (default: bad, good)
Good.
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v9 5/5] bisect: allow any terms set by user
2015-06-25 21:41 ` Junio C Hamano
@ 2015-06-25 22:10 ` Junio C Hamano
2015-06-26 8:20 ` Matthieu Moy
2015-06-26 6:59 ` Matthieu Moy
1 sibling, 1 reply; 48+ messages in thread
From: Junio C Hamano @ 2015-06-25 22:10 UTC (permalink / raw)
To: Matthieu Moy
Cc: git, antoine.delaite, louis--alexandre.stuber, chriscool,
thomasxnguy, valentinduperray, Louis Stuber
Junio C Hamano <gitster@pobox.com> writes:
>> +Only the first bisection following the `git bisect terms` will use the
>> +terms. If you mistyped one of the terms you can do again `git bisect
>> +terms <term-old> <term-new>`.
This paragraph may need further polishing.
The first sentence makes it sound as if this is a valid sequence,
but I do not think that is what you meant:
$ git bisect start master maint
$ git bisect bad
$ git bisect terms new old
$ git bisect old
$ git bisect bad
I think what you wanted to say was that "git bisect terms" is in
effect during the single bisect session, and after you are done with
"git bisect reset", the next bisect session, unless you use "git
bisect terms", will use "bad" and "good", as that is the default
pair of terms.
The second sentence may want to be something like
If you mistyped one of the terms, you can do another "git
bisect terms <term-new> <term-old>" to correct them, but
that is possible only before you start the bisection.
Otherwise, you invite this
$ git bisect start master maint
$ git bisect terms new olf
$ git bisect olf
$ git bisect new
$ git bisect old
... error message that says you can give either "new" and "olf"
$ git bisect terms new old
$ git bisect old
which may not work well.
^ permalink raw reply [flat|nested] 48+ messages in thread* Re: [PATCH v9 5/5] bisect: allow any terms set by user
2015-06-25 22:10 ` Junio C Hamano
@ 2015-06-26 8:20 ` Matthieu Moy
2015-06-26 16:48 ` Junio C Hamano
0 siblings, 1 reply; 48+ messages in thread
From: Matthieu Moy @ 2015-06-26 8:20 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, antoine.delaite, louis--alexandre.stuber, chriscool,
thomasxnguy, valentinduperray, Louis Stuber
Junio C Hamano <gitster@pobox.com> writes:
> The second sentence may want to be something like
>
> If you mistyped one of the terms, you can do another "git
> bisect terms <term-new> <term-old>" to correct them, but
> that is possible only before you start the bisection.
Applied, thanks.
I currently have this in addition to v9 in my branch. I'll resend later
(https://github.com/moy/git/tree/bisect-terms is up to date).
diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
index e783f87..7609cd6 100644
--- a/Documentation/git-bisect.txt
+++ b/Documentation/git-bisect.txt
@@ -143,19 +143,19 @@ set your own terms.
git bisect terms <term-new> <term-old>
------------------------------------------------
-This command has to be used before a bisection has started. <term-old>
-must be associated with the latest revisions and <term-new> with the
-ancestors of <term-old>. For example, if something was buggy in the
+This command has to be used before a bisection has started. <term-new>
+must be associated with the latest revisions and <term-old> with some
+ancestors of <term-new>. For example, if something was buggy in the
old part of the history, you know somewhere the bug was fixed, and you
want to find the exact commit that fixed it, you may want to say `git
-bisect terms fixed broken`; this way, you would mark a commit that
+bisect terms broken fixed`; this way, you would mark a commit that
still has the bug with `broken`, and a newer one after the fix with
`fixed`.
-Only the first bisection following the `git bisect terms` will use the
+Only the bisection following the `git bisect terms` will use the
terms. If you mistyped one of the terms you can do again `git bisect
-terms <term-old> <term-new>`.
-
+terms <term-new> <term-old>`, but that is possible only before you
+start the bisection.
Bisect visualize
~~~~~~~~~~~~~~~~
diff --git a/git-bisect.sh b/git-bisect.sh
index 8fee712..07c64d9 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -549,6 +549,20 @@ get_terms () {
fi
}
+write_terms () {
+ NAME_BAD=$1
+ NAME_GOOD=$2
+ check_term_format "$NAME_BAD"
+ check_term_format "$NAME_GOOD"
+ printf '%s\n%s\n' "$NAME_BAD" "$NAME_GOOD" >"$GIT_DIR/BISECT_TERMS"
+}
+
+check_term_format () {
+ term=$1
+ git check-ref-format refs/bisect/"$term" ||
+ die "$(eval_gettext "'\$term' is not a valid term")"
+}
+
check_and_set_terms () {
cmd="$1"
case "$cmd" in
@@ -579,8 +593,8 @@ check_and_set_terms () {
bisect_voc () {
case "$1" in
- bad) echo "bad|old" ;;
- good) echo "good|new" ;;
+ bad) echo "bad|new" ;;
+ good) echo "good|old" ;;
esac
}
@@ -611,20 +625,6 @@ Otherwise, to start a new bisection with new terms, please use
esac
}
-write_terms () {
- NAME_BAD=$1
- NAME_GOOD=$2
- check_term_format "$NAME_BAD"
- check_term_format "$NAME_GOOD"
- printf '%s\n%s\n' "$NAME_BAD" "$NAME_GOOD" >"$GIT_DIR/BISECT_TERMS"
-}
-
-check_term_format () {
- term=$1
- git check-ref-format refs/bisect/"$term" ||
- die "$(eval_gettext "'\$term' is not a valid term")"
-}
-
case "$#" in
0)
usage ;;
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply related [flat|nested] 48+ messages in thread* Re: [PATCH v9 5/5] bisect: allow any terms set by user
2015-06-26 8:20 ` Matthieu Moy
@ 2015-06-26 16:48 ` Junio C Hamano
2015-06-26 17:08 ` Matthieu Moy
0 siblings, 1 reply; 48+ messages in thread
From: Junio C Hamano @ 2015-06-26 16:48 UTC (permalink / raw)
To: Matthieu Moy
Cc: git, antoine.delaite, louis--alexandre.stuber, chriscool,
thomasxnguy, valentinduperray, Louis Stuber
Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> The second sentence may want to be something like
>>
>> If you mistyped one of the terms, you can do another "git
>> bisect terms <term-new> <term-old>" to correct them, but
>> that is possible only before you start the bisection.
>
> Applied, thanks.
I didn't say it out loud while writing the above, but this (and we
have other places that use the same phrase in the doc) mentions that
you have some point in time where you "start" the bisection, without
having a clear definition of where that bisection starts, and that
bothers me. "You can do X until you do Y", when it is not clear
what Y exactly happens, is not very helpful.
We who know how bisection works internally know that the point of no
return is when we commit to the two terms and write one of the good
or bad bisect refs. At that point, technically we haven't done a
bisection yet ("git bisect good maint" would bisect_autostart, but
without the other end, does not have a graph to bisect yet to find a
midpoint).
And worse yet, majority of users may read "git bisect start" is
where "you start bisection", but "bisect start" (either called
directly, or via bisect_autostart by the first "git bisect good")
is where you set up the machinery, so doing "bisect terms" before
doing "bisect start" does not make much sense.
> I currently have this in addition to v9 in my branch. I'll resend later
> (https://github.com/moy/git/tree/bisect-terms is up to date).
>
> diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
> index e783f87..7609cd6 100644
> --- a/Documentation/git-bisect.txt
> +++ b/Documentation/git-bisect.txt
> @@ -143,19 +143,19 @@ set your own terms.
> git bisect terms <term-new> <term-old>
> ------------------------------------------------
>
> -This command has to be used before a bisection has started. <term-old>
> -must be associated with the latest revisions and <term-new> with the
> -ancestors of <term-old>. For example, if something was buggy in the
> +This command has to be used before a bisection has started. <term-new>
> +must be associated with the latest revisions and <term-old> with some
> +ancestors of <term-new>. For example, if something was buggy in the
> old part of the history, you know somewhere the bug was fixed, and you
> want to find the exact commit that fixed it, you may want to say `git
> -bisect terms fixed broken`; this way, you would mark a commit that
> +bisect terms broken fixed`; this way, you would mark a commit that
The example talks about a bug we used to have that was corrected, so
"broken" is old and "fixed" is new. The earlier parts of this hunk
are correct but the last line should not be changed, no?
There unfortunately are two reasons why we cannot flip the order to
"git bisect terms old new":
* "git bisect start $bad $good" established the
convention to list bad before good (and 'B'ad comes before
'G'ood, so does 'N'ew before 'O'ld).
* "git bisect start $good $bad", even if we could use a time
machine, would not be a good syntax, as you give zero or more
good ones and one and only one bad one.
^ permalink raw reply [flat|nested] 48+ messages in thread* Re: [PATCH v9 5/5] bisect: allow any terms set by user
2015-06-26 16:48 ` Junio C Hamano
@ 2015-06-26 17:08 ` Matthieu Moy
2015-06-26 18:08 ` Junio C Hamano
0 siblings, 1 reply; 48+ messages in thread
From: Matthieu Moy @ 2015-06-26 17:08 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, antoine.delaite, louis--alexandre.stuber, chriscool,
thomasxnguy, valentinduperray, Louis Stuber
Junio C Hamano <gitster@pobox.com> writes:
> And worse yet, majority of users may read "git bisect start" is
> where "you start bisection", but "bisect start" (either called
> directly, or via bisect_autostart by the first "git bisect good")
> is where you set up the machinery, so doing "bisect terms" before
> doing "bisect start" does not make much sense.
This is currently how it's implemented. You need to say
$ git bisect terms foo bar
$ git bisect start
And indeed "git bisect terms foo bar" errors out. I think the reason it
is this way is to allow
$ git bisect terms foo bar
$ git bisect start <sha1> <sha1>
But actually, we can allow "git bisect terms" until BISECT_TERMS is
created, which is your intuition and makes more sense IMHO. I'll try to
do that.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v9 5/5] bisect: allow any terms set by user
2015-06-26 17:08 ` Matthieu Moy
@ 2015-06-26 18:08 ` Junio C Hamano
2015-06-26 20:18 ` Matthieu Moy
0 siblings, 1 reply; 48+ messages in thread
From: Junio C Hamano @ 2015-06-26 18:08 UTC (permalink / raw)
To: Matthieu Moy
Cc: git, antoine.delaite, louis--alexandre.stuber, chriscool,
thomasxnguy, valentinduperray, Louis Stuber
Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
> This is currently how it's implemented. You need to say
>
> $ git bisect terms foo bar
> $ git bisect start
Ahh, it means everything including the description (i.e. "you start
bisection") is consistent and perfectly fine.
I misread the patch, it seems. Sorry for the noise.
Thanks.
> And indeed "git bisect terms foo bar" errors out. I think the reason it
> is this way is to allow
>
> $ git bisect terms foo bar
> $ git bisect start <sha1> <sha1>
>
> But actually, we can allow "git bisect terms" until BISECT_TERMS is
> created, which is your intuition and makes more sense IMHO. I'll try to
> do that.
I am not sure if it is a good idea, though. Matching the intuition
of those who (think they) know how it is implemented is much less
important than providing a behaviour that is simple to explain to
casual users.
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v9 5/5] bisect: allow any terms set by user
2015-06-26 18:08 ` Junio C Hamano
@ 2015-06-26 20:18 ` Matthieu Moy
0 siblings, 0 replies; 48+ messages in thread
From: Matthieu Moy @ 2015-06-26 20:18 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, antoine.delaite, louis--alexandre.stuber, chriscool,
thomasxnguy, valentinduperray, Louis Stuber
Junio C Hamano <gitster@pobox.com> writes:
> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>
>> And indeed "git bisect terms foo bar" errors out. I think the reason it
>> is this way is to allow
>>
>> $ git bisect terms foo bar
>> $ git bisect start <sha1> <sha1>
>>
>> But actually, we can allow "git bisect terms" until BISECT_TERMS is
>> created, which is your intuition and makes more sense IMHO. I'll try to
>> do that.
>
> I am not sure if it is a good idea, though. Matching the intuition
> of those who (think they) know how it is implemented is much less
> important than providing a behaviour that is simple to explain to
> casual users.
I thought about it a bit more. Essentially, we have two options simple
to implement:
1) Allow `git bisect terms` when BISECT_TERMS does not exists. This
prevents running `git bisect terms` twice in a row, but would match
your first guess: we could still run it until the terms are commited.
Pro: it feels more natural to me to write
git bisect start
git bisect terms foo bar
...
git bisect reset
=> everything happens between start and reset.
2) Allow `git bisect terms` when BISECT_START does not exist (the current
implementation).
I'm keeping option 2) for now, but if anyone think option 1) is better,
it shouldn't be hard to change.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v9 5/5] bisect: allow any terms set by user
2015-06-25 21:41 ` Junio C Hamano
2015-06-25 22:10 ` Junio C Hamano
@ 2015-06-26 6:59 ` Matthieu Moy
1 sibling, 0 replies; 48+ messages in thread
From: Matthieu Moy @ 2015-06-26 6:59 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, antoine.delaite, louis--alexandre.stuber, chriscool,
thomasxnguy, valentinduperray, Louis Stuber
Junio C Hamano <gitster@pobox.com> writes:
> Matthieu Moy <Matthieu.Moy@imag.fr> writes:
>
>> +------------------------------------------------
>> +git bisect terms <term-new> <term-old>
>> +------------------------------------------------
>
> The mnemonic for "git bisect start bad good" is Bad comes before
> Good (B = 0x42, G = 0x47) and this is same for "new/old", New comes
> before Old (N = 0x4e, O = 0x4f). "git bisect terms new old" follows
> the same pattern, which is good. Easy to remember.
>
>> +This command has to be used before a bisection has started. <term-old>
>> +must be associated with the latest revisions and <term-new> with the
>> +ancestors of <term-old>.
>
> Whoa? This gets new and old mixed up, doesn't it?
Right. I think it was already the case before, but using term1/term2 in
the text made it hard to spot. The new wording is clearer, and makes it
easier to find bugs in the explanations ;-).
>> +Only the first bisection following the `git bisect terms` will use the
>> +terms. If you mistyped one of the terms you can do again `git bisect
>> +terms <term-old> <term-new>`.
>
> This is also the other way around, no?
Indeed.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply [flat|nested] 48+ messages in thread