{"thread":{"id":"48270","subject":"[PATCH] Create '--merges-only' option for 'git bisect'","startedAt":"2018-04-11T22:56:17Z","lastAt":"2018-05-01T11:31:20Z","messageCount":12,"participants":["Harald Nordgren","Stefan Beller","Christian Couder","Tiago Botelho","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"344503","messageId":"20180411225534.48658-1-haraldnordgren@gmail.com","threadId":"48270","inReplyTo":null,"subject":"[PATCH] Create '--merges-only' option for 'git bisect'","fromName":"Harald Nordgren","fromEmail":"haraldnordgren@gmail.com","sentAt":"2018-04-11T22:55:34Z","receivedAt":"2018-04-11T22:56:17Z","isPatch":true,"sender":{"key":"haraldnordgren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9569897?v=4"},"body":"When ran with '--merges-only', git bisect will only look at merge commits -- commits with 2 or more parents or the initial commit.\n\nSigned-off-by: Harald Nordgren <haraldnordgren@gmail.com>\n---\n\nNotes:\n    Proof of concept of a feature that I have wanted in Git for a while. In my daily work my company uses GitHub, which creates lots of merge commits. In general, tests are only ran on the tip of a branch before merging, so the different commits within a merge commit are allowed not to be buildable. Therefore 'git bisect' often doesn't work since it will give lots of false positives for anything that is not a merge commit. If we could have a feature to only bisect merge commits then it would be easier to pinpoint which merge causes any particular issue. After that, a bisect could be done within the merge to pinpoint futher. As a follow-up to this patch -- we could potentially create a feature that automatically continues into regular bisect within the bad merge commit after completed '--merges-only' bisection.\n\n bisect.c                        |  87 ++++++++++++++++++++++++++----\n bisect.h                        |   4 +-\n builtin/bisect--helper.c        |  28 +++++++++-\n builtin/rev-list.c              |   2 +-\n git-bisect.sh                   |   5 ++\n t/t6070-bisect-merge-commits.sh | 116 ++++++++++++++++++++++++++++++++++++++++\n 6 files changed, 228 insertions(+), 14 deletions(-)\n create mode 100755 t/t6070-bisect-merge-commits.sh\n\ndiff --git a/bisect.c b/bisect.c\nindex a579b5088..e8a2f77c7 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -254,7 +254,7 @@ static struct commit_list *best_bisection_sorted(struct commit_list *list, int n\n  */\n static struct commit_list *do_find_bisection(struct commit_list *list,\n \t\t\t\t\t     int nr, int *weights,\n-\t\t\t\t\t     int find_all)\n+\t\t\t\t\t     int find_all, int only_merge_commits)\n {\n \tint n, counted;\n \tstruct commit_list *p;\n@@ -266,6 +266,13 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\n \t\tunsigned flags = commit->object.flags;\n \n \t\tp->item->util = &weights[n++];\n+\n+\t\tif (only_merge_commits) {\n+\t\t\tweight_set(p, -2);\n+\t\t\tcounted++;\n+\t\t\tcontinue;\n+\t\t}\n+\n \t\tswitch (count_interesting_parents(commit)) {\n \t\tcase 0:\n \t\t\tif (!(flags & TREESAME)) {\n@@ -305,11 +312,17 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\n \t * way, and then fill the blanks using cheaper algorithm.\n \t */\n \tfor (p = list; p; p = p->next) {\n+\t\tint distance;\n \t\tif (p->item->object.flags & UNINTERESTING)\n \t\t\tcontinue;\n \t\tif (weight(p) != -2)\n \t\t\tcontinue;\n-\t\tweight_set(p, count_distance(p));\n+\n+\t\tif (only_merge_commits)\n+\t\t\tdistance = count_distance(p) - 1;\n+\t\telse\n+\t\t\tdistance = count_distance(p);\n+\t\tweight_set(p, distance);\n \t\tclear_distance(list);\n \n \t\t/* Does it happen to be at exactly half-way? */\n@@ -330,11 +343,17 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\n \t\t\tfor (q = p->item->parents; q; q = q->next) {\n \t\t\t\tif (q->item->object.flags & UNINTERESTING)\n \t\t\t\t\tcontinue;\n+\t\t\t\tif (!q->item->util)\n+\t\t\t\t\tbreak;\n \t\t\t\tif (0 <= weight(q))\n \t\t\t\t\tbreak;\n \t\t\t}\n \t\t\tif (!q)\n \t\t\t\tcontinue;\n+\t\t\tif (!q->item->util) {\n+\t\t\t\tcounted++;\n+\t\t\t\tcontinue;\n+\t\t\t}\n \n \t\t\t/*\n \t\t\t * weight for p is unknown but q is known.\n@@ -364,8 +383,43 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\n \t\treturn best_bisection_sorted(list, nr);\n }\n \n+int merge_commit_or_root(const struct commit c)\n+{\n+\tif (!c.parents)\n+\t\treturn 1;\n+\n+\treturn !!c.parents->next;\n+}\n+\n+void filter_non_merge_commits(struct commit_list **commit_list)\n+{\n+\tstruct commit_list *list1 = *commit_list;\n+\tstruct commit_list *list2 = NULL;\n+\t*commit_list = NULL;\n+\n+\tfor ( ; list1; list1 = list1->next) {\n+\t\tif (merge_commit_or_root(*list1->item)) {\n+\t\t\tlist2 = list1;\n+\t\t\tlist1 = list1->next;\n+\t\t\tlist2->next = NULL;\n+\t\t\tlist2->item->parents = NULL;\n+\t\t\t*commit_list = list2;\n+\t\t\tbreak;\n+\t\t}\n+\t}\n+\n+\tfor ( ; list1; list1 = list1->next) {\n+\t\tlist2->next = NULL;\n+\t\tif (merge_commit_or_root(*list1->item)) {\n+\t\t\tlist2->next = list1;\n+\t\t\tlist2 = list2->next;\n+\t\t\tlist2->item->parents = list1;\n+\t\t}\n+\t}\n+}\n+\n void find_bisection(struct commit_list **commit_list, int *reaches,\n-\t\t    int *all, int find_all)\n+\t\t    int *all, int find_all, int only_merge_commits)\n {\n \tint nr, on_list;\n \tstruct commit_list *list, *p, *best, *next, *last;\n@@ -373,6 +427,10 @@ void find_bisection(struct commit_list **commit_list, int *reaches,\n \n \tshow_list(\"bisection 2 entry\", 0, 0, *commit_list);\n \n+\tif (only_merge_commits) {\n+\t\tfilter_non_merge_commits(commit_list);\n+\t}\n+\n \t/*\n \t * Count the number of total and tree-changing items on the\n \t * list, while reversing the list.\n@@ -400,7 +458,7 @@ void find_bisection(struct commit_list **commit_list, int *reaches,\n \tweights = xcalloc(on_list, sizeof(*weights));\n \n \t/* Do the real work of finding bisection commit. */\n-\tbest = do_find_bisection(list, nr, weights, find_all);\n+\tbest = do_find_bisection(list, nr, weights, find_all, only_merge_commits);\n \tif (best) {\n \t\tif (!find_all) {\n \t\t\tlist->item = best->item;\n@@ -878,7 +936,7 @@ static void check_good_are_ancestors_of_bad(const char *prefix, int no_checkout)\n /*\n  * This does \"git diff-tree --pretty COMMIT\" without one fork+exec.\n  */\n-static void show_diff_tree(const char *prefix, struct commit *commit)\n+static void show_diff_tree(const char *prefix, struct commit *commit, int only_merge_commits)\n {\n \tstruct rev_info opt;\n \n@@ -893,6 +951,11 @@ static void show_diff_tree(const char *prefix, struct commit *commit)\n \topt.use_terminator = 0;\n \topt.commit_format = CMIT_FMT_DEFAULT;\n \n+\tif (only_merge_commits) {\n+\t\topt.ignore_merges = 0;\n+\t\topt.combine_merges = 1;\n+\t}\n+\n \t/* diff-tree init */\n \tif (!opt.diffopt.output_format)\n \t\topt.diffopt.output_format = DIFF_FORMAT_RAW;\n@@ -938,7 +1001,7 @@ void read_bisect_terms(const char **read_bad, const char **read_good)\n  * If no_checkout is non-zero, the bisection process does not\n  * checkout the trial commit but instead simply updates BISECT_HEAD.\n  */\n-int bisect_next_all(const char *prefix, int no_checkout)\n+int bisect_next_all(const char *prefix, int no_checkout, int only_merge_commits)\n {\n \tstruct rev_info revs;\n \tstruct commit_list *tried;\n@@ -957,7 +1020,7 @@ int bisect_next_all(const char *prefix, int no_checkout)\n \n \tbisect_common(&revs);\n \n-\tfind_bisection(&revs.commits, &reaches, &all, !!skipped_revs.nr);\n+\tfind_bisection(&revs.commits, &reaches, &all, !!skipped_revs.nr, only_merge_commits);\n \trevs.commits = managed_skipped(revs.commits, &tried);\n \n \tif (!revs.commits) {\n@@ -983,10 +1046,14 @@ int bisect_next_all(const char *prefix, int no_checkout)\n \tbisect_rev = &revs.commits->item->object.oid;\n \n \tif (!oidcmp(bisect_rev, current_bad_oid)) {\n+\t\tchar *format_string = NULL;\n \t\texit_if_skipped_commits(tried, current_bad_oid);\n-\t\tprintf(\"%s is the first %s commit\\n\", oid_to_hex(bisect_rev),\n-\t\t\tterm_bad);\n-\t\tshow_diff_tree(prefix, revs.commits->item);\n+\t\tif (only_merge_commits)\n+\t\t\tformat_string = \"%s is the first %s merge\\n\";\n+\t\telse\n+\t\t\tformat_string = \"%s is the first %s commit\\n\";\n+\t\tprintf(format_string, oid_to_hex(bisect_rev), term_bad);\n+\t\tshow_diff_tree(prefix, revs.commits->item, only_merge_commits);\n \t\t/* This means the bisection process succeeded. */\n \t\texit(10);\n \t}\ndiff --git a/bisect.h b/bisect.h\nindex a5d9248a4..664ada180 100644\n--- a/bisect.h\n+++ b/bisect.h\n@@ -9,7 +9,7 @@\n  * best commit, as chosen by `find_all`.\n  */\n extern void find_bisection(struct commit_list **list, int *reaches, int *all,\n-\t\t\t   int find_all);\n+\t\t\t   int find_all, int only_merge_commits);\n \n extern struct commit_list *filter_skipped(struct commit_list *list,\n \t\t\t\t\t  struct commit_list **tried,\n@@ -28,7 +28,7 @@ struct rev_list_info {\n \tconst char *header_prefix;\n };\n \n-extern int bisect_next_all(const char *prefix, int no_checkout);\n+extern int bisect_next_all(const char *prefix, int no_checkout, int only_merge_commits);\n \n extern int estimate_bisect_steps(int all);\n \ndiff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\nindex 4b5fadcbe..9d7a1dd23 100644\n--- a/builtin/bisect--helper.c\n+++ b/builtin/bisect--helper.c\n@@ -106,6 +106,32 @@ static void check_expected_revs(const char **revs, int rev_nr)\n \t}\n }\n \n+static GIT_PATH_FUNC(git_path_bisect_merges_only, \"MERGES_ONLY_BISECT\")\n+\n+static int merges_only(void)\n+{\n+\tconst char *filename = git_path_bisect_merges_only();\n+\tstruct stat st;\n+\tstruct strbuf str = STRBUF_INIT;\n+\tFILE *fp;\n+\tint res = 0;\n+\n+\tif (stat(filename, &st) || !S_ISREG(st.st_mode))\n+\t\treturn 0;\n+\n+\tfp = fopen_or_warn(filename, \"r\");\n+\tif (!fp)\n+\t\treturn 0;\n+\n+\tif (strbuf_getline_lf(&str, fp) != EOF)\n+\t\tres = atoi(str.buf);\n+\n+\tstrbuf_release(&str);\n+\tfclose(fp);\n+\n+\treturn res;\n+}\n+\n int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n {\n \tenum {\n@@ -137,7 +163,7 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \n \tswitch (cmdmode) {\n \tcase NEXT_ALL:\n-\t\treturn bisect_next_all(prefix, no_checkout);\n+\t\treturn bisect_next_all(prefix, no_checkout, merges_only());\n \tcase WRITE_TERMS:\n \t\tif (argc != 2)\n \t\t\treturn error(_(\"--write-terms requires two arguments\"));\ndiff --git a/builtin/rev-list.c b/builtin/rev-list.c\nindex fadd3ec14..cacffe6c6 100644\n--- a/builtin/rev-list.c\n+++ b/builtin/rev-list.c\n@@ -538,7 +538,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)\n \tif (bisect_list) {\n \t\tint reaches, all;\n \n-\t\tfind_bisection(&revs.commits, &reaches, &all, bisect_find_all);\n+\t\tfind_bisection(&revs.commits, &reaches, &all, bisect_find_all, 0);\n \n \t\tif (bisect_show_vars)\n \t\t\treturn show_bisect_vars(&info, reaches, all);\ndiff --git a/git-bisect.sh b/git-bisect.sh\nindex 54cbfecc5..730c983c5 100755\n--- a/git-bisect.sh\n+++ b/git-bisect.sh\n@@ -82,6 +82,7 @@ bisect_start() {\n \tbad_seen=0\n \teval=''\n \tmust_write_terms=0\n+\tmerges_only=0\n \trevs=''\n \tif test \"z$(git rev-parse --is-bare-repository)\" != zfalse\n \tthen\n@@ -96,6 +97,9 @@ bisect_start() {\n \t\t\tshift\n \t\t\tbreak\n \t\t;;\n+\t\t--merges-only)\n+\t\t\tmerges_only=1\n+\t\t\tshift ;;\n \t\t--no-checkout)\n \t\t\tmode=--no-checkout\n \t\t\tshift ;;\n@@ -212,6 +216,7 @@ bisect_start() {\n \t\tgit bisect--helper --write-terms \"$TERM_BAD\" \"$TERM_GOOD\" || exit\n \tfi &&\n \techo \"git bisect start$orig_args\" >>\"$GIT_DIR/BISECT_LOG\" || exit\n+\techo \"$merges_only\" >\"$GIT_DIR/MERGES_ONLY_BISECT\" || exit\n \t#\n \t# Check if we can proceed to the next bisect state.\n \t#\ndiff --git a/t/t6070-bisect-merge-commits.sh b/t/t6070-bisect-merge-commits.sh\nnew file mode 100755\nindex 000000000..edfae0cfd\n--- /dev/null\n+++ b/t/t6070-bisect-merge-commits.sh\n@@ -0,0 +1,116 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2018 Harald Nordgren\n+#\n+test_description='Tests git bisect merge commit functionality'\n+\n+exec </dev/null\n+\n+. ./test-lib.sh\n+\n+add_line_into_file()\n+{\n+    _line=$1\n+    _file=$2\n+\n+    if [ -f \"$_file\" ]; then\n+        echo \"$_line\" >> $_file || return $?\n+        MSG=\"Add <$_line> into <$_file>.\"\n+    else\n+        echo \"$_line\" > $_file || return $?\n+        git add $_file || return $?\n+        MSG=\"Create file <$_file> with <$_line> inside.\"\n+    fi\n+\n+    test_tick\n+    git commit --quiet -m \"$MSG\" $_file\n+}\n+\n+create_merge_commit()\n+{\n+    _branch=$1\n+    _file=$2\n+\n+     git checkout -b $_branch &&\n+     add_line_into_file \"hello\" $_file &&\n+     git checkout master &&\n+     git merge $_branch --no-edit --no-ff\n+}\n+\n+HASH1=\n+HASH2=\n+HASH3=\n+HASH4=\n+HASH5=\n+HASH6=\n+\n+test_expect_success 'set up basic repo with 3 files and 3 merge commits' '\n+    add_line_into_file \"hello\" hello &&\n+    HASH1=$(git rev-parse --verify HEAD) &&\n+\n+    add_line_into_file \"hello\" hello &&\n+    git checkout -b branch1 &&\n+    add_line_into_file \"hello\" hello1 &&\n+    git checkout master &&\n+    git checkout -b branch2 &&\n+    add_line_into_file \"hello\" hello2 &&\n+    git checkout master &&\n+    git checkout -b branch3 &&\n+    add_line_into_file \"hello\" hello3 &&\n+    git checkout master &&\n+    git merge branch1 --no-edit --no-ff &&\n+    HASH2=$(git rev-parse --verify HEAD) &&\n+\n+    add_line_into_file \"hello\" hello &&\n+    add_line_into_file \"hello\" hello &&\n+    git merge branch2 --no-edit --no-ff &&\n+    git merge branch3 --no-edit --no-ff &&\n+    HASH3=$(git rev-parse --verify HEAD) &&\n+\n+    add_line_into_file \"hello\" hello &&\n+    HASH4=$(git rev-parse --verify HEAD) &&\n+\n+    create_merge_commit branch4 hello4 &&\n+    create_merge_commit branch5 hello5 &&\n+    create_merge_commit branch6 hello6 &&\n+    create_merge_commit branch7 hello7 &&\n+    create_merge_commit branch8 hello8 &&\n+    create_merge_commit branch9 hello9 &&\n+    create_merge_commit branch10 hello10 &&\n+    create_merge_commit branch11 hello11 &&\n+    create_merge_commit branch12 hello12 &&\n+    create_merge_commit branch13 hello13 &&\n+    create_merge_commit branch14 hello14 &&\n+    create_merge_commit branch15 hello15 &&\n+    create_merge_commit branch16 hello16 &&\n+    HASH5=$(git rev-parse --verify HEAD) &&\n+\n+    create_merge_commit branch17 hello17 &&\n+    create_merge_commit branch18 hello18 &&\n+    create_merge_commit branch19 hello19 &&\n+    create_merge_commit branch20 hello20 &&\n+    HASH6=$(git rev-parse --verify HEAD)\n+'\n+\n+test_expect_success 'bisect skip: successful result' '\n+\ttest_when_finished git bisect reset &&\n+\tgit bisect reset &&\n+\tgit bisect start $HASH4 $HASH1 --merges-only &&\n+\tgit bisect good &&\n+\tgit bisect good &&\n+\tgit bisect bad > my_bisect_log.txt &&\n+\tgrep \"$HASH3 is the first bad merge\" my_bisect_log.txt\n+'\n+\n+test_expect_success 'bisect skip: successful result' '\n+\ttest_when_finished git bisect reset &&\n+\tgit bisect reset &&\n+\tgit bisect start $HASH6 $HASH2 --merges-only &&\n+\tgit bisect good &&\n+\tgit bisect good &&\n+\tgit bisect bad &&\n+\tgit bisect bad > my_bisect_log.txt &&\n+\tgrep \"$HASH5 is the first bad merge\" my_bisect_log.txt\n+'\n+\n+test_done\n-- \n2.14.3 (Apple Git-98)\n\n"},{"id":"344508","messageId":"CAGZ79kbt=J5SHsHJTfOZYhgSdn9_gOjVBC3qp_oL0sC-b3ZRmw@mail.gmail.com","threadId":"48270","inReplyTo":"20180411225534.48658-1-haraldnordgren@gmail.com","subject":"Re: [PATCH] Create '--merges-only' option for 'git bisect'","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-04-11T23:33:58Z","receivedAt":"2018-04-11T23:34:05Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Apr 11, 2018 at 3:55 PM, Harald Nordgren\n<haraldnordgren@gmail.com> wrote:\n> When ran with '--merges-only', git bisect will only look at merge commits -- commits with 2 or more parents or the initial commit.\n\nThere has been quite some talk on the mailing list, e.g.\nhttps://public-inbox.org/git/20160427204551.GB4613@virgo.localdomain/\nwhich suggests a --first-parent mode instead. For certain histories\nthese are the same,\nbut merges-only is more restrictive for back-and-forth-cross merges.\n\n\n\n>\n> Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>\n> ---\n>\n> Notes:\n>     Proof of concept of a feature that I have wanted in Git for a while. In my daily work my company uses GitHub, which creates lots of merge commits. In general, tests are only ran on the tip of a branch before merging, so the different commits within a merge commit are allowed not to be buildable. Therefore 'git bisect' often doesn't work since it will give lots of false positives for anything that is not a merge commit. If we could have a feature to only bisect merge commits then it would be easier to pinpoint which merge causes any particular issue. After that, a bisect could be done within the merge to pinpoint futher. As a follow-up to this patch -- we could potentially create a feature that automatically continues into regular bisect within the bad merge commit after completed '--merges-only' bisection.\n\nThe github workflow you mention sounds as if --first-parent would do, too?\n"},{"id":"344531","messageId":"CAHwyqnVg83xSQHRnDVz+aMxPPMj-3bWF4P8YUxEjurxnpDhFEQ@mail.gmail.com","threadId":"48270","inReplyTo":"CAGZ79kbt=J5SHsHJTfOZYhgSdn9_gOjVBC3qp_oL0sC-b3ZRmw@mail.gmail.com","subject":"Re: [PATCH] Create '--merges-only' option for 'git bisect'","fromName":"Harald Nordgren","fromEmail":"haraldnordgren@gmail.com","sentAt":"2018-04-12T07:49:03Z","receivedAt":"2018-04-12T07:49:51Z","isPatch":true,"sender":{"key":"haraldnordgren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9569897?v=4"},"body":"I think it looks similar. But if I'm reading that thread correctly\nthen there was never a patch created, right?\n\nOn Thu, Apr 12, 2018 at 1:33 AM, Stefan Beller <sbeller@google.com> wrote:\n> On Wed, Apr 11, 2018 at 3:55 PM, Harald Nordgren\n> <haraldnordgren@gmail.com> wrote:\n>> When ran with '--merges-only', git bisect will only look at merge commits -- commits with 2 or more parents or the initial commit.\n>\n> There has been quite some talk on the mailing list, e.g.\n> https://public-inbox.org/git/20160427204551.GB4613@virgo.localdomain/\n> which suggests a --first-parent mode instead. For certain histories\n> these are the same,\n> but merges-only is more restrictive for back-and-forth-cross merges.\n>\n>\n>\n>>\n>> Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>\n>> ---\n>>\n>> Notes:\n>>     Proof of concept of a feature that I have wanted in Git for a while. In my daily work my company uses GitHub, which creates lots of merge commits. In general, tests are only ran on the tip of a branch before merging, so the different commits within a merge commit are allowed not to be buildable. Therefore 'git bisect' often doesn't work since it will give lots of false positives for anything that is not a merge commit. If we could have a feature to only bisect merge commits then it would be easier to pinpoint which merge causes any particular issue. After that, a bisect could be done within the merge to pinpoint futher. As a follow-up to this patch -- we could potentially create a feature that automatically continues into regular bisect within the bad merge commit after completed '--merges-only' bisection.\n>\n> The github workflow you mention sounds as if --first-parent would do, too?\n"},{"id":"344533","messageId":"CAP8UFD3j-MGY7jHQK52zVLYMB=1vhCSFKxrvQ1JsRW-Rmnn8Qg@mail.gmail.com","threadId":"48270","inReplyTo":"CAHwyqnVg83xSQHRnDVz+aMxPPMj-3bWF4P8YUxEjurxnpDhFEQ@mail.gmail.com","subject":"Re: [PATCH] Create '--merges-only' option for 'git bisect'","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2018-04-12T08:58:21Z","receivedAt":"2018-04-12T08:58:30Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Thu, Apr 12, 2018 at 9:49 AM, Harald Nordgren\n<haraldnordgren@gmail.com> wrote:\n> I think it looks similar. But if I'm reading that thread correctly\n> then there was never a patch created, right?\n\n(It is customary on this mailing list to reply after the sentences we\nreply to. We don't \"top post\".)\n\nOn the GSoC idea pages (like https://git.github.io/SoC-2018-Ideas/) we\nhave been suggesting \"Implement git bisect --first-parent\" and there\nare the following related links:\n\nhttps://public-inbox.org/git/20150304053333.GA9584@peff.net/\nhttps://public-inbox.org/git/4D3CDDF9.6080405@intel.com/\n\nTiago in Cc also tried at a recent London hackathon to implement it\nand came up with the following:\n\nhttps://github.com/tiagonbotelho/git/pull/1/files\n\nI tried to help him by reworking his commit in the following branch:\n\nhttps://github.com/chriscool/git/commits/myfirstparent\n"},{"id":"344537","messageId":"CAF2Guqum5dak4us_HJtiW_gL0xgsihgUpidFEJzBgw-tEMunkQ@mail.gmail.com","threadId":"48270","inReplyTo":"CAP8UFD3j-MGY7jHQK52zVLYMB=1vhCSFKxrvQ1JsRW-Rmnn8Qg@mail.gmail.com","subject":"Re: [PATCH] Create '--merges-only' option for 'git bisect'","fromName":"Tiago Botelho","fromEmail":"tiago@gitlab.com","sentAt":"2018-04-12T09:27:34Z","receivedAt":"2018-04-12T09:27:40Z","isPatch":true,"sender":{"key":"tiago@gitlab.com","avatar":"https://gravatar.com/avatar/5b6b4ffc9992baf86b0b9d77c0450e63fe8505e7360f7c63f9014e9c246c1d7b?d=mp&s=160"},"body":"On Thu, Apr 12, 2018 at 9:58 AM, Christian Couder\n<christian.couder@gmail.com> wrote:\n> On Thu, Apr 12, 2018 at 9:49 AM, Harald Nordgren\n> <haraldnordgren@gmail.com> wrote:\n>> I think it looks similar. But if I'm reading that thread correctly\n>> then there was never a patch created, right?\n>\n> (It is customary on this mailing list to reply after the sentences we\n> reply to. We don't \"top post\".)\n>\n> On the GSoC idea pages (like https://git.github.io/SoC-2018-Ideas/) we\n> have been suggesting \"Implement git bisect --first-parent\" and there\n> are the following related links:\n>\n> https://public-inbox.org/git/20150304053333.GA9584@peff.net/\n> https://public-inbox.org/git/4D3CDDF9.6080405@intel.com/\n>\n> Tiago in Cc also tried at a recent London hackathon to implement it\n> and came up with the following:\n>\n> https://github.com/tiagonbotelho/git/pull/1/files\n>\n> I tried to help him by reworking his commit in the following branch:\n>\n> https://github.com/chriscool/git/commits/myfirstparent\n\nThank you for the cc Christian, I’ve been quite busy and was not able\nto work on the PR for quite some time.\n\nI intended to pick it back up again next week. If it is ok with Harald\nI would love to finish the PR that I started,\nsince it is quite close to being finished (I think it was just specs\nmissing if I am not mistaken).\n\nKind regards,\n\nTiago Botelho\n"},{"id":"344541","messageId":"nycvar.QRO.7.76.6.1804121143120.65@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz","threadId":"48270","inReplyTo":"20180411225534.48658-1-haraldnordgren@gmail.com","subject":"Re: [PATCH] Create '--merges-only' option for 'git bisect'","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-04-12T11:33:00Z","receivedAt":"2018-04-12T11:33:25Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Harald,\n\nThank you for working on this.\n\nOn Thu, 12 Apr 2018, Harald Nordgren wrote:\n\n> When ran with '--merges-only', git bisect will only look at merge\n> commits -- commits with 2 or more parents or the initial commit.\n\nThis is an excellent idea!\n\n>     Proof of concept of a feature that I have wanted in Git for a while.\n>     In my daily work my company uses GitHub, which creates lots of merge\n>     commits. In general, tests are only ran on the tip of a branch\n>     before merging, so the different commits within a merge commit are\n>     allowed not to be buildable. Therefore 'git bisect' often doesn't\n>     work since it will give lots of false positives for anything that is\n>     not a merge commit. If we could have a feature to only bisect merge\n>     commits then it would be easier to pinpoint which merge causes any\n>     particular issue. After that, a bisect could be done within the\n>     merge to pinpoint futher. As a follow-up to this patch -- we could\n>     potentially create a feature that automatically continues into\n>     regular bisect within the bad merge commit after completed\n>     '--merges-only' bisection.\n\nIt also helps when bisecting breakages in `pu` (mainly because the\nbranches in `pu` use branch points that are often insanely far in the\npast).\n\n> diff --git a/bisect.c b/bisect.c\n> index a579b5088..e8a2f77c7 100644\n> --- a/bisect.c\n> +++ b/bisect.c\n> @@ -254,7 +254,7 @@ static struct commit_list *best_bisection_sorted(struct commit_list *list, int n\n>   */\n>  static struct commit_list *do_find_bisection(struct commit_list *list,\n>  \t\t\t\t\t     int nr, int *weights,\n> -\t\t\t\t\t     int find_all)\n> +\t\t\t\t\t     int find_all, int only_merge_commits)\n\nHow about `int flags`, and defining FIND_ALL and ONLY_MERGE_COMMITS?\n\nThat way, it will be easy to add ONLY_FIRST_PARENTS without changing the\nsignature of do_find_bisection().\n\nIdeally, a preparatory commit would change find_all -> flags (adding\nFIND_ALL), and the second commit would then add support for\nONLY_MERGE_COMMITS.\n\n> @@ -266,6 +266,13 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\n>  \t\tunsigned flags = commit->object.flags;\n>  \n>  \t\tp->item->util = &weights[n++];\n> +\n> +\t\tif (only_merge_commits) {\n> +\t\t\tweight_set(p, -2);\n> +\t\t\tcounted++;\n> +\t\t\tcontinue;\n> +\t\t}\n> +\n\nThis hunk is a little hard to understand when you come from elsewhere, as\nI do. Could you maybe explain a little bit what the `weight_set(p, -2)`\ndoes? This is probably good material for enhancing the commit message\n(seeing as we understand the commit message to be kind of the \"convince me\nthat this is a good change, and explain things that are not immediately\nobvious from the diff\" document).\n\nMaybe it would be enough to describe the role of the weight, and what the\ntypical values look like.\n\n> @@ -305,11 +312,17 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\n>  \t * way, and then fill the blanks using cheaper algorithm.\n>  \t */\n>  \tfor (p = list; p; p = p->next) {\n> +\t\tint distance;\n>  \t\tif (p->item->object.flags & UNINTERESTING)\n>  \t\t\tcontinue;\n>  \t\tif (weight(p) != -2)\n>  \t\t\tcontinue;\n> -\t\tweight_set(p, count_distance(p));\n> +\n> +\t\tif (only_merge_commits)\n> +\t\t\tdistance = count_distance(p) - 1;\n> +\t\telse\n> +\t\t\tdistance = count_distance(p);\n> +\t\tweight_set(p, distance);\n\nA shorter way:\n\n\t\tweight_set(p, count_distance(p) - !!only_merge_commits);\n\nCould you add a code comment above this line to explain why this is\nneeded? (I have to admit that I have no clue what the weights or the\ndistances are in this context, but I think that a 3-line explanation could\nprobably give me enough of an idea that I do not have to study an hour or\nthree to learn enough to understand this change.)\n\n>  \t\tclear_distance(list);\n>  \n>  \t\t/* Does it happen to be at exactly half-way? */\n> @@ -330,11 +343,17 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\n>  \t\t\tfor (q = p->item->parents; q; q = q->next) {\n>  \t\t\t\tif (q->item->object.flags & UNINTERESTING)\n>  \t\t\t\t\tcontinue;\n> +\t\t\t\tif (!q->item->util)\n> +\t\t\t\t\tbreak;\n\nSo we use the `util` field now to do things, probably to flag some\nproperty of the commit.\n\nMaybe the commit message could prepare the reader for this, with a\nparagraph starting with \"We use the commits' `util` field to store the\ninformation that [...]\"?\n\nSeeing as this loop iterates over the commit's parents, I guess we are\nlooking at merge commits, and drop out early if we find a parent that was\nnot marked as interesting earlier?\n\nIf this is the case, why do we not have to look further, at later parents,\nwhether they *do* have a non-NULL `util` attribute?\n\n>  \t\t\t\tif (0 <= weight(q))\n>  \t\t\t\t\tbreak;\n>  \t\t\t}\n>  \t\t\tif (!q)\n>  \t\t\t\tcontinue;\n> +\t\t\tif (!q->item->util) {\n> +\t\t\t\tcounted++;\n> +\t\t\t\tcontinue;\n> +\t\t\t}\n\nOkay, so here we skip the current commit if we found a parent commit that\nhas the `util` field set to NULL...\n\n> @@ -364,8 +383,43 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\n>  \t\treturn best_bisection_sorted(list, nr);\n>  }\n>  \n> +int merge_commit_or_root(const struct commit c)\n\nBy value? Let's use `*c` instead of `c` here.\n\n> +{\n> +\tif (!c.parents)\n> +\t\treturn 1;\n> +\n> +\treturn !!c.parents->next;\n\nUnless you need the return value to be 0 or 1 (instead of 0 or non-0),\nthis is conciser:\n\n\treturn !c.parents || c.parents->next;\n\nHowever, we should not detect root commits here. We are only interested in\nmerges, according to the new option! So we should filter out root commits,\ntoo. So this is what you really want:\n\n\tint is_merge(const struct commit *c)\n\t{\n\t\treturn c->parents && c->parents->next;\n\t}\n\n> +void filter_non_merge_commits(struct commit_list **commit_list)\n\nSo does this keep non_merge_commits (similar to Makefile's $(filter ...)\nfunction?) Or does it remove them?\n\nJudging by the code, maybe \"filter_out_non_merge_commits()\" or\n\"drop_regular_commits()\"? Or \"keep_only_root_and_merge_commits()\"?\n\n> +{\n> +\tstruct commit_list *list1 = *commit_list;\n> +\tstruct commit_list *list2 = NULL;\n> +\t*commit_list = NULL;\n> +\n> +\tfor ( ; list1; list1 = list1->next) {\n> +\t\tif (merge_commit_or_root(*list1->item)) {\n> +\t\t\tlist2 = list1;\n> +\t\t\tlist1 = list1->next;\n> +\t\t\tlist2->next = NULL;\n\nSince commit_lists are malloc()ed, they also need to be free()d when\nfiltering something out.\n\n> +\t\t\tlist2->item->parents = NULL;\n\nWhy do we touch the parents here? This is a bit dangerous, as the item\nrefers to the commit that `lookup_commit()` would produce again. And by\nsetting parents = NULL, we pretend that those commits are root commits\nfrom now on.\n\n> +\t\t\t*commit_list = list2;\n> +\t\t\tbreak;\n\nAlso, using the non-descriptive names `list1` and `list2` makes code a bit\nhard to understand. (And hence it makes it easy for bugs to creep in.)\n\n> +\t\t}\n> +\t}\n> +\n> +\tfor ( ; list1; list1 = list1->next) {\n\nThe previous for loop has the condition `list1` already. So at this point\nwe know that `list1 == NULL`, and therefore this for loop is dead code.\n\nDid you mean `for (list1 = *commit_list; list1; list1 = list1->next)`?\n\n> +\t\tlist2->next = NULL;\n\nWhere is list2 set? Only in the previous loop, if it found a merge or root\ncommit. Otherwise, list2 would still be NULL and dereferenced here!\n\n> +\t\tif (merge_commit_or_root(*list1->item)) {\n> +\t\t\tlist2->next = list1;\n> +\t\t\tlist2 = list2->next;\n> +\t\t\tlist2->item->parents = list1;\n\nAnd here, we set the parents to list1... Again, this is messing with the\ncommits that come back when `lookup_commit()` is called later.\n\nChanging the parents of a commit is something I imagine breaks too many\nassumption to be a safe thing.\n\nI'll read on for a little bit, maybe I get a better understanding what we\nneed to do in this here function.\n\n> +\t\t}\n> +\t}\n> +}\n> +\n>  void find_bisection(struct commit_list **commit_list, int *reaches,\n> -\t\t    int *all, int find_all)\n> +\t\t    int *all, int find_all, int only_merge_commits)\n\nLet's use a flag here, just as above.\n\n>  {\n>  \tint nr, on_list;\n>  \tstruct commit_list *list, *p, *best, *next, *last;\n> @@ -373,6 +427,10 @@ void find_bisection(struct commit_list **commit_list, int *reaches,\n>  \n>  \tshow_list(\"bisection 2 entry\", 0, 0, *commit_list);\n>  \n> +\tif (only_merge_commits) {\n> +\t\tfilter_non_merge_commits(commit_list);\n> +\t}\n> +\n>  \t/*\n>  \t * Count the number of total and tree-changing items on the\n>  \t * list, while reversing the list.\n> @@ -400,7 +458,7 @@ void find_bisection(struct commit_list **commit_list, int *reaches,\n>  \tweights = xcalloc(on_list, sizeof(*weights));\n>  \n>  \t/* Do the real work of finding bisection commit. */\n> -\tbest = do_find_bisection(list, nr, weights, find_all);\n> +\tbest = do_find_bisection(list, nr, weights, find_all, only_merge_commits);\n>  \tif (best) {\n>  \t\tif (!find_all) {\n>  \t\t\tlist->item = best->item;\n> @@ -878,7 +936,7 @@ static void check_good_are_ancestors_of_bad(const char *prefix, int no_checkout)\n>  /*\n>   * This does \"git diff-tree --pretty COMMIT\" without one fork+exec.\n>   */\n> -static void show_diff_tree(const char *prefix, struct commit *commit)\n> +static void show_diff_tree(const char *prefix, struct commit *commit, int only_merge_commits)\n>  {\n>  \tstruct rev_info opt;\n>  \n> @@ -893,6 +951,11 @@ static void show_diff_tree(const char *prefix, struct commit *commit)\n>  \topt.use_terminator = 0;\n>  \topt.commit_format = CMIT_FMT_DEFAULT;\n>  \n> +\tif (only_merge_commits) {\n> +\t\topt.ignore_merges = 0;\n> +\t\topt.combine_merges = 1;\n> +\t}\n\nThis is most likely what we want, whether --only-merge-commits was passed\nor not. Let's make it the default (in a separate commit)?\n\n> @@ -938,7 +1001,7 @@ void read_bisect_terms(const char **read_bad, const char **read_good)\n>   * If no_checkout is non-zero, the bisection process does not\n>   * checkout the trial commit but instead simply updates BISECT_HEAD.\n>   */\n> -int bisect_next_all(const char *prefix, int no_checkout)\n> +int bisect_next_all(const char *prefix, int no_checkout, int only_merge_commits)\n\nLet's use `flags` here, too.\n\n>  {\n>  \tstruct rev_info revs;\n>  \tstruct commit_list *tried;\n> @@ -957,7 +1020,7 @@ int bisect_next_all(const char *prefix, int no_checkout)\n>  \n>  \tbisect_common(&revs);\n>  \n> -\tfind_bisection(&revs.commits, &reaches, &all, !!skipped_revs.nr);\n> +\tfind_bisection(&revs.commits, &reaches, &all, !!skipped_revs.nr, only_merge_commits);\n\nWith the `flags` change, we would probably just introduce a local variable\n`int flags = 0;` and set `flags |= !!skipped_revs.nr;` here at first, and\nonly change the `flags` variable to a parameter when introducing the\n--only-merge-commits flag.\n\n> @@ -983,10 +1046,14 @@ int bisect_next_all(const char *prefix, int no_checkout)\n>  \tbisect_rev = &revs.commits->item->object.oid;\n>  \n>  \tif (!oidcmp(bisect_rev, current_bad_oid)) {\n> +\t\tchar *format_string = NULL;\n>  \t\texit_if_skipped_commits(tried, current_bad_oid);\n> -\t\tprintf(\"%s is the first %s commit\\n\", oid_to_hex(bisect_rev),\n> -\t\t\tterm_bad);\n> -\t\tshow_diff_tree(prefix, revs.commits->item);\n> +\t\tif (only_merge_commits)\n> +\t\t\tformat_string = \"%s is the first %s merge\\n\";\n> +\t\telse\n> +\t\t\tformat_string = \"%s is the first %s commit\\n\";\n\n... why? We can reference merge commits as \"commits\", too... Why not leave\nthe message as-is?\n\n> diff --git a/bisect.h b/bisect.h\n> index a5d9248a4..664ada180 100644\n> --- a/bisect.h\n> +++ b/bisect.h\n> @@ -9,7 +9,7 @@\n>   * best commit, as chosen by `find_all`.\n>   */\n>  extern void find_bisection(struct commit_list **list, int *reaches, int *all,\n> -\t\t\t   int find_all);\n> +\t\t\t   int find_all, int only_merge_commits);\n\nOh, I missed that this is a global function. So the flags should not be\nFIND_ALL and ONLY_MERGE_COMMITS but BISECT_FIND_ALL and\nBISECT_ONLY_MERGE_COMMITS, and `#define`d above this function.\n\n> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\n> index 4b5fadcbe..9d7a1dd23 100644\n> --- a/builtin/bisect--helper.c\n> +++ b/builtin/bisect--helper.c\n> @@ -106,6 +106,32 @@ static void check_expected_revs(const char **revs, int rev_nr)\n>  \t}\n>  }\n>  \n> +static GIT_PATH_FUNC(git_path_bisect_merges_only, \"MERGES_ONLY_BISECT\")\n\nThe other bisect-specific files start with BISECT_, let's do that here,\ntoo (i.e. \"BISECT_ONLY_MERGES\")?\n\n> +static int merges_only(void)\n> +{\n> +\tconst char *filename = git_path_bisect_merges_only();\n> +\tstruct stat st;\n> +\tstruct strbuf str = STRBUF_INIT;\n> +\tFILE *fp;\n> +\tint res = 0;\n> +\n> +\tif (stat(filename, &st) || !S_ISREG(st.st_mode))\n> +\t\treturn 0;\n> +\n> +\tfp = fopen_or_warn(filename, \"r\");\n> +\tif (!fp)\n> +\t\treturn 0;\n> +\n> +\tif (strbuf_getline_lf(&str, fp) != EOF)\n> +\t\tres = atoi(str.buf);\n\nWhy not use the presence of the file as flag, rather than its contents?\nThen the test becomes as simple as\n\n\tint merges_only = file_exists(git_path_bisect_merges_only());\n\n> +\n> +\tstrbuf_release(&str);\n> +\tfclose(fp);\n> +\n> +\treturn res;\n> +}\n> +\n>  int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n>  {\n>  \tenum {\n> @@ -137,7 +163,7 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n>  \n>  \tswitch (cmdmode) {\n>  \tcase NEXT_ALL:\n> -\t\treturn bisect_next_all(prefix, no_checkout);\n> +\t\treturn bisect_next_all(prefix, no_checkout, merges_only());\n>  \tcase WRITE_TERMS:\n>  \t\tif (argc != 2)\n>  \t\t\treturn error(_(\"--write-terms requires two arguments\"));\n> diff --git a/builtin/rev-list.c b/builtin/rev-list.c\n> index fadd3ec14..cacffe6c6 100644\n> --- a/builtin/rev-list.c\n> +++ b/builtin/rev-list.c\n> @@ -538,7 +538,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)\n>  \tif (bisect_list) {\n>  \t\tint reaches, all;\n>  \n> -\t\tfind_bisection(&revs.commits, &reaches, &all, bisect_find_all);\n> +\t\tfind_bisection(&revs.commits, &reaches, &all, bisect_find_all, 0);\n\nThis would become bisect_flags in the preparatory patch.\n\n>  \n>  \t\tif (bisect_show_vars)\n>  \t\t\treturn show_bisect_vars(&info, reaches, all);\n> diff --git a/git-bisect.sh b/git-bisect.sh\n> index 54cbfecc5..730c983c5 100755\n> --- a/git-bisect.sh\n> +++ b/git-bisect.sh\n> @@ -82,6 +82,7 @@ bisect_start() {\n>  \tbad_seen=0\n>  \teval=''\n>  \tmust_write_terms=0\n> +\tmerges_only=0\n>  \trevs=''\n>  \tif test \"z$(git rev-parse --is-bare-repository)\" != zfalse\n>  \tthen\n> @@ -96,6 +97,9 @@ bisect_start() {\n>  \t\t\tshift\n>  \t\t\tbreak\n>  \t\t;;\n> +\t\t--merges-only)\n> +\t\t\tmerges_only=1\n> +\t\t\tshift ;;\n>  \t\t--no-checkout)\n>  \t\t\tmode=--no-checkout\n>  \t\t\tshift ;;\n> @@ -212,6 +216,7 @@ bisect_start() {\n>  \t\tgit bisect--helper --write-terms \"$TERM_BAD\" \"$TERM_GOOD\" || exit\n>  \tfi &&\n>  \techo \"git bisect start$orig_args\" >>\"$GIT_DIR/BISECT_LOG\" || exit\n> +\techo \"$merges_only\" >\"$GIT_DIR/MERGES_ONLY_BISECT\" || exit\n\nThe git-bisect.sh script deviates from our coding convention, it seems.\nTypically, we would write\n\n\tmerges_only=\n\t[...]\n\t\t--merges-only)\n\t\t\tmerges_only=t\n\t\t\t;;\n\t[...]\n\ttest -z \"$merges_only\" || : >\"$GIT_DIR/BISECT_ONLY_MERGES\" || exit\n\nI would prefer that here, too.\n\n> diff --git a/t/t6070-bisect-merge-commits.sh b/t/t6070-bisect-merge-commits.sh\n> new file mode 100755\n> index 000000000..edfae0cfd\n> --- /dev/null\n> +++ b/t/t6070-bisect-merge-commits.sh\n> @@ -0,0 +1,116 @@\n> +#!/bin/sh\n> +#\n> +# Copyright (c) 2018 Harald Nordgren\n> +#\n> +test_description='Tests git bisect merge commit functionality'\n> +\n> +exec </dev/null\n\nThis is unnecessary, as our test-lib.sh already executes `exec 6<&0`.\nFurther, it is not only unnecessary, but also breaks debugging with the\n`debug` shell function defined in test-lib-function.sh. Let's just drop\nthis statement.\n\n> +\n> +. ./test-lib.sh\n> +\n> +add_line_into_file()\n> +{\n> +    _line=$1\n> +    _file=$2\n> +\n> +    if [ -f \"$_file\" ]; then\n> +        echo \"$_line\" >> $_file || return $?\n> +        MSG=\"Add <$_line> into <$_file>.\"\n> +    else\n> +        echo \"$_line\" > $_file || return $?\n> +        git add $_file || return $?\n> +        MSG=\"Create file <$_file> with <$_line> inside.\"\n> +    fi\n> +\n> +    test_tick\n> +    git commit --quiet -m \"$MSG\" $_file\n> +}\n\nIt would appear that the contents of the files written using this function\nare never tested.\n\nLet's just use `test_commit` instead.\n\n> +\n> +create_merge_commit()\n> +{\n> +    _branch=$1\n> +    _file=$2\n> +\n> +     git checkout -b $_branch &&\n> +     add_line_into_file \"hello\" $_file &&\n> +     git checkout master &&\n> +     git merge $_branch --no-edit --no-ff\n> +}\n\nThis not only creates a merge commit but also sets up a branch and adds a\ncommit to it. I'd much rather have a function that performs the merge\nafter calling `test_tick`, and then tags the result just like\n`test_commit` would do.\n\n> +HASH1=\n> +HASH2=\n> +HASH3=\n> +HASH4=\n> +HASH5=\n> +HASH6=\n\nThese are too non-descriptive names to be useful. Remember that the\npurpose of a regression test is to help debugging regressions. If I\nencounter a regression while developing a patch series that breaks this\ntest, I really need to understand the regression test as quickly as\npossible.\n\nSo let's use HEAD~4 and/or the tags creates by `test_commit` instead.\n\n> +test_expect_success 'set up basic repo with 3 files and 3 merge commits' '\n\nIf we say 'setup' here, it is not only conciser, but also less prone to\nbecome incorrect in the future.\n\n> +    add_line_into_file \"hello\" hello &&\n> +    HASH1=$(git rev-parse --verify HEAD) &&\n> +\n> +    add_line_into_file \"hello\" hello &&\n> +    git checkout -b branch1 &&\n> +    add_line_into_file \"hello\" hello1 &&\n> +    git checkout master &&\n> +    git checkout -b branch2 &&\n> +    add_line_into_file \"hello\" hello2 &&\n> +    git checkout master &&\n> +    git checkout -b branch3 &&\n> +    add_line_into_file \"hello\" hello3 &&\n> +    git checkout master &&\n> +    git merge branch1 --no-edit --no-ff &&\n> +    HASH2=$(git rev-parse --verify HEAD) &&\n> +\n> +    add_line_into_file \"hello\" hello &&\n> +    add_line_into_file \"hello\" hello &&\n> +    git merge branch2 --no-edit --no-ff &&\n> +    git merge branch3 --no-edit --no-ff &&\n> +    HASH3=$(git rev-parse --verify HEAD) &&\n> +\n> +    add_line_into_file \"hello\" hello &&\n> +    HASH4=$(git rev-parse --verify HEAD) &&\n> +\n> +    create_merge_commit branch4 hello4 &&\n> +    create_merge_commit branch5 hello5 &&\n> +    create_merge_commit branch6 hello6 &&\n> +    create_merge_commit branch7 hello7 &&\n> +    create_merge_commit branch8 hello8 &&\n> +    create_merge_commit branch9 hello9 &&\n> +    create_merge_commit branch10 hello10 &&\n> +    create_merge_commit branch11 hello11 &&\n> +    create_merge_commit branch12 hello12 &&\n> +    create_merge_commit branch13 hello13 &&\n> +    create_merge_commit branch14 hello14 &&\n> +    create_merge_commit branch15 hello15 &&\n> +    create_merge_commit branch16 hello16 &&\n> +    HASH5=$(git rev-parse --verify HEAD) &&\n> +\n> +    create_merge_commit branch17 hello17 &&\n> +    create_merge_commit branch18 hello18 &&\n> +    create_merge_commit branch19 hello19 &&\n> +    create_merge_commit branch20 hello20 &&\n> +    HASH6=$(git rev-parse --verify HEAD)\n> +'\n\nIt is next to impossible to understand this function in an 80x25 terminal\n(which many of us old-timers still use). And it is not even necessary in\norder to test the functionality you introduced...\n\nSo how about this strategy: first, paint a nice little diagram of the\ncommit graph you want to achieve, and make it not as insanely complex.\nThen, set that up using `test_commit`, `git checkout -b` and `test_tick &&\ngit merge`. Something like this:\n\n# We generate the following commit graph:\n#\n# A - B - C - F\n#   \\   \\   /   \\\n#     D - E - G - H\n\ntest_merge () {\n\ttest_tick &&\n\tgit merge \"$@\" &&\n\tgit tag \"$1\"\n}\n\ntest_expect_success 'setup' '\n\ttest_commit A &&\n\ttest_commit B &&\n\ttest_commit C &&\n\tgit reset --hard A &&\n\ttest_commit D &&\n\ttest_merge -m E B &&\n\tgit reset --hard C &&\n\tgit merge -m F E &&\n\tgit reset --hard G &&\n\tgit merge -m H F\n'\n\n> +test_expect_success 'bisect skip: successful result' '\n\nI do not actually see that `bisect skip` is tested here.\n\n> +\ttest_when_finished git bisect reset &&\n> +\tgit bisect reset &&\n> +\tgit bisect start $HASH4 $HASH1 --merges-only &&\n> +\tgit bisect good &&\n> +\tgit bisect good &&\n> +\tgit bisect bad > my_bisect_log.txt &&\n> +\tgrep \"$HASH3 is the first bad merge\" my_bisect_log.txt\n> +'\n\nWhat we want to verify here is that we only encounter merge commits,\nright? So that is what we should test:\n\ntest_expect_success '--merges-only' '\n\twrite_script find-c.sh <<-\\EOF &&\n\tcase \"$(git show --format=%p -s)\" in\n\t*\\ *) ;; # merge commit: okay\n\t*) git rev-parse HEAD >non-merge-found;;\n\tesac\n\t# detect whether we are \"before\" or \"after\" C\n\ttest ! -f C.t\n\tEOF\n\n\tgit bisect start HEAD A &&\n\tgit bisect run ./find-c.sh >actual &&\n\ttest_path_is_missing non-merge-found &&\n\tgrep \"$(git rev-parse F) is the first bad commit\" actual\n'\n\n> +\n> +test_expect_success 'bisect skip: successful result' '\n> +\ttest_when_finished git bisect reset &&\n> +\tgit bisect reset &&\n> +\tgit bisect start $HASH6 $HASH2 --merges-only &&\n> +\tgit bisect good &&\n> +\tgit bisect good &&\n> +\tgit bisect bad &&\n> +\tgit bisect bad > my_bisect_log.txt &&\n> +\tgrep \"$HASH5 is the first bad merge\" my_bisect_log.txt\n> +'\n> +\n> +test_done\n\nSo after reading this, I am quite a bit more puzzled about that\ncommit->util field we seem to use. Could you clarify that in the commit\nmessage, to give readers a head start?\n\nOkay, now let's come back to that function accepting a commit_list and\ndropping all non-merge commits from it.\n\nFirst of all, I do not think that we actually make use of the parents\ninformation after calling this function, right? If we did, I think it\nwould be horribly broken because all non-merge commits would now be\nlooking like root commits, and all the merge commits would pretend to have\n*all* of the previous merge commits as parents (the first merge commit\nwould therefore look like a root commit, the second merge commit would\nlook like a non-merge, and so on).\n\nSo let's just drop the parent rewriting? If we do need to collapse the\ncommit graph to the merge commit nodes, then a much better idea would be\nto use an oidset with commit_lists representing the collapsed vertices in\nthat graph (i.e. for every commit, it would replace the non-merge parents\nwith the respective closest ancestor that *is* a merge, and this list\nwould be constructed in reverse topological order to give linear\ncomplexity).\n\nUnder the assumption that we can leave the commits' parents alone, this is\nmy proposed function to keep only merge commits in a commit_list:\n\nstatic void keep_only_merge_commits(struct commit_list **p)\n{\n\tstruct commit_list *parents, *next;\n\n\twhile (*p) {\n\t\tparents = (*p)->item->parents;\n\t\tif (parents && parents->next) {\n\t\t\t/* merge commit: keep */\n\t\t\tp = &(*p)->next;\n\t\t\tcontinue;\n\t\t}\n\n\t\t/* non-merge commit: free() and skip */\n\t\tnext = (*p)->next;\n\t\tfree(*p);\n\t\t*p = next;\n\t}\n}\n\nThis makes use of the fact that both the original list of commits, as well\nas the `next` field in the commit_list items, is a pointer to a\ncommit_list. So once we processed the first item of the list, we either\nfree it and replace the list by the remainder of the list, or we advance\nto the \"sub list\" starting with the second item of the list (if there is\nany left).\n\nCiao,\nJohannes\n"},{"id":"344543","messageId":"nycvar.QRO.7.76.6.1804121333340.65@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz","threadId":"48270","inReplyTo":"CAGZ79kbt=J5SHsHJTfOZYhgSdn9_gOjVBC3qp_oL0sC-b3ZRmw@mail.gmail.com","subject":"Re: [PATCH] Create '--merges-only' option for 'git bisect'","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-04-12T11:35:43Z","receivedAt":"2018-04-12T11:36:05Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Stefan,\n\nOn Wed, 11 Apr 2018, Stefan Beller wrote:\n\n> On Wed, Apr 11, 2018 at 3:55 PM, Harald Nordgren\n> <haraldnordgren@gmail.com> wrote:\n> > When ran with '--merges-only', git bisect will only look at merge commits -- commits with 2 or more parents or the initial commit.\n> \n> There has been quite some talk on the mailing list, e.g.\n> https://public-inbox.org/git/20160427204551.GB4613@virgo.localdomain/\n> which suggests a --first-parent mode instead.\n\nI like that mode, but I would love to have *both*. And from what I see, it\nshould be relatively easy to add the --first-parent mode on top of\nHarald's patches.\n\n> For certain histories these are the same, but merges-only is more\n> restrictive for back-and-forth-cross merges.\n\nYou mean merges-only tests *more* in back-and-forth-cross-merges\nscenarios?\n\nCiao,\nDscho\n"},{"id":"344546","messageId":"nycvar.QRO.7.76.6.1804121341590.65@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz","threadId":"48270","inReplyTo":"CAF2Guqum5dak4us_HJtiW_gL0xgsihgUpidFEJzBgw-tEMunkQ@mail.gmail.com","subject":"Re: [PATCH] Create '--merges-only' option for 'git bisect'","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-04-12T11:53:46Z","receivedAt":"2018-04-12T11:54:09Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Tiago,\n\nOn Thu, 12 Apr 2018, Tiago Botelho wrote:\n\n> On Thu, Apr 12, 2018 at 9:58 AM, Christian Couder\n> <christian.couder@gmail.com> wrote:\n> > On Thu, Apr 12, 2018 at 9:49 AM, Harald Nordgren\n> > <haraldnordgren@gmail.com> wrote:\n> >> I think it looks similar. But if I'm reading that thread correctly\n> >> then there was never a patch created, right?\n> >\n> > (It is customary on this mailing list to reply after the sentences we\n> > reply to. We don't \"top post\".)\n> >\n> > On the GSoC idea pages (like https://git.github.io/SoC-2018-Ideas/) we\n> > have been suggesting \"Implement git bisect --first-parent\" and there\n> > are the following related links:\n> >\n> > https://public-inbox.org/git/20150304053333.GA9584@peff.net/\n> > https://public-inbox.org/git/4D3CDDF9.6080405@intel.com/\n> >\n> > Tiago in Cc also tried at a recent London hackathon to implement it\n> > and came up with the following:\n> >\n> > https://github.com/tiagonbotelho/git/pull/1/files\n> >\n> > I tried to help him by reworking his commit in the following branch:\n> >\n> > https://github.com/chriscool/git/commits/myfirstparent\n> \n> Thank you for the cc Christian, I’ve been quite busy and was not able\n> to work on the PR for quite some time.\n> \n> I intended to pick it back up again next week. If it is ok with Harald\n> I would love to finish the PR that I started,\n> since it is quite close to being finished (I think it was just specs\n> missing if I am not mistaken).\n\nThat looks promising. Just like I suggested to Harald in another reply\n[*1*] on this thread, you probably want to use `int flags` instead, and\nturn `find_all` into BISECT_FIND_ALL in a preparatory commit.\n\nAlso, you will definitely want to add a test. Again, my reply to Harald\n[*1*] should give you a head start there... You will want to imitate the\ntest case I outlined there, maybe something like:\n\n# A - B - C - F\n#   \\   \\   /   \\\n#     D - E - G - H\n\n[... 'setup' as in my mail to Harald ...]\n\ntest_expect_success '--first-parent' '\n\twrite_script find-e.sh <<-\\EOF &&\n\tcase \"$(git show --format=%s -s)\" in\n\tB|C|F) ;; # first parent lineage: okay\n\t*) git show -s --oneline HEAD >unexpected;;\n\tesac\n\t# detect whether we are \"before\" or \"after\" E\n\ttest ! -f E.t\n\tEOF\n\n\tgit bisect start --first-parent HEAD A &&\n\tgit bisect run ./find-e.sh >actual &&\n\ttest_path_is_missing unexpected &&\n\tgrep \"$(git rev-parse F) is the first bad commit\" actual\n'\n\nAlso, Tiago, reading through your patch (as on chriscool/git; do you have\nyour own fork? That would make it much easier to collaborate with you by\noffering PRs), it looks more straight-forward than editing the commit_list\nafter the fact and adding magic weights ;-)\n\nExcept for one thing. I wonder why `bisect_next_all()` does not set\nrevs.first_parent_only after calling `bisect_rev_setup()`? You would still\nneed the changes in `count_distance()`, as it performs its own commit\ngraph traversal, but there is no need to enumerate too many commits in the\nfirst place, right?\n\nHarald, maybe --merges-only can be implemented on top of --first-parent,\nwith the `int flags` change I suggested?\n\nCiao,\nJohannes\n\nFootnote *1*:\nhttps://public-inbox.org/git/nycvar.QRO.7.76.6.1804121143120.65@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz/"},{"id":"344547","messageId":"CAF2GuqtooOo0P3nGXNEEu=j2XSuNZ2KRwEOAz82mMC3t3v7bXQ@mail.gmail.com","threadId":"48270","inReplyTo":"nycvar.QRO.7.76.6.1804121341590.65@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz","subject":"Re: [PATCH] Create '--merges-only' option for 'git bisect'","fromName":"Tiago Botelho","fromEmail":"tiago@gitlab.com","sentAt":"2018-04-12T12:19:23Z","receivedAt":"2018-04-12T12:19:31Z","isPatch":true,"sender":{"key":"tiago@gitlab.com","avatar":"https://gravatar.com/avatar/5b6b4ffc9992baf86b0b9d77c0450e63fe8505e7360f7c63f9014e9c246c1d7b?d=mp&s=160"},"body":"On Thu, Apr 12, 2018 at 12:53 PM, Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n> Hi Tiago,\n>\n> On Thu, 12 Apr 2018, Tiago Botelho wrote:\n>\n>> On Thu, Apr 12, 2018 at 9:58 AM, Christian Couder\n>> <christian.couder@gmail.com> wrote:\n>> > On Thu, Apr 12, 2018 at 9:49 AM, Harald Nordgren\n>> > <haraldnordgren@gmail.com> wrote:\n>> >> I think it looks similar. But if I'm reading that thread correctly\n>> >> then there was never a patch created, right?\n>> >\n>> > (It is customary on this mailing list to reply after the sentences we\n>> > reply to. We don't \"top post\".)\n>> >\n>> > On the GSoC idea pages (like https://git.github.io/SoC-2018-Ideas/) we\n>> > have been suggesting \"Implement git bisect --first-parent\" and there\n>> > are the following related links:\n>> >\n>> > https://public-inbox.org/git/20150304053333.GA9584@peff.net/\n>> > https://public-inbox.org/git/4D3CDDF9.6080405@intel.com/\n>> >\n>> > Tiago in Cc also tried at a recent London hackathon to implement it\n>> > and came up with the following:\n>> >\n>> > https://github.com/tiagonbotelho/git/pull/1/files\n>> >\n>> > I tried to help him by reworking his commit in the following branch:\n>> >\n>> > https://github.com/chriscool/git/commits/myfirstparent\n>>\n>> Thank you for the cc Christian, I’ve been quite busy and was not able\n>> to work on the PR for quite some time.\n>>\n>> I intended to pick it back up again next week. If it is ok with Harald\n>> I would love to finish the PR that I started,\n>> since it is quite close to being finished (I think it was just specs\n>> missing if I am not mistaken).\n>\n> That looks promising. Just like I suggested to Harald in another reply\n> [*1*] on this thread, you probably want to use `int flags` instead, and\n> turn `find_all` into BISECT_FIND_ALL in a preparatory commit.\n>\n> Also, you will definitely want to add a test. Again, my reply to Harald\n> [*1*] should give you a head start there... You will want to imitate the\n> test case I outlined there, maybe something like:\n>\n> # A - B - C - F\n> #   \\   \\   /   \\\n> #     D - E - G - H\n>\n> [... 'setup' as in my mail to Harald ...]\n>\n> test_expect_success '--first-parent' '\n>         write_script find-e.sh <<-\\EOF &&\n>         case \"$(git show --format=%s -s)\" in\n>         B|C|F) ;; # first parent lineage: okay\n>         *) git show -s --oneline HEAD >unexpected;;\n>         esac\n>         # detect whether we are \"before\" or \"after\" E\n>         test ! -f E.t\n>         EOF\n>\n>         git bisect start --first-parent HEAD A &&\n>         git bisect run ./find-e.sh >actual &&\n>         test_path_is_missing unexpected &&\n>         grep \"$(git rev-parse F) is the first bad commit\" actual\n> '\n>\n> Also, Tiago, reading through your patch (as on chriscool/git; do you have\n> your own fork? That would make it much easier to collaborate with you by\n> offering PRs), it looks more straight-forward than editing the commit_list\n> after the fact and adding magic weights ;-)\n>\n> Except for one thing. I wonder why `bisect_next_all()` does not set\n> revs.first_parent_only after calling `bisect_rev_setup()`? You would still\n> need the changes in `count_distance()`, as it performs its own commit\n> graph traversal, but there is no need to enumerate too many commits in the\n> first place, right?\n>\n> Harald, maybe --merges-only can be implemented on top of --first-parent,\n> with the `int flags` change I suggested?\n>\n> Ciao,\n> Johannes\n>\n> Footnote *1*:\n> https://public-inbox.org/git/nycvar.QRO.7.76.6.1804121143120.65@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz/\n\nHi Johannes,\n\nThank you for the pointers!\n\nI do have my own fork, you can see it here:\nhttps://github.com/tiagonbotelho/git/pull/1/commits\nwhich applies the changes Chris made to my first draft of the solution.\n\nI still have to add him as co-author of that commit.\n\nCheers,\nTiago Botelho\n"},{"id":"346138","messageId":"nycvar.QRO.7.76.6.1804301330230.79@tvgsbejvaqbjf.bet","threadId":"48270","inReplyTo":"CAF2GuqtooOo0P3nGXNEEu=j2XSuNZ2KRwEOAz82mMC3t3v7bXQ@mail.gmail.com","subject":"Re: [PATCH] Create '--merges-only' option for 'git bisect'","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-04-30T11:31:49Z","receivedAt":"2018-04-30T11:31:55Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Tiago,\n\n> > On Thu, 12 Apr 2018, Tiago Botelho wrote:\n> >\n> >> On Thu, Apr 12, 2018 at 9:58 AM, Christian Couder\n> >> <christian.couder@gmail.com> wrote:\n> >> > On Thu, Apr 12, 2018 at 9:49 AM, Harald Nordgren\n> >> > <haraldnordgren@gmail.com> wrote:\n> >> >> I think it looks similar. But if I'm reading that thread correctly\n> >> >> then there was never a patch created, right?\n> >> >\n> >> > (It is customary on this mailing list to reply after the sentences we\n> >> > reply to. We don't \"top post\".)\n> >> >\n> >> > On the GSoC idea pages (like https://git.github.io/SoC-2018-Ideas/) we\n> >> > have been suggesting \"Implement git bisect --first-parent\" and there\n> >> > are the following related links:\n> >> >\n> >> > https://public-inbox.org/git/20150304053333.GA9584@peff.net/\n> >> > https://public-inbox.org/git/4D3CDDF9.6080405@intel.com/\n> >> >\n> >> > Tiago in Cc also tried at a recent London hackathon to implement it\n> >> > and came up with the following:\n> >> >\n> >> > https://github.com/tiagonbotelho/git/pull/1/files\n> >> >\n> >> > I tried to help him by reworking his commit in the following branch:\n> >> >\n> >> > https://github.com/chriscool/git/commits/myfirstparent\n> >>\n> >> Thank you for the cc Christian, I’ve been quite busy and was not able\n> >> to work on the PR for quite some time.\n> >>\n> >> I intended to pick it back up again next week. If it is ok with\n> >> Harald I would love to finish the PR that I started, since it is\n> >> quite close to being finished (I think it was just specs missing if I\n> >> am not mistaken).\n\nIt is now well after \"next week\". Are there any news? Or could you unblock\nHarald by stating that you won't come back to it any time soon (in\nparticular since the PR is not quite as finished from my reading as you\nmade it sound...)?\n\nCiao,\nJohannes"},{"id":"346139","messageId":"CAF2GuqvTvPPfGPXCwhBkTHJvRQcV1w9VebHkn+Kqw6dZ2b0xKQ@mail.gmail.com","threadId":"48270","inReplyTo":"nycvar.QRO.7.76.6.1804301330230.79@tvgsbejvaqbjf.bet","subject":"Re: [PATCH] Create '--merges-only' option for 'git bisect'","fromName":"Tiago Botelho","fromEmail":"tiago@gitlab.com","sentAt":"2018-04-30T11:34:20Z","receivedAt":"2018-04-30T11:34:26Z","isPatch":true,"sender":{"key":"tiago@gitlab.com","avatar":"https://gravatar.com/avatar/5b6b4ffc9992baf86b0b9d77c0450e63fe8505e7360f7c63f9014e9c246c1d7b?d=mp&s=160"},"body":"On Mon, Apr 30, 2018 at 12:31 PM, Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n> Hi Tiago,\n>\n>> > On Thu, 12 Apr 2018, Tiago Botelho wrote:\n>> >\n>> >> On Thu, Apr 12, 2018 at 9:58 AM, Christian Couder\n>> >> <christian.couder@gmail.com> wrote:\n>> >> > On Thu, Apr 12, 2018 at 9:49 AM, Harald Nordgren\n>> >> > <haraldnordgren@gmail.com> wrote:\n>> >> >> I think it looks similar. But if I'm reading that thread correctly\n>> >> >> then there was never a patch created, right?\n>> >> >\n>> >> > (It is customary on this mailing list to reply after the sentences we\n>> >> > reply to. We don't \"top post\".)\n>> >> >\n>> >> > On the GSoC idea pages (like https://git.github.io/SoC-2018-Ideas/) we\n>> >> > have been suggesting \"Implement git bisect --first-parent\" and there\n>> >> > are the following related links:\n>> >> >\n>> >> > https://public-inbox.org/git/20150304053333.GA9584@peff.net/\n>> >> > https://public-inbox.org/git/4D3CDDF9.6080405@intel.com/\n>> >> >\n>> >> > Tiago in Cc also tried at a recent London hackathon to implement it\n>> >> > and came up with the following:\n>> >> >\n>> >> > https://github.com/tiagonbotelho/git/pull/1/files\n>> >> >\n>> >> > I tried to help him by reworking his commit in the following branch:\n>> >> >\n>> >> > https://github.com/chriscool/git/commits/myfirstparent\n>> >>\n>> >> Thank you for the cc Christian, I’ve been quite busy and was not able\n>> >> to work on the PR for quite some time.\n>> >>\n>> >> I intended to pick it back up again next week. If it is ok with\n>> >> Harald I would love to finish the PR that I started, since it is\n>> >> quite close to being finished (I think it was just specs missing if I\n>> >> am not mistaken).\n>\n> It is now well after \"next week\". Are there any news? Or could you unblock\n> Harald by stating that you won't come back to it any time soon (in\n> particular since the PR is not quite as finished from my reading as you\n> made it sound...)?\n>\n> Ciao,\n> Johannes\n\nI've been working on the feature for the past week\nhttps://github.com/tiagonbotelho/git/commit/709e2e248ebfb1deab12fd7d3da4611002dfaf86#diff-118df990fd68a0929bca5441fea06fc7\n\nI have some comments sent by Christian I plan on fixing this week\n\nThank you,\n\n-- \nTiago Botelho\n"},{"id":"346241","messageId":"nycvar.QRO.7.76.6.1805011330510.79@tvgsbejvaqbjf.bet","threadId":"48270","inReplyTo":"CAF2GuqvTvPPfGPXCwhBkTHJvRQcV1w9VebHkn+Kqw6dZ2b0xKQ@mail.gmail.com","subject":"Re: [PATCH] Create '--merges-only' option for 'git bisect'","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-05-01T11:31:14Z","receivedAt":"2018-05-01T11:31:20Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Tiago,\n\nOn Mon, 30 Apr 2018, Tiago Botelho wrote:\n\n> On Mon, Apr 30, 2018 at 12:31 PM, Johannes Schindelin\n> <Johannes.Schindelin@gmx.de> wrote:\n> >\n> >> > On Thu, 12 Apr 2018, Tiago Botelho wrote:\n> >> >\n> >> >> On Thu, Apr 12, 2018 at 9:58 AM, Christian Couder\n> >> >> <christian.couder@gmail.com> wrote:\n> >> >> > On Thu, Apr 12, 2018 at 9:49 AM, Harald Nordgren\n> >> >> > <haraldnordgren@gmail.com> wrote:\n> >> >> >> I think it looks similar. But if I'm reading that thread correctly\n> >> >> >> then there was never a patch created, right?\n> >> >> >\n> >> >> > (It is customary on this mailing list to reply after the sentences we\n> >> >> > reply to. We don't \"top post\".)\n> >> >> >\n> >> >> > On the GSoC idea pages (like https://git.github.io/SoC-2018-Ideas/) we\n> >> >> > have been suggesting \"Implement git bisect --first-parent\" and there\n> >> >> > are the following related links:\n> >> >> >\n> >> >> > https://public-inbox.org/git/20150304053333.GA9584@peff.net/\n> >> >> > https://public-inbox.org/git/4D3CDDF9.6080405@intel.com/\n> >> >> >\n> >> >> > Tiago in Cc also tried at a recent London hackathon to implement it\n> >> >> > and came up with the following:\n> >> >> >\n> >> >> > https://github.com/tiagonbotelho/git/pull/1/files\n> >> >> >\n> >> >> > I tried to help him by reworking his commit in the following branch:\n> >> >> >\n> >> >> > https://github.com/chriscool/git/commits/myfirstparent\n> >> >>\n> >> >> Thank you for the cc Christian, I’ve been quite busy and was not able\n> >> >> to work on the PR for quite some time.\n> >> >>\n> >> >> I intended to pick it back up again next week. If it is ok with\n> >> >> Harald I would love to finish the PR that I started, since it is\n> >> >> quite close to being finished (I think it was just specs missing if I\n> >> >> am not mistaken).\n> >\n> > It is now well after \"next week\". Are there any news? Or could you unblock\n> > Harald by stating that you won't come back to it any time soon (in\n> > particular since the PR is not quite as finished from my reading as you\n> > made it sound...)?\n> >\n> > Ciao,\n> > Johannes\n> \n> I've been working on the feature for the past week\n> https://github.com/tiagonbotelho/git/commit/709e2e248ebfb1deab12fd7d3da4611002dfaf86#diff-118df990fd68a0929bca5441fea06fc7\n> \n> I have some comments sent by Christian I plan on fixing this week\n\nOkay, great! I was concerned because of the long silence.\n\nCiao,\nJohannes"}]}