{"thread":{"id":"39732","subject":"[PATCH v10 2/7] Documentation/bisect: move getting help section to the end","startedAt":"2015-06-26T16:58:09Z","lastAt":"2015-06-29T16:40:10Z","messageCount":38,"participants":["Matthieu Moy","Junio C Hamano","Christian Couder","Michael Haggerty"],"isPatch":true,"patchVersion":10,"patchTotal":7},"messages":[{"id":"264964","messageId":"1435337896-20709-1-git-send-email-Matthieu.Moy@imag.fr","threadId":"39732","inReplyTo":null,"subject":"[PATCH v10 0/7] bisect terms","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2015-06-26T16:58:09Z","receivedAt":"2015-06-26T16:58:09Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"This version include Michael Haggerty's improvement to the doc. The\ngeneral ones are at the beginning of the series, and the\nterms-specific ones are squashed into the appropriate commits.\n\nI added more tests to \"git bisect terms\", and there are a few code\nimprovement.\n\nAntoine Delaite (5):\n bisect: correction of typo\n bisect: replace hardcoded \"bad|good\" by variables\n bisect: simplify the addition of new bisect terms\n bisect: add the terms old/new\n bisect: allow any terms set by user\n\nMatthieu Moy (1):\n Documentation/bisect: move getting help section to the end\n\nMichael Haggerty (1):\n Documentation/bisect: revise overall content\n\n Documentation/git-bisect.txt | 225 +++++++++++++++++++++++++++++++------------\n bisect.c | 94 ++++++++++++++----\n git-bisect.sh | 210 ++++++++++++++++++++++++++++++++--------\n revision.c | 20 +++-\n t/t6030-bisect-porcelain.sh | 119 ++++++++++++++++++++++-\n 5 files changed, 544 insertions(+), 124 deletions(-)\n\n-- \n2.4.4.414.g318df7a.dirty\n"},{"id":"264967","messageId":"1435337896-20709-2-git-send-email-Matthieu.Moy@imag.fr","threadId":"39732","inReplyTo":"1435337896-20709-1-git-send-email-Matthieu.Moy@imag.fr","subject":"[PATCH v10 1/7] bisect: correction of typo","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2015-06-26T16:58:10Z","receivedAt":"2015-06-26T16:58:10Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"From: Antoine Delaite \n\nSigned-off-by: Antoine Delaite \nSigned-off-by: Matthieu Moy \n---\n bisect.c | 2 +-\n t/t6030-bisect-porcelain.sh | 2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex 03d5cd9..5b8357d 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -743,7 +743,7 @@ static void handle_bad_merge_base(void)\n \n \tfprintf(stderr, \"Some good revs are not ancestor of the bad rev.\\n\"\n \t\t\"git bisect cannot work properly in this case.\\n\"\n-\t\t\"Maybe you mistake good and bad revs?\\n\");\n+\t\t\"Maybe you mistook good and bad revs?\\n\");\n \texit(1);\n }\n \ndiff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh\nindex 06b4868..9e2c203 100755\n--- a/t/t6030-bisect-porcelain.sh\n+++ b/t/t6030-bisect-porcelain.sh\n@@ -362,7 +362,7 @@ test_expect_success 'bisect starting with a detached HEAD' '\n test_expect_success 'bisect errors out if bad and good are mistaken' '\n \tgit bisect reset &&\n \ttest_must_fail git bisect start $HASH2 $HASH4 2> rev_list_error &&\n-\tgrep \"mistake good and bad\" rev_list_error &&\n+\tgrep \"mistook good and bad\" rev_list_error &&\n \tgit bisect reset\n '\n \n-- \n2.4.4.414.g318df7a.dirty\n"},{"id":"264963","messageId":"1435337896-20709-3-git-send-email-Matthieu.Moy@imag.fr","threadId":"39732","inReplyTo":"1435337896-20709-1-git-send-email-Matthieu.Moy@imag.fr","subject":"[PATCH v10 2/7] Documentation/bisect: move getting help section to the end","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2015-06-26T16:58:11Z","receivedAt":"2015-06-26T16:58:11Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Signed-off-by: Matthieu Moy \n---\n Documentation/git-bisect.txt | 11 +++++------\n 1 file changed, 5 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt\nindex 4cb52a7..2bdc3b8 100644\n--- a/Documentation/git-bisect.txt\n+++ b/Documentation/git-bisect.txt\n@@ -31,12 +31,6 @@ This command uses 'git rev-list --bisect' to help drive the\n binary search process to find which change introduced a bug, given an\n old \"good\" commit object name and a later \"bad\" commit object name.\n \n-Getting help\n-~~~~~~~~~~~~\n-\n-Use \"git bisect\" to get a short usage description, and \"git bisect\n-help\" or \"git bisect -h\" to get a long usage description.\n-\n Basic bisect commands: start, bad, good\n ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n \n@@ -379,6 +373,11 @@ In this case, when 'git bisect run' finishes, bisect/bad will refer to a commit\n has at least one parent whose reachable graph is fully traversable in the sense\n required by 'git pack objects'.\n \n+Getting help\n+~~~~~~~~~~~~\n+\n+Use `git bisect` to get a short usage description, and `git bisect\n+help` or `git bisect -h` to get a long usage description.\n \n SEE ALSO\n --------\n-- \n2.4.4.414.g318df7a.dirty\n"},{"id":"264969","messageId":"1435337896-20709-4-git-send-email-Matthieu.Moy@imag.fr","threadId":"39732","inReplyTo":"1435337896-20709-1-git-send-email-Matthieu.Moy@imag.fr","subject":"[PATCH v10 3/7] Documentation/bisect: revise overall content","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2015-06-26T16:58:12Z","receivedAt":"2015-06-26T16:58:12Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"From: Michael Haggerty \n\nThoroughly revise the \"git bisect\" manpage, including:\n\n* Beef up the \"Description\" section.\n\n* Make the first long example less specific to kernel development.\n\n* De-emphasize implementation details in a couple of places.\n\n* Add \"(roughly N steps)\" in the places where example output is shown.\n\n* Properly markup code within the prose.\n\n* Lots of wordsmithing.\n\nSigned-off-by: Michael Haggerty \nSigned-off-by: Matthieu Moy \n---\n Documentation/git-bisect.txt | 122 ++++++++++++++++++++++++-------------------\n 1 file changed, 68 insertions(+), 54 deletions(-)\n\ndiff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt\nindex 2bdc3b8..8e8a073 100644\n--- a/Documentation/git-bisect.txt\n+++ b/Documentation/git-bisect.txt\n@@ -3,7 +3,7 @@ git-bisect(1)\n \n NAME\n ----\n-git-bisect - Find by binary search the change that introduced a bug\n+git-bisect - Use binary search to find the commit that introduced a bug\n \n \n SYNOPSIS\n@@ -16,7 +16,6 @@ DESCRIPTION\n The command takes various subcommands, and different options depending\n on the subcommand:\n \n- git bisect help\n git bisect start [--no-checkout] [ [...]] [--] [...]\n git bisect bad []\n git bisect good [...]\n@@ -26,58 +25,71 @@ on the subcommand:\n git bisect replay \n git bisect log\n git bisect run ...\n+ git bisect help\n \n-This command uses 'git rev-list --bisect' to help drive the\n-binary search process to find which change introduced a bug, given an\n-old \"good\" commit object name and a later \"bad\" commit object name.\n+This command uses a binary search algorithm to find which commit in\n+your project's history introduced a bug. You use it by first telling\n+it a \"bad\" commit that is known to contain the bug, and a \"good\"\n+commit that is known to be before the bug was introduced. Then `git\n+bisect` picks a commit between those two endpoints and asks you\n+whether the selected commit is \"good\" or \"bad\". It continues narrowing\n+down the range until it finds the exact commit that introduced the\n+change.\n \n Basic bisect commands: start, bad, good\n ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n \n-Using the Linux kernel tree as an example, basic use of the bisect\n-command is as follows:\n+As an example, suppose you are trying to find the commit that broke a\n+feature that was known to work in version `v2.6.13-rc2` of your\n+project. You start a bisect session as follows:\n \n ------------------------------------------------\n $ git bisect start\n $ git bisect bad # Current version is bad\n-$ git bisect good v2.6.13-rc2 # v2.6.13-rc2 was the last version\n- # tested that was good\n+$ git bisect good v2.6.13-rc2 # v2.6.13-rc2 is known to be good\n+------------------------------------------------\n+\n+Once you have specified at least one bad and one good commit, `git\n+bisect` selects a commit in the middle of that range of history,\n+checks it out, and outputs something similar to the following:\n+\n+------------------------------------------------\n+Bisecting: 675 revisions left to test after this (roughly 10 steps)\n ------------------------------------------------\n \n-When you have specified at least one bad and one good version, the\n-command bisects the revision tree and outputs something similar to\n-the following:\n+You should now compile the checked-out version and test it. If that\n+version works correctly, type\n \n ------------------------------------------------\n-Bisecting: 675 revisions left to test after this\n+$ git bisect good\n ------------------------------------------------\n \n-The state in the middle of the set of revisions is then checked out.\n-You would now compile that kernel and boot it. If the booted kernel\n-works correctly, you would then issue the following command:\n+If that version is broken, type\n \n ------------------------------------------------\n-$ git bisect good\t\t\t# this one is good\n+$ git bisect bad\n ------------------------------------------------\n \n-The output of this command would be something similar to the following:\n+Then `git bisect` will respond with something like\n \n ------------------------------------------------\n-Bisecting: 337 revisions left to test after this\n+Bisecting: 337 revisions left to test after this (roughly 9 steps)\n ------------------------------------------------\n \n-You keep repeating this process, compiling the tree, testing it, and\n-depending on whether it is good or bad issuing the command \"git bisect good\"\n-or \"git bisect bad\" to ask for the next bisection.\n+Keep repeating the process: compile the tree, test it, and depending\n+on whether it is good or bad run `git bisect good` or `git bisect bad`\n+to ask for the next commit that needs testing.\n+\n+Eventually there will be no more revisions left to bisect, and the\n+command will print out a description of the first bad commit. The\n+reference `refs/bisect/bad` will be left pointing at that commit.\n \n-Eventually there will be no more revisions left to bisect, and you\n-will have been left with the first bad kernel revision in \"refs/bisect/bad\".\n \n Bisect reset\n ~~~~~~~~~~~~\n \n After a bisect session, to clean up the bisection state and return to\n-the original HEAD (i.e., to quit bisecting), issue the following command:\n+the original HEAD, issue the following command:\n \n ------------------------------------------------\n $ git bisect reset\n@@ -94,9 +106,10 @@ instead:\n $ git bisect reset \n ------------------------------------------------\n \n-For example, `git bisect reset HEAD` will leave you on the current\n-bisection commit and avoid switching commits at all, while `git bisect\n-reset bisect/bad` will check out the first bad revision.\n+For example, `git bisect reset bisect/bad` will check out the first\n+bad revision, while `git bisect reset HEAD` will leave you on the\n+current bisection commit and avoid switching commits at all.\n+\n \n Bisect visualize\n ~~~~~~~~~~~~~~~~\n@@ -141,17 +154,17 @@ $ git bisect replay that-file\n Avoiding testing a commit\n ~~~~~~~~~~~~~~~~~~~~~~~~~\n \n-If, in the middle of a bisect session, you know that the next suggested\n-revision is not a good one to test (e.g. the change the commit\n-introduces is known not to work in your environment and you know it\n-does not have anything to do with the bug you are chasing), you may\n-want to find a nearby commit and try that instead.\n+If, in the middle of a bisect session, you know that the suggested\n+revision is not a good one to test (e.g. it fails to build and you\n+know that the failure does not have anything to do with the bug you\n+are chasing), you can manually select a nearby commit and test that\n+one instead.\n \n For example:\n \n ------------\n $ git bisect good/bad\t\t\t# previous round was good or bad.\n-Bisecting: 337 revisions left to test after this\n+Bisecting: 337 revisions left to test after this (roughly 9 steps)\n $ git bisect visualize\t\t\t# oops, that is uninteresting.\n $ git reset --hard HEAD~3\t\t# try 3 revisions before what\n \t\t\t\t\t# was suggested\n@@ -163,18 +176,19 @@ the revision as good or bad in the usual manner.\n Bisect skip\n ~~~~~~~~~~~~\n \n-Instead of choosing by yourself a nearby commit, you can ask Git\n-to do it for you by issuing the command:\n+Instead of choosing a nearby commit by yourself, you can ask Git to do\n+it for you by issuing the command:\n \n ------------\n $ git bisect skip # Current version cannot be tested\n ------------\n \n-But Git may eventually be unable to tell the first bad commit among\n-a bad commit and one or more skipped commits.\n+However, if you skip a commit adjacent to the one you are looking for,\n+Git will be unable to tell exactly which of those commits was the\n+first bad one.\n \n-You can even skip a range of commits, instead of just one commit,\n-using the \"''..''\" notation. For example:\n+You can also skip a range of commits, instead of just one commit,\n+using range notation. For example:\n \n ------------\n $ git bisect skip v2.5..v2.6\n@@ -190,8 +204,8 @@ would issue the command:\n $ git bisect skip v2.5 v2.5..v2.6\n ------------\n \n-This tells the bisect process that the commits between `v2.5` included\n-and `v2.6` included should be skipped.\n+This tells the bisect process that the commits between `v2.5` and\n+`v2.6` (inclusive) should be skipped.\n \n \n Cutting down bisection by giving more parameters to bisect start\n@@ -225,14 +239,14 @@ or bad, you can bisect by issuing the command:\n $ git bisect run my_script arguments\n ------------\n \n-Note that the script (`my_script` in the above example) should\n-exit with code 0 if the current source code is good, and exit with a\n-code between 1 and 127 (inclusive), except 125, if the current\n-source code is bad.\n+Note that the script (`my_script` in the above example) should exit\n+with code 0 if the current source code is good/old, and exit with a\n+code between 1 and 127 (inclusive), except 125, if the current source\n+code is bad/new.\n \n Any other exit code will abort the bisect process. It should be noted\n-that a program that terminates via \"exit(-1)\" leaves $? = 255, (see the\n-exit(3) manual page), as the value is chopped with \"& 0377\".\n+that a program that terminates via `exit(-1)` leaves $? = 255, (see the\n+exit(3) manual page), as the value is chopped with `& 0377`.\n \n The special exit code 125 should be used when the current source code\n cannot be tested. If the script exits with this code, the current\n@@ -241,7 +255,7 @@ as the highest sensible value to use for this purpose, because 126 and 127\n are used by POSIX shells to signal specific error status (127 is for\n command not found, 126 is for command found but not executable---these\n details do not matter, as they are normal errors in the script, as far as\n-\"bisect run\" is concerned).\n+`bisect run` is concerned).\n \n You may often find that during a bisect session you want to have\n temporary modifications (e.g. s/#define DEBUG 0/#define DEBUG 1/ in a\n@@ -254,7 +268,7 @@ next revision to test, the script can apply the patch\n before compiling, run the real test, and afterwards decide if the\n revision (possibly with the needed patch) passed the test and then\n rewind the tree to the pristine state. Finally the script should exit\n-with the status of the real test to let the \"git bisect run\" command loop\n+with the status of the real test to let the `git bisect run` command loop\n determine the eventual outcome of the bisect session.\n \n OPTIONS\n@@ -301,12 +315,12 @@ $ git bisect run ~/test.sh\n $ git bisect reset # quit the bisect session\n ------------\n +\n-Here we use a \"test.sh\" custom script. In this script, if \"make\"\n+Here we use a `test.sh` custom script. In this script, if `make`\n fails, we skip the current commit.\n-\"check_test_case.sh\" should \"exit 0\" if the test case passes,\n-and \"exit 1\" otherwise.\n+`check_test_case.sh` should `exit 0` if the test case passes,\n+and `exit 1` otherwise.\n +\n-It is safer if both \"test.sh\" and \"check_test_case.sh\" are\n+It is safer if both `test.sh` and `check_test_case.sh` are\n outside the repository to prevent interactions between the bisect,\n make and test processes and the scripts.\n \n-- \n2.4.4.414.g318df7a.dirty\n"},{"id":"264966","messageId":"1435337896-20709-5-git-send-email-Matthieu.Moy@imag.fr","threadId":"39732","inReplyTo":"1435337896-20709-1-git-send-email-Matthieu.Moy@imag.fr","subject":"[PATCH v10 4/7] bisect: replace hardcoded \"bad|good\" by variables","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2015-06-26T16:58:13Z","receivedAt":"2015-06-26T16:58:13Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"From: Antoine Delaite \n\nTo add new tags like old/new and have keywords less confusing, the\nfirst step is to avoid hardcoding the keywords.\n\nThe default mode is still bad/good.\n\nSigned-off-by: Antoine Delaite \nSigned-off-by: Louis Stuber \nSigned-off-by: Valentin Duperray \nSigned-off-by: Franck Jonas \nSigned-off-by: Lucien Kong \nSigned-off-by: Thomas Nguy \nSigned-off-by: Huynh Khoi Nguyen Nguyen \nSigned-off-by: Matthieu Moy \n---\n bisect.c | 54 +++++++++++++++++++++++++++++++++++++-----------------\n git-bisect.sh | 57 +++++++++++++++++++++++++++++++--------------------------\n 2 files changed, 68 insertions(+), 43 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex 5b8357d..9368f6c 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -21,6 +21,9 @@ static const char *argv_checkout[] = {\"checkout\", \"-q\", NULL, \"--\", NULL};\n static const char *argv_show_branch[] = {\"show-branch\", NULL, NULL};\n static const char *argv_update_ref[] = {\"update-ref\", \"--no-deref\", \"BISECT_HEAD\", NULL, NULL};\n \n+static const char *name_bad;\n+static const char *name_good;\n+\n /* Remember to update object flag allocation in object.h */\n #define COUNTED\t\t(1u<<16)\n \n@@ -403,15 +406,21 @@ struct commit_list *find_bisection(struct commit_list *list,\n static int register_ref(const char *refname, const struct object_id *oid,\n \t\t\tint flags, void *cb_data)\n {\n-\tif (!strcmp(refname, \"bad\")) {\n+\tstruct strbuf good_prefix = STRBUF_INIT;\n+\tstrbuf_addstr(&good_prefix, name_good);\n+\tstrbuf_addstr(&good_prefix, \"-\");\n+\n+\tif (!strcmp(refname, name_bad)) {\n \t\tcurrent_bad_oid = xmalloc(sizeof(*current_bad_oid));\n \t\toidcpy(current_bad_oid, oid);\n-\t} else if (starts_with(refname, \"good-\")) {\n+\t} else if (starts_with(refname, good_prefix.buf)) {\n \t\tsha1_array_append(&good_revs, oid->hash);\n \t} else if (starts_with(refname, \"skip-\")) {\n \t\tsha1_array_append(&skipped_revs, oid->hash);\n \t}\n \n+\tstrbuf_release(&good_prefix);\n+\n \treturn 0;\n }\n \n@@ -634,7 +643,7 @@ static void exit_if_skipped_commits(struct commit_list *tried,\n \t\treturn;\n \n \tprintf(\"There are only 'skip'ped commits left to test.\\n\"\n-\t \"The first bad commit could be any of:\\n\");\n+\t \"The first %s commit could be any of:\\n\", name_bad);\n \tprint_commit_list(tried, \"%s\\n\", \"%s\\n\");\n \tif (bad)\n \t\tprintf(\"%s\\n\", oid_to_hex(bad));\n@@ -732,18 +741,24 @@ static void handle_bad_merge_base(void)\n \tif (is_expected_rev(current_bad_oid)) {\n \t\tchar *bad_hex = oid_to_hex(current_bad_oid);\n \t\tchar *good_hex = join_sha1_array_hex(&good_revs, ' ');\n-\n-\t\tfprintf(stderr, \"The merge base %s is bad.\\n\"\n-\t\t\t\"This means the bug has been fixed \"\n-\t\t\t\"between %s and [%s].\\n\",\n-\t\t\tbad_hex, bad_hex, good_hex);\n-\n+\t\tif (!strcmp(name_bad, \"bad\") && !strcmp(name_good, \"good\")) {\n+\t\t\tfprintf(stderr, \"The merge base %s is bad.\\n\"\n+\t\t\t\t\"This means the bug has been fixed \"\n+\t\t\t\t\"between %s and [%s].\\n\",\n+\t\t\t\tbad_hex, bad_hex, good_hex);\n+\t\t} else {\n+\t\t\tfprintf(stderr, \"The merge base %s is %s.\\n\"\n+\t\t\t\t\"This means the first '%s' commit is \"\n+\t\t\t\t\"between %s and [%s].\\n\",\n+\t\t\t\tbad_hex, name_bad, name_good, bad_hex, good_hex);\n+\t\t}\n \t\texit(3);\n \t}\n \n-\tfprintf(stderr, \"Some good revs are not ancestor of the bad rev.\\n\"\n+\tfprintf(stderr, \"Some %s revs are not ancestor of the %s rev.\\n\"\n \t\t\"git bisect cannot work properly in this case.\\n\"\n-\t\t\"Maybe you mistook good and bad revs?\\n\");\n+\t\t\"Maybe you mistook %s and %s revs?\\n\",\n+\t\tname_good, name_bad, name_good, name_bad);\n \texit(1);\n }\n \n@@ -755,10 +770,10 @@ static void handle_skipped_merge_base(const unsigned char *mb)\n \n \twarning(\"the merge base between %s and [%s] \"\n \t\t\"must be skipped.\\n\"\n-\t\t\"So we cannot be sure the first bad commit is \"\n+\t\t\"So we cannot be sure the first %s commit is \"\n \t\t\"between %s and %s.\\n\"\n \t\t\"We continue anyway.\",\n-\t\tbad_hex, good_hex, mb_hex, bad_hex);\n+\t\tbad_hex, good_hex, name_bad, mb_hex, bad_hex);\n \tfree(good_hex);\n }\n \n@@ -839,7 +854,7 @@ static void check_good_are_ancestors_of_bad(const char *prefix, int no_checkout)\n \tint fd;\n \n \tif (!current_bad_oid)\n-\t\tdie(\"a bad revision is needed\");\n+\t\tdie(\"a %s revision is needed\", name_bad);\n \n \t/* Check if file BISECT_ANCESTORS_OK exists. */\n \tif (!stat(filename, &st) && S_ISREG(st.st_mode))\n@@ -905,6 +920,8 @@ int bisect_next_all(const char *prefix, int no_checkout)\n \tconst unsigned char *bisect_rev;\n \tchar bisect_rev_hex[GIT_SHA1_HEXSZ + 1];\n \n+\tname_bad = \"bad\";\n+\tname_good = \"good\";\n \tif (read_bisect_refs())\n \t\tdie(\"reading bisect refs failed\");\n \n@@ -926,8 +943,10 @@ int bisect_next_all(const char *prefix, int no_checkout)\n \t\t */\n \t\texit_if_skipped_commits(tried, NULL);\n \n-\t\tprintf(\"%s was both good and bad\\n\",\n-\t\t oid_to_hex(current_bad_oid));\n+\t\tprintf(\"%s was both %s and %s\\n\",\n+\t\t oid_to_hex(current_bad_oid),\n+\t\t name_good,\n+\t\t name_bad);\n \t\texit(1);\n \t}\n \n@@ -942,7 +961,8 @@ int bisect_next_all(const char *prefix, int no_checkout)\n \n \tif (!hashcmp(bisect_rev, current_bad_oid->hash)) {\n \t\texit_if_skipped_commits(tried, current_bad_oid);\n-\t\tprintf(\"%s is the first bad commit\\n\", bisect_rev_hex);\n+\t\tprintf(\"%s is the first %s commit\\n\", bisect_rev_hex,\n+\t\t\tname_bad);\n \t\tshow_diff_tree(prefix, revs.commits->item);\n \t\t/* This means the bisection process succeeded. */\n \t\texit(10);\ndiff --git a/git-bisect.sh b/git-bisect.sh\nindex ae3fec2..ce6412f 100755\n--- a/git-bisect.sh\n+++ b/git-bisect.sh\n@@ -32,6 +32,8 @@ OPTIONS_SPEC=\n \n _x40='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]'\n _x40=\"$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40\"\n+NAME_BAD=bad\n+NAME_GOOD=good\n \n bisect_head()\n {\n@@ -100,8 +102,8 @@ bisect_start() {\n \t\t\t\tbreak\n \t\t\t}\n \t\t\tcase $bad_seen in\n-\t\t\t0) state='bad' ; bad_seen=1 ;;\n-\t\t\t*) state='good' ;;\n+\t\t\t0) state=$NAME_BAD ; bad_seen=1 ;;\n+\t\t\t*) state=$NAME_GOOD ;;\n \t\t\tesac\n \t\t\teval=\"$eval bisect_write '$state' '$rev' 'nolog' &&\"\n \t\t\tshift\n@@ -184,9 +186,12 @@ bisect_write() {\n \trev=\"$2\"\n \tnolog=\"$3\"\n \tcase \"$state\" in\n-\t\tbad)\t\ttag=\"$state\" ;;\n-\t\tgood|skip)\ttag=\"$state\"-\"$rev\" ;;\n-\t\t*)\t\tdie \"$(eval_gettext \"Bad bisect_write argument: \\$state\")\" ;;\n+\t\t\"$NAME_BAD\")\n+\t\t\ttag=\"$state\" ;;\n+\t\t\"$NAME_GOOD\"|skip)\n+\t\t\ttag=\"$state\"-\"$rev\" ;;\n+\t\t*)\n+\t\t\tdie \"$(eval_gettext \"Bad bisect_write argument: \\$state\")\" ;;\n \tesac\n \tgit update-ref \"refs/bisect/$tag\" \"$rev\" || exit\n \techo \"# $state: $(git show-branch $rev)\" >>\"$GIT_DIR/BISECT_LOG\"\n@@ -230,12 +235,12 @@ bisect_state() {\n \tcase \"$#,$state\" in\n \t0,*)\n \t\tdie \"$(gettext \"Please call 'bisect_state' with at least one argument.\")\" ;;\n-\t1,bad|1,good|1,skip)\n+\t1,\"$NAME_BAD\"|1,\"$NAME_GOOD\"|1,skip)\n \t\trev=$(git rev-parse --verify $(bisect_head)) ||\n \t\t\tdie \"$(gettext \"Bad rev input: $(bisect_head)\")\"\n \t\tbisect_write \"$state\" \"$rev\"\n \t\tcheck_expected_revs \"$rev\" ;;\n-\t2,bad|*,good|*,skip)\n+\t2,\"$NAME_BAD\"|*,\"$NAME_GOOD\"|*,skip)\n \t\tshift\n \t\thash_list=''\n \t\tfor rev in \"$@\"\n@@ -249,8 +254,8 @@ bisect_state() {\n \t\t\tbisect_write \"$state\" \"$rev\"\n \t\tdone\n \t\tcheck_expected_revs $hash_list ;;\n-\t*,bad)\n-\t\tdie \"$(gettext \"'git bisect bad' can take only one argument.\")\" ;;\n+\t*,\"$NAME_BAD\")\n+\t\tdie \"$(eval_gettext \"'git bisect \\$NAME_BAD' can take only one argument.\")\" ;;\n \t*)\n \t\tusage ;;\n \tesac\n@@ -259,21 +264,21 @@ bisect_state() {\n \n bisect_next_check() {\n \tmissing_good= missing_bad=\n-\tgit show-ref -q --verify refs/bisect/bad || missing_bad=t\n-\ttest -n \"$(git for-each-ref \"refs/bisect/good-*\")\" || missing_good=t\n+\tgit show-ref -q --verify refs/bisect/$NAME_BAD || missing_bad=t\n+\ttest -n \"$(git for-each-ref \"refs/bisect/$NAME_GOOD-*\")\" || missing_good=t\n \n \tcase \"$missing_good,$missing_bad,$1\" in\n \t,,*)\n-\t\t: have both good and bad - ok\n+\t\t: have both $NAME_GOOD and $NAME_BAD - ok\n \t\t;;\n \t*,)\n \t\t# do not have both but not asked to fail - just report.\n \t\tfalse\n \t\t;;\n-\tt,,good)\n+\tt,,\"$NAME_GOOD\")\n \t\t# have bad but not good. we could bisect although\n \t\t# this is less optimum.\n-\t\tgettextln \"Warning: bisecting only with a bad commit.\" >&2\n+\t\teval_gettextln \"Warning: bisecting only with a \\$NAME_BAD commit.\" >&2\n \t\tif test -t 0\n \t\tthen\n \t\t\t# TRANSLATORS: Make sure to include [Y] and [n] in your\n@@ -283,7 +288,7 @@ bisect_next_check() {\n \t\t\tread yesno\n \t\t\tcase \"$yesno\" in [Nn]*) exit 1 ;; esac\n \t\tfi\n-\t\t: bisect without good...\n+\t\t: bisect without $NAME_GOOD...\n \t\t;;\n \t*)\n \n@@ -307,7 +312,7 @@ bisect_auto_next() {\n bisect_next() {\n \tcase \"$#\" in 0) ;; *) usage ;; esac\n \tbisect_autostart\n-\tbisect_next_check good\n+\tbisect_next_check $NAME_GOOD\n \n \t# Perform all bisection computation, display and checkout\n \tgit bisect--helper --next-all $(test -f \"$GIT_DIR/BISECT_HEAD\" && echo --no-checkout)\n@@ -316,18 +321,18 @@ bisect_next() {\n \t# Check if we should exit because bisection is finished\n \tif test $res -eq 10\n \tthen\n-\t\tbad_rev=$(git show-ref --hash --verify refs/bisect/bad)\n+\t\tbad_rev=$(git show-ref --hash --verify refs/bisect/$NAME_BAD)\n \t\tbad_commit=$(git show-branch $bad_rev)\n-\t\techo \"# first bad commit: $bad_commit\" >>\"$GIT_DIR/BISECT_LOG\"\n+\t\techo \"# first $NAME_BAD commit: $bad_commit\" >>\"$GIT_DIR/BISECT_LOG\"\n \t\texit 0\n \telif test $res -eq 2\n \tthen\n \t\techo \"# only skipped commits left to test\" >>\"$GIT_DIR/BISECT_LOG\"\n-\t\tgood_revs=$(git for-each-ref --format=\"%(objectname)\" \"refs/bisect/good-*\")\n-\t\tfor skipped in $(git rev-list refs/bisect/bad --not $good_revs)\n+\t\tgood_revs=$(git for-each-ref --format=\"%(objectname)\" \"refs/bisect/$NAME_GOOD-*\")\n+\t\tfor skipped in $(git rev-list refs/bisect/$NAME_BAD --not $good_revs)\n \t\tdo\n \t\t\tskipped_commit=$(git show-branch $skipped)\n-\t\t\techo \"# possible first bad commit: $skipped_commit\" >>\"$GIT_DIR/BISECT_LOG\"\n+\t\t\techo \"# possible first $NAME_BAD commit: $skipped_commit\" >>\"$GIT_DIR/BISECT_LOG\"\n \t\tdone\n \t\texit $res\n \tfi\n@@ -421,7 +426,7 @@ bisect_replay () {\n \t\tstart)\n \t\t\tcmd=\"bisect_start $rev\"\n \t\t\teval \"$cmd\" ;;\n-\t\tgood|bad|skip)\n+\t\t$NAME_GOOD|$NAME_BAD|skip)\n \t\t\tbisect_write \"$command\" \"$rev\" ;;\n \t\t*)\n \t\t\tdie \"$(gettext \"?? what are you talking about?\")\" ;;\n@@ -455,9 +460,9 @@ exit code \\$res from '\\$command' is < 0 or >= 128\" >&2\n \t\t\tstate='skip'\n \t\telif [ $res -gt 0 ]\n \t\tthen\n-\t\t\tstate='bad'\n+\t\t\tstate=\"$NAME_BAD\"\n \t\telse\n-\t\t\tstate='good'\n+\t\t\tstate=\"$NAME_GOOD\"\n \t\tfi\n \n \t\t# We have to use a subshell because \"bisect_state\" can exit.\n@@ -466,7 +471,7 @@ exit code \\$res from '\\$command' is < 0 or >= 128\" >&2\n \n \t\tcat \"$GIT_DIR/BISECT_RUN\"\n \n-\t\tif sane_grep \"first bad commit could be any of\" \"$GIT_DIR/BISECT_RUN\" \\\n+\t\tif sane_grep \"first $NAME_BAD commit could be any of\" \"$GIT_DIR/BISECT_RUN\" \\\n \t\t\t>/dev/null\n \t\tthen\n \t\t\tgettextln \"bisect run cannot continue any more\" >&2\n@@ -480,7 +485,7 @@ exit code \\$res from '\\$command' is < 0 or >= 128\" >&2\n \t\t\texit $res\n \t\tfi\n \n-\t\tif sane_grep \"is the first bad commit\" \"$GIT_DIR/BISECT_RUN\" >/dev/null\n+\t\tif sane_grep \"is the first $NAME_BAD commit\" \"$GIT_DIR/BISECT_RUN\" >/dev/null\n \t\tthen\n \t\t\tgettextln \"bisect run success\"\n \t\t\texit 0;\n-- \n2.4.4.414.g318df7a.dirty\n"},{"id":"264970","messageId":"1435337896-20709-6-git-send-email-Matthieu.Moy@imag.fr","threadId":"39732","inReplyTo":"1435337896-20709-1-git-send-email-Matthieu.Moy@imag.fr","subject":"[PATCH v10 5/7] bisect: simplify the addition of new bisect terms","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2015-06-26T16:58:14Z","receivedAt":"2015-06-26T16:58:14Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"From: Antoine Delaite \n\nWe create a file BISECT_TERMS in the repository .git to be read during a\nbisection. The fonctions to be changed if we add new terms are quite\nfew.\nIn git-bisect.sh :\n\tcheck_and_set_terms\n\tbisect_voc\n\nCo-authored-by: Louis Stuber \nTweaked-by: Matthieu Moy \nSigned-off-by: Antoine Delaite \nSigned-off-by: Louis Stuber \nSigned-off-by: Valentin Duperray \nSigned-off-by: Franck Jonas \nSigned-off-by: Lucien Kong \nSigned-off-by: Thomas Nguy \nSigned-off-by: Huynh Khoi Nguyen Nguyen \nSigned-off-by: Matthieu Moy \n---\n bisect.c | 33 ++++++++++++++++++++++++--\n git-bisect.sh | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++++------\n revision.c | 20 ++++++++++++++--\n 3 files changed, 118 insertions(+), 11 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex 9368f6c..c5f96eb 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -905,6 +905,36 @@ static void show_diff_tree(const char *prefix, struct commit *commit)\n }\n \n /*\n+ * The terms used for this bisect session are stored in BISECT_TERMS.\n+ * We read them and store them to adapt the messages accordingly.\n+ * Default is bad/good.\n+ */\n+void read_bisect_terms(const char **read_bad, const char **read_good)\n+{\n+\tstruct strbuf str = STRBUF_INIT;\n+\tconst char *filename = git_path(\"BISECT_TERMS\");\n+\tFILE *fp = fopen(filename, \"r\");\n+\n+\tif (!fp) {\n+\t\tif (errno == ENOENT) {\n+\t\t\t*read_bad = \"bad\";\n+\t\t\t*read_good = \"good\";\n+\t\t\treturn;\n+\t\t} else {\n+\t\t\tdie(\"could not read file '%s': %s\", filename,\n+\t\t\t\tstrerror(errno));\n+\t\t}\n+\t} else {\n+\t\tstrbuf_getline(&str, fp, '\\n');\n+\t\t*read_bad = strbuf_detach(&str, NULL);\n+\t\tstrbuf_getline(&str, fp, '\\n');\n+\t\t*read_good = strbuf_detach(&str, NULL);\n+\t}\n+\tstrbuf_release(&str);\n+\tfclose(fp);\n+}\n+\n+/*\n * We use the convention that exiting with an exit code 10 means that\n * the bisection process finished successfully.\n * In this case the calling shell script should exit 0.\n@@ -920,8 +950,7 @@ int bisect_next_all(const char *prefix, int no_checkout)\n \tconst unsigned char *bisect_rev;\n \tchar bisect_rev_hex[GIT_SHA1_HEXSZ + 1];\n \n-\tname_bad = \"bad\";\n-\tname_good = \"good\";\n+\tread_bisect_terms(&name_bad, &name_good);\n \tif (read_bisect_refs())\n \t\tdie(\"reading bisect refs failed\");\n \ndiff --git a/git-bisect.sh b/git-bisect.sh\nindex ce6412f..52e41e8 100755\n--- a/git-bisect.sh\n+++ b/git-bisect.sh\n@@ -77,6 +77,7 @@ bisect_start() {\n \torig_args=$(git rev-parse --sq-quote \"$@\")\n \tbad_seen=0\n \teval=''\n+\tmust_write_terms=0\n \tif test \"z$(git rev-parse --is-bare-repository)\" != zfalse\n \tthen\n \t\tmode=--no-checkout\n@@ -101,6 +102,14 @@ bisect_start() {\n \t\t\t\tdie \"$(eval_gettext \"'\\$arg' does not appear to be a valid revision\")\"\n \t\t\t\tbreak\n \t\t\t}\n+\n+\t\t\t# The user ran \"git bisect start \n+\t\t\t# \", hence did not explicitly specify\n+\t\t\t# the terms, but we are already starting to\n+\t\t\t# set references named with the default terms,\n+\t\t\t# and won't be able to change afterwards.\n+\t\t\tmust_write_terms=1\n+\n \t\t\tcase $bad_seen in\n \t\t\t0) state=$NAME_BAD ; bad_seen=1 ;;\n \t\t\t*) state=$NAME_GOOD ;;\n@@ -172,6 +181,10 @@ bisect_start() {\n \t} &&\n \tgit rev-parse --sq-quote \"$@\" >\"$GIT_DIR/BISECT_NAMES\" &&\n \teval \"$eval true\" &&\n+\tif test $must_write_terms -eq 1\n+\tthen\n+\t\twrite_terms \"$NAME_BAD\" \"$NAME_GOOD\"\n+\tfi &&\n \techo \"git bisect start$orig_args\" >>\"$GIT_DIR/BISECT_LOG\" || exit\n \t#\n \t# Check if we can proceed to the next bisect state.\n@@ -232,6 +245,7 @@ bisect_skip() {\n bisect_state() {\n \tbisect_autostart\n \tstate=$1\n+\tcheck_and_set_terms $state\n \tcase \"$#,$state\" in\n \t0,*)\n \t\tdie \"$(gettext \"Please call 'bisect_state' with at least one argument.\")\" ;;\n@@ -291,15 +305,17 @@ bisect_next_check() {\n \t\t: bisect without $NAME_GOOD...\n \t\t;;\n \t*)\n-\n+\t\tbad_syn=$(bisect_voc bad)\n+\t\tgood_syn=$(bisect_voc good)\n \t\tif test -s \"$GIT_DIR/BISECT_START\"\n \t\tthen\n-\t\t\tgettextln \"You need to give me at least one good and one bad revision.\n-(You can use \\\"git bisect bad\\\" and \\\"git bisect good\\\" for that.)\" >&2\n+\n+\t\t\teval_gettextln \"You need to give me at least one \\$bad_syn and one \\$good_syn revision.\n+(You can use \\\"git bisect \\$bad_syn\\\" and \\\"git bisect \\$good_syn\\\" for that.)\" >&2\n \t\telse\n-\t\t\tgettextln \"You need to start by \\\"git bisect start\\\".\n-You then need to give me at least one good and one bad revision.\n-(You can use \\\"git bisect bad\\\" and \\\"git bisect good\\\" for that.)\" >&2\n+\t\t\teval_gettextln \"You need to start by \\\"git bisect start\\\".\n+You then need to give me at least one \\$good_syn and one \\$bad_syn revision.\n+(You can use \\\"git bisect \\$bad_syn\\\" and \\\"git bisect \\$good_syn\\\" for that.)\" >&2\n \t\tfi\n \t\texit 1 ;;\n \tesac\n@@ -402,6 +418,7 @@ bisect_clean_state() {\n \trm -f \"$GIT_DIR/BISECT_LOG\" &&\n \trm -f \"$GIT_DIR/BISECT_NAMES\" &&\n \trm -f \"$GIT_DIR/BISECT_RUN\" &&\n+\trm -f \"$GIT_DIR/BISECT_TERMS\" &&\n \t# Cleanup head-name if it got left by an old version of git-bisect\n \trm -f \"$GIT_DIR/head-name\" &&\n \tgit update-ref -d --no-deref BISECT_HEAD &&\n@@ -422,11 +439,13 @@ bisect_replay () {\n \t\t\trev=\"$command\"\n \t\t\tcommand=\"$bisect\"\n \t\tfi\n+\t\tget_terms\n+\t\tcheck_and_set_terms \"$command\"\n \t\tcase \"$command\" in\n \t\tstart)\n \t\t\tcmd=\"bisect_start $rev\"\n \t\t\teval \"$cmd\" ;;\n-\t\t$NAME_GOOD|$NAME_BAD|skip)\n+\t\t\"$NAME_GOOD\"|\"$NAME_BAD\"|skip)\n \t\t\tbisect_write \"$command\" \"$rev\" ;;\n \t\t*)\n \t\t\tdie \"$(gettext \"?? what are you talking about?\")\" ;;\n@@ -499,11 +518,54 @@ bisect_log () {\n \tcat \"$GIT_DIR/BISECT_LOG\"\n }\n \n+get_terms () {\n+\tif test -s \"$GIT_DIR/BISECT_TERMS\"\n+\tthen\n+\t\t{\n+\t\tread NAME_BAD\n+\t\tread NAME_GOOD\n+\t\t} <\"$GIT_DIR/BISECT_TERMS\"\n+\tfi\n+}\n+\n+write_terms () {\n+\tNAME_BAD=$1\n+\tNAME_GOOD=$2\n+\tprintf '%s\\n%s\\n' \"$NAME_BAD\" \"$NAME_GOOD\" >\"$GIT_DIR/BISECT_TERMS\"\n+}\n+\n+check_and_set_terms () {\n+\tcmd=\"$1\"\n+\tcase \"$cmd\" in\n+\tbad|good)\n+\t\tif test -s \"$GIT_DIR/BISECT_TERMS\" && test \"$cmd\" != \"$NAME_BAD\" && test \"$cmd\" != \"$NAME_GOOD\"\n+\t\tthen\n+\t\t\tdie \"$(eval_gettext \"Invalid command: you're currently in a \\$NAME_BAD/\\$NAME_GOOD bisect.\")\"\n+\t\tfi\n+\t\tcase \"$cmd\" in\n+\t\tbad|good)\n+\t\t\tif ! test -s \"$GIT_DIR/BISECT_TERMS\"\n+\t\t\tthen\n+\t\t\t\twrite_terms bad good\n+\t\t\tfi\n+\t\t\t;;\n+\t\tesac ;;\n+\tesac\n+}\n+\n+bisect_voc () {\n+\tcase \"$1\" in\n+\tbad) echo \"bad\" ;;\n+\tgood) echo \"good\" ;;\n+\tesac\n+}\n+\n case \"$#\" in\n 0)\n \tusage ;;\n *)\n \tcmd=\"$1\"\n+\tget_terms\n \tshift\n \tcase \"$cmd\" in\n \thelp)\ndiff --git a/revision.c b/revision.c\nindex 3ff8723..24ce842 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -21,6 +21,9 @@\n \n volatile show_early_output_fn_t show_early_output;\n \n+static const char *name_bad;\n+static const char *name_good;\n+\n char *path_name(const struct name_path *path, const char *name)\n {\n \tconst struct name_path *p;\n@@ -2076,14 +2079,26 @@ void parse_revision_opt(struct rev_info *revs, struct parse_opt_ctx_t *ctx,\n \tctx->argc -= n;\n }\n \n+extern void read_bisect_terms(const char **bad, const char **good);\n+\n static int for_each_bad_bisect_ref(const char *submodule, each_ref_fn fn, void *cb_data)\n {\n-\treturn for_each_ref_in_submodule(submodule, \"refs/bisect/bad\", fn, cb_data);\n+\tstruct strbuf bisect_refs = STRBUF_INIT;\n+\tint status;\n+\tstrbuf_addf(&bisect_refs, \"refs/bisect/%s\", name_bad);\n+\tstatus = for_each_ref_in_submodule(submodule, bisect_refs.buf, fn, cb_data);\n+\tstrbuf_release(&bisect_refs);\n+\treturn status;\n }\n \n static int for_each_good_bisect_ref(const char *submodule, each_ref_fn fn, void *cb_data)\n {\n-\treturn for_each_ref_in_submodule(submodule, \"refs/bisect/good\", fn, cb_data);\n+\tstruct strbuf bisect_refs = STRBUF_INIT;\n+\tint status;\n+\tstrbuf_addf(&bisect_refs, \"refs/bisect/%s\", name_good);\n+\tstatus = for_each_ref_in_submodule(submodule, bisect_refs.buf, fn, cb_data);\n+\tstrbuf_release(&bisect_refs);\n+\treturn status;\n }\n \n static int handle_revision_pseudo_opt(const char *submodule,\n@@ -2112,6 +2127,7 @@ static int handle_revision_pseudo_opt(const char *submodule,\n \t\thandle_refs(submodule, revs, *flags, for_each_branch_ref_submodule);\n \t\tclear_ref_exclusion(&revs->ref_excludes);\n \t} else if (!strcmp(arg, \"--bisect\")) {\n+\t\tread_bisect_terms(&name_bad, &name_good);\n \t\thandle_refs(submodule, revs, *flags, for_each_bad_bisect_ref);\n \t\thandle_refs(submodule, revs, *flags ^ (UNINTERESTING | BOTTOM), for_each_good_bisect_ref);\n \t\trevs->bisect = 1;\n-- \n2.4.4.414.g318df7a.dirty\n"},{"id":"264965","messageId":"1435337896-20709-7-git-send-email-Matthieu.Moy@imag.fr","threadId":"39732","inReplyTo":"1435337896-20709-1-git-send-email-Matthieu.Moy@imag.fr","subject":"[PATCH v10 6/7] bisect: add the terms old/new","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2015-06-26T16:58:15Z","receivedAt":"2015-06-26T16:58:15Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"From: Antoine Delaite \n\nWhen not looking for a regression during a bisect but for a fix or a\nchange in another given property, it can be confusing to use 'good'\nand 'bad'.\n\nThis patch introduce `git bisect new` and `git bisect old` as an\nalternative to 'bad' and good': the commits which have a certain\nproperty must be marked as `new` and the ones which do not as `old`.\n\nThe output will be the first commit after the change in the property.\nDuring a new/old bisect session you cannot use bad/good commands and\nvice-versa.\n\nSome commands are still not available for old/new:\n * git rev-list --bisect does not treat the revs/bisect/new and\n revs/bisect/old-SHA1 files.\n\nOld discussions:\n\t- http://thread.gmane.org/gmane.comp.version-control.git/86063\n\t\tintroduced bisect fix unfixed to find fix.\n\t- http://thread.gmane.org/gmane.comp.version-control.git/182398\n\t\tdiscussion around bisect yes/no or old/new.\n\t- http://thread.gmane.org/gmane.comp.version-control.git/199758\n\t\tlast discussion and reviews\nNew discussions:\n\t- http://thread.gmane.org/gmane.comp.version-control.git/271320\n\t\t( v2 1/7-4/7 )\n\t- http://comments.gmane.org/gmane.comp.version-control.git/271343\n\t\t( v2 5/7-7/7 )\n\nSigned-off-by: Antoine Delaite \nSigned-off-by: Louis Stuber \nSigned-off-by: Valentin Duperray \nSigned-off-by: Franck Jonas \nSigned-off-by: Lucien Kong \nSigned-off-by: Thomas Nguy \nSigned-off-by: Huynh Khoi Nguyen Nguyen \nSigned-off-by: Matthieu Moy \n---\n Documentation/git-bisect.txt | 58 ++++++++++++++++++++++++++++++++++++++++++--\n bisect.c | 11 ++++++---\n git-bisect.sh | 28 +++++++++++++--------\n t/t6030-bisect-porcelain.sh | 38 +++++++++++++++++++++++++++++\n 4 files changed, 120 insertions(+), 15 deletions(-)\n\ndiff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt\nindex 8e8a073..24171a5 100644\n--- a/Documentation/git-bisect.txt\n+++ b/Documentation/git-bisect.txt\n@@ -17,8 +17,8 @@ The command takes various subcommands, and different options depending\n on the subcommand:\n \n git bisect start [--no-checkout] [ [...]] [--] [...]\n- git bisect bad []\n- git bisect good [...]\n+ git bisect (bad|new) []\n+ git bisect (good|old) [...]\n git bisect skip [(|)...]\n git bisect reset []\n git bisect visualize\n@@ -36,6 +36,13 @@ whether the selected commit is \"good\" or \"bad\". It continues narrowing\n down the range until it finds the exact commit that introduced the\n change.\n \n+In fact, `git bisect` can be used to find the commit that changed\n+*any* property of your project; e.g., the commit that fixed a bug, or\n+the commit that caused a benchmark's performance to improve. To\n+support this more general usage, the terms \"old\" and \"new\" can be used\n+in place of \"good\" and \"bad\". See\n+section \"Alternate terms\" below for more information.\n+\n Basic bisect commands: start, bad, good\n ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n \n@@ -111,6 +118,45 @@ bad revision, while `git bisect reset HEAD` will leave you on the\n current bisection commit and avoid switching commits at all.\n \n \n+Alternate terms\n+~~~~~~~~~~~~~~~\n+\n+Sometimes you are not looking for the commit that introduced a\n+breakage, but rather for a commit that caused a change between some\n+other \"old\" state and \"new\" state. For example, you might be looking\n+for the commit that introduced a particular fix. Or you might be\n+looking for the first commit in which the source-code filenames were\n+finally all converted to your company's naming standard. Or whatever.\n+\n+In such cases it can be very confusing to use the terms \"good\" and\n+\"bad\" to refer to \"the state before the change\" and \"the state after\n+the change\". So instead, you can use the terms \"old\" and \"new\",\n+respectively, in place of \"good\" and \"bad\". (But note that you cannot\n+mix \"good\" and \"bad\" with \"old\" and \"new\" in a single session.)\n+\n+In this more general usage, you provide `git bisect` with a \"new\"\n+commit has some property and an \"old\" commit that doesn't have that\n+property. Each time `git bisect` checks out a commit, you test if that\n+commit has the property. If it does, mark the commit as \"new\";\n+otherwise, mark it as \"old\". When the bisection is done, `git bisect`\n+will report which commit introduced the property.\n+\n+To use \"old\" and \"new\" instead of \"good\" and bad, you must run `git\n+bisect start` without commits as argument and then run the following\n+commands to add the commits:\n+\n+------------------------------------------------\n+git bisect old []\n+------------------------------------------------\n+\n+to indicate that a commit was before the sought change, or\n+\n+------------------------------------------------\n+git bisect new [...]\n+------------------------------------------------\n+\n+to indicate that it was after.\n+\n Bisect visualize\n ~~~~~~~~~~~~~~~~\n \n@@ -387,6 +433,14 @@ In this case, when 'git bisect run' finishes, bisect/bad will refer to a commit\n has at least one parent whose reachable graph is fully traversable in the sense\n required by 'git pack objects'.\n \n+* Look for a fix instead of a regression in the code\n++\n+------------\n+$ git bisect start\n+$ git bisect new HEAD # current commit is marked as new\n+$ git bisect old HEAD~10 # the tenth commit from now is marked as old\n+------------\n+\n Getting help\n ~~~~~~~~~~~~\n \ndiff --git a/bisect.c b/bisect.c\nindex c5f96eb..d447b65 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -746,6 +746,11 @@ static void handle_bad_merge_base(void)\n \t\t\t\t\"This means the bug has been fixed \"\n \t\t\t\t\"between %s and [%s].\\n\",\n \t\t\t\tbad_hex, bad_hex, good_hex);\n+\t\t} else if (!strcmp(name_bad, \"new\") && !strcmp(name_good, \"old\")) {\n+\t\t\tfprintf(stderr, \"The merge base %s is new.\\n\"\n+\t\t\t\t\"The property has changed \"\n+\t\t\t\t\"between %s and [%s].\\n\",\n+\t\t\t\tbad_hex, bad_hex, good_hex);\n \t\t} else {\n \t\t\tfprintf(stderr, \"The merge base %s is %s.\\n\"\n \t\t\t\t\"This means the first '%s' commit is \"\n@@ -778,11 +783,11 @@ static void handle_skipped_merge_base(const unsigned char *mb)\n }\n \n /*\n- * \"check_merge_bases\" checks that merge bases are not \"bad\".\n+ * \"check_merge_bases\" checks that merge bases are not \"bad\" (or \"new\").\n *\n- * - If one is \"bad\", it means the user assumed something wrong\n+ * - If one is \"bad\" (or \"new\"), it means the user assumed something wrong\n * and we must exit with a non 0 error code.\n- * - If one is \"good\", that's good, we have nothing to do.\n+ * - If one is \"good\" (or \"old\"), that's good, we have nothing to do.\n * - If one is \"skipped\", we can't know but we should warn.\n * - If we don't know, we should check it out and ask the user to test.\n */\ndiff --git a/git-bisect.sh b/git-bisect.sh\nindex 52e41e8..5769eaf 100755\n--- a/git-bisect.sh\n+++ b/git-bisect.sh\n@@ -1,14 +1,16 @@\n #!/bin/sh\n \n-USAGE='[help|start|bad|good|skip|next|reset|visualize|replay|log|run]'\n+USAGE='[help|start|bad|good|new|old|skip|next|reset|visualize|replay|log|run]'\n LONG_USAGE='git bisect help\n \tprint this long help message.\n git bisect start [--no-checkout] [ [...]] [--] [...]\n \treset bisect state and start bisection.\n-git bisect bad []\n-\tmark a known-bad revision.\n-git bisect good [...]\n-\tmark ... known-good revisions.\n+git bisect (bad|new) []\n+\tmark a known-bad revision/\n+\t\ta revision after change in a given property.\n+git bisect (good|old) [...]\n+\tmark ... known-good revisions/\n+\t\trevisions before change in a given property.\n git bisect skip [(|)...]\n \tmark ... untestable revisions.\n git bisect next\n@@ -290,7 +292,7 @@ bisect_next_check() {\n \t\tfalse\n \t\t;;\n \tt,,\"$NAME_GOOD\")\n-\t\t# have bad but not good. we could bisect although\n+\t\t# have bad (or new) but not good (or old). we could bisect although\n \t\t# this is less optimum.\n \t\teval_gettextln \"Warning: bisecting only with a \\$NAME_BAD commit.\" >&2\n \t\tif test -t 0\n@@ -537,7 +539,7 @@ write_terms () {\n check_and_set_terms () {\n \tcmd=\"$1\"\n \tcase \"$cmd\" in\n-\tbad|good)\n+\tbad|good|new|old)\n \t\tif test -s \"$GIT_DIR/BISECT_TERMS\" && test \"$cmd\" != \"$NAME_BAD\" && test \"$cmd\" != \"$NAME_GOOD\"\n \t\tthen\n \t\t\tdie \"$(eval_gettext \"Invalid command: you're currently in a \\$NAME_BAD/\\$NAME_GOOD bisect.\")\"\n@@ -549,14 +551,20 @@ check_and_set_terms () {\n \t\t\t\twrite_terms bad good\n \t\t\tfi\n \t\t\t;;\n+\t\tnew|old)\n+\t\t\tif ! test -s \"$GIT_DIR/BISECT_TERMS\"\n+\t\t\tthen\n+\t\t\t\twrite_terms new old\n+\t\t\tfi\n+\t\t\t;;\n \t\tesac ;;\n \tesac\n }\n \n bisect_voc () {\n \tcase \"$1\" in\n-\tbad) echo \"bad\" ;;\n-\tgood) echo \"good\" ;;\n+\tbad) echo \"bad|new\" ;;\n+\tgood) echo \"good|old\" ;;\n \tesac\n }\n \n@@ -572,7 +580,7 @@ case \"$#\" in\n \t\tgit bisect -h ;;\n \tstart)\n \t\tbisect_start \"$@\" ;;\n-\tbad|good)\n+\tbad|good|new|old)\n \t\tbisect_state \"$cmd\" \"$@\" ;;\n \tskip)\n \t\tbisect_skip \"$@\" ;;\ndiff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh\nindex 9e2c203..983c503 100755\n--- a/t/t6030-bisect-porcelain.sh\n+++ b/t/t6030-bisect-porcelain.sh\n@@ -759,4 +759,42 @@ test_expect_success '\"git bisect bad HEAD\" behaves as \"git bisect bad\"' '\n \tgit bisect reset\n '\n \n+test_expect_success 'bisect starts with only one new' '\n+\tgit bisect reset &&\n+\tgit bisect start &&\n+\tgit bisect new $HASH4 &&\n+\tgit bisect next\n+'\n+\n+test_expect_success 'bisect does not start with only one old' '\n+\tgit bisect reset &&\n+\tgit bisect start &&\n+\tgit bisect old $HASH1 &&\n+\ttest_must_fail git bisect next\n+'\n+\n+test_expect_success 'bisect start with one new and old' '\n+\tgit bisect reset &&\n+\tgit bisect start &&\n+\tgit bisect old $HASH1 &&\n+\tgit bisect new $HASH4 &&\n+\tgit bisect new &&\n+\tgit bisect new >bisect_result &&\n+\tgrep \"$HASH2 is the first new commit\" bisect_result &&\n+\tgit bisect log >log_to_replay.txt &&\n+\tgit bisect reset\n+'\n+\n+test_expect_success 'bisect replay with old and new' '\n+\tgit bisect replay log_to_replay.txt >bisect_result &&\n+\tgrep \"$HASH2 is the first new commit\" bisect_result &&\n+\tgit bisect reset\n+'\n+\n+test_expect_success 'bisect cannot mix old/new and good/bad' '\n+\tgit bisect start &&\n+\tgit bisect bad $HASH4 &&\n+\ttest_must_fail git bisect old $HASH1\n+'\n+\n test_done\n-- \n2.4.4.414.g318df7a.dirty\n"},{"id":"264968","messageId":"1435337896-20709-8-git-send-email-Matthieu.Moy@imag.fr","threadId":"39732","inReplyTo":"1435337896-20709-1-git-send-email-Matthieu.Moy@imag.fr","subject":"[PATCH v10 7/7] bisect: allow any terms set by user","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2015-06-26T16:58:16Z","receivedAt":"2015-06-26T16:58:16Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"From: Antoine Delaite \n\nIntroduction of the git bisect terms command. The user can set his own\nterms. It will work exactly like before. The terms must be set before the\nstart.\n\nSigned-off-by: Antoine Delaite \nSigned-off-by: Louis Stuber \nSigned-off-by: Matthieu Moy \n---\n Documentation/git-bisect.txt | 36 +++++++++++++++++++-\n git-bisect.sh | 65 +++++++++++++++++++++++++++++++++---\n t/t6030-bisect-porcelain.sh | 79 ++++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 175 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt\nindex 24171a5..b1ef41c 100644\n--- a/Documentation/git-bisect.txt\n+++ b/Documentation/git-bisect.txt\n@@ -19,6 +19,7 @@ on the subcommand:\n git bisect start [--no-checkout] [ [...]] [--] [...]\n git bisect (bad|new) []\n git bisect (good|old) [...]\n+ git bisect terms \n git bisect skip [(|)...]\n git bisect reset []\n git bisect visualize\n@@ -40,7 +41,7 @@ In fact, `git bisect` can be used to find the commit that changed\n *any* property of your project; e.g., the commit that fixed a bug, or\n the commit that caused a benchmark's performance to improve. To\n support this more general usage, the terms \"old\" and \"new\" can be used\n-in place of \"good\" and \"bad\". See\n+in place of \"good\" and \"bad\", or you can choose your own terms. See\n section \"Alternate terms\" below for more information.\n \n Basic bisect commands: start, bad, good\n@@ -157,6 +158,31 @@ git bisect new [...]\n \n to indicate that it was after.\n \n+If you would like to use your own terms instead of \"bad\"/\"good\" or\n+\"new\"/\"old\", you can choose any names you like by typing\n+\n+------------------------------------------------\n+git bisect terms \n+------------------------------------------------\n+\n+before starting a bisection session. For example, if you are looking\n+for a commit that introduced a performance regression, you might use\n+\n+------------------------------------------------\n+git bisect terms slow fast\n+------------------------------------------------\n+\n+Or if you are looking for the commit that fixed a bug, you might use\n+\n+------------------------------------------------\n+git bisect terms fixed broken\n+------------------------------------------------\n+\n+Only the bisection following the `git bisect terms` will use the\n+terms. If you mistyped one of the terms you can do again `git bisect\n+terms `, but that is possible only before you\n+start the bisection.\n+\n Bisect visualize\n ~~~~~~~~~~~~~~~~\n \n@@ -440,6 +466,14 @@ $ git bisect start\n $ git bisect new HEAD # current commit is marked as new\n $ git bisect old HEAD~10 # the tenth commit from now is marked as old\n ------------\n++\n+or:\n+------------\n+$ git bisect terms fixed broken\n+$ git bisect start\n+$ git bisect fixed\n+$ git bisect broken HEAD~10\n+------------\n \n Getting help\n ~~~~~~~~~~~~\ndiff --git a/git-bisect.sh b/git-bisect.sh\nindex 5769eaf..64ea122 100755\n--- a/git-bisect.sh\n+++ b/git-bisect.sh\n@@ -1,6 +1,6 @@\n #!/bin/sh\n \n-USAGE='[help|start|bad|good|new|old|skip|next|reset|visualize|replay|log|run]'\n+USAGE='[help|start|bad|good|new|old|terms|skip|next|reset|visualize|replay|log|run]'\n LONG_USAGE='git bisect help\n \tprint this long help message.\n git bisect start [--no-checkout] [ [...]] [--] [...]\n@@ -11,6 +11,8 @@ git bisect (bad|new) []\n git bisect (good|old) [...]\n \tmark ... known-good revisions/\n \t\trevisions before change in a given property.\n+git bisect terms \n+\tset up and as terms (default: bad, good)\n git bisect skip [(|)...]\n \tmark ... untestable revisions.\n git bisect next\n@@ -80,6 +82,16 @@ bisect_start() {\n \tbad_seen=0\n \teval=''\n \tmust_write_terms=0\n+\tmust_log_terms=0\n+\tif test -s \"$GIT_DIR/BISECT_TERMS\"\n+\tthen\n+\t\t# We're going to restart from a clean state and the\n+\t\t# file will be deleted. Record the old state in\n+\t\t# variables and restore it below.\n+\t\tmust_write_terms=1\n+\t\tmust_log_terms=1\n+\t\tget_terms\n+\tfi\n \tif test \"z$(git rev-parse --is-bare-repository)\" != zfalse\n \tthen\n \t\tmode=--no-checkout\n@@ -185,7 +197,12 @@ bisect_start() {\n \teval \"$eval true\" &&\n \tif test $must_write_terms -eq 1\n \tthen\n-\t\twrite_terms \"$NAME_BAD\" \"$NAME_GOOD\"\n+\t\twrite_terms \"$NAME_BAD\" \"$NAME_GOOD\" &&\n+\t\tif test $must_log_terms -eq 1\n+\t\tthen\n+\t\t\techo \"git bisect terms $NAME_BAD $NAME_GOOD\" \\\n+\t\t\t >>\"$GIT_DIR/BISECT_LOG\"\n+\t\tfi\n \tfi &&\n \techo \"git bisect start$orig_args\" >>\"$GIT_DIR/BISECT_LOG\" || exit\n \t#\n@@ -449,6 +466,8 @@ bisect_replay () {\n \t\t\teval \"$cmd\" ;;\n \t\t\"$NAME_GOOD\"|\"$NAME_BAD\"|skip)\n \t\t\tbisect_write \"$command\" \"$rev\" ;;\n+\t\tterms)\n+\t\t\tbisect_terms $rev ;;\n \t\t*)\n \t\t\tdie \"$(gettext \"?? what are you talking about?\")\" ;;\n \t\tesac\n@@ -533,13 +552,22 @@ get_terms () {\n write_terms () {\n \tNAME_BAD=$1\n \tNAME_GOOD=$2\n+\tcheck_term_format \"$NAME_BAD\"\n+\tcheck_term_format \"$NAME_GOOD\"\n \tprintf '%s\\n%s\\n' \"$NAME_BAD\" \"$NAME_GOOD\" >\"$GIT_DIR/BISECT_TERMS\"\n }\n \n+check_term_format () {\n+\tterm=$1\n+\tgit check-ref-format refs/bisect/\"$term\" ||\n+\tdie \"$(eval_gettext \"'\\$term' is not a valid term\")\"\n+}\n+\n check_and_set_terms () {\n \tcmd=\"$1\"\n \tcase \"$cmd\" in\n-\tbad|good|new|old)\n+\tskip|start|terms) ;;\n+\t*)\n \t\tif test -s \"$GIT_DIR/BISECT_TERMS\" && test \"$cmd\" != \"$NAME_BAD\" && test \"$cmd\" != \"$NAME_GOOD\"\n \t\tthen\n \t\t\tdie \"$(eval_gettext \"Invalid command: you're currently in a \\$NAME_BAD/\\$NAME_GOOD bisect.\")\"\n@@ -568,6 +596,33 @@ bisect_voc () {\n \tesac\n }\n \n+bisect_terms () {\n+\tcase \"$#\" in\n+\t0)\n+\t\tif test -s \"$GIT_DIR/BISECT_TERMS\"\n+\t\tthen\n+\t\t\tget_terms\n+\t\t\tgettextln \"Your current terms are $NAME_GOOD for the old state\n+and $NAME_BAD for the new state.\"\n+\t\telse\n+\t\t\tdie \"$(gettext \"No terms defined.\")\"\n+\t\tfi ;;\n+\t2)\n+\t\tif ! test -s \"$GIT_DIR/BISECT_START\"\n+\t\tthen\n+\t\t\twrite_terms \"$1\" \"$2\"\n+\t\t\techo \"git bisect terms $NAME_BAD $NAME_GOOD\" >>\"$GIT_DIR/BISECT_LOG\" || exit\n+\t\telse\n+\t\t\tdie \"$(gettext \"A bisection has already started, and you can't change terms in the middle of it.\n+Use 'git bisect terms' to see the current terms.\n+Otherwise, to start a new bisection with new terms, please use\n+'git bisect reset' and set the terms before the start\")\"\n+\t\tfi ;;\n+\t*)\n+\t\tusage ;;\n+\tesac\n+}\n+\n case \"$#\" in\n 0)\n \tusage ;;\n@@ -580,7 +635,7 @@ case \"$#\" in\n \t\tgit bisect -h ;;\n \tstart)\n \t\tbisect_start \"$@\" ;;\n-\tbad|good|new|old)\n+\tbad|good|new|old|\"$NAME_BAD\"|\"$NAME_GOOD\")\n \t\tbisect_state \"$cmd\" \"$@\" ;;\n \tskip)\n \t\tbisect_skip \"$@\" ;;\n@@ -597,6 +652,8 @@ case \"$#\" in\n \t\tbisect_log ;;\n \trun)\n \t\tbisect_run \"$@\" ;;\n+\tterms)\n+\t\tbisect_terms \"$@\" ;;\n \t*)\n \t\tusage ;;\n \tesac\ndiff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh\nindex 983c503..83254d2 100755\n--- a/t/t6030-bisect-porcelain.sh\n+++ b/t/t6030-bisect-porcelain.sh\n@@ -797,4 +797,83 @@ test_expect_success 'bisect cannot mix old/new and good/bad' '\n \ttest_must_fail git bisect old $HASH1\n '\n \n+test_expect_success 'bisect start with one term1 and term2' '\n+\tgit bisect reset &&\n+\tgit bisect terms term1 term2 &&\n+\tgit bisect start &&\n+\tgit bisect term2 $HASH1 &&\n+\tgit bisect term1 $HASH4 &&\n+\tgit bisect term1 &&\n+\tgit bisect term1 >bisect_result &&\n+\tgrep \"$HASH2 is the first term1 commit\" bisect_result &&\n+\tgit bisect log >log_to_replay.txt &&\n+\tgit bisect reset\n+'\n+\n+test_expect_success 'bisect replay with term1 and term2' '\n+\tgit bisect replay log_to_replay.txt >bisect_result &&\n+\tgrep \"$HASH2 is the first term1 commit\" bisect_result &&\n+\tgit bisect reset\n+'\n+\n+test_expect_success 'bisect start term1 term2' '\n+\tgit bisect reset &&\n+\tgit bisect terms term1 term2 &&\n+\tgit bisect start $HASH4 $HASH1 &&\n+\tgit bisect term1 &&\n+\tgit bisect term1 >bisect_result &&\n+\tgrep \"$HASH2 is the first term1 commit\" bisect_result &&\n+\tgit bisect log >log_to_replay.txt &&\n+\tgit bisect reset\n+'\n+\n+test_expect_success 'bisect cannot mix terms' '\n+\tgit bisect reset &&\n+\tgit bisect terms a b &&\n+\tgit bisect terms term1 term2 &&\n+\tgit bisect start $HASH4 $HASH1 &&\n+\ttest_must_fail git bisect a &&\n+\ttest_must_fail git bisect b &&\n+\ttest_must_fail git bisect bad &&\n+\ttest_must_fail git bisect good &&\n+\ttest_must_fail git bisect new &&\n+\ttest_must_fail git bisect old\n+'\n+\n+test_expect_success 'bisect terms rejects invalid terms' '\n+\tgit bisect reset &&\n+\ttest_must_fail git bisect terms invalid..term term2 &&\n+\ttest_must_fail git bisect terms term1 invalid..term &&\n+\ttest_path_is_missing .git/BISECT_TERMS\n+'\n+\n+test_expect_success 'bisect terms needs 0 or 2 arguments' '\n+\tgit bisect reset &&\n+\ttest_must_fail git bisect terms only-one &&\n+\ttest_must_fail git bisect terms 1 2 3 &&\n+\ttest_must_fail git bisect terms 2>actual\n+'\n+\n+test_expect_success 'bisect terms does store terms' '\n+\techo \"No terms defined.\" >expected &&\n+\ttest_cmp expected actual &&\n+\tgit bisect terms one two &&\n+\tgit bisect terms >actual &&\n+\tcat <<-EOF >expected &&\n+\tYour current terms are two for the old state\n+\tand one for the new state.\n+\tEOF\n+\ttest_cmp expected actual'\n+\n+test_expect_success 'bisect start keeps defined terms' '\n+\tgit bisect terms one two &&\n+\tgit bisect start &&\n+\tgit bisect terms >actual &&\n+\tcat <<-EOF >expected &&\n+\tYour current terms are two for the old state\n+\tand one for the new state.\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n test_done\n-- \n2.4.4.414.g318df7a.dirty\n"},{"id":"264982","messageId":"xmqqsi9etjwy.fsf@gitster.dls.corp.google.com","threadId":"39732","inReplyTo":"1435337896-20709-8-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH v10 7/7] bisect: allow any terms set by user","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-26T18:16:13Z","receivedAt":"2015-06-26T18:16:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy writes:\n\n> diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt\n> index 24171a5..b1ef41c 100644\n> --- a/Documentation/git-bisect.txt\n> +++ b/Documentation/git-bisect.txt\n> @@ -19,6 +19,7 @@ on the subcommand:\n> git bisect start [--no-checkout] [ [...]] [--] [...]\n> git bisect (bad|new) []\n> git bisect (good|old) [...]\n> + git bisect terms \n\nI think this is the other way around.\n\nOther than that, this round looked OK to me.\n\nThanks.\n"},{"id":"264986","messageId":"CAP8UFD1ofg01R1rEWk3MJweGAQsVc-yrgCH=fjJ_JeU_81yyTA@mail.gmail.com","threadId":"39732","inReplyTo":"1435337896-20709-6-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH v10 5/7] bisect: simplify the addition of new bisect terms","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2015-06-26T19:22:26Z","receivedAt":"2015-06-26T19:22:26Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Fri, Jun 26, 2015 at 6:58 PM, Matthieu Moy wrote:\n>\n> static int for_each_bad_bisect_ref(const char *submodule, each_ref_fn fn, void *cb_data)\n> {\n> - return for_each_ref_in_submodule(submodule, \"refs/bisect/bad\", fn, cb_data);\n> + struct strbuf bisect_refs = STRBUF_INIT;\n> + int status;\n> + strbuf_addf(&bisect_refs, \"refs/bisect/%s\", name_bad);\n> + status = for_each_ref_in_submodule(submodule, bisect_refs.buf, fn, cb_data);\n> + strbuf_release(&bisect_refs);\n> + return status;\n> }\n>\n> static int for_each_good_bisect_ref(const char *submodule, each_ref_fn fn, void *cb_data)\n> {\n> - return for_each_ref_in_submodule(submodule, \"refs/bisect/good\", fn, cb_data);\n> + struct strbuf bisect_refs = STRBUF_INIT;\n> + int status;\n> + strbuf_addf(&bisect_refs, \"refs/bisect/%s\", name_good);\n> + status = for_each_ref_in_submodule(submodule, bisect_refs.buf, fn, cb_data);\n> + strbuf_release(&bisect_refs);\n> + return status;\n> }\n\nI wonder if it would not be better to just have:\n\nstatic int for_each_bisect_ref(const char *submodule, each_ref_fn fn,\nconst char *term, void *cb_data)\n{\n struct strbuf bisect_refs = STRBUF_INIT;\n int status;\n strbuf_addf(&bisect_refs, \"refs/bisect/%s\", term);\n status = for_each_ref_in_submodule(submodule, bisect_refs.buf,\nfn, cb_data);\n strbuf_release(&bisect_refs);\n return status;\n}\n\nThis way it can be used with either name_good, name_bad or \"skip\" as\nthe term argument.\n"},{"id":"264992","messageId":"CAP8UFD1Donr4v3Z8bpawv0_heDUQ+-EkepZF6UvuXOed7TQBXA@mail.gmail.com","threadId":"39732","inReplyTo":"1435337896-20709-8-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH v10 7/7] bisect: allow any terms set by user","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2015-06-26T20:29:04Z","receivedAt":"2015-06-26T20:29:04Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Fri, Jun 26, 2015 at 6:58 PM, Matthieu Moy wrote:\n> From: Antoine Delaite \n>\n> Introduction of the git bisect terms command. The user can set his own\n> terms. It will work exactly like before. The terms must be set before the\n> start.\n\nAfter looking a bit at the code, I think that for now existing\npredefined terms (\"bad\", \"good\", \"new\" and \"old\") as well as some\nother terms that look like bisect subcommands like \"skip\", \"start\" and\n\"terms\" should be disallowed as arguments to \"git bisect terms\", and\nthis should be stated in the commit message and in the documentation\nas well as checked and tested.\n\nFor example a user might want to search for a fix by using \"git bisect\nterms good bad\" (which should swap \"good\" and \"bad\"), but we should\nnot at least for now allow that.\n\n> @@ -185,7 +197,12 @@ bisect_start() {\n> eval \"$eval true\" &&\n> if test $must_write_terms -eq 1\n> then\n> - write_terms \"$NAME_BAD\" \"$NAME_GOOD\"\n> + write_terms \"$NAME_BAD\" \"$NAME_GOOD\" &&\n> + if test $must_log_terms -eq 1\n> + then\n> + echo \"git bisect terms $NAME_BAD $NAME_GOOD\" \\\n> + >>\"$GIT_DIR/BISECT_LOG\"\n> + fi\n\nMaybe you could move appending to the log into write_terms() though\nyou might need to pass an additional argument to enable or disable\nlogging.\n"},{"id":"264993","messageId":"1435350769-10973-1-git-send-email-Matthieu.Moy@imag.fr","threadId":"39732","inReplyTo":"CAP8UFD1ofg01R1rEWk3MJweGAQsVc-yrgCH=fjJ_JeU_81yyTA@mail.gmail.com","subject":"[PATCH v10.1 5/7] bisect: simplify the addition of new bisect terms","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2015-06-26T20:32:49Z","receivedAt":"2015-06-26T20:32:49Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"From: Antoine Delaite \n\nWe create a file BISECT_TERMS in the repository .git to be read during a\nbisection. The fonctions to be changed if we add new terms are quite few.\n\nIn git-bisect.sh:\n\tcheck_and_set_terms\n\tbisect_voc\n\nCo-authored-by: Louis Stuber \nTweaked-by: Matthieu Moy \nSigned-off-by: Antoine Delaite \nSigned-off-by: Louis Stuber \nSigned-off-by: Valentin Duperray \nSigned-off-by: Franck Jonas \nSigned-off-by: Lucien Kong \nSigned-off-by: Thomas Nguy \nSigned-off-by: Huynh Khoi Nguyen Nguyen \nSigned-off-by: Matthieu Moy \n---\n> I wonder if it would not be better to just have:\n> \n> static int for_each_bisect_ref(const char *submodule, each_ref_fn fn,\n> const char *term, void *cb_data)\n\nWe do need two functions because we pass the pointer as callback, but\nit reads nicer with a third helper function.\n\n bisect.c | 33 ++++++++++++++++++++++++--\n git-bisect.sh | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++++------\n revision.c | 19 +++++++++++++--\n 3 files changed, 117 insertions(+), 11 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex 9368f6c..c5f96eb 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -905,6 +905,36 @@ static void show_diff_tree(const char *prefix, struct commit *commit)\n }\n \n /*\n+ * The terms used for this bisect session are stored in BISECT_TERMS.\n+ * We read them and store them to adapt the messages accordingly.\n+ * Default is bad/good.\n+ */\n+void read_bisect_terms(const char **read_bad, const char **read_good)\n+{\n+\tstruct strbuf str = STRBUF_INIT;\n+\tconst char *filename = git_path(\"BISECT_TERMS\");\n+\tFILE *fp = fopen(filename, \"r\");\n+\n+\tif (!fp) {\n+\t\tif (errno == ENOENT) {\n+\t\t\t*read_bad = \"bad\";\n+\t\t\t*read_good = \"good\";\n+\t\t\treturn;\n+\t\t} else {\n+\t\t\tdie(\"could not read file '%s': %s\", filename,\n+\t\t\t\tstrerror(errno));\n+\t\t}\n+\t} else {\n+\t\tstrbuf_getline(&str, fp, '\\n');\n+\t\t*read_bad = strbuf_detach(&str, NULL);\n+\t\tstrbuf_getline(&str, fp, '\\n');\n+\t\t*read_good = strbuf_detach(&str, NULL);\n+\t}\n+\tstrbuf_release(&str);\n+\tfclose(fp);\n+}\n+\n+/*\n * We use the convention that exiting with an exit code 10 means that\n * the bisection process finished successfully.\n * In this case the calling shell script should exit 0.\n@@ -920,8 +950,7 @@ int bisect_next_all(const char *prefix, int no_checkout)\n \tconst unsigned char *bisect_rev;\n \tchar bisect_rev_hex[GIT_SHA1_HEXSZ + 1];\n \n-\tname_bad = \"bad\";\n-\tname_good = \"good\";\n+\tread_bisect_terms(&name_bad, &name_good);\n \tif (read_bisect_refs())\n \t\tdie(\"reading bisect refs failed\");\n \ndiff --git a/git-bisect.sh b/git-bisect.sh\nindex ce6412f..52e41e8 100755\n--- a/git-bisect.sh\n+++ b/git-bisect.sh\n@@ -77,6 +77,7 @@ bisect_start() {\n \torig_args=$(git rev-parse --sq-quote \"$@\")\n \tbad_seen=0\n \teval=''\n+\tmust_write_terms=0\n \tif test \"z$(git rev-parse --is-bare-repository)\" != zfalse\n \tthen\n \t\tmode=--no-checkout\n@@ -101,6 +102,14 @@ bisect_start() {\n \t\t\t\tdie \"$(eval_gettext \"'\\$arg' does not appear to be a valid revision\")\"\n \t\t\t\tbreak\n \t\t\t}\n+\n+\t\t\t# The user ran \"git bisect start \n+\t\t\t# \", hence did not explicitly specify\n+\t\t\t# the terms, but we are already starting to\n+\t\t\t# set references named with the default terms,\n+\t\t\t# and won't be able to change afterwards.\n+\t\t\tmust_write_terms=1\n+\n \t\t\tcase $bad_seen in\n \t\t\t0) state=$NAME_BAD ; bad_seen=1 ;;\n \t\t\t*) state=$NAME_GOOD ;;\n@@ -172,6 +181,10 @@ bisect_start() {\n \t} &&\n \tgit rev-parse --sq-quote \"$@\" >\"$GIT_DIR/BISECT_NAMES\" &&\n \teval \"$eval true\" &&\n+\tif test $must_write_terms -eq 1\n+\tthen\n+\t\twrite_terms \"$NAME_BAD\" \"$NAME_GOOD\"\n+\tfi &&\n \techo \"git bisect start$orig_args\" >>\"$GIT_DIR/BISECT_LOG\" || exit\n \t#\n \t# Check if we can proceed to the next bisect state.\n@@ -232,6 +245,7 @@ bisect_skip() {\n bisect_state() {\n \tbisect_autostart\n \tstate=$1\n+\tcheck_and_set_terms $state\n \tcase \"$#,$state\" in\n \t0,*)\n \t\tdie \"$(gettext \"Please call 'bisect_state' with at least one argument.\")\" ;;\n@@ -291,15 +305,17 @@ bisect_next_check() {\n \t\t: bisect without $NAME_GOOD...\n \t\t;;\n \t*)\n-\n+\t\tbad_syn=$(bisect_voc bad)\n+\t\tgood_syn=$(bisect_voc good)\n \t\tif test -s \"$GIT_DIR/BISECT_START\"\n \t\tthen\n-\t\t\tgettextln \"You need to give me at least one good and one bad revision.\n-(You can use \\\"git bisect bad\\\" and \\\"git bisect good\\\" for that.)\" >&2\n+\n+\t\t\teval_gettextln \"You need to give me at least one \\$bad_syn and one \\$good_syn revision.\n+(You can use \\\"git bisect \\$bad_syn\\\" and \\\"git bisect \\$good_syn\\\" for that.)\" >&2\n \t\telse\n-\t\t\tgettextln \"You need to start by \\\"git bisect start\\\".\n-You then need to give me at least one good and one bad revision.\n-(You can use \\\"git bisect bad\\\" and \\\"git bisect good\\\" for that.)\" >&2\n+\t\t\teval_gettextln \"You need to start by \\\"git bisect start\\\".\n+You then need to give me at least one \\$good_syn and one \\$bad_syn revision.\n+(You can use \\\"git bisect \\$bad_syn\\\" and \\\"git bisect \\$good_syn\\\" for that.)\" >&2\n \t\tfi\n \t\texit 1 ;;\n \tesac\n@@ -402,6 +418,7 @@ bisect_clean_state() {\n \trm -f \"$GIT_DIR/BISECT_LOG\" &&\n \trm -f \"$GIT_DIR/BISECT_NAMES\" &&\n \trm -f \"$GIT_DIR/BISECT_RUN\" &&\n+\trm -f \"$GIT_DIR/BISECT_TERMS\" &&\n \t# Cleanup head-name if it got left by an old version of git-bisect\n \trm -f \"$GIT_DIR/head-name\" &&\n \tgit update-ref -d --no-deref BISECT_HEAD &&\n@@ -422,11 +439,13 @@ bisect_replay () {\n \t\t\trev=\"$command\"\n \t\t\tcommand=\"$bisect\"\n \t\tfi\n+\t\tget_terms\n+\t\tcheck_and_set_terms \"$command\"\n \t\tcase \"$command\" in\n \t\tstart)\n \t\t\tcmd=\"bisect_start $rev\"\n \t\t\teval \"$cmd\" ;;\n-\t\t$NAME_GOOD|$NAME_BAD|skip)\n+\t\t\"$NAME_GOOD\"|\"$NAME_BAD\"|skip)\n \t\t\tbisect_write \"$command\" \"$rev\" ;;\n \t\t*)\n \t\t\tdie \"$(gettext \"?? what are you talking about?\")\" ;;\n@@ -499,11 +518,54 @@ bisect_log () {\n \tcat \"$GIT_DIR/BISECT_LOG\"\n }\n \n+get_terms () {\n+\tif test -s \"$GIT_DIR/BISECT_TERMS\"\n+\tthen\n+\t\t{\n+\t\tread NAME_BAD\n+\t\tread NAME_GOOD\n+\t\t} <\"$GIT_DIR/BISECT_TERMS\"\n+\tfi\n+}\n+\n+write_terms () {\n+\tNAME_BAD=$1\n+\tNAME_GOOD=$2\n+\tprintf '%s\\n%s\\n' \"$NAME_BAD\" \"$NAME_GOOD\" >\"$GIT_DIR/BISECT_TERMS\"\n+}\n+\n+check_and_set_terms () {\n+\tcmd=\"$1\"\n+\tcase \"$cmd\" in\n+\tbad|good)\n+\t\tif test -s \"$GIT_DIR/BISECT_TERMS\" && test \"$cmd\" != \"$NAME_BAD\" && test \"$cmd\" != \"$NAME_GOOD\"\n+\t\tthen\n+\t\t\tdie \"$(eval_gettext \"Invalid command: you're currently in a \\$NAME_BAD/\\$NAME_GOOD bisect.\")\"\n+\t\tfi\n+\t\tcase \"$cmd\" in\n+\t\tbad|good)\n+\t\t\tif ! test -s \"$GIT_DIR/BISECT_TERMS\"\n+\t\t\tthen\n+\t\t\t\twrite_terms bad good\n+\t\t\tfi\n+\t\t\t;;\n+\t\tesac ;;\n+\tesac\n+}\n+\n+bisect_voc () {\n+\tcase \"$1\" in\n+\tbad) echo \"bad\" ;;\n+\tgood) echo \"good\" ;;\n+\tesac\n+}\n+\n case \"$#\" in\n 0)\n \tusage ;;\n *)\n \tcmd=\"$1\"\n+\tget_terms\n \tshift\n \tcase \"$cmd\" in\n \thelp)\ndiff --git a/revision.c b/revision.c\nindex 3ff8723..5cd08e9 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -21,6 +21,9 @@\n \n volatile show_early_output_fn_t show_early_output;\n \n+static const char *name_bad;\n+static const char *name_good;\n+\n char *path_name(const struct name_path *path, const char *name)\n {\n \tconst struct name_path *p;\n@@ -2076,14 +2079,25 @@ void parse_revision_opt(struct rev_info *revs, struct parse_opt_ctx_t *ctx,\n \tctx->argc -= n;\n }\n \n+extern void read_bisect_terms(const char **bad, const char **good);\n+\n+static int for_each_bisect_ref(const char *submodule, each_ref_fn fn, void *cb_data, const char *term) {\n+\tstruct strbuf bisect_refs = STRBUF_INIT;\n+\tint status;\n+\tstrbuf_addf(&bisect_refs, \"refs/bisect/%s\", term);\n+\tstatus = for_each_ref_in_submodule(submodule, bisect_refs.buf, fn, cb_data);\n+\tstrbuf_release(&bisect_refs);\n+\treturn status;\n+}\n+\n static int for_each_bad_bisect_ref(const char *submodule, each_ref_fn fn, void *cb_data)\n {\n-\treturn for_each_ref_in_submodule(submodule, \"refs/bisect/bad\", fn, cb_data);\n+\treturn for_each_bisect_ref(submodule, fn, cb_data, \"bad\");\n }\n \n static int for_each_good_bisect_ref(const char *submodule, each_ref_fn fn, void *cb_data)\n {\n-\treturn for_each_ref_in_submodule(submodule, \"refs/bisect/good\", fn, cb_data);\n+\treturn for_each_bisect_ref(submodule, fn, cb_data, \"good\");\n }\n \n static int handle_revision_pseudo_opt(const char *submodule,\n@@ -2112,6 +2126,7 @@ static int handle_revision_pseudo_opt(const char *submodule,\n \t\thandle_refs(submodule, revs, *flags, for_each_branch_ref_submodule);\n \t\tclear_ref_exclusion(&revs->ref_excludes);\n \t} else if (!strcmp(arg, \"--bisect\")) {\n+\t\tread_bisect_terms(&name_bad, &name_good);\n \t\thandle_refs(submodule, revs, *flags, for_each_bad_bisect_ref);\n \t\thandle_refs(submodule, revs, *flags ^ (UNINTERESTING | BOTTOM), for_each_good_bisect_ref);\n \t\trevs->bisect = 1;\n-- \n2.5.0.rc0.7.g0f487ca.dirty\n"},{"id":"264995","messageId":"1435351183-27100-1-git-send-email-Matthieu.Moy@imag.fr","threadId":"39732","inReplyTo":"xmqqsi9etjwy.fsf@gitster.dls.corp.google.com","subject":"[PATCH v10.1 7/7] bisect: allow any terms set by user","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2015-06-26T20:39:43Z","receivedAt":"2015-06-26T20:39:43Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"From: Antoine Delaite \n\nIntroduction of the git bisect terms command. The user can set his own\nterms. It will work exactly like before. The terms must be set before the\nstart.\n\nSigned-off-by: Antoine Delaite \nSigned-off-by: Louis Stuber \nSigned-off-by: Matthieu Moy \n---\nJunio C Hamano writes:\n\n> Matthieu Moy writes:\n>\n>> + git bisect terms \n>\n> I think this is the other way around.\n\nIndeed.\n\nThe test scripts were messed up, I sent too quickly after refactoring.\n\nThere was also a small bug: \"git bisect reset\" was not reseting the\nterms when ran before \"git bisect start\".\n\nMy branch https://github.com/moy/git/commits/bisect-terms is up to\ndate in case the individual resend are too messy. I can also resend\nlater, but I was too ashamed of my bugs with tests to leave it as\nis ;-).\n\n Documentation/git-bisect.txt | 36 +++++++++++++++++++-\n git-bisect.sh | 67 +++++++++++++++++++++++++++++++++---\n t/t6030-bisect-porcelain.sh | 81 ++++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 179 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt\nindex abaf462..28f6104 100644\n--- a/Documentation/git-bisect.txt\n+++ b/Documentation/git-bisect.txt\n@@ -19,6 +19,7 @@ on the subcommand:\n git bisect start [--no-checkout] [ [...]] [--] [...]\n git bisect (bad|new) []\n git bisect (good|old) [...]\n+ git bisect terms \n git bisect skip [(|)...]\n git bisect reset []\n git bisect visualize\n@@ -40,7 +41,7 @@ In fact, `git bisect` can be used to find the commit that changed\n *any* property of your project; e.g., the commit that fixed a bug, or\n the commit that caused a benchmark's performance to improve. To\n support this more general usage, the terms \"old\" and \"new\" can be used\n-in place of \"good\" and \"bad\". See\n+in place of \"good\" and \"bad\", or you can choose your own terms. See\n section \"Alternate terms\" below for more information.\n \n Basic bisect commands: start, bad, good\n@@ -157,6 +158,31 @@ git bisect new [...]\n \n to indicate that it was after.\n \n+If you would like to use your own terms instead of \"bad\"/\"good\" or\n+\"new\"/\"old\", you can choose any names you like by typing\n+\n+------------------------------------------------\n+git bisect terms \n+------------------------------------------------\n+\n+before starting a bisection session. For example, if you are looking\n+for a commit that introduced a performance regression, you might use\n+\n+------------------------------------------------\n+git bisect terms slow fast\n+------------------------------------------------\n+\n+Or if you are looking for the commit that fixed a bug, you might use\n+\n+------------------------------------------------\n+git bisect terms fixed broken\n+------------------------------------------------\n+\n+Only the bisection following the `git bisect terms` will use the\n+terms. If you mistyped one of the terms you can do again `git bisect\n+terms `, but that is possible only before you\n+start the bisection.\n+\n Bisect visualize\n ~~~~~~~~~~~~~~~~\n \n@@ -440,6 +466,14 @@ $ git bisect start\n $ git bisect new HEAD # current commit is marked as new\n $ git bisect old HEAD~10 # the tenth commit from now is marked as old\n ------------\n++\n+or:\n+------------\n+$ git bisect terms fixed broken\n+$ git bisect start\n+$ git bisect fixed\n+$ git bisect broken HEAD~10\n+------------\n \n Getting help\n ~~~~~~~~~~~~\ndiff --git a/git-bisect.sh b/git-bisect.sh\nindex 5769eaf..cf07a91 100755\n--- a/git-bisect.sh\n+++ b/git-bisect.sh\n@@ -1,6 +1,6 @@\n #!/bin/sh\n \n-USAGE='[help|start|bad|good|new|old|skip|next|reset|visualize|replay|log|run]'\n+USAGE='[help|start|bad|good|new|old|terms|skip|next|reset|visualize|replay|log|run]'\n LONG_USAGE='git bisect help\n \tprint this long help message.\n git bisect start [--no-checkout] [ [...]] [--] [...]\n@@ -11,6 +11,8 @@ git bisect (bad|new) []\n git bisect (good|old) [...]\n \tmark ... known-good revisions/\n \t\trevisions before change in a given property.\n+git bisect terms \n+\tset up and as terms (default: bad, good)\n git bisect skip [(|)...]\n \tmark ... untestable revisions.\n git bisect next\n@@ -80,6 +82,16 @@ bisect_start() {\n \tbad_seen=0\n \teval=''\n \tmust_write_terms=0\n+\tmust_log_terms=0\n+\tif test -s \"$GIT_DIR/BISECT_TERMS\"\n+\tthen\n+\t\t# We're going to restart from a clean state and the\n+\t\t# file will be deleted. Record the old state in\n+\t\t# variables and restore it below.\n+\t\tmust_write_terms=1\n+\t\tmust_log_terms=1\n+\t\tget_terms\n+\tfi\n \tif test \"z$(git rev-parse --is-bare-repository)\" != zfalse\n \tthen\n \t\tmode=--no-checkout\n@@ -185,7 +197,12 @@ bisect_start() {\n \teval \"$eval true\" &&\n \tif test $must_write_terms -eq 1\n \tthen\n-\t\twrite_terms \"$NAME_BAD\" \"$NAME_GOOD\"\n+\t\twrite_terms \"$NAME_BAD\" \"$NAME_GOOD\" &&\n+\t\tif test $must_log_terms -eq 1\n+\t\tthen\n+\t\t\techo \"git bisect terms $NAME_BAD $NAME_GOOD\" \\\n+\t\t\t >>\"$GIT_DIR/BISECT_LOG\"\n+\t\tfi\n \tfi &&\n \techo \"git bisect start$orig_args\" >>\"$GIT_DIR/BISECT_LOG\" || exit\n \t#\n@@ -387,6 +404,8 @@ bisect_visualize() {\n bisect_reset() {\n \ttest -s \"$GIT_DIR/BISECT_START\" || {\n \t\tgettextln \"We are not bisecting.\"\n+\t\t# We may still have leftovers like BISECT_TERMS\n+\t\tbisect_clean_state\n \t\treturn\n \t}\n \tcase \"$#\" in\n@@ -449,6 +468,8 @@ bisect_replay () {\n \t\t\teval \"$cmd\" ;;\n \t\t\"$NAME_GOOD\"|\"$NAME_BAD\"|skip)\n \t\t\tbisect_write \"$command\" \"$rev\" ;;\n+\t\tterms)\n+\t\t\tbisect_terms $rev ;;\n \t\t*)\n \t\t\tdie \"$(gettext \"?? what are you talking about?\")\" ;;\n \t\tesac\n@@ -533,13 +554,22 @@ get_terms () {\n write_terms () {\n \tNAME_BAD=$1\n \tNAME_GOOD=$2\n+\tcheck_term_format \"$NAME_BAD\"\n+\tcheck_term_format \"$NAME_GOOD\"\n \tprintf '%s\\n%s\\n' \"$NAME_BAD\" \"$NAME_GOOD\" >\"$GIT_DIR/BISECT_TERMS\"\n }\n \n+check_term_format () {\n+\tterm=$1\n+\tgit check-ref-format refs/bisect/\"$term\" ||\n+\tdie \"$(eval_gettext \"'\\$term' is not a valid term\")\"\n+}\n+\n check_and_set_terms () {\n \tcmd=\"$1\"\n \tcase \"$cmd\" in\n-\tbad|good|new|old)\n+\tskip|start|terms) ;;\n+\t*)\n \t\tif test -s \"$GIT_DIR/BISECT_TERMS\" && test \"$cmd\" != \"$NAME_BAD\" && test \"$cmd\" != \"$NAME_GOOD\"\n \t\tthen\n \t\t\tdie \"$(eval_gettext \"Invalid command: you're currently in a \\$NAME_BAD/\\$NAME_GOOD bisect.\")\"\n@@ -568,6 +598,33 @@ bisect_voc () {\n \tesac\n }\n \n+bisect_terms () {\n+\tcase \"$#\" in\n+\t0)\n+\t\tif test -s \"$GIT_DIR/BISECT_TERMS\"\n+\t\tthen\n+\t\t\tget_terms\n+\t\t\tgettextln \"Your current terms are $NAME_GOOD for the old state\n+and $NAME_BAD for the new state.\"\n+\t\telse\n+\t\t\tdie \"$(gettext \"No terms defined.\")\"\n+\t\tfi ;;\n+\t2)\n+\t\tif ! test -s \"$GIT_DIR/BISECT_START\"\n+\t\tthen\n+\t\t\twrite_terms \"$1\" \"$2\"\n+\t\t\techo \"git bisect terms $NAME_BAD $NAME_GOOD\" >>\"$GIT_DIR/BISECT_LOG\" || exit\n+\t\telse\n+\t\t\tdie \"$(gettext \"A bisection has already started, and you can't change terms in the middle of it.\n+Use 'git bisect terms' to see the current terms.\n+Otherwise, to start a new bisection with new terms, please use\n+'git bisect reset' and set the terms before the start\")\"\n+\t\tfi ;;\n+\t*)\n+\t\tusage ;;\n+\tesac\n+}\n+\n case \"$#\" in\n 0)\n \tusage ;;\n@@ -580,7 +637,7 @@ case \"$#\" in\n \t\tgit bisect -h ;;\n \tstart)\n \t\tbisect_start \"$@\" ;;\n-\tbad|good|new|old)\n+\tbad|good|new|old|\"$NAME_BAD\"|\"$NAME_GOOD\")\n \t\tbisect_state \"$cmd\" \"$@\" ;;\n \tskip)\n \t\tbisect_skip \"$@\" ;;\n@@ -597,6 +654,8 @@ case \"$#\" in\n \t\tbisect_log ;;\n \trun)\n \t\tbisect_run \"$@\" ;;\n+\tterms)\n+\t\tbisect_terms \"$@\" ;;\n \t*)\n \t\tusage ;;\n \tesac\ndiff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh\nindex 983c503..eb8cc80 100755\n--- a/t/t6030-bisect-porcelain.sh\n+++ b/t/t6030-bisect-porcelain.sh\n@@ -797,4 +797,85 @@ test_expect_success 'bisect cannot mix old/new and good/bad' '\n \ttest_must_fail git bisect old $HASH1\n '\n \n+test_expect_success 'bisect start with one term1 and term2' '\n+\tgit bisect reset &&\n+\tgit bisect terms term1 term2 &&\n+\tgit bisect start &&\n+\tgit bisect term2 $HASH1 &&\n+\tgit bisect term1 $HASH4 &&\n+\tgit bisect term1 &&\n+\tgit bisect term1 >bisect_result &&\n+\tgrep \"$HASH2 is the first term1 commit\" bisect_result &&\n+\tgit bisect log >log_to_replay.txt &&\n+\tgit bisect reset\n+'\n+\n+test_expect_success 'bisect replay with term1 and term2' '\n+\tgit bisect replay log_to_replay.txt >bisect_result &&\n+\tgrep \"$HASH2 is the first term1 commit\" bisect_result &&\n+\tgit bisect reset\n+'\n+\n+test_expect_success 'bisect start term1 term2' '\n+\tgit bisect reset &&\n+\tgit bisect terms term1 term2 &&\n+\tgit bisect start $HASH4 $HASH1 &&\n+\tgit bisect term1 &&\n+\tgit bisect term1 >bisect_result &&\n+\tgrep \"$HASH2 is the first term1 commit\" bisect_result &&\n+\tgit bisect log >log_to_replay.txt &&\n+\tgit bisect reset\n+'\n+\n+test_expect_success 'bisect cannot mix terms' '\n+\tgit bisect reset &&\n+\tgit bisect terms a b &&\n+\tgit bisect terms term1 term2 &&\n+\tgit bisect start $HASH4 $HASH1 &&\n+\ttest_must_fail git bisect a &&\n+\ttest_must_fail git bisect b &&\n+\ttest_must_fail git bisect bad &&\n+\ttest_must_fail git bisect good &&\n+\ttest_must_fail git bisect new &&\n+\ttest_must_fail git bisect old\n+'\n+\n+test_expect_success 'bisect terms rejects invalid terms' '\n+\tgit bisect reset &&\n+\ttest_must_fail git bisect terms invalid..term term2 &&\n+\ttest_must_fail git bisect terms term1 invalid..term &&\n+\ttest_path_is_missing .git/BISECT_TERMS\n+'\n+\n+test_expect_success 'bisect terms needs 0 or 2 arguments' '\n+\tgit bisect reset &&\n+\ttest_must_fail git bisect terms only-one &&\n+\ttest_must_fail git bisect terms 1 2 3 &&\n+\ttest_must_fail git bisect terms 2>actual &&\n+\techo \"No terms defined.\" >expected &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'bisect terms does store terms' '\n+\tgit bisect reset &&\n+\tgit bisect terms one two &&\n+\tgit bisect terms >actual &&\n+\tcat <<-EOF >expected &&\n+\tYour current terms are two for the old state\n+\tand one for the new state.\n+\tEOF\n+\ttest_cmp expected actual'\n+\n+test_expect_success 'bisect start keeps defined terms' '\n+\tgit bisect reset &&\n+\tgit bisect terms one two &&\n+\tgit bisect start &&\n+\tgit bisect terms >actual &&\n+\tcat <<-EOF >expected &&\n+\tYour current terms are two for the old state\n+\tand one for the new state.\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n test_done\n-- \n2.5.0.rc0.7.g0f487ca.dirty\n"},{"id":"264998","messageId":"vpq7fqqqj7f.fsf@anie.imag.fr","threadId":"39732","inReplyTo":"CAP8UFD1Donr4v3Z8bpawv0_heDUQ+-EkepZF6UvuXOed7TQBXA@mail.gmail.com","subject":"Re: [PATCH v10 7/7] bisect: allow any terms set by user","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-06-26T20:59:48Z","receivedAt":"2015-06-26T20:59:48Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Christian Couder writes:\n\n> On Fri, Jun 26, 2015 at 6:58 PM, Matthieu Moy wrote:\n>> From: Antoine Delaite \n>>\n>> Introduction of the git bisect terms command. The user can set his own\n>> terms. It will work exactly like before. The terms must be set before the\n>> start.\n>\n> After looking a bit at the code, I think that for now existing\n> predefined terms (\"bad\", \"good\", \"new\" and \"old\") as well as some\n> other terms that look like bisect subcommands like \"skip\", \"start\" and\n> \"terms\" should be disallowed as arguments to \"git bisect terms\",\n\nMore importantly, subcommands should be disallowed, or the user may\ncompletely break bisect (e.g. running 'git bisect terms reset bar'\nprevents you from running 'git bisect reset' later).\n\nAnd they should be different, or some more funny situation will occur.\n\nI've just squashed this into my last patch:\n\ndiff --git a/git-bisect.sh b/git-bisect.sh\nindex cf07a91..f6be218 100755\n--- a/git-bisect.sh\n+++ b/git-bisect.sh\n@@ -607,9 +607,21 @@ bisect_terms () {\n \t\t\tgettextln \"Your current terms are $NAME_GOOD for the old state\n and $NAME_BAD for the new state.\"\n \t\telse\n-\t\t\tdie \"$(gettext \"No terms defined.\")\"\n+\t\t\tdie \"$(gettext \"no terms defined\")\"\n \t\tfi ;;\n \t2)\n+\t\tfor term in \"$@\"\n+\t\tdo\n+\t\t\tcase \"$term\" in\n+\t\t\t\thelp|start|bad|good|new|old|terms|skip|next|reset|visualize|replay|log|run)\n+\t\t\t\tdie \"$(eval_gettext \"can't use the builtin command '\\$term' as a term\")\"\n+\t\t\t\t;;\n+\t\t\tesac\n+\t\tdone\n+\t\tif test \"$1\" = \"$2\"\n+\t\tthen\n+\t\t\tdie \"$(gettext \"please use two different terms\")\"\n+\t\tfi\n \t\tif ! test -s \"$GIT_DIR/BISECT_START\"\n \t\tthen\n \t\t\twrite_terms \"$1\" \"$2\"\ndiff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh\nindex eb8cc80..5a7243b 100755\n--- a/t/t6030-bisect-porcelain.sh\n+++ b/t/t6030-bisect-porcelain.sh\n@@ -852,7 +852,7 @@ test_expect_success 'bisect terms needs 0 or 2 arguments' '\n \ttest_must_fail git bisect terms only-one &&\n \ttest_must_fail git bisect terms 1 2 3 &&\n \ttest_must_fail git bisect terms 2>actual &&\n-\techo \"No terms defined.\" >expected &&\n+\techo \"no terms defined\" >expected &&\n \ttest_cmp expected actual\n '\n \nUpdated my GitHub branch, but I'll stop spamming the list with git\nsend-email for a while ;-).\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"265001","messageId":"xmqq381etb1e.fsf@gitster.dls.corp.google.com","threadId":"39732","inReplyTo":"1435350769-10973-1-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH v10.1 5/7] bisect: simplify the addition of new bisect terms","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-26T21:27:57Z","receivedAt":"2015-06-26T21:27:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy writes:\n\n> We do need two functions because we pass the pointer as callback, but\n> it reads nicer with a third helper function.\n>\n> diff --git a/revision.c b/revision.c\n> index 3ff8723..5cd08e9 100644\n> --- a/revision.c\n> +++ b/revision.c\n> @@ -21,6 +21,9 @@\n> \n> volatile show_early_output_fn_t show_early_output;\n> \n> +static const char *name_bad;\n> +static const char *name_good;\n> +\n> ...\n> +\n> +static int for_each_bisect_ref(const char *submodule, each_ref_fn fn, void *cb_data, const char *term) {\n> +\tstruct strbuf bisect_refs = STRBUF_INIT;\n> +\tint status;\n> +\tstrbuf_addf(&bisect_refs, \"refs/bisect/%s\", term);\n> +\tstatus = for_each_ref_in_submodule(submodule, bisect_refs.buf, fn, cb_data);\n> +\tstrbuf_release(&bisect_refs);\n> +\treturn status;\n> +}\n> +\n> static int for_each_bad_bisect_ref(const char *submodule, each_ref_fn fn, void *cb_data)\n> {\n> -\treturn for_each_ref_in_submodule(submodule, \"refs/bisect/bad\", fn, cb_data);\n> +\treturn for_each_bisect_ref(submodule, fn, cb_data, \"bad\");\n> }\n\nShouldn't this be passing name_bad instead of \"bad\"?\n\n> \n> static int for_each_good_bisect_ref(const char *submodule, each_ref_fn fn, void *cb_data)\n> {\n> -\treturn for_each_ref_in_submodule(submodule, \"refs/bisect/good\", fn, cb_data);\n> +\treturn for_each_bisect_ref(submodule, fn, cb_data, \"good\");\n> }\n\nLikewise.\n"},{"id":"265002","messageId":"vpqioaap2vv.fsf@anie.imag.fr","threadId":"39732","inReplyTo":"xmqq381etb1e.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v10.1 5/7] bisect: simplify the addition of new bisect terms","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-06-26T21:37:40Z","receivedAt":"2015-06-26T21:37:40Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano writes:\n\n>> static int for_each_bad_bisect_ref(const char *submodule, each_ref_fn fn, void *cb_data)\n>> {\n>> -\treturn for_each_ref_in_submodule(submodule, \"refs/bisect/bad\", fn, cb_data);\n>> +\treturn for_each_bisect_ref(submodule, fn, cb_data, \"bad\");\n>> }\n>\n> Shouldn't this be passing name_bad instead of \"bad\"?\n>\n>> \n>> static int for_each_good_bisect_ref(const char *submodule, each_ref_fn fn, void *cb_data)\n>> {\n>> -\treturn for_each_ref_in_submodule(submodule, \"refs/bisect/good\", fn, cb_data);\n>> +\treturn for_each_bisect_ref(submodule, fn, cb_data, \"good\");\n>> }\n>\n> Likewise.\n\nIndeed. Fixed.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"265009","messageId":"xmqqa8vmrtsh.fsf@gitster.dls.corp.google.com","threadId":"39732","inReplyTo":"1435351183-27100-1-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH v10.1 7/7] bisect: allow any terms set by user","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-26T22:25:50Z","receivedAt":"2015-06-26T22:25:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy writes:\n\n>> Matthieu Moy writes:\n>>\n>>> + git bisect terms \n>>\n>> I think this is the other way around.\n>\n> Indeed.\n\nI hate to be saying this, but this is a strong indication that\nconsistency with \"start $bad $good...\" must be broken. If the\nperson who has been working on this topic for a few iterations in\nthe past few days cannot get it right, no ordinary user can. With\nor without a mnemonic hint \"N comes before O, so does B before G\".\n\nOf course we cannot just say \"git bisect terms old new\". That would\nonly invite \"eh, I do not remember, which between terms and start\ntake the old one first?\" without helping people.\n\nThe best I can come up with is to forbid positional arguments to\nthis subcommand and always require them to be given like so:\n\n\tgit bisect terms --old=fast --new=slow\n\tgit bisect terms --new=slow --old=fast\n\nWe may want to start supporting\n\n\tgit bisect start --new=master --old=maint\n\nwhile at it and then gently nudging people to stop using\n\n\tgit bisect start master maint\n\nby showing depreation notice.\n"},{"id":"265013","messageId":"CAP8UFD1PyS_qM3EHW_Nzmo=3aeTDkZ0M3hnhRQANAO+ShF3H1Q@mail.gmail.com","threadId":"39732","inReplyTo":"xmqqa8vmrtsh.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v10.1 7/7] bisect: allow any terms set by user","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2015-06-27T04:10:07Z","receivedAt":"2015-06-27T04:10:07Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Sat, Jun 27, 2015 at 12:25 AM, Junio C Hamano wrote:\n> Matthieu Moy writes:\n>\n>>> Matthieu Moy writes:\n>>>\n>>>> + git bisect terms \n>>>\n>>> I think this is the other way around.\n>>\n>> Indeed.\n>\n> I hate to be saying this, but this is a strong indication that\n> consistency with \"start $bad $good...\" must be broken. If the\n> person who has been working on this topic for a few iterations in\n> the past few days cannot get it right, no ordinary user can. With\n> or without a mnemonic hint \"N comes before O, so does B before G\".\n>\n> Of course we cannot just say \"git bisect terms old new\". That would\n> only invite \"eh, I do not remember, which between terms and start\n> take the old one first?\" without helping people.\n>\n> The best I can come up with is to forbid positional arguments to\n> this subcommand and always require them to be given like so:\n>\n> git bisect terms --old=fast --new=slow\n> git bisect terms --new=slow --old=fast\n\nIf we don't want to support positional arguments, then I would suggest\nsupporting first the following instead:\n\n git bisect terms --name-good=fast --name-bad=slow\n git bisect terms --name-bad=slow --name-good=fast\n\nThis would make the interface consistent with the code.\n\nOf course we could also accept --name-old and --name-new as synonyms\nfor --name-good and --name-bad.\n\n> We may want to start supporting\n>\n> git bisect start --new=master --old=maint\n\nMaybe we could also support:\n\ngit bisect start --name-good=fast --name-bad=slow --fast=maint --slow=master\n"},{"id":"265021","messageId":"CAPc5daXSnovQPxR5kVoeRy4kApH7DiuTDvAz1ooQFp=5DZZg4Q@mail.gmail.com","threadId":"39732","inReplyTo":"CAP8UFD1PyS_qM3EHW_Nzmo=3aeTDkZ0M3hnhRQANAO+ShF3H1Q@mail.gmail.com","subject":"Re: [PATCH v10.1 7/7] bisect: allow any terms set by user","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-27T04:25:12Z","receivedAt":"2015-06-27T04:25:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"On Fri, Jun 26, 2015 at 9:10 PM, Christian Couder\n wrote:\n>\n> If we don't want to support positional arguments, then I would suggest\n> supporting first the following instead:\n>\n> git bisect terms --name-good=fast --name-bad=slow\n> git bisect terms --name-bad=slow --name-good=fast\n>\n> This would make the interface consistent with the code.\n\nWhich somewhat defeats the point of introducing \"old\" and \"new\", though.\nThe \"terms\" support is for people who feel that good/bad would be too confusing\nfor the particular bisect session (e.g. because they are hunting for a fix).\n\n>> We may want to start supporting\n>>\n>> git bisect start --new=master --old=maint\n>\n> Maybe we could also support:\n>\n> git bisect start --name-good=fast --name-bad=slow --fast=maint --slow=master\n\nThe same comment for the token after --name-, but allowing the terms to be set\nat \"start\" could be a type-saver. With need for added \"--name-\"\nprefix (worse, twice),\nI am not sure if it would be seen as a useful type-saver, though.\n\nThanks.\n"},{"id":"265022","messageId":"CAP8UFD0Zzjcc94qcUJOpfSrTej-o6hRKzZOuTufzAbOPkjUr_A@mail.gmail.com","threadId":"39732","inReplyTo":"CAPc5daXSnovQPxR5kVoeRy4kApH7DiuTDvAz1ooQFp=5DZZg4Q@mail.gmail.com","subject":"Re: [PATCH v10.1 7/7] bisect: allow any terms set by user","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2015-06-27T04:51:20Z","receivedAt":"2015-06-27T04:51:20Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Sat, Jun 27, 2015 at 6:25 AM, Junio C Hamano wrote:\n> On Fri, Jun 26, 2015 at 9:10 PM, Christian Couder\n> wrote:\n>>\n>> If we don't want to support positional arguments, then I would suggest\n>> supporting first the following instead:\n>>\n>> git bisect terms --name-good=fast --name-bad=slow\n>> git bisect terms --name-bad=slow --name-good=fast\n>>\n>> This would make the interface consistent with the code.\n>\n> Which somewhat defeats the point of introducing \"old\" and \"new\", though.\n> The \"terms\" support is for people who feel that good/bad would be too confusing\n> for the particular bisect session (e.g. because they are hunting for a fix).\n\nWell if --name-old and --name-new are also available as synonyms, it\nwould not be too bad I think.\nPeople could use the option names that fit their mental model or their\nuse case better.\n\n>>> We may want to start supporting\n>>>\n>>> git bisect start --new=master --old=maint\n>>\n>> Maybe we could also support:\n>>\n>> git bisect start --name-good=fast --name-bad=slow --fast=maint --slow=master\n>\n> The same comment for the token after --name-, but allowing the terms to be set\n> at \"start\" could be a type-saver. With need for added \"--name-\"\n> prefix (worse, twice),\n> I am not sure if it would be seen as a useful type-saver, though.\n\nAt least people don't need to remember if they have to use \"git bisect\nterm\" before or after starting :-)\n"},{"id":"265026","messageId":"vpq7fqppn5h.fsf@anie.imag.fr","threadId":"39732","inReplyTo":"CAP8UFD0Zzjcc94qcUJOpfSrTej-o6hRKzZOuTufzAbOPkjUr_A@mail.gmail.com","subject":"Re: [PATCH v10.1 7/7] bisect: allow any terms set by user","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-06-27T08:32:10Z","receivedAt":"2015-06-27T08:32:10Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Christian Couder writes:\n\n> On Sat, Jun 27, 2015 at 6:25 AM, Junio C Hamano wrote:\n>> On Fri, Jun 26, 2015 at 9:10 PM, Christian Couder\n>> wrote:\n>>>\n>>> If we don't want to support positional arguments, then I would suggest\n>>> supporting first the following instead:\n>>>\n>>> git bisect terms --name-good=fast --name-bad=slow\n>>> git bisect terms --name-bad=slow --name-good=fast\n>>>\n>>> This would make the interface consistent with the code.\n>>\n>> Which somewhat defeats the point of introducing \"old\" and \"new\", though.\n>> The \"terms\" support is for people who feel that good/bad would be too confusing\n>> for the particular bisect session (e.g. because they are hunting for a fix).\n>\n> Well if --name-old and --name-new are also available as synonyms, it\n> would not be too bad I think.\n> People could use the option names that fit their mental model or their\n> use case better.\n\nOK, I'll add both.\n\n>>>> We may want to start supporting\n>>>>\n>>>> git bisect start --new=master --old=maint\n>>>\n>>> Maybe we could also support:\n>>>\n>>> git bisect start --name-good=fast --name-bad=slow --fast=maint --slow=master\n>>\n>> The same comment for the token after --name-, but allowing the terms to be set\n>> at \"start\" could be a type-saver. With need for added \"--name-\"\n>> prefix (worse, twice),\n>> I am not sure if it would be seen as a useful type-saver, though.\n>\n> At least people don't need to remember if they have to use \"git bisect\n> term\" before or after starting :-)\n\nOK, first lesson learnt: the UI is controversial. So, I'm working on a\nseries that splits the last patch into several preparation steps, and a\nvery simple patch to implement the UI.\n\nI have a draft here https://github.com/moy/git/commits/bisect-terms\nI'll go through it once more and send it later.\n\nAs a teaser, the patch implementing the UI is just:\n\n--- a/git-bisect.sh\n+++ b/git-bisect.sh\n@@ -70,6 +70,26 @@ bisect_autostart() {\n \t}\n }\n \n+parse_name_args() {\n+\twhile [ $# -gt 0 ]; do\n+\t\targ=\"$1\"\n+\t\tcase \"$arg\" in\n+\t\t--name-good|--name-old)\n+\t\t\tshift\n+\t\t\tmust_write_terms=1\n+\t\t\tNAME_GOOD=$1\n+\t\t\tshift ;;\n+\t\t--name-bad|--name-new)\n+\t\t\tshift\n+\t\t\tmust_write_terms=1\n+\t\t\tNAME_BAD=$1\n+\t\t\tshift ;;\n+\t\t*)\n+\t\t\tshift ;;\n+\t\tesac\n+\tdone\n+}\n+\n bisect_start() {\n \t#\n \t# Check for one bad and then some good revisions.\n@@ -88,6 +108,9 @@ bisect_start() {\n \telse\n \t\tmode=''\n \tfi\n+\t# Parse --name-* options before the other to allow any mix of\n+\t# --name-* and revisions on the command-line.\n+\tparse_name_args \"$@\"\n \twhile [ $# -gt 0 ]; do\n \t\targ=\"$1\"\n \t\tcase \"$arg\" in\n@@ -98,6 +121,8 @@ bisect_start() {\n \t\t--no-checkout)\n \t\t\tmode=--no-checkout\n \t\t\tshift ;;\n+\t\t--name-good|--name-old|--name-bad|--name-new)\n+\t\t\tshift 2 ;;\n \t\t--*)\n \t\t\tdie \"$(eval_gettext \"unrecognised option: '\\$arg'\")\" ;;\n \t\t*)\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"265032","messageId":"xmqq8ub5f0z2.fsf@gitster.dls.corp.google.com","threadId":"39732","inReplyTo":"vpq7fqppn5h.fsf@anie.imag.fr","subject":"Re: [PATCH v10.1 7/7] bisect: allow any terms set by user","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-27T18:41:21Z","receivedAt":"2015-06-27T18:41:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy writes:\n\n> Christian Couder writes:\n>\n>> On Sat, Jun 27, 2015 at 6:25 AM, Junio C Hamano wrote:\n>>> On Fri, Jun 26, 2015 at 9:10 PM, Christian Couder\n>>> wrote:\n>>>>\n>>>> If we don't want to support positional arguments, then I would suggest\n>>>> supporting first the following instead:\n>>>>\n>>>> git bisect terms --name-good=fast --name-bad=slow\n>>>> git bisect terms --name-bad=slow --name-good=fast\n>>>>\n>>>> This would make the interface consistent with the code.\n>>>\n>>> Which somewhat defeats the point of introducing \"old\" and \"new\", though.\n>>> The \"terms\" support is for people who feel that good/bad would be too confusing\n>>> for the particular bisect session (e.g. because they are hunting for a fix).\n>>\n>> Well if --name-old and --name-new are also available as synonyms, it\n>> would not be too bad I think.\n>> People could use the option names that fit their mental model or their\n>> use case better.\n>\n> OK, I'll add both.\n\nI moderately hate to see both from aesthetics point of view, but can\nwe at least lose \"--name-\" prefix? Having to repeat that twice does\nnot add any value to the subcommand.\n"},{"id":"265039","messageId":"558F8B55.1070708@alum.mit.edu","threadId":"39732","inReplyTo":"CAPc5daXSnovQPxR5kVoeRy4kApH7DiuTDvAz1ooQFp=5DZZg4Q@mail.gmail.com","subject":"Re: [PATCH v10.1 7/7] bisect: allow any terms set by user","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2015-06-28T05:51:17Z","receivedAt":"2015-06-28T05:51:17Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 06/27/2015 06:25 AM, Junio C Hamano wrote:\n> On Fri, Jun 26, 2015 at 9:10 PM, Christian Couder\n> wrote:\n>>\n>> If we don't want to support positional arguments, then I would suggest\n>> supporting first the following instead:\n>>\n>> git bisect terms --name-good=fast --name-bad=slow\n>> git bisect terms --name-bad=slow --name-good=fast\n>>\n>> This would make the interface consistent with the code.\n> \n> Which somewhat defeats the point of introducing \"old\" and \"new\", though.\n> The \"terms\" support is for people who feel that good/bad would be too confusing\n> for the particular bisect session (e.g. because they are hunting for a fix).\n> \n>>> We may want to start supporting\n>>>\n>>> git bisect start --new=master --old=maint\n>>\n>> Maybe we could also support:\n>>\n>> git bisect start --name-good=fast --name-bad=slow --fast=maint --slow=master\n> \n> The same comment for the token after --name-, but allowing the terms to be set\n> at \"start\" could be a type-saver. With need for added \"--name-\"\n> prefix (worse, twice),\n> I am not sure if it would be seen as a useful type-saver, though.\n\nI would like to remind everybody of my old claim that it would be\npossible to teach `git bisect` to infer by itself which term means\n\"older\" and which term means \"newer\":\n\n http://article.gmane.org/gmane.comp.version-control.git/244036\n\nI think that making `bisect` smarter could make the UI simpler, though\nadmittedly it would be more work than the current proposal.\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\n"},{"id":"265040","messageId":"CAPc5daWmhkqDL0pNYne4-kRoxWK7ObcOKxtE5DsfHA2cnMM1pQ@mail.gmail.com","threadId":"39732","inReplyTo":"558F8B55.1070708@alum.mit.edu","subject":"Re: [PATCH v10.1 7/7] bisect: allow any terms set by user","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-28T06:15:49Z","receivedAt":"2015-06-28T06:15:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"On Sat, Jun 27, 2015 at 10:51 PM, Michael Haggerty wrote:\n>\n> I would like to remind everybody of my old claim that it would be\n> possible to teach `git bisect` to infer by itself which term means\n> \"older\" and which term means \"newer\":\n>\n> http://article.gmane.org/gmane.comp.version-control.git/244036\n\nBut then one mistake at the beginning and the user will be on a wrong\ntrack during the whole bisect session, no? Unless you make absolutely\nclear when making the \"intelligent\" decision what Git inferred, that is.\n\nFor something complex like bisect, I highly suspect that a tool that is\nmore intelligent than the end users (more precisely, a tool that it thinks\nit is more intelligent) would hurt them more than it helps them.\n\nOf course, that is only my claim ;-)\n"},{"id":"265041","messageId":"558F9854.5080605@alum.mit.edu","threadId":"39732","inReplyTo":"CAPc5daWmhkqDL0pNYne4-kRoxWK7ObcOKxtE5DsfHA2cnMM1pQ@mail.gmail.com","subject":"Re: [PATCH v10.1 7/7] bisect: allow any terms set by user","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2015-06-28T06:46:44Z","receivedAt":"2015-06-28T06:46:44Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 06/28/2015 08:15 AM, Junio C Hamano wrote:\n> On Sat, Jun 27, 2015 at 10:51 PM, Michael Haggerty wrote:\n>>\n>> I would like to remind everybody of my old claim that it would be\n>> possible to teach `git bisect` to infer by itself which term means\n>> \"older\" and which term means \"newer\":\n>>\n>> http://article.gmane.org/gmane.comp.version-control.git/244036\n> \n> But then one mistake at the beginning and the user will be on a wrong\n> track during the whole bisect session, no? Unless you make absolutely\n> clear when making the \"intelligent\" decision what Git inferred, that is.\n\nDefinitely, `git bisect` should tell the user what it inferred.\n\n> For something complex like bisect, I highly suspect that a tool that is\n> more intelligent than the end users (more precisely, a tool that it thinks\n> it is more intelligent) would hurt them more than it helps them.\n\nThis isn't about making bisect \"more intelligent than the end users\". It\nis about not forcing the user cumbersomely to spell out redundant\ninformation because the tool is too stupid.\n\nIf I mark one commit \"broken\" and another commit \"fixed\", and the\n\"broken\" commit is an ancestor of the \"fixed\" commit, then it is pretty\nobvious that I am looking for the commit that caused the transition\n\"broken\" -> \"fixed\". The same if I mark one commit \"xyzzy\" and the other\none \"plugh\".\n\nI understand that the user might make a mistake when marking the initial\ncommits, but as soon as bisect says\n\n Commit is an ancestor of , so I\n will look for the commit that caused the transition from\n \"xyzzy\" to \"plugh\".\n\nthen I hope the user will notice and correct her/his mistake.\n\nFor example, a session could be started with\n\n git bisect start --mark=broken --mark=fixed \n\nand from then on\n\n git bisect broken\n git bisect fixed\n\nOr, if the user doesn't want to specify both endpoints on the `start` line,\n\n git bisect start\n git bisect --mark=broken []\n git bisect --mark=fixed []\n\nEssentially, specifying `--mark=` once would make `` a\nshorthand for `--mark=`.\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\n"},{"id":"265043","messageId":"xmqqwpyoe1aj.fsf@gitster.dls.corp.google.com","threadId":"39732","inReplyTo":"558F9854.5080605@alum.mit.edu","subject":"Re: [PATCH v10.1 7/7] bisect: allow any terms set by user","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-28T07:32:04Z","receivedAt":"2015-06-28T07:32:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Haggerty writes:\n\n> I understand that the user might make a mistake when marking the initial\n> commits, but as soon as bisect says\n>\n> Commit is an ancestor of , so I\n> will look for the commit that caused the transition from\n> \"xyzzy\" to \"plugh\".\n>\n> then I hope the user will notice and correct her/his mistake.\n>\n> For example, a session could be started with\n>\n> git bisect start --mark=broken --mark=fixed \n\nInteresting.\n\nIf we extend that line of thought further, maybe we do not even need\nto add new/old, fixed/broken, or slow/fast.\n\nYou just _always_ say \"good\" or \"bad\". If something is slow, you\nsay \"bad\" and if something is fast, you say \"good\".\n\nIf you start \"git bisect start maint master\" (and as always, \"bad\"\ncomes before \"good\") because some operation is very slow in maint\nbut somehow is usably fast in master, you are automatically hunting\nfor a performance fix that you can cherry-pick to the maintenance\ntrack.\n\nNo need for \"bisect new\", \"bisect old\", or \"bisect terms\", let alone\n\"bisect terms --new=fast --old=slow\". The tool just does the right\nthing because it already has the information necessary to infer what\nthe user means by 'good' and 'bad', and the initial topology determines\nwhich transition, either from 'good' to 'bad' or from 'bad' to 'good',\nthe user is hunting for.\n\nI really like that simplicity.\n"},{"id":"265047","messageId":"558FDAF9.3010300@alum.mit.edu","threadId":"39732","inReplyTo":"xmqqwpyoe1aj.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v10.1 7/7] bisect: allow any terms set by user","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2015-06-28T11:31:05Z","receivedAt":"2015-06-28T11:31:05Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 06/28/2015 09:32 AM, Junio C Hamano wrote:\n> Michael Haggerty writes:\n> \n>> I understand that the user might make a mistake when marking the initial\n>> commits, but as soon as bisect says\n>>\n>> Commit is an ancestor of , so I\n>> will look for the commit that caused the transition from\n>> \"xyzzy\" to \"plugh\".\n>>\n>> then I hope the user will notice and correct her/his mistake.\n>>\n>> For example, a session could be started with\n>>\n>> git bisect start --mark=broken --mark=fixed \n> \n> Interesting.\n> \n> If we extend that line of thought further, maybe we do not even need\n> to add new/old, fixed/broken, or slow/fast.\n> \n> You just _always_ say \"good\" or \"bad\". If something is slow, you\n> say \"bad\" and if something is fast, you say \"good\".\n\nYes, I think \"good\" and \"bad\" would usually be perfectly intuitive and\nwould almost always be usable.\n\n> [...]\n> No need for \"bisect new\", \"bisect old\", or \"bisect terms\", let alone\n> \"bisect terms --new=fast --old=slow\". The tool just does the right\n> thing because it already has the information necessary to infer what\n> the user means by 'good' and 'bad', and the initial topology determines\n> which transition, either from 'good' to 'bad' or from 'bad' to 'good',\n> the user is hunting for.\n\nCorrect. The only caveat is if the initial \"good\" and \"bad\" commits are\nnot ancestrally related to each other. But in this case, I think\n\"bisect\" asks the user to test a merge base anyway, and once that one\nhas been tested it will be clear which of the labels comes \"before\" the\nother.\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\n"},{"id":"265096","messageId":"xmqqh9prekdw.fsf@gitster.dls.corp.google.com","threadId":"39732","inReplyTo":"558FDAF9.3010300@alum.mit.edu","subject":"Re: [PATCH v10.1 7/7] bisect: allow any terms set by user","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-28T18:51:55Z","receivedAt":"2015-06-28T18:51:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Haggerty writes:\n\n> On 06/28/2015 09:32 AM, Junio C Hamano wrote:\n>> \n>> You just _always_ say \"good\" or \"bad\". If something is slow, you\n>> say \"bad\" and if something is fast, you say \"good\".\n>\n> Yes, I think \"good\" and \"bad\" would usually be perfectly intuitive and\n> would almost always be usable.\n>\n>> [...]\n>> No need for \"bisect new\", \"bisect old\", or \"bisect terms\", let alone\n>> \"bisect terms --new=fast --old=slow\". The tool just does the right\n>> thing because it already has the information necessary to infer what\n>> the user means by 'good' and 'bad', and the initial topology determines\n>> which transition, either from 'good' to 'bad' or from 'bad' to 'good',\n>> the user is hunting for.\n>\n> Correct. The only caveat is if the initial \"good\" and \"bad\" commits are\n> not ancestrally related to each other. But in this case, I think\n> \"bisect\" asks the user to test a merge base anyway, and once that one\n> has been tested it will be clear which of the labels comes \"before\" the\n> other.\n\nThe more I look at the proposal, the more I like it. The old way of\nthinking is that we need to keep 'bad' for newer one and 'good' for\nolder one, that required us to invent 'broken' vs 'fixed', or value\nneutral 'old' vs 'new'. Then we extend it to a random pair of\n'terms', but we reserve 'good', 'bad', etc. and do not allow the\nuser to say \"old was bad, new is now good\". With your proposal, the\nuser can just say \"oh this is good\", vs \"oh this is bad\". The\nmental model becomes much simpler.\n\nI _think_ bulk of Antoine and Matthieu's work can be salvaged/reused\nto implement the proposal, but now it would be more clear that\n$name_good and $name_bad is a bad way to name internal variables and\nfiles in $GIT_DIR. The inferred 'ah you are hunting for regression'\nmode would call old ones 'bad' and new ones 'good', they have to be\ngiven value neutral names, e.g. $name_old and $name_new.\n"},{"id":"265109","messageId":"CAP8UFD0NjExQJHQWix1+zc4_k15e3+aC5vAv=dHipg-X+2y1sQ@mail.gmail.com","threadId":"39732","inReplyTo":"558F9854.5080605@alum.mit.edu","subject":"Re: [PATCH v10.1 7/7] bisect: allow any terms set by user","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2015-06-29T05:08:26Z","receivedAt":"2015-06-29T05:08:26Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Sun, Jun 28, 2015 at 8:46 AM, Michael Haggerty wrote:\n> On 06/28/2015 08:15 AM, Junio C Hamano wrote:\n>> On Sat, Jun 27, 2015 at 10:51 PM, Michael Haggerty wrote:\n>>>\n>>> I would like to remind everybody of my old claim that it would be\n>>> possible to teach `git bisect` to infer by itself which term means\n>>> \"older\" and which term means \"newer\":\n>>>\n>>> http://article.gmane.org/gmane.comp.version-control.git/244036\n>>\n>> But then one mistake at the beginning and the user will be on a wrong\n>> track during the whole bisect session, no? Unless you make absolutely\n>> clear when making the \"intelligent\" decision what Git inferred, that is.\n>\n> Definitely, `git bisect` should tell the user what it inferred.\n>\n>> For something complex like bisect, I highly suspect that a tool that is\n>> more intelligent than the end users (more precisely, a tool that it thinks\n>> it is more intelligent) would hurt them more than it helps them.\n>\n> This isn't about making bisect \"more intelligent than the end users\". It\n> is about not forcing the user cumbersomely to spell out redundant\n> information because the tool is too stupid.\n>\n> If I mark one commit \"broken\" and another commit \"fixed\", and the\n> \"broken\" commit is an ancestor of the \"fixed\" commit, then it is pretty\n> obvious that I am looking for the commit that caused the transition\n> \"broken\" -> \"fixed\". The same if I mark one commit \"xyzzy\" and the other\n> one \"plugh\".\n>\n> I understand that the user might make a mistake when marking the initial\n> commits, but as soon as bisect says\n>\n> Commit is an ancestor of , so I\n> will look for the commit that caused the transition from\n> \"xyzzy\" to \"plugh\".\n>\n> then I hope the user will notice and correct her/his mistake.\n\nThis looks fragile to me. Unfortunately many users will probably not\nread it and continue, and then spend a lot of time later trying to\nunderstand what went wrong, not remembering about the message at all.\n\nThe message looks like an informative message. At least we should add\nsomething like \"Please check that it is what you want to do and abort\nwith 'git bisect reset' if it is not.\"\n\n> For example, a session could be started with\n>\n> git bisect start --mark=broken --mark=fixed \n\nThis look nearly the same as:\n\ngit bisect start --name-old=broken --broken=\n--name-new=fixed --fixed=\n\nexcept that it looks safer and more backward compatible to me with\n--name-old and --name-new.\n\nBy the way we could use \"mark\" or \"term\" instead of \"name\" in the\noption name (like --mark-old or --term-old) and in the code too if it\nlooks clearer.\n\n> and from then on\n>\n> git bisect broken\n> git bisect fixed\n>\n> Or, if the user doesn't want to specify both endpoints on the `start` line,\n>\n> git bisect start\n> git bisect --mark=broken []\n> git bisect --mark=fixed []\n\nWe could do that too with:\n\n git bisect start\n git bisect --name-old=broken broken []\n git bisect --name-new=fixed fixed []\n\nand/or:\n\n git bisect start\n git bisect --name-old=broken --broken=[]\n git bisect --name-new=fixed --fixed=[]\n"},{"id":"265112","messageId":"vpqegkvnfe5.fsf@anie.imag.fr","threadId":"39732","inReplyTo":"xmqqh9prekdw.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v10.1 7/7] bisect: allow any terms set by user","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-06-29T07:27:14Z","receivedAt":"2015-06-29T07:27:14Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano writes:\n\n> I _think_ bulk of Antoine and Matthieu's work can be salvaged/reused\n> to implement the proposal,\n\nI'm obviously biaised since I already spent time on the \"bisect terms\"\nidea, and I would hate to see my work and Antoine & Louis' thrown away.\nBut I have to admit that I do like the idea of \"git bisect\" figuring out\nwhich commit is the old and which commit is the new one.\n\nIt's much easier to implement after the series. I'm currently forbidding\nredefining \"good\" as \"bad\" and vice versa, but that's just to avoid\nconfusion and because I didn't test this case, but it should just work.\nSo, essentially an implementation of this \"guess who's old and who's\nnew\" could be: if we find a good commit that is an ancestor of an old\ncommit, swap the terms in BISECT_TERMS. When this happens before we\nstarted to set any refs, this should do the trick. In general, we should\nrename the good-$sha1 reference to good and the bad to bad-$sha1 (there\nare corner-cases where the user already specified several good-$sha1\nrefs, in which case we would need to discard some of them).\n\nI'm getting out of Git time budget, so I can't be the one doing this, at\nleast not soon.\n\nSo, one option is to take the series as-is, and wait for someone to\nimplement the \"guess who's old and who's new\" later on top of it. One\ndrawback would be that we'd end up having the not-so-useful feature\n\"bisect terms\" in the user-interface. At least, I am now convinced that\nhardcoding the pair old/new is not needed. In the short term, we can\nhave \"git bisect start --name-old foo --name-new bar\" which avoids the\n\"One needs to remember in which order to give terms\" issue we used to\nhave, so we don't need to clutter the user-interface with many ways to\ndo the same thing. OTOH, the \"bisect terms\" feature wouldn't be\ncompletely useless: not everything is good or bad, so leaving the option\nto the user to tag \"slow/fast\", \"present/absent\", ... still makes sense.\n\nSo, my proposal would be to remove the \"old/new\" patch from the series,\nand keep the other patches.\n\nWhat do you think?\n\n> but now it would be more clear that $name_good and $name_bad is a bad\n> way to name internal variables and files in $GIT_DIR. The inferred 'ah\n> you are hunting for regression' mode would call old ones 'bad' and new\n> ones 'good', they have to be given value neutral names, e.g. $name_old\n> and $name_new.\n\nIdeally, the whole code would be ported to use old/new, but the more I\nread the code the more I agree with Christian actually: we still have\nmany more instances of good/bad in the code (e.g. functions\ncheck_good_are_ancestors_of_bad, for_each_good_bisect_ref, ...), so\nhaving just name_new/name_old would make the code very inconsistant.\nIt's easier to read the code thinking \"good revs are old, bad revs are\nrecent; maybe some magic swapped the terms but I don't need to worry\nabout this\" for now.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"265113","messageId":"vpq1tgvnf2g.fsf@anie.imag.fr","threadId":"39732","inReplyTo":"CAP8UFD0NjExQJHQWix1+zc4_k15e3+aC5vAv=dHipg-X+2y1sQ@mail.gmail.com","subject":"Re: [PATCH v10.1 7/7] bisect: allow any terms set by user","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-06-29T07:34:15Z","receivedAt":"2015-06-29T07:34:15Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Christian Couder writes:\n\n> On Sun, Jun 28, 2015 at 8:46 AM, Michael Haggerty wrote:\n>> I understand that the user might make a mistake when marking the initial\n>> commits, but as soon as bisect says\n>>\n>> Commit is an ancestor of , so I\n>> will look for the commit that caused the transition from\n>> \"xyzzy\" to \"plugh\".\n>>\n>> then I hope the user will notice and correct her/his mistake.\n>\n> This looks fragile to me. Unfortunately many users will probably not\n> read it and continue, and then spend a lot of time later trying to\n> understand what went wrong,\n\nI don't understand what you mean by \"went wrong\". As a user, when I\ndiscovered \"git bisect\", I was actually surprised that it expected one\nparticular order between good and bad. I would have expected to be able\nto say \"this is good, this is bad, tell me where it changed\" without\nhaving an idea of who's good and who's bad. In particular when bisecting\nfrom two branches, the user knows that branch A is good, and branch B is\nbad, but does not necessarily know whether it's a regression in B or a\nfix in A. The fact that bisect can find out should be just \"normal\" from\nthe user point of view. There's no mistake involved, nothing to fix, and\nnothing that went wrong.\n\n> By the way we could use \"mark\" or \"term\" instead of \"name\" in the\n> option name (like --mark-old or --term-old) and in the code too if it\n> looks clearer.\n\nI prefer \"term\" to \"mark\" because \"mark\" is both a verb and a noun, so\n--mark-old=foo could mean both \"mark foo as old\" or \"the name of the\nmarks for old commits is foo\".\n\nI think I prefer \"term\" to \"name\".\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"265114","messageId":"CAP8UFD1gWXOZHfiELZYgvGP1dA-aeaxbyaokKCut9pfWghy9uQ@mail.gmail.com","threadId":"39732","inReplyTo":"vpq1tgvnf2g.fsf@anie.imag.fr","subject":"Re: [PATCH v10.1 7/7] bisect: allow any terms set by user","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2015-06-29T08:08:42Z","receivedAt":"2015-06-29T08:08:42Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Mon, Jun 29, 2015 at 9:34 AM, Matthieu Moy\n wrote:\n> Christian Couder writes:\n>\n>> On Sun, Jun 28, 2015 at 8:46 AM, Michael Haggerty wrote:\n>>> I understand that the user might make a mistake when marking the initial\n>>> commits, but as soon as bisect says\n>>>\n>>> Commit is an ancestor of , so I\n>>> will look for the commit that caused the transition from\n>>> \"xyzzy\" to \"plugh\".\n>>>\n>>> then I hope the user will notice and correct her/his mistake.\n>>\n>> This looks fragile to me. Unfortunately many users will probably not\n>> read it and continue, and then spend a lot of time later trying to\n>> understand what went wrong,\n>\n> I don't understand what you mean by \"went wrong\".\n\nIt happens that users mistake the \"good\" and the \"bad\" commits when\ngiving them to git bisect.\n\nRight now in the most common case, we can error out because we know\nthat a \"bad\" commit cannot be an ancestor of a \"good\" commit.\n\n> As a user, when I\n> discovered \"git bisect\", I was actually surprised that it expected one\n> particular order between good and bad. I would have expected to be able\n> to say \"this is good, this is bad, tell me where it changed\" without\n> having an idea of who's good and who's bad.\n\nMaybe, but it's not how it has been developed.\n\n> In particular when bisecting\n> from two branches, the user knows that branch A is good, and branch B is\n> bad, but does not necessarily know whether it's a regression in B or a\n> fix in A. The fact that bisect can find out should be just \"normal\" from\n> the user point of view. There's no mistake involved, nothing to fix, and\n> nothing that went wrong.\n\nWell in this case, it's possible that the merge base is bad and what\nthe user is interested in is the first bad commit that was commited\nbefore the merge base. We just don't know, in the case the merge base\nis bad, what is more interesting for the user.\n\nSo I disagree with you and Michael that we should decide that the user\nis interested by the fix in this case. It's better to error out like\nwe do now and let the user decide what he/she wants rather than decide\nfor him/her that he/she is interested by the fix.\n\n>> By the way we could use \"mark\" or \"term\" instead of \"name\" in the\n>> option name (like --mark-old or --term-old) and in the code too if it\n>> looks clearer.\n>\n> I prefer \"term\" to \"mark\" because \"mark\" is both a verb and a noun, so\n> --mark-old=foo could mean both \"mark foo as old\" or \"the name of the\n> marks for old commits is foo\".\n>\n> I think I prefer \"term\" to \"name\".\n\nOk with that. I agree that it would be more consistent to have a \"git\nbisect terms\" and \"--term-{old,new,bad,good}\".\n\nThanks,\nChristian.\n"},{"id":"265117","messageId":"vpqsi9alv1l.fsf@anie.imag.fr","threadId":"39732","inReplyTo":"CAP8UFD1gWXOZHfiELZYgvGP1dA-aeaxbyaokKCut9pfWghy9uQ@mail.gmail.com","subject":"Re: [PATCH v10.1 7/7] bisect: allow any terms set by user","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-06-29T09:32:06Z","receivedAt":"2015-06-29T09:32:06Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Christian Couder writes:\n\n> On Mon, Jun 29, 2015 at 9:34 AM, Matthieu Moy\n>\n>> As a user, when I\n>> discovered \"git bisect\", I was actually surprised that it expected one\n>> particular order between good and bad. I would have expected to be able\n>> to say \"this is good, this is bad, tell me where it changed\" without\n>> having an idea of who's good and who's bad.\n>\n> Maybe, but it's not how it has been developed.\n\n... but it's how it would behave if we had this guessing feature. The\nuser does not have to care whether we internally need \"good is an\nancestor of bad\" if we can provide a user-interface which does not need\nthat.\n\nI find it particularly frustrating that we issue the message:\n\n \"The merge base %s is bad.\\n\"\n \"This means the bug has been fixed \"\n \"between %s and [%s].\\n\"\n\nbisect is all about finding the commit where a property has changed, and\nhere it stops, saying \"I know it's between A and B, but I won't go\nfurther\".\n\n>> In particular when bisecting from two branches, the user knows that\n>> branch A is good, and branch B is bad, but does not necessarily know\n>> whether it's a regression in B or a\n>> fix in A. The fact that bisect can find out should be just \"normal\" from\n>> the user point of view. There's no mistake involved, nothing to fix, and\n>> nothing that went wrong.\n>\n> Well in this case, it's possible that the merge base is bad and what\n> the user is interested in is the first bad commit that was commited\n> before the merge base. We just don't know, in the case the merge base\n> is bad, what is more interesting for the user.\n\nThe question asked by the user is \"X is good, Y is bad, tell me where\nexactly it changed\". We can't know for sure what is \"interesting\" for\nthe user, but we can answer his question anyway.\n\nSimilarly, there can be several commits that introduce the same\nregression (this may happen if you cherry picked the guilty commit from\nbranch A to branch B, and then merged A and B, or if you broke, fixed,\nand then broke again). bisect finds one transition from good to bad, but\nnot all. It may or may not be the one the user wanted, but we can't\nknow.\n\n>> I think I prefer \"term\" to \"name\".\n>\n> Ok with that. I agree that it would be more consistent to have a \"git\n> bisect terms\" and \"--term-{old,new,bad,good}\".\n\nOK, I've applied it.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"265118","messageId":"vpq381aetas.fsf@anie.imag.fr","threadId":"39732","inReplyTo":"xmqq8ub5f0z2.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v10.1 7/7] bisect: allow any terms set by user","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-06-29T09:51:39Z","receivedAt":"2015-06-29T09:51:39Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano writes:\n\n> I moderately hate to see both from aesthetics point of view, but can\n> we at least lose \"--name-\" prefix?\n\nI changed it to --term- prefix, but I'd rather not drop it. When reading\n\"--old=foo\", it is not clear to me whether the meaning should be \"the\nterm used for old is foo\" or \"mark foo as old\". The longer version does\nnot have this problem.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"265119","messageId":"CAP8UFD23RTCzrv8j8Jbcx5ajMUjgVsO3P5_Fiv3Vwx5uaPwXrw@mail.gmail.com","threadId":"39732","inReplyTo":"vpqsi9alv1l.fsf@anie.imag.fr","subject":"Re: [PATCH v10.1 7/7] bisect: allow any terms set by user","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2015-06-29T10:55:26Z","receivedAt":"2015-06-29T10:55:26Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Mon, Jun 29, 2015 at 11:32 AM, Matthieu Moy\n wrote:\n> Christian Couder writes:\n\n[...]\n\n> I find it particularly frustrating that we issue the message:\n>\n> \"The merge base %s is bad.\\n\"\n> \"This means the bug has been fixed \"\n> \"between %s and [%s].\\n\"\n\nI find it a good safety feature.\n\n> bisect is all about finding the commit where a property has changed,\n\nThat is your interpretation of this command. On the man page there is:\n\n git-bisect - Find by binary search the change that introduced a bug\n\nSo its stated purpose is to find the first \"bad\" commit. Not to find a fix.\n\nThis doesn't mean that we cannot or should not make it possible to\nfind a fix, but we shoudn't try to find a fix when there is doubt\nabout whether the user wants to find a fix or the first bad commit.\n\n> and\n> here it stops, saying \"I know it's between A and B, but I won't go\n> further\".\n\nYou can interpret it that way, but my interpretation of this is that\nthe real reason why we stop is because we don't know what the users\nreally wants to do, find the fix, or really find the first bad commit.\n\n>>> In particular when bisecting from two branches, the user knows that\n>>> branch A is good, and branch B is bad, but does not necessarily know\n>>> whether it's a regression in B or a\n>>> fix in A. The fact that bisect can find out should be just \"normal\" from\n>>> the user point of view. There's no mistake involved, nothing to fix, and\n>>> nothing that went wrong.\n>>\n>> Well in this case, it's possible that the merge base is bad and what\n>> the user is interested in is the first bad commit that was commited\n>> before the merge base. We just don't know, in the case the merge base\n>> is bad, what is more interesting for the user.\n>\n> The question asked by the user is \"X is good, Y is bad, tell me where\n> exactly it changed\".\n\nThat's your interpretation of the question asked by the user.\nIt can be interpreted otherwise.\n\n> We can't know for sure what is \"interesting\" for\n> the user, but we can answer his question anyway.\n\nYou can answer your interpretation of the question, yes, but this\nmeans taking risks that we don't need to take in my opinion.\n\nIf people really want it, we can still have an option called for\nexample --find-fix that could automatically try to find the fix when\nthe merge base is \"bad\" without displaying the message that annoys\nyou. It would make it explicit that it is ok to find a fix.\n\n> Similarly, there can be several commits that introduce the same\n> regression (this may happen if you cherry picked the guilty commit from\n> branch A to branch B, and then merged A and B, or if you broke, fixed,\n> and then broke again). bisect finds one transition from good to bad, but\n> not all. It may or may not be the one the user wanted, but we can't\n> know.\n\nYeah, but this is different as we would still find a first \"bad\"\ncommit, not a fix.\n\n>>> I think I prefer \"term\" to \"name\".\n>>\n>> Ok with that. I agree that it would be more consistent to have a \"git\n>> bisect terms\" and \"--term-{old,new,bad,good}\".\n>\n> OK, I've applied it.\n\nGreat!\n"},{"id":"265125","messageId":"vpqlhf2czk6.fsf@anie.imag.fr","threadId":"39732","inReplyTo":"CAP8UFD23RTCzrv8j8Jbcx5ajMUjgVsO3P5_Fiv3Vwx5uaPwXrw@mail.gmail.com","subject":"Re: [PATCH v10.1 7/7] bisect: allow any terms set by user","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-06-29T15:19:21Z","receivedAt":"2015-06-29T15:19:21Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Christian Couder writes:\n\n> On Mon, Jun 29, 2015 at 11:32 AM, Matthieu Moy\n> wrote:\n>> bisect is all about finding the commit where a property has changed,\n>\n> That is your interpretation of this command. On the man page there is:\n>\n> git-bisect - Find by binary search the change that introduced a bug\n>\n> So its stated purpose is to find the first \"bad\" commit. Not to find a fix.\n\nThis is a limitation of the current bisect, but the discussion is\nprecisely about removing this limitation.\n\nI still don't understand what \"risk\" we are taking by doing the\nbisection anyway. I can't imagine a case where we would harm the user by\ndoing so.\n\nI just tested with Mercurial, and looking for a fix instead of a\nregression just works:\n\n$ hg bisect --good 4 \n$ hg bisect --bad 1 \nTesting changeset 2:d75a2d042c99 (3 changesets remaining, ~1 tests)\n1 files updated, 0 files merged, 0 files removed, 0 files unresolved\n$ hg bisect --bad \nTesting changeset 3:9d27d9c02e28 (2 changesets remaining, ~1 tests)\n1 files updated, 0 files merged, 0 files removed, 0 files unresolved\n$ hg bisect --bad\nThe first good revision is:\nchangeset: 4:1dd9bb959eb6\ntag: tip\nuser: Matthieu Moy \ndate: Mon Jun 29 17:07:51 2015 +0200\nsummary: foo\n\nI don't see anything wrong with this.\n\n(OTOH, \"hg bisect\" does not accept revisions which aren't parent of each\nother)\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"265140","messageId":"xmqqd20ecw1v.fsf@gitster.dls.corp.google.com","threadId":"39732","inReplyTo":"vpq381aetas.fsf@anie.imag.fr","subject":"Re: [PATCH v10.1 7/7] bisect: allow any terms set by user","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-29T16:35:08Z","receivedAt":"2015-06-29T16:35:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy writes:\n\n> Junio C Hamano writes:\n>\n>> I moderately hate to see both from aesthetics point of view, but can\n>> we at least lose \"--name-\" prefix?\n>\n> I changed it to --term- prefix, but I'd rather not drop it. When reading\n> \"--old=foo\", it is not clear to me whether the meaning should be \"the\n> term used for old is foo\" or \"mark foo as old\". The longer version does\n> not have this problem.\n\nYeah, my suggestion was based on one assumption I did not mention,\nwhich is that we do not need to crowd \"bisect start\" with this\noption when we have \"bisect terms\", as long as \"bisect start\" does\nnot have to take both \"what are the terms\" and \"which commits are\npainted using which one of the two terms\" on its command line, the\n\"--name-\" prefix was unnecessary.\n\nBut if you are dropping \"bisect terms\" and allowing the terms\nspecified only from \"bisect start\" as you mentioned in the cover\nletter of v11, that changes the equation. And I think \"start is the\nplace that sets up a clean slate, and that is the only place where\nyou can optionally declare your custom terms\" is a very sensible\ndesign. I do not have a problem with \"--terms\" prefix in that case.\n\nThanks.\n"},{"id":"265141","messageId":"xmqq8ub2cvth.fsf@gitster.dls.corp.google.com","threadId":"39732","inReplyTo":"vpqegkvnfe5.fsf@anie.imag.fr","subject":"Re: [PATCH v10.1 7/7] bisect: allow any terms set by user","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-29T16:40:10Z","receivedAt":"2015-06-29T16:40:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy writes:\n\n> So, my proposal would be to remove the \"old/new\" patch from the series,\n> and keep the other patches.\n>\n> What do you think?\n\nLet me answer after reading v11 through.\n\n>> but now it would be more clear that $name_good and $name_bad is a bad\n>> way to name internal variables and files in $GIT_DIR. The inferred 'ah\n>> you are hunting for regression' mode would call old ones 'bad' and new\n>> ones 'good', they have to be given value neutral names, e.g. $name_old\n>> and $name_new.\n>\n> Ideally, the whole code would be ported to use old/new, but the more I\n> read the code the more I agree with Christian actually: we still have\n> many more instances of good/bad in the code (e.g. functions\n> check_good_are_ancestors_of_bad, for_each_good_bisect_ref, ...), so\n> having just name_new/name_old would make the code very inconsistant.\n\nOh, no question about that. I was hoping that we would at least get\nthe concensus that we should move all to old/new and these good/bad\nin code no longer make sense.\n\nIt just was that introducing new variables and functions whose names\nfollow the convention that reflects the world view that is no longer\nvalid (i.e. good is always old and bad is always new) in a series\nthat introduces the new world view somehow felt wrong.\n"}]}