[PATCH v3 1/6] bisect: correction of typo

classic Classic list List threaded Threaded
48 messages Options
123
Reply | Threaded
Open this post in threaded view
|

[PATCH v3 1/6] bisect: correction of typo

Antoine Delaite
Signed-off-by: Antoine Delaite <[hidden email]>
---
 bisect.c                    |    2 +-
 t/t6030-bisect-porcelain.sh |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/bisect.c b/bisect.c
index 03d5cd9..5b8357d 100644
--- a/bisect.c
+++ b/bisect.c
@@ -743,7 +743,7 @@ static void handle_bad_merge_base(void)
 
  fprintf(stderr, "Some good revs are not ancestor of the bad rev.\n"
  "git bisect cannot work properly in this case.\n"
- "Maybe you mistake good and bad revs?\n");
+ "Maybe you mistook good and bad revs?\n");
  exit(1);
 }
 
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index 06b4868..9e2c203 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -362,7 +362,7 @@ test_expect_success 'bisect starting with a detached HEAD' '
 test_expect_success 'bisect errors out if bad and good are mistaken' '
  git bisect reset &&
  test_must_fail git bisect start $HASH2 $HASH4 2> rev_list_error &&
- grep "mistake good and bad" rev_list_error &&
+ grep "mistook good and bad" rev_list_error &&
  git bisect reset
 '
 
--
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe git" in
Reply | Threaded
Open this post in threaded view
|

[PATCH v3 2/6] bisect: replace hardcoded "bad|good" by variables

Antoine Delaite
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 <[hidden email]>
Signed-off-by: Louis Stuber <[hidden email]>
Signed-off-by: Valentin Duperray <[hidden email]>
Signed-off-by: Franck Jonas <[hidden email]>
Signed-off-by: Lucien Kong <[hidden email]>
Signed-off-by: Thomas Nguy <[hidden email]>
Signed-off-by: Huynh Khoi Nguyen Nguyen <[hidden email]>
Signed-off-by: Matthieu Moy <[hidden email]>
---
 bisect.c      |   51 ++++++++++++++++++++++++++++++++++-----------------
 git-bisect.sh |   57 +++++++++++++++++++++++++++++++--------------------------
 2 files changed, 65 insertions(+), 43 deletions(-)
 mode change 100755 => 100644 git-bisect.sh

diff --git a/bisect.c b/bisect.c
index 5b8357d..cda30fa 100644
--- a/bisect.c
+++ b/bisect.c
@@ -21,6 +21,9 @@ static const char *argv_checkout[] = {"checkout", "-q", NULL, "--", NULL};
 static const char *argv_show_branch[] = {"show-branch", NULL, NULL};
 static const char *argv_update_ref[] = {"update-ref", "--no-deref", "BISECT_HEAD", NULL, NULL};
 
+static const char *name_bad;
+static const char *name_good;
+
 /* Remember to update object flag allocation in object.h */
 #define COUNTED (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;
--
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe git" in
Reply | Threaded
Open this post in threaded view
|

[PATCH v3 3/6] bisect: simplify the addition of new bisect terms

Antoine Delaite
In reply to this post by Antoine Delaite
We create a file BISECT_TERMS in the repository .git to be read during a
bisection. The fonctions to be changed if we add new terms are quite
few.
In git-bisect.sh :
        check_and_set_terms
        bisect_voc

Signed-off-by: Antoine Delaite <[hidden email]>
Signed-off-by: Louis Stuber <[hidden email]>
Signed-off-by: Valentin Duperray <[hidden email]>
Signed-off-by: Franck Jonas <[hidden email]>
Signed-off-by: Lucien Kong <[hidden email]>
Signed-off-by: Thomas Nguy <[hidden email]>
Signed-off-by: Huynh Khoi Nguyen Nguyen <[hidden email]>
Signed-off-by: Matthieu Moy <[hidden email]>
---
 bisect.c      |   38 ++++++++++++++++++++++++++++--
 git-bisect.sh |   70 +++++++++++++++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 98 insertions(+), 10 deletions(-)

diff --git a/bisect.c b/bisect.c
index cda30fa..2fc8a78 100644
--- a/bisect.c
+++ b/bisect.c
@@ -747,7 +747,10 @@ static void handle_bad_merge_base(void)
  "between %s and [%s].\n",
  bad_hex, bad_hex, good_hex);
  } else {
- die("BUG: terms %s/%s not managed", name_bad, name_good);
+ fprintf(stderr, "The merge base %s is %s.\n"
+ "This means the first commit marked %s is "
+ "between %s and [%s].\n",
+ bad_hex, name_bad, name_bad, bad_hex, good_hex);
  }
  exit(3);
  }
@@ -902,6 +905,36 @@ static void show_diff_tree(const char *prefix, struct commit *commit)
 }
 
 /*
+ * The terms used for this bisect session are stored in BISECT_TERMS.
+ * We read them and store them to adapt the messages accordingly.
+ * Default is bad/good.
+ */
+void read_bisect_terms(const char **read_bad, const char **read_good)
+{
+ struct strbuf str = STRBUF_INIT;
+ const char *filename = git_path("BISECT_TERMS");
+ FILE *fp = fopen(filename, "r");
+
+ if (!fp) {
+ if (errno==2) {
+ *read_bad = "bad";
+ *read_good = "good";
+ return;
+ } else {
+ die("could not read file '%s': %s", filename,
+ strerror(errno));
+ }
+ } else {
+ strbuf_getline(&str, fp, '\n');
+ *read_bad = strbuf_detach(&str, NULL);
+ strbuf_getline(&str, fp, '\n');
+ *read_good = strbuf_detach(&str, NULL);
+ }
+ strbuf_release(&str);
+ fclose(fp);
+}
+
+/*
  * We use the convention that exiting with an exit code 10 means that
  * the bisection process finished successfully.
  * In this case the calling shell script should exit 0.
@@ -917,8 +950,7 @@ int bisect_next_all(const char *prefix, int no_checkout)
  const unsigned char *bisect_rev;
  char bisect_rev_hex[GIT_SHA1_HEXSZ + 1];
 
- name_bad="bad";
- name_good="good";
+ read_bisect_terms(&name_bad, &name_good);
  if (read_bisect_refs())
  die("reading bisect refs failed");
 
diff --git a/git-bisect.sh b/git-bisect.sh
index ce6412f..55b9ebd 100644
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -77,6 +77,9 @@ bisect_start() {
  orig_args=$(git rev-parse --sq-quote "$@")
  bad_seen=0
  eval=''
+ # revision_seen is true if a git bisect start
+ # has revision as arguments
+ revision_seen=0
  if test "z$(git rev-parse --is-bare-repository)" != zfalse
  then
  mode=--no-checkout
@@ -101,6 +104,9 @@ bisect_start() {
  die "$(eval_gettext "'\$arg' does not appear to be a valid revision")"
  break
  }
+
+ revision_seen=1
+
  case $bad_seen in
  0) state=$NAME_BAD ; bad_seen=1 ;;
  *) state=$NAME_GOOD ;;
@@ -172,6 +178,11 @@ bisect_start() {
  } &&
  git rev-parse --sq-quote "$@" >"$GIT_DIR/BISECT_NAMES" &&
  eval "$eval true" &&
+ if test $revision_seen -eq 1 && test ! -s "$GIT_DIR/BISECT_TERMS"
+ then
+ echo "$NAME_BAD" >"$GIT_DIR/BISECT_TERMS" &&
+ echo "$NAME_GOOD" >>"$GIT_DIR/BISECT_TERMS"
+ fi &&
  echo "git bisect start$orig_args" >>"$GIT_DIR/BISECT_LOG" || exit
  #
  # Check if we can proceed to the next bisect state.
@@ -232,6 +243,7 @@ bisect_skip() {
 bisect_state() {
  bisect_autostart
  state=$1
+ check_and_set_terms $state
  case "$#,$state" in
  0,*)
  die "$(gettext "Please call 'bisect_state' with at least one argument.")" ;;
@@ -291,15 +303,17 @@ bisect_next_check() {
  : bisect without $NAME_GOOD...
  ;;
  *)
-
+ bad_syn=$(bisect_voc bad)
+ 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)
--
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe git" in
Reply | Threaded
Open this post in threaded view
|

[PATCH v3 4/6] bisect: add the terms old/new

Antoine Delaite
In reply to this post by Antoine Delaite
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 <[hidden email]>
Signed-off-by: Louis Stuber <[hidden email]>
Signed-off-by: Valentin Duperray <[hidden email]>
Signed-off-by: Franck Jonas <[hidden email]>
Signed-off-by: Lucien Kong <[hidden email]>
Signed-off-by: Thomas Nguy <[hidden email]>
Signed-off-by: Huynh Khoi Nguyen Nguyen <[hidden email]>
Signed-off-by: Matthieu Moy <[hidden email]>
---
 Documentation/git-bisect.txt |   48 ++++++++++++++++++++++++++++++++++++++++-
 bisect.c                     |   11 +++++++--
 git-bisect.sh                |   30 +++++++++++++++++--------
 t/t6030-bisect-porcelain.sh  |   38 +++++++++++++++++++++++++++++++++
 4 files changed, 112 insertions(+), 15 deletions(-)

diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
index 4cb52a7..3c3021a 100644
--- a/Documentation/git-bisect.txt
+++ b/Documentation/git-bisect.txt
@@ -18,8 +18,8 @@ on the subcommand:
 
  git bisect help
  git bisect start [--no-checkout] [<bad> [<good>...]] [--] [<paths>...]
- git bisect bad [<rev>]
- git bisect good [<rev>...]
+ git bisect (bad|new) [<rev>]
+ git bisect (good|old) [<rev>...]
  git bisect skip [(<rev>|<range>)...]
  git bisect reset [<commit>]
  git bisect visualize
@@ -104,6 +104,35 @@ For example, `git bisect reset HEAD` will leave you on the current
 bisection commit and avoid switching commits at all, while `git bisect
 reset bisect/bad` will check out the first bad revision.
 
+
+Alternative terms: bisect new and bisect old
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+If you are not at ease with the terms "bad" and "good", perhaps
+because you are looking for the commit that introduced a fix, you can
+alternatively use "new" and "old" instead.
+But note that you cannot mix "bad" and good" with "new" and "old".
+
+------------------------------------------------
+git bisect new [<rev>]
+------------------------------------------------
+
+Marks the commit as new, e.g. "the bug is no longer there", if you are looking
+for a commit that fixed a bug, or "the feature that used to work is now broken
+at this point", if you are looking for a commit that introduced a bug.
+It is the equivalent of "git bisect bad [<rev>]".
+
+------------------------------------------------
+git bisect old [<rev>...]
+------------------------------------------------
+
+Marks the commit as old, as the opposite of 'git bisect new'.
+It is the equivalent of "git bisect good [<rev>...]".
+
+You must run `git bisect start` without commits as argument and run
+`git bisect new <rev>`/`git bisect old <rev>...` after to add the
+commits.
+
 Bisect visualize
 ~~~~~~~~~~~~~~~~
 
@@ -379,6 +408,21 @@ In this case, when 'git bisect run' finishes, bisect/bad will refer to a commit
 has at least one parent whose reachable graph is fully traversable in the sense
 required by 'git pack objects'.
 
+* Look for a fix instead of a regression in the code
++
+------------
+$ git bisect start
+$ git bisect new HEAD    # current commit is marked as new
+$ git bisect old HEAD~10 # the tenth commit from now is marked as old
+------------
++
+Let's consider the last commit has a given property, and that we are looking
+for the commit which introduced this property. For each commit the bisection
+guide us to, we will test if the property is present. If it is we will mark
+the commit as new with 'git bisect new', otherwise we will mark it as old.
+At the end of the bisect session, the result will be the first new commit (e.g
+the first one with the property).
+
 
 SEE ALSO
 --------
diff --git a/bisect.c b/bisect.c
index 2fc8a78..7492fdc 100644
--- a/bisect.c
+++ b/bisect.c
@@ -746,6 +746,11 @@ static 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 55b9ebd..a11ca06 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..2f2143b 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
--
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe git" in
Reply | Threaded
Open this post in threaded view
|

[PATCH v3 5/6] revision: fix rev-list --bisect in old/new mode

Antoine Delaite
In reply to this post by Antoine Delaite
From: Louis Stuber <[hidden email]>

Calling git rev-list --bisect when an old/new mode bisection was started
shows the help notice. This has been fixed by reading BISECT_TERMS in
revision.c to find the correct bisect refs path (which was always
refs/bisect/bad (or good) before and can be refs/bisect/new (old) now).

Signed-off-by: Louis Stuber <[hidden email]>
Signed-off-by: Antoine Delaite <[hidden email]>
---
 revision.c |   16 ++++++++++++++--
 1 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/revision.c b/revision.c
index 3ff8723..27750ac 100644
--- a/revision.c
+++ b/revision.c
@@ -21,6 +21,9 @@
 
 volatile show_early_output_fn_t show_early_output;
 
+static const char *name_bad;
+static const char *name_good;
+
 char *path_name(const struct name_path *path, const char *name)
 {
  const struct name_path *p;
@@ -2076,14 +2079,22 @@ void parse_revision_opt(struct rev_info *revs, struct parse_opt_ctx_t *ctx,
  ctx->argc -= n;
 }
 
+extern void read_bisect_terms(const char **bad, const char **good);
+
 static int for_each_bad_bisect_ref(const char *submodule, each_ref_fn fn, void *cb_data)
 {
- return for_each_ref_in_submodule(submodule, "refs/bisect/bad", fn, cb_data);
+ char bisect_refs_path[256];
+ strcpy(bisect_refs_path, "refs/bisect/");
+ strcat(bisect_refs_path, name_bad);
+ return for_each_ref_in_submodule(submodule, bisect_refs_path, fn, cb_data);
 }
 
 static int for_each_good_bisect_ref(const char *submodule, each_ref_fn fn, void *cb_data)
 {
- return for_each_ref_in_submodule(submodule, "refs/bisect/good", fn, cb_data);
+ char bisect_refs_path[256];
+ strcpy(bisect_refs_path, "refs/bisect/");
+ strcat(bisect_refs_path, name_good);
+ return for_each_ref_in_submodule(submodule, bisect_refs_path, fn, cb_data);
 }
 
 static int handle_revision_pseudo_opt(const char *submodule,
@@ -2112,6 +2123,7 @@ static int handle_revision_pseudo_opt(const char *submodule,
  handle_refs(submodule, revs, *flags, for_each_branch_ref_submodule);
  clear_ref_exclusion(&revs->ref_excludes);
  } else if (!strcmp(arg, "--bisect")) {
+ read_bisect_terms(&name_bad, &name_good);
  handle_refs(submodule, revs, *flags, for_each_bad_bisect_ref);
  handle_refs(submodule, revs, *flags ^ (UNINTERESTING | BOTTOM), for_each_good_bisect_ref);
  revs->bisect = 1;
--
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe git" in
Reply | Threaded
Open this post in threaded view
|

[PATCH v3 6/6] bisect: allows any terms set by user

Antoine Delaite
In reply to this post by Antoine Delaite
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 <[hidden email]>
Signed-off-by: Louis Stuber <[hidden email]>
---
 Documentation/git-bisect.txt |   19 ++++++++++++
 git-bisect.sh                |   68 ++++++++++++++++++++++++++++++++++++++---
 t/t6030-bisect-porcelain.sh  |   43 ++++++++++++++++++++++++++
 3 files changed, 125 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
index 3c3021a..ef0c03c 100644
--- a/Documentation/git-bisect.txt
+++ b/Documentation/git-bisect.txt
@@ -133,6 +133,25 @@ You must run `git bisect start` without commits as argument and run
 `git bisect new <rev>`/`git bisect old <rev>...` after to add the
 commits.
 
+Alternative terms: use your own terms
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+If the builtins terms bad/good and new/old do not satisfy you, you can
+set your own terms.
+
+------------------------------------------------
+git bisect terms term1 term2
+------------------------------------------------
+
+This command has to be used before a bisection has started.
+The term1 must be associated with the latest revisions and term2 with the
+ancestors of term1.
+
+Only the first bisection following the 'git bisect terms' will use the terms.
+If you mistyped one of the terms you can do again 'git bisect terms term1
+term2'.
+
+
 Bisect visualize
 ~~~~~~~~~~~~~~~~
 
diff --git a/git-bisect.sh b/git-bisect.sh
index a11ca06..7da22b1 100644
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -1,6 +1,6 @@
 #!/bin/sh
 
-USAGE='[help|start|bad|good|new|old|skip|next|reset|visualize|replay|log|run]'
+USAGE='[help|start|bad|good|new|old|terms|skip|next|reset|visualize|replay|log|run]'
 LONG_USAGE='git bisect help
  print this long help message.
 git bisect start [--no-checkout] [<bad> [<good>...]] [--] [<pathspec>...]
@@ -11,6 +11,8 @@ git bisect (bad|new) [<rev>]
 git bisect (good|old) [<rev>...]
  mark <rev>... known-good revisions/
  revisions before change in a given property.
+git bisect terms term1 term2
+ set up term1 and term2 as bisection terms.
 git bisect skip [(<rev>|<range>)...]
  mark <rev>... untestable revisions.
 git bisect next
@@ -82,6 +84,14 @@ bisect_start() {
  # revision_seen is true if a git bisect start
  # has revision as arguments
  revision_seen=0
+ # terms_defined is used to detect if the user
+ # defined his own terms with git bisect terms
+ terms_defined=0
+ if test -s "$GIT_DIR/TERMS_DEFINED"
+ then
+ terms_defined=1
+ get_terms
+ fi
  if test "z$(git rev-parse --is-bare-repository)" != zfalse
  then
  mode=--no-checkout
@@ -180,10 +190,14 @@ bisect_start() {
  } &&
  git rev-parse --sq-quote "$@" >"$GIT_DIR/BISECT_NAMES" &&
  eval "$eval true" &&
- if test $revision_seen -eq 1 && test ! -s "$GIT_DIR/BISECT_TERMS"
+ if test $revision_seen -eq 1 && test ! -s "$GIT_DIR/BISECT_TERMS" || test $terms_defined -eq 1
  then
  echo "$NAME_BAD" >"$GIT_DIR/BISECT_TERMS" &&
- echo "$NAME_GOOD" >>"$GIT_DIR/BISECT_TERMS"
+ echo "$NAME_GOOD" >>"$GIT_DIR/BISECT_TERMS" &&
+ if test $terms_defined -eq 1
+ then
+ echo "git bisect terms $NAME_BAD $NAME_GOOD" >>"$GIT_DIR/BISECT_LOG" || exit
+ fi
  fi &&
  echo "git bisect start$orig_args" >>"$GIT_DIR/BISECT_LOG" || exit
  #
@@ -419,6 +433,7 @@ bisect_clean_state() {
  rm -f "$GIT_DIR/BISECT_NAMES" &&
  rm -f "$GIT_DIR/BISECT_RUN" &&
  rm -f "$GIT_DIR/BISECT_TERMS" &&
+ rm -f "$GIT_DIR/TERMS_DEFINED" &&
  # Cleanup head-name if it got left by an old version of git-bisect
  rm -f "$GIT_DIR/head-name" &&
  git update-ref -d --no-deref BISECT_HEAD &&
@@ -447,6 +462,8 @@ bisect_replay () {
  eval "$cmd" ;;
  "$NAME_GOOD"|"$NAME_BAD"|skip)
  bisect_write "$command" "$rev" ;;
+ terms)
+ bisect_terms $rev ;;
  *)
  die "$(gettext "?? what are you talking about?")" ;;
  esac
@@ -531,7 +548,8 @@ get_terms () {
 check_and_set_terms () {
  cmd="$1"
  case "$cmd" in
- bad|good|new|old)
+ skip|start|terms) ;;
+ *)
  if test -s "$GIT_DIR/BISECT_TERMS" && test "$cmd" != "$NAME_BAD" && test "$cmd" != "$NAME_GOOD"
  then
  die "$(eval_gettext "Invalid command: you're currently in a \$NAME_BAD/\$NAME_GOOD bisect.")"
@@ -564,6 +582,44 @@ bisect_voc () {
  esac
 }
 
+bisect_terms () {
+ case "$#" in
+ 0)
+ if test -s "$GIT_DIR/BISECT_TERMS"
+ then
+ {
+ read term1
+ read term2
+ }<"$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 2f2143b..d91116e 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
--
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe git" in
Reply | Threaded
Open this post in threaded view
|

[PATCH v7 0/5] git bisect old/new

Matthieu Moy
In reply to this post by Antoine Delaite
Hi,

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

Not visible in the patch below: I squashed PATCH 5 into PATCH 3 to
avoid the pattern "break stuff and then repair it".

The first two patches seem ready.

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.

diff --git a/bisect.c b/bisect.c
index 7492fdc..ab09650 100644
--- a/bisect.c
+++ b/bisect.c
@@ -921,7 +921,7 @@ void read_bisect_terms(const char **read_bad, const char **read_good)
  FILE *fp = fopen(filename, "r");
 
  if (!fp) {
- if (errno==2) {
+ if (errno == ENOENT) {
  *read_bad = "bad";
  *read_good = "good";
  return;
diff --git a/git-bisect.sh b/git-bisect.sh
index 7da22b1..8ef2b94 100644
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -541,7 +541,7 @@ get_terms () {
  {
  read NAME_BAD
  read NAME_GOOD
- }<"$GIT_DIR/BISECT_TERMS"
+ } <"$GIT_DIR/BISECT_TERMS"
  fi
 }
 
@@ -605,8 +605,8 @@ 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.
+ 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 ;;
  *)
diff --git a/revision.c b/revision.c
index 27750ac..f22923f 100644
--- a/revision.c
+++ b/revision.c
@@ -2083,18 +2083,28 @@ 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)
 {
- char bisect_refs_path[256];
- strcpy(bisect_refs_path, "refs/bisect/");
- strcat(bisect_refs_path, name_bad);
- return for_each_ref_in_submodule(submodule, bisect_refs_path, fn, cb_data);
+ 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)
 {
- char bisect_refs_path[256];
- strcpy(bisect_refs_path, "refs/bisect/");
- strcat(bisect_refs_path, name_good);
- return for_each_ref_in_submodule(submodule, bisect_refs_path, fn, cb_data);
+ 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,
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index d91116e..289dbb0 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -781,12 +781,12 @@ test_expect_success 'bisect start with one new and old' '
  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 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 &&
+ git bisect replay log_to_replay.txt >bisect_result &&
  grep "$HASH2 is the first new commit" bisect_result &&
  git bisect reset
 '
@@ -806,12 +806,12 @@ test_expect_success 'bisect start with one term1 and term2' '
  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 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 &&
+ git bisect replay log_to_replay.txt >bisect_result &&
  grep "$HASH2 is the first term1 commit" bisect_result &&
  git bisect reset
 '
@@ -823,7 +823,7 @@ test_expect_success 'bisect start term1 term2' '
  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 log >log_to_replay.txt &&
  git bisect reset
 '
 

Subject: [PATCH v7 0/5] git bisect old/new

Hi,

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

diff --git a/bisect.c b/bisect.c
index 7492fdc..ab09650 100644
--- a/bisect.c
+++ b/bisect.c
@@ -921,7 +921,7 @@ void read_bisect_terms(const char **read_bad, const char **read_good)
  FILE *fp = fopen(filename, "r");
 
  if (!fp) {
- if (errno==2) {
+ if (errno == ENOENT) {
  *read_bad = "bad";
  *read_good = "good";
  return;
diff --git a/git-bisect.sh b/git-bisect.sh
index 7da22b1..8ef2b94 100644
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -541,7 +541,7 @@ get_terms () {
  {
  read NAME_BAD
  read NAME_GOOD
- }<"$GIT_DIR/BISECT_TERMS"
+ } <"$GIT_DIR/BISECT_TERMS"
  fi
 }
 
@@ -605,8 +605,8 @@ 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.
+ 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 ;;
  *)
diff --git a/revision.c b/revision.c
index 27750ac..f22923f 100644
--- a/revision.c
+++ b/revision.c
@@ -2083,18 +2083,28 @@ 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)
 {
- char bisect_refs_path[256];
- strcpy(bisect_refs_path, "refs/bisect/");
- strcat(bisect_refs_path, name_bad);
- return for_each_ref_in_submodule(submodule, bisect_refs_path, fn, cb_data);
+ 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)
 {
- char bisect_refs_path[256];
- strcpy(bisect_refs_path, "refs/bisect/");
- strcat(bisect_refs_path, name_good);
- return for_each_ref_in_submodule(submodule, bisect_refs_path, fn, cb_data);
+ 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,
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index d91116e..289dbb0 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -781,12 +781,12 @@ test_expect_success 'bisect start with one new and old' '
  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 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 &&
+ git bisect replay log_to_replay.txt >bisect_result &&
  grep "$HASH2 is the first new commit" bisect_result &&
  git bisect reset
 '
@@ -806,12 +806,12 @@ test_expect_success 'bisect start with one term1 and term2' '
  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 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 &&
+ git bisect replay log_to_replay.txt >bisect_result &&
  grep "$HASH2 is the first term1 commit" bisect_result &&
  git bisect reset
 '
@@ -823,7 +823,7 @@ test_expect_success 'bisect start term1 term2' '
  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 log >log_to_replay.txt &&
  git bisect reset
 '
 
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: allows any terms set by user

 Documentation/git-bisect.txt |  67 +++++++++++++-
 bisect.c                     |  94 +++++++++++++++-----
 git-bisect.sh                | 207 +++++++++++++++++++++++++++++++++++--------
 revision.c                   |  26 +++++-
 t/t6030-bisect-porcelain.sh  |  83 ++++++++++++++++-
 5 files changed, 413 insertions(+), 64 deletions(-)
 mode change 100755 => 100644 git-bisect.sh

--
2.4.4.414.ge37915c

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

[PATCH v7 1/5] bisect: correction of typo

Matthieu Moy
From: Antoine Delaite <[hidden email]>

Signed-off-by: Antoine Delaite <[hidden email]>
Signed-off-by: Matthieu Moy <[hidden email]>
---
 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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

[PATCH v7 2/5] bisect: replace hardcoded "bad|good" by variables

Matthieu Moy
In reply to this post by Matthieu Moy
From: Antoine Delaite <[hidden email]>

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 <[hidden email]>
Signed-off-by: Louis Stuber <[hidden email]>
Signed-off-by: Valentin Duperray <[hidden email]>
Signed-off-by: Franck Jonas <[hidden email]>
Signed-off-by: Lucien Kong <[hidden email]>
Signed-off-by: Thomas Nguy <[hidden email]>
Signed-off-by: Huynh Khoi Nguyen Nguyen <[hidden email]>
Signed-off-by: Matthieu Moy <[hidden email]>
Signed-off-by: Matthieu Moy <[hidden email]>
---
 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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

[PATCH v7 3/5] bisect: simplify the addition of new bisect terms

Matthieu Moy
In reply to this post by Matthieu Moy
From: Antoine Delaite <[hidden email]>

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 <[hidden email]>
Tweaked-by: Matthieu Moy <[hidden email]>
Signed-off-by: Antoine Delaite <[hidden email]>
Signed-off-by: Louis Stuber <[hidden email]>
Signed-off-by: Valentin Duperray <[hidden email]>
Signed-off-by: Franck Jonas <[hidden email]>
Signed-off-by: Lucien Kong <[hidden email]>
Signed-off-by: Thomas Nguy <[hidden email]>
Signed-off-by: Huynh Khoi Nguyen Nguyen <[hidden email]>
Signed-off-by: Matthieu Moy <[hidden email]>
Signed-off-by: Matthieu Moy <[hidden email]>
---
 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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

[PATCH v7 4/5] bisect: add the terms old/new

Matthieu Moy
In reply to this post by Matthieu Moy
From: Antoine Delaite <[hidden email]>

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 <[hidden email]>
Signed-off-by: Louis Stuber <[hidden email]>
Signed-off-by: Valentin Duperray <[hidden email]>
Signed-off-by: Franck Jonas <[hidden email]>
Signed-off-by: Lucien Kong <[hidden email]>
Signed-off-by: Thomas Nguy <[hidden email]>
Signed-off-by: Huynh Khoi Nguyen Nguyen <[hidden email]>
Signed-off-by: Matthieu Moy <[hidden email]>
Signed-off-by: Matthieu Moy <[hidden email]>
---
 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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

[PATCH v7 5/5] bisect: allows any terms set by user

Matthieu Moy
In reply to this post by Matthieu Moy
From: Antoine Delaite <[hidden email]>

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 <[hidden email]>
Signed-off-by: Louis Stuber <[hidden email]>
Signed-off-by: Matthieu Moy <[hidden email]>
---
 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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v7 3/5] bisect: simplify the addition of new bisect terms

Eric Sunshine
In reply to this post by Matthieu Moy
On Tue, Jun 23, 2015 at 8:54 AM, Matthieu Moy <[hidden email]> 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;
>  }
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v7 3/5] bisect: simplify the addition of new bisect terms

Matthieu Moy-2
Eric Sunshine <[hidden email]> writes:

> On Tue, Jun 23, 2015 at 8:54 AM, Matthieu Moy <[hidden email]> 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/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v7 5/5] bisect: allows any terms set by user

Junio C Hamano
In reply to this post by Matthieu Moy
Matthieu Moy <[hidden email]> writes:

>Subject: Re: [PATCH v7 5/5] bisect: allows any terms set by user

s/allows/allow/;

> +Alternative terms: use your own terms
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Lengths of underline and the text must match.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v7 0/5] git bisect old/new

Junio C Hamano
In reply to this post by Matthieu Moy
Matthieu Moy <[hidden email]> 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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v7 4/5] bisect: add the terms old/new

Remi Galan Alfonso
In reply to this post by Matthieu Moy
Matthieu Moy <[hidden email]> writes:
> Signed-off-by: Matthieu Moy <[hidden email]>
> Signed-off-by: Matthieu Moy <[hidden email]>

Sounds like you went all out with this patch.

RĂ©mi
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v7 0/5] git bisect old/new

Matthieu Moy-2
In reply to this post by Junio C Hamano
Junio C Hamano <[hidden email]> writes:

> Matthieu Moy <[hidden email]> 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/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v7 4/5] bisect: add the terms old/new

Matthieu Moy-2
In reply to this post by Remi Galan Alfonso
Remi Galan Alfonso <[hidden email]> writes:

> Matthieu Moy <[hidden email]> writes:
>> Signed-off-by: Matthieu Moy <[hidden email]>
>> Signed-off-by: Matthieu Moy <[hidden email]>
>
> 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/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v7 0/5] git bisect old/new

Junio C Hamano
In reply to this post by Matthieu Moy-2
Matthieu Moy <[hidden email]> 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.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
123