{"thread":{"id":"53937","subject":"[PATCH 0/3] Introduce --first-parent flag for git bisect","startedAt":"2020-07-28T15:44:21Z","lastAt":"2020-08-07T22:20:44Z","messageCount":42,"participants":["Aaron Lipman via GitGitGadget","Junio C Hamano","Aaron Lipman","Martin Ågren","Eric Sunshine","Christian Couder"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"402199","messageId":"pull.686.git.1595951056.gitgitgadget@gmail.com","threadId":"53937","inReplyTo":null,"subject":"[PATCH 0/3] Introduce --first-parent flag for git bisect","fromName":"Aaron Lipman via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-07-28T15:44:13Z","receivedAt":"2020-07-28T15:44:21Z","isPatch":true,"sender":{"key":"alipman88@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3922511?v=4"},"body":"I've always wished git bisect had a first-parent flag, e.g. \"git bisect\nstart --first-parent\"\n\nThis option would be particularly useful in avoiding false positives when\nbisecting, if a merged branch contained broken or non-buildable commits, but\nthe merge itself was OK.\n\nThere have been a couple attempts/iterations towards this functionality in\nthe past two years or so, and I'd like to get this across the finish line.\n\nThe previous iterations have focussed on the preliminary step of editing\nfunctions in bisect.c to accept a first_parent_only parameter, enabling the\n--bisect and --first-parent flags to be used in conjunction with git\nrev-list. In addition to updating git rev-list, I intend to enable the\n--first-parent flag on git bisect as well.\n\nI've taken some of the feedback from previous iterations into account,\nspecifically tidying up the code that assigns weights to commits in\ndo_find_bisection(), utilizing existing commit graphs when running tests,\nand clarifying the test for \"git rev-list --bisect-all --first-parent\"\n\nPrevious two iterations (most recent first):\nhttps://lore.kernel.org/git/20191105052141.15913-1-workingjubilee@gmail.com/\nhttps://lore.kernel.org/git/20180828123234.44582-1-tiagonbotelho@hotmail.com/\n\nRelated discussion about combining multiple boolean params passed to\nfind_bisection() into a single unsigned integer:\nhttps://lore.kernel.org/git/20180415085841.1269-1-haraldnordgren@gmail.com/\n\nAaron Lipman (3):\n rev-list: allow bisect and first-parent flags\n bisect: introduce first-parent flag\n bisect: combine args passed to find_bisection()\n\n Documentation/git-bisect.txt | 13 +++++-\n Documentation/rev-list-options.txt | 7 ++--\n bisect.c | 66 +++++++++++++++++++++---------\n bisect.h | 7 +++-\n builtin/bisect--helper.c | 16 ++++++--\n builtin/rev-list.c | 9 +++-\n revision.c | 3 --\n t/t6000-rev-list-misc.sh | 4 +-\n t/t6002-rev-list-bisect.sh | 45 ++++++++++++++++++++\n t/t6030-bisect-porcelain.sh | 17 ++++++++\n 10 files changed, 151 insertions(+), 36 deletions(-)\n\n\nbase-commit: 00a7a21b97a47889cf66de32f8defed023c14c2c\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-686%2Falipman88%2Fbisect_first_parent_final_draft-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-686/alipman88/bisect_first_parent_final_draft-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/686\n-- \ngitgitgadget\n"},{"id":"402200","messageId":"25263842832eeb09769b55cec025978b5f361eca.1595951056.git.gitgitgadget@gmail.com","threadId":"53937","inReplyTo":"pull.686.git.1595951056.gitgitgadget@gmail.com","subject":"[PATCH 1/3] rev-list: allow bisect and first-parent flags","fromName":"Aaron Lipman via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-07-28T15:44:14Z","receivedAt":"2020-07-28T15:44:22Z","isPatch":true,"sender":{"key":"alipman88@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3922511?v=4"},"body":"From: Aaron Lipman \n\nAdd first_parent_only parameter to find_bisection(), removing the\nbarrier that prevented combining the --bisect and --first-parent flags\nwhen using git rev-list\n\nSigned-off-by: Aaron Lipman \nBased-on-patch-by: Tiago Botelho \n---\n Documentation/rev-list-options.txt | 7 ++---\n bisect.c | 43 +++++++++++++++++-----------\n bisect.h | 2 +-\n builtin/rev-list.c | 2 +-\n revision.c | 3 --\n t/t6000-rev-list-misc.sh | 4 +--\n t/t6002-rev-list-bisect.sh | 45 ++++++++++++++++++++++++++++++\n 7 files changed, 79 insertions(+), 27 deletions(-)\n\ndiff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\nindex b01b2b6773..d3117ce51b 100644\n--- a/Documentation/rev-list-options.txt\n+++ b/Documentation/rev-list-options.txt\n@@ -128,8 +128,7 @@ parents) and `--max-parents=-1` (negative numbers denote no upper limit).\n \tbecause merges into a topic branch tend to be only about\n \tadjusting to updated upstream from time to time, and\n \tthis option allows you to ignore the individual commits\n-\tbrought in to your history by such a merge. Cannot be\n-\tcombined with --bisect.\n+\tbrought in to your history by such a merge.\n \n --not::\n \tReverses the meaning of the '{caret}' prefix (or lack thereof)\n@@ -207,7 +206,7 @@ ifndef::git-rev-list[]\n \tPretend as if the bad bisection ref `refs/bisect/bad`\n \twas listed and as if it was followed by `--not` and the good\n \tbisection refs `refs/bisect/good-*` on the command\n-\tline. Cannot be combined with --first-parent.\n+\tline.\n endif::git-rev-list[]\n \n --stdin::\n@@ -743,7 +742,7 @@ outputs 'midpoint', the output of the two commands\n would be of roughly the same length. Finding the change which\n introduces a regression is thus reduced to a binary search: repeatedly\n generate and test new 'midpoint's until the commit chain is of length\n-one. Cannot be combined with --first-parent.\n+one.\n \n --bisect-vars::\n \tThis calculates the same as `--bisect`, except that refs in\ndiff --git a/bisect.c b/bisect.c\nindex d5e830410f..762f68c8e9 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -37,7 +37,7 @@ static const char *term_good;\n * We care just barely enough to avoid recursing for\n * non-merge entries.\n */\n-static int count_distance(struct commit_list *entry)\n+static int count_distance(struct commit_list *entry, int first_parent_only)\n {\n \tint nr = 0;\n \n@@ -52,10 +52,10 @@ static int count_distance(struct commit_list *entry)\n \t\tcommit->object.flags |= COUNTED;\n \t\tp = commit->parents;\n \t\tentry = p;\n-\t\tif (p) {\n+\t\tif (p && !first_parent_only) {\n \t\t\tp = p->next;\n \t\t\twhile (p) {\n-\t\t\t\tnr += count_distance(p);\n+\t\t\t\tnr += count_distance(p, first_parent_only);\n \t\t\t\tp = p->next;\n \t\t\t}\n \t\t}\n@@ -88,15 +88,16 @@ static inline void weight_set(struct commit_list *elem, int weight)\n \t**commit_weight_at(&commit_weight, elem->item) = weight;\n }\n \n-static int count_interesting_parents(struct commit *commit)\n+static int count_interesting_parents(struct commit *commit, int first_parent_only)\n {\n \tstruct commit_list *p;\n \tint count;\n \n \tfor (count = 0, p = commit->parents; p; p = p->next) {\n-\t\tif (p->item->object.flags & UNINTERESTING)\n-\t\t\tcontinue;\n-\t\tcount++;\n+\t\tif (!(p->item->object.flags & UNINTERESTING))\n+\t\t\tcount++;\n+\t\tif (first_parent_only)\n+\t\t\tbreak;\n \t}\n \treturn count;\n }\n@@ -259,7 +260,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 first_parent_only)\n {\n \tint n, counted;\n \tstruct commit_list *p;\n@@ -271,7 +272,7 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\n \t\tunsigned flags = commit->object.flags;\n \n \t\t*commit_weight_at(&commit_weight, p->item) = &weights[n++];\n-\t\tswitch (count_interesting_parents(commit)) {\n+\t\tswitch (count_interesting_parents(commit, first_parent_only)) {\n \t\tcase 0:\n \t\t\tif (!(flags & TREESAME)) {\n \t\t\t\tweight_set(p, 1);\n@@ -314,7 +315,7 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\n \t\t\tcontinue;\n \t\tif (weight(p) != -2)\n \t\t\tcontinue;\n-\t\tweight_set(p, count_distance(p));\n+\t\tweight_set(p, count_distance(p, first_parent_only));\n \t\tclear_distance(list);\n \n \t\t/* Does it happen to be at exactly half-way? */\n@@ -330,13 +331,21 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\n \t\t\tstruct commit_list *q;\n \t\t\tunsigned flags = p->item->object.flags;\n \n+\t\t\t/* if commit p has already been weighted, continue. */\n \t\t\tif (0 <= weight(p))\n \t\t\t\tcontinue;\n+\n+\t\t\t/*\n+\t\t\t * otherwise, find the first interesting\n+\t\t\t * already-weighted parent of p and store as q.\n+\t\t\t */\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 (0 <= weight(q))\n+\t\t\t\tif (!(q->item->object.flags & UNINTERESTING) && 0 <= weight(q)) {\n+\t\t\t\t\tbreak;\n+\t\t\t\t} else if (first_parent_only) {\n+\t\t\t\t\tq = NULL;\n \t\t\t\t\tbreak;\n+\t\t\t\t}\n \t\t\t}\n \t\t\tif (!q)\n \t\t\t\tcontinue;\n@@ -370,7 +379,7 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\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 first_parent_only)\n {\n \tint nr, on_list;\n \tstruct commit_list *list, *p, *best, *next, *last;\n@@ -406,7 +415,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, first_parent_only);\n \tif (best) {\n \t\tif (!find_all) {\n \t\t\tlist->item = best->item;\n@@ -991,6 +1000,7 @@ enum bisect_error bisect_next_all(struct repository *r, const char *prefix, int\n \tenum bisect_error res = BISECT_OK;\n \tstruct object_id *bisect_rev;\n \tchar *steps_msg;\n+\tint first_parent_only = 0; /* TODO: pass --first-parent flag from git bisect start */\n \n \tread_bisect_terms(&term_bad, &term_good);\n \tif (read_bisect_refs())\n@@ -1001,11 +1011,12 @@ enum bisect_error bisect_next_all(struct repository *r, const char *prefix, int\n \t\treturn res;\n \n \tbisect_rev_setup(r, &revs, prefix, \"%s\", \"^%s\", 1);\n+\trevs.first_parent_only = first_parent_only;\n \trevs.limited = 1;\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, first_parent_only);\n \trevs.commits = managed_skipped(revs.commits, &tried);\n \n \tif (!revs.commits) {\ndiff --git a/bisect.h b/bisect.h\nindex 8bad8d8391..a63af0505f 100644\n--- a/bisect.h\n+++ b/bisect.h\n@@ -12,7 +12,7 @@ struct repository;\n * best commit, as chosen by `find_all`.\n */\n void find_bisection(struct commit_list **list, int *reaches, int *all,\n-\t\t int find_all);\n+\t\t int find_all, int first_parent_only);\n \n struct commit_list *filter_skipped(struct commit_list *list,\n \t\t\t\t struct commit_list **tried,\ndiff --git a/builtin/rev-list.c b/builtin/rev-list.c\nindex f520111eda..d1a14596b2 100644\n--- a/builtin/rev-list.c\n+++ b/builtin/rev-list.c\n@@ -638,7 +638,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, revs.first_parent_only);\n \n \t\tif (bisect_show_vars)\n \t\t\treturn show_bisect_vars(&info, reaches, all);\ndiff --git a/revision.c b/revision.c\nindex 6aa7f4f567..f0ad2603c1 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2869,9 +2869,6 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n \tif (!revs->reflog_info && revs->grep_filter.use_reflog_filter)\n \t\tdie(\"cannot use --grep-reflog without --walk-reflogs\");\n \n-\tif (revs->first_parent_only && revs->bisect)\n-\t\tdie(_(\"--first-parent is incompatible with --bisect\"));\n-\n \tif (revs->line_level_traverse &&\n \t (revs->diffopt.output_format & ~(DIFF_FORMAT_PATCH | DIFF_FORMAT_NO_OUTPUT)))\n \t\tdie(_(\"-L does not yet support diff formats besides -p and -s\"));\ndiff --git a/t/t6000-rev-list-misc.sh b/t/t6000-rev-list-misc.sh\nindex 3dc1ad8f71..26e3533562 100755\n--- a/t/t6000-rev-list-misc.sh\n+++ b/t/t6000-rev-list-misc.sh\n@@ -125,8 +125,8 @@ test_expect_success 'rev-list can negate index objects' '\n \ttest_cmp expect actual\n '\n \n-test_expect_success '--bisect and --first-parent can not be combined' '\n-\ttest_must_fail git rev-list --bisect --first-parent HEAD\n+test_expect_success '--bisect and --first-parent can be combined' '\n+\tgit rev-list --bisect --first-parent HEAD\n '\n \n test_expect_success '--header shows a NUL after each commit' '\ndiff --git a/t/t6002-rev-list-bisect.sh b/t/t6002-rev-list-bisect.sh\nindex a661408038..7fc0f75ca5 100755\n--- a/t/t6002-rev-list-bisect.sh\n+++ b/t/t6002-rev-list-bisect.sh\n@@ -263,4 +263,49 @@ test_expect_success 'rev-parse --bisect can default to good/bad refs' '\n \ttest_cmp expect.sorted actual.sorted\n '\n \n+test_output_expect_success \"--bisect --first-parent\" 'git rev-list --bisect --first-parent E ^F' < expect.unsorted < expect &&\n+git rev-list --bisect-all --first-parent E ^F > actual &&\n+test_cmp actual expect\n+'\n+\n test_done\n-- \ngitgitgadget\n\n"},{"id":"402202","messageId":"66720c6425d7e497832f9291804de1e433909535.1595951056.git.gitgitgadget@gmail.com","threadId":"53937","inReplyTo":"pull.686.git.1595951056.gitgitgadget@gmail.com","subject":"[PATCH 2/3] bisect: introduce first-parent flag","fromName":"Aaron Lipman via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-07-28T15:44:15Z","receivedAt":"2020-07-28T15:44:25Z","isPatch":true,"sender":{"key":"alipman88@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3922511?v=4"},"body":"From: Aaron Lipman \n\nUpon seeing a merge commit when bisecting, this option may be used to\nfollow only the first parent.\n\nIn detecting regressions introduced through the merging of a branch, the\nmerge commit will be identified as introduction of the bug and its\nancestors will be ignored.\n\nThis option is particularly useful in avoiding false positives when a\nmerged branch contained broken or non-buildable commits, but the merge\nitself was OK.\n\nSigned-off-by: Aaron Lipman \n---\n Documentation/git-bisect.txt | 13 ++++++++++++-\n bisect.c | 10 +++++++++-\n bisect.h | 2 ++\n builtin/bisect--helper.c | 16 ++++++++++++----\n t/t6030-bisect-porcelain.sh | 17 +++++++++++++++++\n 5 files changed, 52 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt\nindex 7586c5a843..0e993e4587 100644\n--- a/Documentation/git-bisect.txt\n+++ b/Documentation/git-bisect.txt\n@@ -17,7 +17,7 @@ The command takes various subcommands, and different options depending\n on the subcommand:\n \n git bisect start [--term-{old,good}= --term-{new,bad}=]\n-\t\t [--no-checkout] [ [...]] [--] [...]\n+\t\t [--no-checkout] [--first-parent] [ [...]] [--] [...]\n git bisect (bad|new|) []\n git bisect (good|old|) [...]\n git bisect terms [--term-good | --term-bad]\n@@ -365,6 +365,17 @@ does not require a checked out tree.\n +\n If the repository is bare, `--no-checkout` is assumed.\n \n+--first-parent::\n++\n+Follow only the first parent commit upon seeing a merge commit.\n++\n+In detecting regressions introduced through the merging of a branch, the merge\n+commit will be identified as introduction of the bug and its ancestors will be\n+ignored.\n++\n+This option is particularly useful in avoiding false positives when a merged\n+branch contained broken or non-buildable commits, but the merge itself was OK.\n+\n EXAMPLES\n --------\n \ndiff --git a/bisect.c b/bisect.c\nindex 762f68c8e9..eee1f6f138 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -463,6 +463,7 @@ static GIT_PATH_FUNC(git_path_bisect_run, \"BISECT_RUN\")\n static GIT_PATH_FUNC(git_path_bisect_start, \"BISECT_START\")\n static GIT_PATH_FUNC(git_path_bisect_log, \"BISECT_LOG\")\n static GIT_PATH_FUNC(git_path_bisect_terms, \"BISECT_TERMS\")\n+static GIT_PATH_FUNC(git_path_bisect_first_parent, \"BISECT_FIRST_PARENT\")\n static GIT_PATH_FUNC(git_path_head_name, \"head-name\")\n \n static void read_bisect_paths(struct argv_array *array)\n@@ -984,6 +985,12 @@ void read_bisect_terms(const char **read_bad, const char **read_good)\n \tfclose(fp);\n }\n \n+int read_first_parent_option(void)\n+{\n+\tconst char *filename = git_path_bisect_first_parent();\n+\treturn !access(filename, F_OK);\n+}\n+\n /*\n * We use the convention that return BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND (-10) means\n * the bisection process finished successfully.\n@@ -1000,7 +1007,7 @@ enum bisect_error bisect_next_all(struct repository *r, const char *prefix, int\n \tenum bisect_error res = BISECT_OK;\n \tstruct object_id *bisect_rev;\n \tchar *steps_msg;\n-\tint first_parent_only = 0; /* TODO: pass --first-parent flag from git bisect start */\n+\tint first_parent_only = read_first_parent_option();\n \n \tread_bisect_terms(&term_bad, &term_good);\n \tif (read_bisect_refs())\n@@ -1144,6 +1151,7 @@ int bisect_clean_state(void)\n \tunlink_or_warn(git_path_bisect_names());\n \tunlink_or_warn(git_path_bisect_run());\n \tunlink_or_warn(git_path_bisect_terms());\n+\tunlink_or_warn(git_path_bisect_first_parent());\n \t/* Cleanup head-name if it got left by an old version of git-bisect */\n \tunlink_or_warn(git_path_head_name());\n \t/*\ndiff --git a/bisect.h b/bisect.h\nindex a63af0505f..8ee80f5b48 100644\n--- a/bisect.h\n+++ b/bisect.h\n@@ -66,6 +66,8 @@ int estimate_bisect_steps(int all);\n \n void read_bisect_terms(const char **bad, const char **good);\n \n+int read_first_parent_option(void);\n+\n int bisect_clean_state(void);\n \n #endif\ndiff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\nindex ec4996282e..1236f5df1d 100644\n--- a/builtin/bisect--helper.c\n+++ b/builtin/bisect--helper.c\n@@ -17,6 +17,7 @@ static GIT_PATH_FUNC(git_path_bisect_head, \"BISECT_HEAD\")\n static GIT_PATH_FUNC(git_path_bisect_log, \"BISECT_LOG\")\n static GIT_PATH_FUNC(git_path_head_name, \"head-name\")\n static GIT_PATH_FUNC(git_path_bisect_names, \"BISECT_NAMES\")\n+static GIT_PATH_FUNC(git_path_bisect_first_parent, \"BISECT_FIRST_PARENT\")\n \n static const char * const git_bisect_helper_usage[] = {\n \tN_(\"git bisect--helper --next-all [--no-checkout]\"),\n@@ -28,7 +29,7 @@ static const char * const git_bisect_helper_usage[] = {\n \tN_(\"git bisect--helper --bisect-next-check []\"),\n \tN_(\"git bisect--helper --bisect-terms [--term-good | --term-old | --term-bad | --term-new]\"),\n \tN_(\"git bisect--helper --bisect-start [--term-{old,good}= --term-{new,bad}=]\"\n-\t\t\t\t\t \"[--no-checkout] [ [...]] [--] [...]\"),\n+\t\t\t\t\t \" [--no-checkout] [--first-parent] [ [...]] [--] [...]\"),\n \tNULL\n };\n \n@@ -422,7 +423,7 @@ static int bisect_append_log_quoted(const char **argv)\n }\n \n static int bisect_start(struct bisect_terms *terms, int no_checkout,\n-\t\t\tconst char **argv, int argc)\n+\t\t\tint first_parent_only, const char **argv, int argc)\n {\n \tint i, has_double_dash = 0, must_write_terms = 0, bad_seen = 0;\n \tint flags, pathspec_pos, res = 0;\n@@ -453,6 +454,8 @@ static int bisect_start(struct bisect_terms *terms, int no_checkout,\n \t\t\tbreak;\n \t\t} else if (!strcmp(arg, \"--no-checkout\")) {\n \t\t\tno_checkout = 1;\n+\t\t} else if (!strcmp(arg, \"--first-parent\")) {\n+\t\t\tfirst_parent_only = 1;\n \t\t} else if (!strcmp(arg, \"--term-good\") ||\n \t\t\t !strcmp(arg, \"--term-old\")) {\n \t\t\ti++;\n@@ -577,6 +580,9 @@ static int bisect_start(struct bisect_terms *terms, int no_checkout,\n \t */\n \twrite_file(git_path_bisect_start(), \"%s\\n\", start_head.buf);\n \n+\tif (first_parent_only)\n+\t\twrite_file(git_path_bisect_first_parent(), \"\\n\");\n+\n \tif (no_checkout) {\n \t\tif (get_oid(start_head.buf, &oid) < 0) {\n \t\t\tres = error(_(\"invalid ref: '%s'\"), start_head.buf);\n@@ -632,7 +638,7 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\tBISECT_TERMS,\n \t\tBISECT_START\n \t} cmdmode = 0;\n-\tint no_checkout = 0, res = 0, nolog = 0;\n+\tint no_checkout = 0, first_parent_only = 0, res = 0, nolog = 0;\n \tstruct option options[] = {\n \t\tOPT_CMDMODE(0, \"next-all\", &cmdmode,\n \t\t\t N_(\"perform 'git bisect next'\"), NEXT_ALL),\n@@ -656,6 +662,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"start the bisect session\"), BISECT_START),\n \t\tOPT_BOOL(0, \"no-checkout\", &no_checkout,\n \t\t\t N_(\"update BISECT_HEAD instead of checking out the current commit\")),\n+\t\tOPT_BOOL(0, \"first-parent\", &first_parent_only,\n+\t\t\t N_(\"only trace the first parent of merge commits\")),\n \t\tOPT_BOOL(0, \"no-log\", &nolog,\n \t\t\t N_(\"no log for BISECT_WRITE\")),\n \t\tOPT_END()\n@@ -713,7 +721,7 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\tbreak;\n \tcase BISECT_START:\n \t\tset_terms(&terms, \"bad\", \"good\");\n-\t\tres = bisect_start(&terms, no_checkout, argv, argc);\n+\t\tres = bisect_start(&terms, no_checkout, first_parent_only, argv, argc);\n \t\tbreak;\n \tdefault:\n \t\treturn error(\"BUG: unknown subcommand '%d'\", cmdmode);\ndiff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh\nindex 36d9b2b2e4..7965698310 100755\n--- a/t/t6030-bisect-porcelain.sh\n+++ b/t/t6030-bisect-porcelain.sh\n@@ -458,6 +458,23 @@ test_expect_success 'many merge bases creation' '\n \tgrep \"$SIDE_HASH5\" merge_bases.txt\n '\n \n+# We want to automatically find the merge that\n+# introduced \"line\" into hello.\n+test_expect_success \\\n+ '\"git bisect run --first-parent\" simple case' \\\n+ 'echo \"#\"\\!\"/bin/sh\" > test_script.sh &&\n+ echo \"grep line hello > /dev/null\" >> test_script.sh &&\n+ echo \"test \\$? -ne 0\" >> test_script.sh &&\n+ chmod +x test_script.sh &&\n+ git bisect start --first-parent &&\n+ test_path_is_file \".git/BISECT_FIRST_PARENT\" &&\n+ git bisect good $HASH4 &&\n+ git bisect bad $B_HASH &&\n+ git bisect run ./test_script.sh > my_bisect_log.txt &&\n+ grep \"$B_HASH is the first bad commit\" my_bisect_log.txt &&\n+ git bisect reset &&\n+ test_path_is_missing \".git/BISECT_FIRST_PARENT\"'\n+\n test_expect_success 'good merge bases when good and bad are siblings' '\n \tgit bisect start \"$B_HASH\" \"$A_HASH\" > my_bisect_log.txt &&\n \ttest_i18ngrep \"merge base must be tested\" my_bisect_log.txt &&\n-- \ngitgitgadget\n\n"},{"id":"402201","messageId":"24d116870e5c9ae03b585737a197f38d679fedb3.1595951056.git.gitgitgadget@gmail.com","threadId":"53937","inReplyTo":"pull.686.git.1595951056.gitgitgadget@gmail.com","subject":"[PATCH 3/3] bisect: combine args passed to find_bisection()","fromName":"Aaron Lipman via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-07-28T15:44:16Z","receivedAt":"2020-07-28T15:44:27Z","isPatch":true,"sender":{"key":"alipman88@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3922511?v=4"},"body":"From: Aaron Lipman \n\nNow that find_bisection() accepts multiple boolean arguments, these may\nbe combined into a single unsigned integer in order to declutter some of\nthe code in bisect.c\n\nSigned-off-by: Aaron Lipman \nBased-on-patch-by: Harald Nordgren \n---\n bisect.c | 43 +++++++++++++++++++++++++------------------\n bisect.h | 5 ++++-\n builtin/rev-list.c | 9 ++++++++-\n 3 files changed, 37 insertions(+), 20 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex eee1f6f138..9599c58fd8 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -37,7 +37,7 @@ static const char *term_good;\n * We care just barely enough to avoid recursing for\n * non-merge entries.\n */\n-static int count_distance(struct commit_list *entry, int first_parent_only)\n+static int count_distance(struct commit_list *entry, unsigned bisect_flags)\n {\n \tint nr = 0;\n \n@@ -52,10 +52,10 @@ static int count_distance(struct commit_list *entry, int first_parent_only)\n \t\tcommit->object.flags |= COUNTED;\n \t\tp = commit->parents;\n \t\tentry = p;\n-\t\tif (p && !first_parent_only) {\n+\t\tif (p && !(bisect_flags & BISECT_FIRST_PARENT)) {\n \t\t\tp = p->next;\n \t\t\twhile (p) {\n-\t\t\t\tnr += count_distance(p, first_parent_only);\n+\t\t\t\tnr += count_distance(p, bisect_flags);\n \t\t\t\tp = p->next;\n \t\t\t}\n \t\t}\n@@ -88,7 +88,7 @@ static inline void weight_set(struct commit_list *elem, int weight)\n \t**commit_weight_at(&commit_weight, elem->item) = weight;\n }\n \n-static int count_interesting_parents(struct commit *commit, int first_parent_only)\n+static int count_interesting_parents(struct commit *commit, unsigned bisect_flags)\n {\n \tstruct commit_list *p;\n \tint count;\n@@ -96,7 +96,7 @@ static int count_interesting_parents(struct commit *commit, int first_parent_onl\n \tfor (count = 0, p = commit->parents; p; p = p->next) {\n \t\tif (!(p->item->object.flags & UNINTERESTING))\n \t\t\tcount++;\n-\t\tif (first_parent_only)\n+\t\tif (bisect_flags & BISECT_FIRST_PARENT)\n \t\t\tbreak;\n \t}\n \treturn count;\n@@ -260,7 +260,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, int first_parent_only)\n+\t\t\t\t\t unsigned bisect_flags)\n {\n \tint n, counted;\n \tstruct commit_list *p;\n@@ -272,7 +272,7 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\n \t\tunsigned flags = commit->object.flags;\n \n \t\t*commit_weight_at(&commit_weight, p->item) = &weights[n++];\n-\t\tswitch (count_interesting_parents(commit, first_parent_only)) {\n+\t\tswitch (count_interesting_parents(commit, bisect_flags)) {\n \t\tcase 0:\n \t\t\tif (!(flags & TREESAME)) {\n \t\t\t\tweight_set(p, 1);\n@@ -315,11 +315,11 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\n \t\t\tcontinue;\n \t\tif (weight(p) != -2)\n \t\t\tcontinue;\n-\t\tweight_set(p, count_distance(p, first_parent_only));\n+\t\tweight_set(p, count_distance(p, bisect_flags));\n \t\tclear_distance(list);\n \n \t\t/* Does it happen to be at exactly half-way? */\n-\t\tif (!find_all && halfway(p, nr))\n+\t\tif (!(bisect_flags & BISECT_FIND_ALL) && halfway(p, nr))\n \t\t\treturn p;\n \t\tcounted++;\n \t}\n@@ -342,7 +342,7 @@ 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) && 0 <= weight(q)) {\n \t\t\t\t\tbreak;\n-\t\t\t\t} else if (first_parent_only) {\n+\t\t\t\t} else if (bisect_flags & BISECT_FIRST_PARENT) {\n \t\t\t\t\tq = NULL;\n \t\t\t\t\tbreak;\n \t\t\t\t}\n@@ -365,21 +365,21 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\n \t\t\t\tweight_set(p, weight(q));\n \n \t\t\t/* Does it happen to be at exactly half-way? */\n-\t\t\tif (!find_all && halfway(p, nr))\n+\t\t\tif (!(bisect_flags & BISECT_FIND_ALL) && halfway(p, nr))\n \t\t\t\treturn p;\n \t\t}\n \t}\n \n \tshow_list(\"bisection 2 counted all\", counted, nr, list);\n \n-\tif (!find_all)\n+\tif (!(bisect_flags & BISECT_FIND_ALL))\n \t\treturn best_bisection(list, nr);\n \telse\n \t\treturn best_bisection_sorted(list, nr);\n }\n \n void find_bisection(struct commit_list **commit_list, int *reaches,\n-\t\t int *all, int find_all, int first_parent_only)\n+\t\t int *all, unsigned bisect_flags)\n {\n \tint nr, on_list;\n \tstruct commit_list *list, *p, *best, *next, *last;\n@@ -415,9 +415,9 @@ 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, first_parent_only);\n+\tbest = do_find_bisection(list, nr, weights, bisect_flags);\n \tif (best) {\n-\t\tif (!find_all) {\n+\t\tif (!(bisect_flags & BISECT_FIND_ALL)) {\n \t\t\tlist->item = best->item;\n \t\t\tfree_commit_list(list->next);\n \t\t\tbest = list;\n@@ -1007,23 +1007,30 @@ enum bisect_error bisect_next_all(struct repository *r, const char *prefix, int\n \tenum bisect_error res = BISECT_OK;\n \tstruct object_id *bisect_rev;\n \tchar *steps_msg;\n-\tint first_parent_only = read_first_parent_option();\n+\tunsigned bisect_flags = 0;\n \n \tread_bisect_terms(&term_bad, &term_good);\n \tif (read_bisect_refs())\n \t\tdie(_(\"reading bisect refs failed\"));\n \n+\tif (read_first_parent_option())\n+\t\tbisect_flags |= BISECT_FIRST_PARENT;\n+\n+\tif (!!skipped_revs.nr)\n+\t\tbisect_flags |= BISECT_FIND_ALL;\n+\n \tres = check_good_are_ancestors_of_bad(r, prefix, no_checkout);\n \tif (res)\n \t\treturn res;\n \n \tbisect_rev_setup(r, &revs, prefix, \"%s\", \"^%s\", 1);\n-\trevs.first_parent_only = first_parent_only;\n+\n+\trevs.first_parent_only = !!(bisect_flags & BISECT_FIRST_PARENT);\n \trevs.limited = 1;\n \n \tbisect_common(&revs);\n \n-\tfind_bisection(&revs.commits, &reaches, &all, !!skipped_revs.nr, first_parent_only);\n+\tfind_bisection(&revs.commits, &reaches, &all, bisect_flags);\n \trevs.commits = managed_skipped(revs.commits, &tried);\n \n \tif (!revs.commits) {\ndiff --git a/bisect.h b/bisect.h\nindex 8ee80f5b48..402ddcfb5e 100644\n--- a/bisect.h\n+++ b/bisect.h\n@@ -12,7 +12,7 @@ struct repository;\n * best commit, as chosen by `find_all`.\n */\n void find_bisection(struct commit_list **list, int *reaches, int *all,\n-\t\t int find_all, int first_parent_only);\n+\t\t unsigned bisect_flags);\n \n struct commit_list *filter_skipped(struct commit_list *list,\n \t\t\t\t struct commit_list **tried,\n@@ -23,6 +23,9 @@ struct commit_list *filter_skipped(struct commit_list *list,\n #define BISECT_SHOW_ALL\t\t(1<<0)\n #define REV_LIST_QUIET\t\t(1<<1)\n \n+#define BISECT_FIND_ALL\t\t(1u<<0)\n+#define BISECT_FIRST_PARENT\t(1u<<1)\n+\n struct rev_list_info {\n \tstruct rev_info *revs;\n \tint flags;\ndiff --git a/builtin/rev-list.c b/builtin/rev-list.c\nindex d1a14596b2..1536ea6f28 100644\n--- a/builtin/rev-list.c\n+++ b/builtin/rev-list.c\n@@ -637,8 +637,15 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)\n \n \tif (bisect_list) {\n \t\tint reaches, all;\n+\t\tunsigned bisect_flags = 0;\n \n-\t\tfind_bisection(&revs.commits, &reaches, &all, bisect_find_all, revs.first_parent_only);\n+\t\tif (bisect_find_all)\n+\t\t\tbisect_flags |= BISECT_FIND_ALL;\n+\n+\t\tif (revs.first_parent_only)\n+\t\t\tbisect_flags |= BISECT_FIRST_PARENT;\n+\n+\t\tfind_bisection(&revs.commits, &reaches, &all, bisect_flags);\n \n \t\tif (bisect_show_vars)\n \t\t\treturn show_bisect_vars(&info, reaches, all);\n-- \ngitgitgadget\n"},{"id":"402232","messageId":"xmqq4kpre6cl.fsf@gitster.c.googlers.com","threadId":"53937","inReplyTo":"25263842832eeb09769b55cec025978b5f361eca.1595951056.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/3] rev-list: allow bisect and first-parent flags","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-07-28T20:23:54Z","receivedAt":"2020-07-28T20:24:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Aaron Lipman via GitGitGadget\" writes:\n\n> From: Aaron Lipman \n>\n> Add first_parent_only parameter to find_bisection(), removing the\n> barrier that prevented combining the --bisect and --first-parent flags\n> when using git rev-list\n>\n> Signed-off-by: Aaron Lipman \n> Based-on-patch-by: Tiago Botelho \n\nIn chronological order, please.\n\n> diff --git a/bisect.c b/bisect.c\n> index d5e830410f..762f68c8e9 100644\n> --- a/bisect.c\n> +++ b/bisect.c\n> @@ -37,7 +37,7 @@ static const char *term_good;\n> * We care just barely enough to avoid recursing for\n> * non-merge entries.\n> */\n> -static int count_distance(struct commit_list *entry)\n> +static int count_distance(struct commit_list *entry, int first_parent_only)\n> {\n> \tint nr = 0;\n> \n> @@ -52,10 +52,10 @@ static int count_distance(struct commit_list *entry)\n> \t\tcommit->object.flags |= COUNTED;\n> \t\tp = commit->parents;\n> \t\tentry = p;\n> -\t\tif (p) {\n> +\t\tif (p && !first_parent_only) {\n> \t\t\tp = p->next;\n> \t\t\twhile (p) {\n> -\t\t\t\tnr += count_distance(p);\n> +\t\t\t\tnr += count_distance(p, first_parent_only);\n> \t\t\t\tp = p->next;\n> \t\t\t}\n> \t\t}\n\nIf you are running a first-parent-only bisection, by default, each\ncommit in the \"goodness not yet known\" region is treated as a single\nparent commit for the purpose of bisection. As do_find_bisection()\navoids the cost of calling stupid-and-expensive count_distance() by\ntreating such a single-strand-of-pearls specially, wouldn't it be a\nBUG() if this funtion is called under first_parent_only mode in the\nfirst place?\n\n> @@ -88,15 +88,16 @@ static inline void weight_set(struct commit_list *elem, int weight)\n> \t**commit_weight_at(&commit_weight, elem->item) = weight;\n> }\n\n\n> -static int count_interesting_parents(struct commit *commit)\n> +static int count_interesting_parents(struct commit *commit, int first_parent_only)\n> {\n> \tstruct commit_list *p;\n> \tint count;\n> \n> \tfor (count = 0, p = commit->parents; p; p = p->next) {\n> -\t\tif (p->item->object.flags & UNINTERESTING)\n> -\t\t\tcontinue;\n> -\t\tcount++;\n> +\t\tif (!(p->item->object.flags & UNINTERESTING))\n> +\t\t\tcount++;\n> +\t\tif (first_parent_only)\n> +\t\t\tbreak;\n> \t}\n> \treturn count;\n> }\n\nAny merge will be checked for interesting-ness of its first parent;\nthere is either zero or one interesting parent and no other case\nunder first-parent-only. OK.\n\n> @@ -259,7 +260,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 first_parent_only)\n> {\n> \tint n, counted;\n> \tstruct commit_list *p;\n> @@ -271,7 +272,7 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\n> \t\tunsigned flags = commit->object.flags;\n> \n> \t\t*commit_weight_at(&commit_weight, p->item) = &weights[n++];\n> -\t\tswitch (count_interesting_parents(commit)) {\n> +\t\tswitch (count_interesting_parents(commit, first_parent_only)) {\n> \t\tcase 0:\n> \t\t\tif (!(flags & TREESAME)) {\n> \t\t\t\tweight_set(p, 1);\n> @@ -314,7 +315,7 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\n> \t\t\tcontinue;\n> \t\tif (weight(p) != -2)\n> \t\t\tcontinue;\n\nIf count-interesting-parents can never be 2 or more, you will never\nsee weight(p) == -2 (see the switch statement in the previous hunk).\n\nSo it your first-parent-only mode allowed this continue not to\ntrigger, you have a BUG(), I think.\n\n> -\t\tweight_set(p, count_distance(p));\n> +\t\tweight_set(p, count_distance(p, first_parent_only));\n\nHence, you do not need to touch count_distance() at all, no?\n\n> @@ -330,13 +331,21 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\n> \t\t\tstruct commit_list *q;\n> \t\t\tunsigned flags = p->item->object.flags;\n> \n> +\t\t\t/* if commit p has already been weighted, continue. */\n> \t\t\tif (0 <= weight(p))\n> \t\t\t\tcontinue;\n> +\n> +\t\t\t/*\n> +\t\t\t * otherwise, find the first interesting\n> +\t\t\t * already-weighted parent of p and store as q.\n> +\t\t\t */\n\nUnnecessary changes above.\n\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 (0 <= weight(q))\n> +\t\t\t\tif (!(q->item->object.flags & UNINTERESTING) && 0 <= weight(q)) {\n> +\t\t\t\t\tbreak;\n\nOverlong line. Fold it after \"&&\", like this:\n\n\t\t\t\tif (!(q->item->object.flags & UNINTERESTING) &&\n\t\t\t\t 0 <= weight(q)) {\n\nWhat is this change about?\n\nAh, if the first parent is uninteresting, we do not want to\ncontinue, but with the original loop, we may go on and check\nq->next. We just want to break out of the loop instead.\n\nI strongly suspect that\n\n\t\t\tfor (q = p->item->parents;\n\t\t\t q;\n\t\t\t q = first_parent_only ? NULL : q->next) {\n\nwithout any change inside the loop is much easier to reason about.\nWe follow exactly the same logic as the usual case. We just pretend\nthat all commits have only one single parent which is the first one.\n\nThanks.\n"},{"id":"402254","messageId":"xmqqa6zjcnni.fsf@gitster.c.googlers.com","threadId":"53937","inReplyTo":"xmqq4kpre6cl.fsf@gitster.c.googlers.com","subject":"Re: [PATCH 1/3] rev-list: allow bisect and first-parent flags","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-07-28T21:53:05Z","receivedAt":"2020-07-28T21:53:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taking all the suggestions and speculations I made in the message I\nam responding to, I think squashing the following changes into the\n[PATCH 1/3] would make the code simpler and much easier to follow.\n\nThe first two hunks revert the changes made to count_distance(); the\nthird hunk makes sure that count_distance() is not called under the\nfirst-parent mode, since the \"single-strand-of-pearls\" optimization\nis the only counting method needed in the first-parent mode. The\nlast hunk is to revert the changes to the single-strand-of-pearls\noptimized counting and tweak the loop control to take only the first\nparent into account.\n\nYour new tests in 6000 and 6002 of course still pass with this\nclean-up.\n\n bisect.c | 27 ++++++++++++---------------\n 1 file changed, 12 insertions(+), 15 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex 762f68c8e9..a11fdb1473 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -37,7 +37,7 @@ static const char *term_good;\n * We care just barely enough to avoid recursing for\n * non-merge entries.\n */\n-static int count_distance(struct commit_list *entry, int first_parent_only)\n+static int count_distance(struct commit_list *entry)\n {\n \tint nr = 0;\n \n@@ -52,10 +52,10 @@ static int count_distance(struct commit_list *entry, int first_parent_only)\n \t\tcommit->object.flags |= COUNTED;\n \t\tp = commit->parents;\n \t\tentry = p;\n-\t\tif (p && !first_parent_only) {\n+\t\tif (p) {\n \t\t\tp = p->next;\n \t\t\twhile (p) {\n-\t\t\t\tnr += count_distance(p, first_parent_only);\n+\t\t\t\tnr += count_distance(p);\n \t\t\t\tp = p->next;\n \t\t\t}\n \t\t}\n@@ -315,7 +315,9 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\n \t\t\tcontinue;\n \t\tif (weight(p) != -2)\n \t\t\tcontinue;\n-\t\tweight_set(p, count_distance(p, first_parent_only));\n+\t\tif (first_parent_only)\n+\t\t\tBUG(\"shouldn't be calling count-distance in fp mode\");\n+\t\tweight_set(p, count_distance(p));\n \t\tclear_distance(list);\n \n \t\t/* Does it happen to be at exactly half-way? */\n@@ -331,21 +333,16 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\n \t\t\tstruct commit_list *q;\n \t\t\tunsigned flags = p->item->object.flags;\n \n-\t\t\t/* if commit p has already been weighted, continue. */\n \t\t\tif (0 <= weight(p))\n \t\t\t\tcontinue;\n \n-\t\t\t/*\n-\t\t\t * otherwise, find the first interesting\n-\t\t\t * already-weighted parent of p and store as q.\n-\t\t\t */\n-\t\t\tfor (q = p->item->parents; q; q = q->next) {\n-\t\t\t\tif (!(q->item->object.flags & UNINTERESTING) && 0 <= weight(q)) {\n-\t\t\t\t\tbreak;\n-\t\t\t\t} else if (first_parent_only) {\n-\t\t\t\t\tq = NULL;\n+\t\t\tfor (q = p->item->parents;\n+\t\t\t q;\n+\t\t\t q = first_parent_only ? NULL : q->next) {\n+\t\t\t\tif (q->item->object.flags & UNINTERESTING)\n+\t\t\t\t\tcontinue;\n+\t\t\t\tif (0 <= weight(q))\n \t\t\t\t\tbreak;\n-\t\t\t\t}\n \t\t\t}\n \t\t\tif (!q)\n \t\t\t\tcontinue;\n\n\n"},{"id":"402441","messageId":"20200730002735.87655-1-alipman88@gmail.com","threadId":"53937","inReplyTo":"pull.686.git.1595951056.gitgitgadget@gmail.com","subject":"[PATCH v2 0/3] Introduce --first-parent flag for git bisect","fromName":"Aaron Lipman","fromEmail":"alipman88@gmail.com","sentAt":"2020-07-30T00:27:32Z","receivedAt":"2020-07-30T00:27:45Z","isPatch":true,"sender":{"key":"alipman88@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3922511?v=4"},"body":"Junio, thank you for the feedback and the explanation regarding\ncount_distance(). I've gone ahead and implemented all your suggestions.\n\n> In chronological order, please.\nI submitted my initial patch through GitGitGadget. I don't see a way to\ncontrol the order GitGitGadget sends emails, so I'm switching to\ngit send-email for this iteration. Please continue to let me know if I'm\nmissing a convention.\n\nAaron Lipman (3):\n rev-list: allow bisect and first-parent flags\n bisect: introduce first-parent flag\n bisect: combine args passed to find_bisection()\n\n Documentation/git-bisect.txt | 13 +++++++-\n Documentation/rev-list-options.txt | 7 ++--\n bisect.c | 51 ++++++++++++++++++++++--------\n bisect.h | 7 +++-\n builtin/bisect--helper.c | 16 +++++++---\n builtin/rev-list.c | 9 +++++-\n revision.c | 3 --\n t/t6000-rev-list-misc.sh | 4 +--\n t/t6002-rev-list-bisect.sh | 45 ++++++++++++++++++++++++++\n t/t6030-bisect-porcelain.sh | 17 ++++++++++\n 10 files changed, 142 insertions(+), 30 deletions(-)\n\n-- \n2.24.3 (Apple Git-128)\n\n"},{"id":"402442","messageId":"20200730002735.87655-2-alipman88@gmail.com","threadId":"53937","inReplyTo":"20200730002735.87655-1-alipman88@gmail.com","subject":"[PATCH v2 1/3] rev-list: allow bisect and first-parent flags","fromName":"Aaron Lipman","fromEmail":"alipman88@gmail.com","sentAt":"2020-07-30T00:27:33Z","receivedAt":"2020-07-30T00:27:47Z","isPatch":true,"sender":{"key":"alipman88@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3922511?v=4"},"body":"Add first_parent_only parameter to find_bisection(), removing the\nbarrier that prevented combining the --bisect and --first-parent flags\nwhen using git rev-list\n\nSigned-off-by: Aaron Lipman \nBased-on-patch-by: Tiago Botelho \n---\n Documentation/rev-list-options.txt | 7 ++---\n bisect.c | 28 ++++++++++++-------\n bisect.h | 2 +-\n builtin/rev-list.c | 2 +-\n revision.c | 3 --\n t/t6000-rev-list-misc.sh | 4 +--\n t/t6002-rev-list-bisect.sh | 45 ++++++++++++++++++++++++++++++\n 7 files changed, 70 insertions(+), 21 deletions(-)\n\ndiff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\nindex b01b2b6773..d3117ce51b 100644\n--- a/Documentation/rev-list-options.txt\n+++ b/Documentation/rev-list-options.txt\n@@ -128,8 +128,7 @@ parents) and `--max-parents=-1` (negative numbers denote no upper limit).\n \tbecause merges into a topic branch tend to be only about\n \tadjusting to updated upstream from time to time, and\n \tthis option allows you to ignore the individual commits\n-\tbrought in to your history by such a merge. Cannot be\n-\tcombined with --bisect.\n+\tbrought in to your history by such a merge.\n \n --not::\n \tReverses the meaning of the '{caret}' prefix (or lack thereof)\n@@ -207,7 +206,7 @@ ifndef::git-rev-list[]\n \tPretend as if the bad bisection ref `refs/bisect/bad`\n \twas listed and as if it was followed by `--not` and the good\n \tbisection refs `refs/bisect/good-*` on the command\n-\tline. Cannot be combined with --first-parent.\n+\tline.\n endif::git-rev-list[]\n \n --stdin::\n@@ -743,7 +742,7 @@ outputs 'midpoint', the output of the two commands\n would be of roughly the same length. Finding the change which\n introduces a regression is thus reduced to a binary search: repeatedly\n generate and test new 'midpoint's until the commit chain is of length\n-one. Cannot be combined with --first-parent.\n+one.\n \n --bisect-vars::\n \tThis calculates the same as `--bisect`, except that refs in\ndiff --git a/bisect.c b/bisect.c\nindex d5e830410f..a11fdb1473 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -88,15 +88,16 @@ static inline void weight_set(struct commit_list *elem, int weight)\n \t**commit_weight_at(&commit_weight, elem->item) = weight;\n }\n \n-static int count_interesting_parents(struct commit *commit)\n+static int count_interesting_parents(struct commit *commit, int first_parent_only)\n {\n \tstruct commit_list *p;\n \tint count;\n \n \tfor (count = 0, p = commit->parents; p; p = p->next) {\n-\t\tif (p->item->object.flags & UNINTERESTING)\n-\t\t\tcontinue;\n-\t\tcount++;\n+\t\tif (!(p->item->object.flags & UNINTERESTING))\n+\t\t\tcount++;\n+\t\tif (first_parent_only)\n+\t\t\tbreak;\n \t}\n \treturn count;\n }\n@@ -259,7 +260,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 first_parent_only)\n {\n \tint n, counted;\n \tstruct commit_list *p;\n@@ -271,7 +272,7 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\n \t\tunsigned flags = commit->object.flags;\n \n \t\t*commit_weight_at(&commit_weight, p->item) = &weights[n++];\n-\t\tswitch (count_interesting_parents(commit)) {\n+\t\tswitch (count_interesting_parents(commit, first_parent_only)) {\n \t\tcase 0:\n \t\t\tif (!(flags & TREESAME)) {\n \t\t\t\tweight_set(p, 1);\n@@ -314,6 +315,8 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\n \t\t\tcontinue;\n \t\tif (weight(p) != -2)\n \t\t\tcontinue;\n+\t\tif (first_parent_only)\n+\t\t\tBUG(\"shouldn't be calling count-distance in fp mode\");\n \t\tweight_set(p, count_distance(p));\n \t\tclear_distance(list);\n \n@@ -332,7 +335,10 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\n \n \t\t\tif (0 <= weight(p))\n \t\t\t\tcontinue;\n-\t\t\tfor (q = p->item->parents; q; q = q->next) {\n+\n+\t\t\tfor (q = p->item->parents;\n+\t\t\t q;\n+\t\t\t q = first_parent_only ? NULL : q->next) {\n \t\t\t\tif (q->item->object.flags & UNINTERESTING)\n \t\t\t\t\tcontinue;\n \t\t\t\tif (0 <= weight(q))\n@@ -370,7 +376,7 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\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 first_parent_only)\n {\n \tint nr, on_list;\n \tstruct commit_list *list, *p, *best, *next, *last;\n@@ -406,7 +412,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, first_parent_only);\n \tif (best) {\n \t\tif (!find_all) {\n \t\t\tlist->item = best->item;\n@@ -991,6 +997,7 @@ enum bisect_error bisect_next_all(struct repository *r, const char *prefix, int\n \tenum bisect_error res = BISECT_OK;\n \tstruct object_id *bisect_rev;\n \tchar *steps_msg;\n+\tint first_parent_only = 0; /* TODO: pass --first-parent flag from git bisect start */\n \n \tread_bisect_terms(&term_bad, &term_good);\n \tif (read_bisect_refs())\n@@ -1001,11 +1008,12 @@ enum bisect_error bisect_next_all(struct repository *r, const char *prefix, int\n \t\treturn res;\n \n \tbisect_rev_setup(r, &revs, prefix, \"%s\", \"^%s\", 1);\n+\trevs.first_parent_only = first_parent_only;\n \trevs.limited = 1;\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, first_parent_only);\n \trevs.commits = managed_skipped(revs.commits, &tried);\n \n \tif (!revs.commits) {\ndiff --git a/bisect.h b/bisect.h\nindex 8bad8d8391..a63af0505f 100644\n--- a/bisect.h\n+++ b/bisect.h\n@@ -12,7 +12,7 @@ struct repository;\n * best commit, as chosen by `find_all`.\n */\n void find_bisection(struct commit_list **list, int *reaches, int *all,\n-\t\t int find_all);\n+\t\t int find_all, int first_parent_only);\n \n struct commit_list *filter_skipped(struct commit_list *list,\n \t\t\t\t struct commit_list **tried,\ndiff --git a/builtin/rev-list.c b/builtin/rev-list.c\nindex f520111eda..d1a14596b2 100644\n--- a/builtin/rev-list.c\n+++ b/builtin/rev-list.c\n@@ -638,7 +638,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, revs.first_parent_only);\n \n \t\tif (bisect_show_vars)\n \t\t\treturn show_bisect_vars(&info, reaches, all);\ndiff --git a/revision.c b/revision.c\nindex 6aa7f4f567..f0ad2603c1 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2869,9 +2869,6 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n \tif (!revs->reflog_info && revs->grep_filter.use_reflog_filter)\n \t\tdie(\"cannot use --grep-reflog without --walk-reflogs\");\n \n-\tif (revs->first_parent_only && revs->bisect)\n-\t\tdie(_(\"--first-parent is incompatible with --bisect\"));\n-\n \tif (revs->line_level_traverse &&\n \t (revs->diffopt.output_format & ~(DIFF_FORMAT_PATCH | DIFF_FORMAT_NO_OUTPUT)))\n \t\tdie(_(\"-L does not yet support diff formats besides -p and -s\"));\ndiff --git a/t/t6000-rev-list-misc.sh b/t/t6000-rev-list-misc.sh\nindex 3dc1ad8f71..26e3533562 100755\n--- a/t/t6000-rev-list-misc.sh\n+++ b/t/t6000-rev-list-misc.sh\n@@ -125,8 +125,8 @@ test_expect_success 'rev-list can negate index objects' '\n \ttest_cmp expect actual\n '\n \n-test_expect_success '--bisect and --first-parent can not be combined' '\n-\ttest_must_fail git rev-list --bisect --first-parent HEAD\n+test_expect_success '--bisect and --first-parent can be combined' '\n+\tgit rev-list --bisect --first-parent HEAD\n '\n \n test_expect_success '--header shows a NUL after each commit' '\ndiff --git a/t/t6002-rev-list-bisect.sh b/t/t6002-rev-list-bisect.sh\nindex a661408038..7fc0f75ca5 100755\n--- a/t/t6002-rev-list-bisect.sh\n+++ b/t/t6002-rev-list-bisect.sh\n@@ -263,4 +263,49 @@ test_expect_success 'rev-parse --bisect can default to good/bad refs' '\n \ttest_cmp expect.sorted actual.sorted\n '\n \n+test_output_expect_success \"--bisect --first-parent\" 'git rev-list --bisect --first-parent E ^F' < expect.unsorted < expect &&\n+git rev-list --bisect-all --first-parent E ^F > actual &&\n+test_cmp actual expect\n+'\n+\n test_done\n-- \n2.24.3 (Apple Git-128)\n\n"},{"id":"402443","messageId":"20200730002735.87655-3-alipman88@gmail.com","threadId":"53937","inReplyTo":"20200730002735.87655-1-alipman88@gmail.com","subject":"[PATCH v2 2/3] bisect: introduce first-parent flag","fromName":"Aaron Lipman","fromEmail":"alipman88@gmail.com","sentAt":"2020-07-30T00:27:34Z","receivedAt":"2020-07-30T00:27:48Z","isPatch":true,"sender":{"key":"alipman88@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3922511?v=4"},"body":"Upon seeing a merge commit when bisecting, this option may be used to\nfollow only the first parent.\n\nIn detecting regressions introduced through the merging of a branch, the\nmerge commit will be identified as introduction of the bug and its\nancestors will be ignored.\n\nThis option is particularly useful in avoiding false positives when a\nmerged branch contained broken or non-buildable commits, but the merge\nitself was OK.\n\nSigned-off-by: Aaron Lipman \n---\n Documentation/git-bisect.txt | 13 ++++++++++++-\n bisect.c | 10 +++++++++-\n bisect.h | 2 ++\n builtin/bisect--helper.c | 16 ++++++++++++----\n t/t6030-bisect-porcelain.sh | 17 +++++++++++++++++\n 5 files changed, 52 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt\nindex 7586c5a843..0e993e4587 100644\n--- a/Documentation/git-bisect.txt\n+++ b/Documentation/git-bisect.txt\n@@ -17,7 +17,7 @@ The command takes various subcommands, and different options depending\n on the subcommand:\n \n git bisect start [--term-{old,good}= --term-{new,bad}=]\n-\t\t [--no-checkout] [ [...]] [--] [...]\n+\t\t [--no-checkout] [--first-parent] [ [...]] [--] [...]\n git bisect (bad|new|) []\n git bisect (good|old|) [...]\n git bisect terms [--term-good | --term-bad]\n@@ -365,6 +365,17 @@ does not require a checked out tree.\n +\n If the repository is bare, `--no-checkout` is assumed.\n \n+--first-parent::\n++\n+Follow only the first parent commit upon seeing a merge commit.\n++\n+In detecting regressions introduced through the merging of a branch, the merge\n+commit will be identified as introduction of the bug and its ancestors will be\n+ignored.\n++\n+This option is particularly useful in avoiding false positives when a merged\n+branch contained broken or non-buildable commits, but the merge itself was OK.\n+\n EXAMPLES\n --------\n \ndiff --git a/bisect.c b/bisect.c\nindex a11fdb1473..b495a19192 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -460,6 +460,7 @@ static GIT_PATH_FUNC(git_path_bisect_run, \"BISECT_RUN\")\n static GIT_PATH_FUNC(git_path_bisect_start, \"BISECT_START\")\n static GIT_PATH_FUNC(git_path_bisect_log, \"BISECT_LOG\")\n static GIT_PATH_FUNC(git_path_bisect_terms, \"BISECT_TERMS\")\n+static GIT_PATH_FUNC(git_path_bisect_first_parent, \"BISECT_FIRST_PARENT\")\n static GIT_PATH_FUNC(git_path_head_name, \"head-name\")\n \n static void read_bisect_paths(struct argv_array *array)\n@@ -981,6 +982,12 @@ void read_bisect_terms(const char **read_bad, const char **read_good)\n \tfclose(fp);\n }\n \n+int read_first_parent_option(void)\n+{\n+\tconst char *filename = git_path_bisect_first_parent();\n+\treturn !access(filename, F_OK);\n+}\n+\n /*\n * We use the convention that return BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND (-10) means\n * the bisection process finished successfully.\n@@ -997,7 +1004,7 @@ enum bisect_error bisect_next_all(struct repository *r, const char *prefix, int\n \tenum bisect_error res = BISECT_OK;\n \tstruct object_id *bisect_rev;\n \tchar *steps_msg;\n-\tint first_parent_only = 0; /* TODO: pass --first-parent flag from git bisect start */\n+\tint first_parent_only = read_first_parent_option();\n \n \tread_bisect_terms(&term_bad, &term_good);\n \tif (read_bisect_refs())\n@@ -1141,6 +1148,7 @@ int bisect_clean_state(void)\n \tunlink_or_warn(git_path_bisect_names());\n \tunlink_or_warn(git_path_bisect_run());\n \tunlink_or_warn(git_path_bisect_terms());\n+\tunlink_or_warn(git_path_bisect_first_parent());\n \t/* Cleanup head-name if it got left by an old version of git-bisect */\n \tunlink_or_warn(git_path_head_name());\n \t/*\ndiff --git a/bisect.h b/bisect.h\nindex a63af0505f..8ee80f5b48 100644\n--- a/bisect.h\n+++ b/bisect.h\n@@ -66,6 +66,8 @@ int estimate_bisect_steps(int all);\n \n void read_bisect_terms(const char **bad, const char **good);\n \n+int read_first_parent_option(void);\n+\n int bisect_clean_state(void);\n \n #endif\ndiff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\nindex ec4996282e..1236f5df1d 100644\n--- a/builtin/bisect--helper.c\n+++ b/builtin/bisect--helper.c\n@@ -17,6 +17,7 @@ static GIT_PATH_FUNC(git_path_bisect_head, \"BISECT_HEAD\")\n static GIT_PATH_FUNC(git_path_bisect_log, \"BISECT_LOG\")\n static GIT_PATH_FUNC(git_path_head_name, \"head-name\")\n static GIT_PATH_FUNC(git_path_bisect_names, \"BISECT_NAMES\")\n+static GIT_PATH_FUNC(git_path_bisect_first_parent, \"BISECT_FIRST_PARENT\")\n \n static const char * const git_bisect_helper_usage[] = {\n \tN_(\"git bisect--helper --next-all [--no-checkout]\"),\n@@ -28,7 +29,7 @@ static const char * const git_bisect_helper_usage[] = {\n \tN_(\"git bisect--helper --bisect-next-check []\"),\n \tN_(\"git bisect--helper --bisect-terms [--term-good | --term-old | --term-bad | --term-new]\"),\n \tN_(\"git bisect--helper --bisect-start [--term-{old,good}= --term-{new,bad}=]\"\n-\t\t\t\t\t \"[--no-checkout] [ [...]] [--] [...]\"),\n+\t\t\t\t\t \" [--no-checkout] [--first-parent] [ [...]] [--] [...]\"),\n \tNULL\n };\n \n@@ -422,7 +423,7 @@ static int bisect_append_log_quoted(const char **argv)\n }\n \n static int bisect_start(struct bisect_terms *terms, int no_checkout,\n-\t\t\tconst char **argv, int argc)\n+\t\t\tint first_parent_only, const char **argv, int argc)\n {\n \tint i, has_double_dash = 0, must_write_terms = 0, bad_seen = 0;\n \tint flags, pathspec_pos, res = 0;\n@@ -453,6 +454,8 @@ static int bisect_start(struct bisect_terms *terms, int no_checkout,\n \t\t\tbreak;\n \t\t} else if (!strcmp(arg, \"--no-checkout\")) {\n \t\t\tno_checkout = 1;\n+\t\t} else if (!strcmp(arg, \"--first-parent\")) {\n+\t\t\tfirst_parent_only = 1;\n \t\t} else if (!strcmp(arg, \"--term-good\") ||\n \t\t\t !strcmp(arg, \"--term-old\")) {\n \t\t\ti++;\n@@ -577,6 +580,9 @@ static int bisect_start(struct bisect_terms *terms, int no_checkout,\n \t */\n \twrite_file(git_path_bisect_start(), \"%s\\n\", start_head.buf);\n \n+\tif (first_parent_only)\n+\t\twrite_file(git_path_bisect_first_parent(), \"\\n\");\n+\n \tif (no_checkout) {\n \t\tif (get_oid(start_head.buf, &oid) < 0) {\n \t\t\tres = error(_(\"invalid ref: '%s'\"), start_head.buf);\n@@ -632,7 +638,7 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\tBISECT_TERMS,\n \t\tBISECT_START\n \t} cmdmode = 0;\n-\tint no_checkout = 0, res = 0, nolog = 0;\n+\tint no_checkout = 0, first_parent_only = 0, res = 0, nolog = 0;\n \tstruct option options[] = {\n \t\tOPT_CMDMODE(0, \"next-all\", &cmdmode,\n \t\t\t N_(\"perform 'git bisect next'\"), NEXT_ALL),\n@@ -656,6 +662,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"start the bisect session\"), BISECT_START),\n \t\tOPT_BOOL(0, \"no-checkout\", &no_checkout,\n \t\t\t N_(\"update BISECT_HEAD instead of checking out the current commit\")),\n+\t\tOPT_BOOL(0, \"first-parent\", &first_parent_only,\n+\t\t\t N_(\"only trace the first parent of merge commits\")),\n \t\tOPT_BOOL(0, \"no-log\", &nolog,\n \t\t\t N_(\"no log for BISECT_WRITE\")),\n \t\tOPT_END()\n@@ -713,7 +721,7 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\tbreak;\n \tcase BISECT_START:\n \t\tset_terms(&terms, \"bad\", \"good\");\n-\t\tres = bisect_start(&terms, no_checkout, argv, argc);\n+\t\tres = bisect_start(&terms, no_checkout, first_parent_only, argv, argc);\n \t\tbreak;\n \tdefault:\n \t\treturn error(\"BUG: unknown subcommand '%d'\", cmdmode);\ndiff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh\nindex 36d9b2b2e4..7965698310 100755\n--- a/t/t6030-bisect-porcelain.sh\n+++ b/t/t6030-bisect-porcelain.sh\n@@ -458,6 +458,23 @@ test_expect_success 'many merge bases creation' '\n \tgrep \"$SIDE_HASH5\" merge_bases.txt\n '\n \n+# We want to automatically find the merge that\n+# introduced \"line\" into hello.\n+test_expect_success \\\n+ '\"git bisect run --first-parent\" simple case' \\\n+ 'echo \"#\"\\!\"/bin/sh\" > test_script.sh &&\n+ echo \"grep line hello > /dev/null\" >> test_script.sh &&\n+ echo \"test \\$? -ne 0\" >> test_script.sh &&\n+ chmod +x test_script.sh &&\n+ git bisect start --first-parent &&\n+ test_path_is_file \".git/BISECT_FIRST_PARENT\" &&\n+ git bisect good $HASH4 &&\n+ git bisect bad $B_HASH &&\n+ git bisect run ./test_script.sh > my_bisect_log.txt &&\n+ grep \"$B_HASH is the first bad commit\" my_bisect_log.txt &&\n+ git bisect reset &&\n+ test_path_is_missing \".git/BISECT_FIRST_PARENT\"'\n+\n test_expect_success 'good merge bases when good and bad are siblings' '\n \tgit bisect start \"$B_HASH\" \"$A_HASH\" > my_bisect_log.txt &&\n \ttest_i18ngrep \"merge base must be tested\" my_bisect_log.txt &&\n-- \n2.24.3 (Apple Git-128)\n\n"},{"id":"402444","messageId":"20200730002735.87655-4-alipman88@gmail.com","threadId":"53937","inReplyTo":"20200730002735.87655-1-alipman88@gmail.com","subject":"[PATCH v2 3/3] bisect: combine args passed to find_bisection()","fromName":"Aaron Lipman","fromEmail":"alipman88@gmail.com","sentAt":"2020-07-30T00:27:35Z","receivedAt":"2020-07-30T00:27:49Z","isPatch":true,"sender":{"key":"alipman88@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3922511?v=4"},"body":"Now that find_bisection() accepts multiple boolean arguments, these may\nbe combined into a single unsigned integer in order to declutter some of\nthe code in bisect.c\n\nSigned-off-by: Aaron Lipman \nBased-on-patch-by: Harald Nordgren \n---\n bisect.c | 37 ++++++++++++++++++++++---------------\n bisect.h | 5 ++++-\n builtin/rev-list.c | 9 ++++++++-\n 3 files changed, 34 insertions(+), 17 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex b495a19192..07a7760a72 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -88,7 +88,7 @@ static inline void weight_set(struct commit_list *elem, int weight)\n \t**commit_weight_at(&commit_weight, elem->item) = weight;\n }\n \n-static int count_interesting_parents(struct commit *commit, int first_parent_only)\n+static int count_interesting_parents(struct commit *commit, unsigned bisect_flags)\n {\n \tstruct commit_list *p;\n \tint count;\n@@ -96,7 +96,7 @@ static int count_interesting_parents(struct commit *commit, int first_parent_onl\n \tfor (count = 0, p = commit->parents; p; p = p->next) {\n \t\tif (!(p->item->object.flags & UNINTERESTING))\n \t\t\tcount++;\n-\t\tif (first_parent_only)\n+\t\tif (bisect_flags & BISECT_FIRST_PARENT)\n \t\t\tbreak;\n \t}\n \treturn count;\n@@ -260,7 +260,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, int first_parent_only)\n+\t\t\t\t\t unsigned bisect_flags)\n {\n \tint n, counted;\n \tstruct commit_list *p;\n@@ -272,7 +272,7 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\n \t\tunsigned flags = commit->object.flags;\n \n \t\t*commit_weight_at(&commit_weight, p->item) = &weights[n++];\n-\t\tswitch (count_interesting_parents(commit, first_parent_only)) {\n+\t\tswitch (count_interesting_parents(commit, bisect_flags)) {\n \t\tcase 0:\n \t\t\tif (!(flags & TREESAME)) {\n \t\t\t\tweight_set(p, 1);\n@@ -315,13 +315,13 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\n \t\t\tcontinue;\n \t\tif (weight(p) != -2)\n \t\t\tcontinue;\n-\t\tif (first_parent_only)\n+\t\tif (bisect_flags & BISECT_FIRST_PARENT)\n \t\t\tBUG(\"shouldn't be calling count-distance in fp mode\");\n \t\tweight_set(p, count_distance(p));\n \t\tclear_distance(list);\n \n \t\t/* Does it happen to be at exactly half-way? */\n-\t\tif (!find_all && halfway(p, nr))\n+\t\tif (!(bisect_flags & BISECT_FIND_ALL) && halfway(p, nr))\n \t\t\treturn p;\n \t\tcounted++;\n \t}\n@@ -338,7 +338,7 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\n \n \t\t\tfor (q = p->item->parents;\n \t\t\t q;\n-\t\t\t q = first_parent_only ? NULL : q->next) {\n+\t\t\t q = bisect_flags & BISECT_FIRST_PARENT ? NULL : q->next) {\n \t\t\t\tif (q->item->object.flags & UNINTERESTING)\n \t\t\t\t\tcontinue;\n \t\t\t\tif (0 <= weight(q))\n@@ -362,21 +362,21 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\n \t\t\t\tweight_set(p, weight(q));\n \n \t\t\t/* Does it happen to be at exactly half-way? */\n-\t\t\tif (!find_all && halfway(p, nr))\n+\t\t\tif (!(bisect_flags & BISECT_FIND_ALL) && halfway(p, nr))\n \t\t\t\treturn p;\n \t\t}\n \t}\n \n \tshow_list(\"bisection 2 counted all\", counted, nr, list);\n \n-\tif (!find_all)\n+\tif (!(bisect_flags & BISECT_FIND_ALL))\n \t\treturn best_bisection(list, nr);\n \telse\n \t\treturn best_bisection_sorted(list, nr);\n }\n \n void find_bisection(struct commit_list **commit_list, int *reaches,\n-\t\t int *all, int find_all, int first_parent_only)\n+\t\t int *all, unsigned bisect_flags)\n {\n \tint nr, on_list;\n \tstruct commit_list *list, *p, *best, *next, *last;\n@@ -412,9 +412,9 @@ 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, first_parent_only);\n+\tbest = do_find_bisection(list, nr, weights, bisect_flags);\n \tif (best) {\n-\t\tif (!find_all) {\n+\t\tif (!(bisect_flags & BISECT_FIND_ALL)) {\n \t\t\tlist->item = best->item;\n \t\t\tfree_commit_list(list->next);\n \t\t\tbest = list;\n@@ -1004,23 +1004,30 @@ enum bisect_error bisect_next_all(struct repository *r, const char *prefix, int\n \tenum bisect_error res = BISECT_OK;\n \tstruct object_id *bisect_rev;\n \tchar *steps_msg;\n-\tint first_parent_only = read_first_parent_option();\n+\tunsigned bisect_flags = 0;\n \n \tread_bisect_terms(&term_bad, &term_good);\n \tif (read_bisect_refs())\n \t\tdie(_(\"reading bisect refs failed\"));\n \n+\tif (read_first_parent_option())\n+\t\tbisect_flags |= BISECT_FIRST_PARENT;\n+\n+\tif (!!skipped_revs.nr)\n+\t\tbisect_flags |= BISECT_FIND_ALL;\n+\n \tres = check_good_are_ancestors_of_bad(r, prefix, no_checkout);\n \tif (res)\n \t\treturn res;\n \n \tbisect_rev_setup(r, &revs, prefix, \"%s\", \"^%s\", 1);\n-\trevs.first_parent_only = first_parent_only;\n+\n+\trevs.first_parent_only = !!(bisect_flags & BISECT_FIRST_PARENT);\n \trevs.limited = 1;\n \n \tbisect_common(&revs);\n \n-\tfind_bisection(&revs.commits, &reaches, &all, !!skipped_revs.nr, first_parent_only);\n+\tfind_bisection(&revs.commits, &reaches, &all, bisect_flags);\n \trevs.commits = managed_skipped(revs.commits, &tried);\n \n \tif (!revs.commits) {\ndiff --git a/bisect.h b/bisect.h\nindex 8ee80f5b48..402ddcfb5e 100644\n--- a/bisect.h\n+++ b/bisect.h\n@@ -12,7 +12,7 @@ struct repository;\n * best commit, as chosen by `find_all`.\n */\n void find_bisection(struct commit_list **list, int *reaches, int *all,\n-\t\t int find_all, int first_parent_only);\n+\t\t unsigned bisect_flags);\n \n struct commit_list *filter_skipped(struct commit_list *list,\n \t\t\t\t struct commit_list **tried,\n@@ -23,6 +23,9 @@ struct commit_list *filter_skipped(struct commit_list *list,\n #define BISECT_SHOW_ALL\t\t(1<<0)\n #define REV_LIST_QUIET\t\t(1<<1)\n \n+#define BISECT_FIND_ALL\t\t(1u<<0)\n+#define BISECT_FIRST_PARENT\t(1u<<1)\n+\n struct rev_list_info {\n \tstruct rev_info *revs;\n \tint flags;\ndiff --git a/builtin/rev-list.c b/builtin/rev-list.c\nindex d1a14596b2..1536ea6f28 100644\n--- a/builtin/rev-list.c\n+++ b/builtin/rev-list.c\n@@ -637,8 +637,15 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)\n \n \tif (bisect_list) {\n \t\tint reaches, all;\n+\t\tunsigned bisect_flags = 0;\n \n-\t\tfind_bisection(&revs.commits, &reaches, &all, bisect_find_all, revs.first_parent_only);\n+\t\tif (bisect_find_all)\n+\t\t\tbisect_flags |= BISECT_FIND_ALL;\n+\n+\t\tif (revs.first_parent_only)\n+\t\t\tbisect_flags |= BISECT_FIRST_PARENT;\n+\n+\t\tfind_bisection(&revs.commits, &reaches, &all, bisect_flags);\n \n \t\tif (bisect_show_vars)\n \t\t\treturn show_bisect_vars(&info, reaches, all);\n-- \n2.24.3 (Apple Git-128)\n\n"},{"id":"402449","messageId":"xmqq1rkt96aa.fsf@gitster.c.googlers.com","threadId":"53937","inReplyTo":"20200730002735.87655-1-alipman88@gmail.com","subject":"Re: [PATCH v2 0/3] Introduce --first-parent flag for git bisect","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-07-30T00:48:45Z","receivedAt":"2020-07-30T00:48:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Aaron Lipman writes:\n\n> Junio, thank you for the feedback and the explanation regarding\n> count_distance(). I've gone ahead and implemented all your suggestions.\n>\n>> In chronological order, please.\n> I submitted my initial patch through GitGitGadget. I don't see a way to\n> control the order GitGitGadget sends emails,...\n\nAh, I was referring to the order of sign-off, helped-by, etc.\nSomebody writes a patch that is left on the list, you pick it up and\npolish it for perfection, and send the result to the list. \n\nSo based-on would come first because it happened first, and then you\nworked further until the result deserves to be signed-off and\nfinally you sent it out. So your sign-off comes the last.\n\n"},{"id":"402453","messageId":"xmqqv9i57i26.fsf@gitster.c.googlers.com","threadId":"53937","inReplyTo":"20200730002735.87655-3-alipman88@gmail.com","subject":"Re: [PATCH v2 2/3] bisect: introduce first-parent flag","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-07-30T04:17:21Z","receivedAt":"2020-07-30T04:17:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Aaron Lipman writes:\n\n> diff --git a/bisect.c b/bisect.c\n> index a11fdb1473..b495a19192 100644\n> --- a/bisect.c\n> +++ b/bisect.c\n> @@ -460,6 +460,7 @@ static GIT_PATH_FUNC(git_path_bisect_run, \"BISECT_RUN\")\n> static GIT_PATH_FUNC(git_path_bisect_start, \"BISECT_START\")\n> static GIT_PATH_FUNC(git_path_bisect_log, \"BISECT_LOG\")\n> static GIT_PATH_FUNC(git_path_bisect_terms, \"BISECT_TERMS\")\n> +static GIT_PATH_FUNC(git_path_bisect_first_parent, \"BISECT_FIRST_PARENT\")\n> static GIT_PATH_FUNC(git_path_head_name, \"head-name\")\n\nThis is not a new issue, but duplication of the above and the same\nset of PATH_FUNC in builtin/bisect-helper.c looks ugly. We may want\nto do something about it after this topic is done.\n\n> @@ -981,6 +982,12 @@ void read_bisect_terms(const char **read_bad, const char **read_good)\n> \tfclose(fp);\n> }\n> \n> +int read_first_parent_option(void)\n\n\"static int\", no? I do not think we need to be able to call this\nfrom anywhere outside this file.\n\n> +{\n> +\tconst char *filename = git_path_bisect_first_parent();\n> +\treturn !access(filename, F_OK);\n> +}\n> +\n> /*\n> * We use the convention that return BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND (-10) means\n> * the bisection process finished successfully.\n> @@ -997,7 +1004,7 @@ enum bisect_error bisect_next_all(struct repository *r, const char *prefix, int\n> \tenum bisect_error res = BISECT_OK;\n> \tstruct object_id *bisect_rev;\n> \tchar *steps_msg;\n> -\tint first_parent_only = 0; /* TODO: pass --first-parent flag from git bisect start */\n> +\tint first_parent_only = read_first_parent_option();\n> \n> \tread_bisect_terms(&term_bad, &term_good);\n> \tif (read_bisect_refs())\n> @@ -1141,6 +1148,7 @@ int bisect_clean_state(void)\n> \tunlink_or_warn(git_path_bisect_names());\n> \tunlink_or_warn(git_path_bisect_run());\n> \tunlink_or_warn(git_path_bisect_terms());\n> +\tunlink_or_warn(git_path_bisect_first_parent());\n> \t/* Cleanup head-name if it got left by an old version of git-bisect */\n> \tunlink_or_warn(git_path_head_name());\n> \t/*\n> diff --git a/bisect.h b/bisect.h\n> index a63af0505f..8ee80f5b48 100644\n> --- a/bisect.h\n> +++ b/bisect.h\n> @@ -66,6 +66,8 @@ int estimate_bisect_steps(int all);\n> \n> void read_bisect_terms(const char **bad, const char **good);\n> \n> +int read_first_parent_option(void);\n> +\n\nSo this won't be needed, right?\n\n> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\n> index ec4996282e..1236f5df1d 100644\n> --- a/builtin/bisect--helper.c\n> +++ b/builtin/bisect--helper.c\n> @@ -422,7 +423,7 @@ static int bisect_append_log_quoted(const char **argv)\n> }\n> \n> static int bisect_start(struct bisect_terms *terms, int no_checkout,\n> -\t\t\tconst char **argv, int argc)\n> +\t\t\tint first_parent_only, const char **argv, int argc)\n\nWhy do we need to pass this new parameter from cmd_bisect__helper(),\nwhen we are going to parse it out of argv/argc outselves anyway?\n\nI suspect that you would ask the same question to whoever added\nno_checkout to this thing, and I wouldn't be surprised if we end up\nremoving both of these parameters (and parsing of the options inside\ncmd_bisect__helper()) after thinking about them, but anyway, let's\nread on.\n\n> +# We want to automatically find the merge that\n> +# introduced \"line\" into hello.\n\n'introduced'? \n\n> +test_expect_success \\\n> + '\"git bisect run --first-parent\" simple case' \\\n\nLet's not revert back to ancient line-folding style. The next test\nyou can see in the postimage, i.e.\n\n\ttest_expect_success 'title in single quote' '\n\t\tthe first command &&\n\t\tthe second command &&\n\t\t...\n\t\tthe last command\n\t'\n\nis what we want to mimic.\n\n> + 'echo \"#\"\\!\"/bin/sh\" > test_script.sh &&\n> + echo \"grep line hello > /dev/null\" >> test_script.sh &&\n> + echo \"test \\$? -ne 0\" >> test_script.sh &&\n> + chmod +x test_script.sh &&\n\nSo, we want to say \"bad\" if $? is 0, i.e. the file 'hello' has a\nstring \"line\" in it and \"good\" if $? is not 0, i.e. the file 'hello'\ndoes not have such a line?\n\nA few things:\n\n - Use \"write_script\" to abstract away platform-specific details\n such as which $SHELL_PATH to use on \"#!\" line, and \"chmod +x\".\n\n - There is no SP between a redirection operator and its target\n file.\n\ni.e. Ssomething like this instead of the above four lines:\n\n\twrite_script test_script.sh <<-\\EOF\n\t! grep line hello >/dev/null\n\tEOF\n\n> + git bisect start --first-parent &&\n> + test_path_is_file \".git/BISECT_FIRST_PARENT\" &&\n> + git bisect good $HASH4 &&\n> + git bisect bad $B_HASH &&\n> + git bisect run ./test_script.sh > my_bisect_log.txt &&\n\nStyle: no SP between '>' and its target file.\n\n> + grep \"$B_HASH is the first bad commit\" my_bisect_log.txt &&\n> + git bisect reset &&\n> + test_path_is_missing \".git/BISECT_FIRST_PARENT\"'\n\nThe final blame must lie on a commit on the first-parent chain,\nwhich this test tries to ensure, but I wonder if it is also required\nthat all commits offered to be tested by \"git bisect\" are on the\nfirst-parent chain, and if so, shouldn't we be make sure each and\nevery time \"test_script\" is run, the commit that is at HEAD is on\nthe first parent chain between the initial good..bad range?\n\n> test_expect_success 'good merge bases when good and bad are siblings' '\n> \tgit bisect start \"$B_HASH\" \"$A_HASH\" > my_bisect_log.txt &&\n> \ttest_i18ngrep \"merge base must be tested\" my_bisect_log.txt &&\n\nOther than that, overall this seems to be going in the right\ndirection.\n\nThanks.\n"},{"id":"402454","messageId":"xmqqr1st7hdd.fsf@gitster.c.googlers.com","threadId":"53937","inReplyTo":"20200730002735.87655-4-alipman88@gmail.com","subject":"Re: [PATCH v2 3/3] bisect: combine args passed to find_bisection()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-07-30T04:32:14Z","receivedAt":"2020-07-30T04:32:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Aaron Lipman writes:\n\n> Now that find_bisection() accepts multiple boolean arguments, these may\n> be combined into a single unsigned integer in order to declutter some of\n> the code in bisect.c\n\nI'd rather call them \"flags\" without \"bisect_\". If we are passing\ntwo sets of flags, one set about \"bisect\" and the other set about\nsomething else, it would make quite a lot of sense to call the first\nset \"bisect_flags\", but what we are seeing here is not like that.\n\n> +\tif (read_first_parent_option())\n> +\t\tbisect_flags |= BISECT_FIRST_PARENT;\n> +\n> +\tif (!!skipped_revs.nr)\n> +\t\tbisect_flags |= BISECT_FIND_ALL;\n> +\n\nYou do not care what kind of \"true\" you are feeding \"if()\", so I do\nnot think you would want to keep !! prefix here. Older API into\nfind_bisection() may wanted to say \"pass 1 to find_all\", in which\ncase, normalizing with !! prefix may have made perfect sense, but \nthat no longer holds here.\n\n> +\n> +\trevs.first_parent_only = !!(bisect_flags & BISECT_FIRST_PARENT);\n\nOn the other hand, this !! may make sense; especially since .first_parent_only\ncould just be a one-bit unsigned bitfield.\n\n> \trevs.limited = 1;\n> \n> \tbisect_common(&revs);\n> \n> -\tfind_bisection(&revs.commits, &reaches, &all, !!skipped_revs.nr, first_parent_only);\n> +\tfind_bisection(&revs.commits, &reaches, &all, bisect_flags);\n\n> @@ -23,6 +23,9 @@ struct commit_list *filter_skipped(struct commit_list *list,\n> #define BISECT_SHOW_ALL\t\t(1<<0)\n> #define REV_LIST_QUIET\t\t(1<<1)\n> \n> +#define BISECT_FIND_ALL\t\t(1u<<0)\n> +#define BISECT_FIRST_PARENT\t(1u<<1)\n\nThe set of bits to go to your \"bisect_flags\" are only these two new\nones, and the existing BISECT_SHOW_ALL does not belong to it (it is\na bit in rev_list_info.flags), but it is not apparent. I wonder if\nthere is a good way to help readers easily tell these two sets apart.\nThese are flags passed to find_bisection(), so perhaps\n\n #define FIND_BISECTION_ALL (1U<<0)\n #define FIND_BISECTION_FIRST_PARENT_ONLY (1U<<1)\n\n unsigned flags = 0;\n if (first_parent)\n\tflags |= FIND_BISECTION_FIRST_PARENT_ONLY;\n ...\n find_bisection(..., flags);\n\nwould be a reasonable compromise?\n\nThanks.\n"},{"id":"402619","messageId":"20200801175840.1877-1-alipman88@gmail.com","threadId":"53937","inReplyTo":"20200730002735.87655-1-alipman88@gmail.com","subject":"[PATCH v3 0/4] Introduce --first-parent flag for git bisect","fromName":"Aaron Lipman","fromEmail":"alipman88@gmail.com","sentAt":"2020-08-01T17:58:36Z","receivedAt":"2020-08-01T17:58:51Z","isPatch":true,"sender":{"key":"alipman88@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3922511?v=4"},"body":"> Ah, I was referring to the order of sign-off, helped-by, etc.\n\nWhoops, re-ordered correctly.\n\n> This is not a new issue, but duplication of the above and the same\n> set of PATH_FUNC in builtin/bisect-helper.c looks ugly. We may want\n> to do something about it after this topic is done.\n\nHappy to pick that up next!\n\n> > +int read_first_parent_option(void)\n\n> \"static int\", no? I do not think we need to be able to call this\n> from anywhere outside this file.\n\nAdded static keyword and removed the redundant line from bisect.h\n\n> > static int bisect_start(struct bisect_terms *terms, int no_checkout,\n> > - const char **argv, int argc)\n> > + int first_parent_only, const char **argv, int argc)\n\n> Why do we need to pass this new parameter from cmd_bisect__helper(),\n> when we are going to parse it out of argv/argc outselves anyway?\n\n> I suspect that you would ask the same question to whoever added\n> no_checkout to this thing, and I wouldn't be surprised if we end up\n> removing both of these parameters (and parsing of the options inside\n> cmd_bisect__helper()) after thinking about them, but anyway, let's\n> read on.\n\nHmm. Is there a preferred way to to add a --first-parent line to the\noutput of \"git bisect start --help\" via the parse-options API without\nremoving the --first-parent option from argv in the process?\n\nAs long as we're capturing the --first-parent option in\ncmd_bisect__helper(), checking argv for a --first-parent flag in\nbisect_start() is pointless. So I've deleted the following lines from my\npatch inside bisect_start() (for the time being, at least):\n\ngit diff v2 v3 -- builtin/bisect--helper.c\n- } else if (!strcmp(arg, \"--first-parent\")) {\n- first_parent_only = 1;\n\n> > +# We want to automatically find the merge that\n> > +# introduced \"line\" into hello.\n\n> 'introduced'?\n\nThat's the language used by other \"git bisect run\" tests. If\n\"introduced\" sounds too clinical, perhaps \"added\" instead?\n\n> Let's not revert back to ancient line-folding style.\n\nThank you for the template. I've updated my test accordingly.\n\n> So, we want to say \"bad\" if $? is 0, i.e. the file 'hello' has a\n> string \"line\" in it and \"good\" if $? is not 0, i.e. the file 'hello'\n> does not have such a line?\n\nI think the other way around - an exit code of 0 from the script passed\nto \"git bisect run\" marks a commit as good, 1 as bad.\n\n> - Use \"write_script\" to abstract away platform-specific details\n> such as which $SHELL_PATH to use on \"#!\" line, and \"chmod +x\".\n\n> - There is no SP between a redirection operator and its target\n> file.\n\nNoted. Can we rewrite the other \"git bisect run\" tests in\nt6030-bisect-porcelain.sh so that they're all consisent? That way, when\nsomeone adds another test for \"git bisect run\", they'll have a few more\nproper examples. (I've added a fourth commit that does this, if that's\nOK.)\n\n> The final blame must lie on a commit on the first-parent chain,\n> which this test tries to ensure, but I wonder if it is also required\n> that all commits offered to be tested by \"git bisect\" are on the\n> first-parent chain, and if so, shouldn't we be make sure each and\n> every time \"test_script\" is run, the commit that is at HEAD is on\n> the first parent chain between the initial good..bad range?\n\nThat is prudent. I've altered test_script.sh to use the special -1 exit\ncode to abort the bisection process if it encounters a commit outside\nthe first parent chain.\n\nAlthernatively, if we prefer not to depend on the special -1 exit code,\nwe can append any tested commits outside the first parent chain to a\nfile and ensure that it's empty after \"git bisect run\" finishes.\n\n> I'd rather call them \"flags\" without \"bisect_\". If we are passing\n> two sets of flags, one set about \"bisect\" and the other set about\n> something else, it would make quite a lot of sense to call the first\n> set \"bisect_flags\", but what we are seeing here is not like that.\n\nbisect.c already uses a \"flags\" variable in several locations that would\ncollide with this. Perhaps rename the existing \"flags\" variable to\n\"commit_flags\" to explicitly differentiate?\n\n> > + if (!!skipped_revs.nr)\n> > + bisect_flags |= BISECT_FIND_ALL;\n> > +\n\n> You do not care what kind of \"true\" you are feeding \"if()\", so I do\n> not think you would want to keep !! prefix here.\n\nOK, removed.\n\n> The set of bits to go to your \"bisect_flags\" are only these two new\n> ones, and the existing BISECT_SHOW_ALL does not belong to it (it is\n> a bit in rev_list_info.flags), but it is not apparent. I wonder if\n> there is a good way to help readers easily tell these two sets apart.\n> These are flags passed to find_bisection(), so perhaps\n> \n> #define FIND_BISECTION_ALL (1U<<0)\n> #define FIND_BISECTION_FIRST_PARENT_ONLY (1U<<1)\n> // ...\n>\n> would be a reasonable compromise?\n\nSounds good, renamed.\n\nAaron Lipman (4):\n rev-list: allow bisect and first-parent flags\n bisect: introduce first-parent flag\n bisect: combine args passed to find_bisection()\n bisect: consistent style for git bisect run tests\n\n Documentation/git-bisect.txt | 13 ++++-\n Documentation/rev-list-options.txt | 7 ++-\n bisect.c | 81 +++++++++++++++++++-----------\n bisect.h | 5 +-\n builtin/bisect--helper.c | 14 ++++--\n builtin/rev-list.c | 9 +++-\n revision.c | 3 --\n t/t6000-rev-list-misc.sh | 4 +-\n t/t6002-rev-list-bisect.sh | 45 +++++++++++++++++\n t/t6030-bisect-porcelain.sh | 64 ++++++++++++++---------\n 10 files changed, 176 insertions(+), 69 deletions(-)\n\n-- \n2.24.3 (Apple Git-128)\n\n"},{"id":"402620","messageId":"20200801175840.1877-2-alipman88@gmail.com","threadId":"53937","inReplyTo":"20200801175840.1877-1-alipman88@gmail.com","subject":"[PATCH v3 1/4] rev-list: allow bisect and first-parent flags","fromName":"Aaron Lipman","fromEmail":"alipman88@gmail.com","sentAt":"2020-08-01T17:58:37Z","receivedAt":"2020-08-01T17:58:52Z","isPatch":true,"sender":{"key":"alipman88@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3922511?v=4"},"body":"Add first_parent_only parameter to find_bisection(), removing the\nbarrier that prevented combining the --bisect and --first-parent flags\nwhen using git rev-list\n\nBased-on-patch-by: Tiago Botelho \nSigned-off-by: Aaron Lipman \n---\n Documentation/rev-list-options.txt | 7 ++---\n bisect.c | 28 ++++++++++++-------\n bisect.h | 2 +-\n builtin/rev-list.c | 2 +-\n revision.c | 3 --\n t/t6000-rev-list-misc.sh | 4 +--\n t/t6002-rev-list-bisect.sh | 45 ++++++++++++++++++++++++++++++\n 7 files changed, 70 insertions(+), 21 deletions(-)\n\ndiff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\nindex b01b2b6773..d3117ce51b 100644\n--- a/Documentation/rev-list-options.txt\n+++ b/Documentation/rev-list-options.txt\n@@ -128,8 +128,7 @@ parents) and `--max-parents=-1` (negative numbers denote no upper limit).\n \tbecause merges into a topic branch tend to be only about\n \tadjusting to updated upstream from time to time, and\n \tthis option allows you to ignore the individual commits\n-\tbrought in to your history by such a merge. Cannot be\n-\tcombined with --bisect.\n+\tbrought in to your history by such a merge.\n \n --not::\n \tReverses the meaning of the '{caret}' prefix (or lack thereof)\n@@ -207,7 +206,7 @@ ifndef::git-rev-list[]\n \tPretend as if the bad bisection ref `refs/bisect/bad`\n \twas listed and as if it was followed by `--not` and the good\n \tbisection refs `refs/bisect/good-*` on the command\n-\tline. Cannot be combined with --first-parent.\n+\tline.\n endif::git-rev-list[]\n \n --stdin::\n@@ -743,7 +742,7 @@ outputs 'midpoint', the output of the two commands\n would be of roughly the same length. Finding the change which\n introduces a regression is thus reduced to a binary search: repeatedly\n generate and test new 'midpoint's until the commit chain is of length\n-one. Cannot be combined with --first-parent.\n+one.\n \n --bisect-vars::\n \tThis calculates the same as `--bisect`, except that refs in\ndiff --git a/bisect.c b/bisect.c\nindex d5e830410f..a11fdb1473 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -88,15 +88,16 @@ static inline void weight_set(struct commit_list *elem, int weight)\n \t**commit_weight_at(&commit_weight, elem->item) = weight;\n }\n \n-static int count_interesting_parents(struct commit *commit)\n+static int count_interesting_parents(struct commit *commit, int first_parent_only)\n {\n \tstruct commit_list *p;\n \tint count;\n \n \tfor (count = 0, p = commit->parents; p; p = p->next) {\n-\t\tif (p->item->object.flags & UNINTERESTING)\n-\t\t\tcontinue;\n-\t\tcount++;\n+\t\tif (!(p->item->object.flags & UNINTERESTING))\n+\t\t\tcount++;\n+\t\tif (first_parent_only)\n+\t\t\tbreak;\n \t}\n \treturn count;\n }\n@@ -259,7 +260,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 first_parent_only)\n {\n \tint n, counted;\n \tstruct commit_list *p;\n@@ -271,7 +272,7 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\n \t\tunsigned flags = commit->object.flags;\n \n \t\t*commit_weight_at(&commit_weight, p->item) = &weights[n++];\n-\t\tswitch (count_interesting_parents(commit)) {\n+\t\tswitch (count_interesting_parents(commit, first_parent_only)) {\n \t\tcase 0:\n \t\t\tif (!(flags & TREESAME)) {\n \t\t\t\tweight_set(p, 1);\n@@ -314,6 +315,8 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\n \t\t\tcontinue;\n \t\tif (weight(p) != -2)\n \t\t\tcontinue;\n+\t\tif (first_parent_only)\n+\t\t\tBUG(\"shouldn't be calling count-distance in fp mode\");\n \t\tweight_set(p, count_distance(p));\n \t\tclear_distance(list);\n \n@@ -332,7 +335,10 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\n \n \t\t\tif (0 <= weight(p))\n \t\t\t\tcontinue;\n-\t\t\tfor (q = p->item->parents; q; q = q->next) {\n+\n+\t\t\tfor (q = p->item->parents;\n+\t\t\t q;\n+\t\t\t q = first_parent_only ? NULL : q->next) {\n \t\t\t\tif (q->item->object.flags & UNINTERESTING)\n \t\t\t\t\tcontinue;\n \t\t\t\tif (0 <= weight(q))\n@@ -370,7 +376,7 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\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 first_parent_only)\n {\n \tint nr, on_list;\n \tstruct commit_list *list, *p, *best, *next, *last;\n@@ -406,7 +412,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, first_parent_only);\n \tif (best) {\n \t\tif (!find_all) {\n \t\t\tlist->item = best->item;\n@@ -991,6 +997,7 @@ enum bisect_error bisect_next_all(struct repository *r, const char *prefix, int\n \tenum bisect_error res = BISECT_OK;\n \tstruct object_id *bisect_rev;\n \tchar *steps_msg;\n+\tint first_parent_only = 0; /* TODO: pass --first-parent flag from git bisect start */\n \n \tread_bisect_terms(&term_bad, &term_good);\n \tif (read_bisect_refs())\n@@ -1001,11 +1008,12 @@ enum bisect_error bisect_next_all(struct repository *r, const char *prefix, int\n \t\treturn res;\n \n \tbisect_rev_setup(r, &revs, prefix, \"%s\", \"^%s\", 1);\n+\trevs.first_parent_only = first_parent_only;\n \trevs.limited = 1;\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, first_parent_only);\n \trevs.commits = managed_skipped(revs.commits, &tried);\n \n \tif (!revs.commits) {\ndiff --git a/bisect.h b/bisect.h\nindex 8bad8d8391..a63af0505f 100644\n--- a/bisect.h\n+++ b/bisect.h\n@@ -12,7 +12,7 @@ struct repository;\n * best commit, as chosen by `find_all`.\n */\n void find_bisection(struct commit_list **list, int *reaches, int *all,\n-\t\t int find_all);\n+\t\t int find_all, int first_parent_only);\n \n struct commit_list *filter_skipped(struct commit_list *list,\n \t\t\t\t struct commit_list **tried,\ndiff --git a/builtin/rev-list.c b/builtin/rev-list.c\nindex f520111eda..d1a14596b2 100644\n--- a/builtin/rev-list.c\n+++ b/builtin/rev-list.c\n@@ -638,7 +638,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, revs.first_parent_only);\n \n \t\tif (bisect_show_vars)\n \t\t\treturn show_bisect_vars(&info, reaches, all);\ndiff --git a/revision.c b/revision.c\nindex 6aa7f4f567..f0ad2603c1 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2869,9 +2869,6 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n \tif (!revs->reflog_info && revs->grep_filter.use_reflog_filter)\n \t\tdie(\"cannot use --grep-reflog without --walk-reflogs\");\n \n-\tif (revs->first_parent_only && revs->bisect)\n-\t\tdie(_(\"--first-parent is incompatible with --bisect\"));\n-\n \tif (revs->line_level_traverse &&\n \t (revs->diffopt.output_format & ~(DIFF_FORMAT_PATCH | DIFF_FORMAT_NO_OUTPUT)))\n \t\tdie(_(\"-L does not yet support diff formats besides -p and -s\"));\ndiff --git a/t/t6000-rev-list-misc.sh b/t/t6000-rev-list-misc.sh\nindex 3dc1ad8f71..26e3533562 100755\n--- a/t/t6000-rev-list-misc.sh\n+++ b/t/t6000-rev-list-misc.sh\n@@ -125,8 +125,8 @@ test_expect_success 'rev-list can negate index objects' '\n \ttest_cmp expect actual\n '\n \n-test_expect_success '--bisect and --first-parent can not be combined' '\n-\ttest_must_fail git rev-list --bisect --first-parent HEAD\n+test_expect_success '--bisect and --first-parent can be combined' '\n+\tgit rev-list --bisect --first-parent HEAD\n '\n \n test_expect_success '--header shows a NUL after each commit' '\ndiff --git a/t/t6002-rev-list-bisect.sh b/t/t6002-rev-list-bisect.sh\nindex a661408038..7fc0f75ca5 100755\n--- a/t/t6002-rev-list-bisect.sh\n+++ b/t/t6002-rev-list-bisect.sh\n@@ -263,4 +263,49 @@ test_expect_success 'rev-parse --bisect can default to good/bad refs' '\n \ttest_cmp expect.sorted actual.sorted\n '\n \n+test_output_expect_success \"--bisect --first-parent\" 'git rev-list --bisect --first-parent E ^F' < expect.unsorted < expect &&\n+git rev-list --bisect-all --first-parent E ^F > actual &&\n+test_cmp actual expect\n+'\n+\n test_done\n-- \n2.24.3 (Apple Git-128)\n\n"},{"id":"402622","messageId":"20200801175840.1877-4-alipman88@gmail.com","threadId":"53937","inReplyTo":"20200801175840.1877-1-alipman88@gmail.com","subject":"[PATCH v3 3/4] bisect: combine args passed to find_bisection()","fromName":"Aaron Lipman","fromEmail":"alipman88@gmail.com","sentAt":"2020-08-01T17:58:39Z","receivedAt":"2020-08-01T17:58:54Z","isPatch":true,"sender":{"key":"alipman88@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3922511?v=4"},"body":"Now that find_bisection() accepts multiple boolean arguments, these may\nbe combined into a single unsigned integer in order to declutter some of\nthe code in bisect.c\n\nAlso, rename existing the \"flags\" bitfield to \"commit_flags\", to\nexplicitly differentiate between the new \"bisect_flags\" bitfield.\n\nBased-on-patch-by: Harald Nordgren \nSigned-off-by: Aaron Lipman \n---\n bisect.c | 67 +++++++++++++++++++++++++---------------------\n bisect.h | 5 +++-\n builtin/rev-list.c | 9 ++++++-\n 3 files changed, 49 insertions(+), 32 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex afbb5aea32..b14a2edc38 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -88,7 +88,7 @@ static inline void weight_set(struct commit_list *elem, int weight)\n \t**commit_weight_at(&commit_weight, elem->item) = weight;\n }\n \n-static int count_interesting_parents(struct commit *commit, int first_parent_only)\n+static int count_interesting_parents(struct commit *commit, unsigned bisect_flags)\n {\n \tstruct commit_list *p;\n \tint count;\n@@ -96,7 +96,7 @@ static int count_interesting_parents(struct commit *commit, int first_parent_onl\n \tfor (count = 0, p = commit->parents; p; p = p->next) {\n \t\tif (!(p->item->object.flags & UNINTERESTING))\n \t\t\tcount++;\n-\t\tif (first_parent_only)\n+\t\tif (bisect_flags & FIND_BISECTION_FIRST_PARENT_ONLY)\n \t\t\tbreak;\n \t}\n \treturn count;\n@@ -136,7 +136,7 @@ static void show_list(const char *debug, int counted, int nr,\n \tfor (p = list; p; p = p->next) {\n \t\tstruct commit_list *pp;\n \t\tstruct commit *commit = p->item;\n-\t\tunsigned flags = commit->object.flags;\n+\t\tunsigned commit_flags = commit->object.flags;\n \t\tenum object_type type;\n \t\tunsigned long size;\n \t\tchar *buf = read_object_file(&commit->object.oid, &type,\n@@ -145,9 +145,9 @@ static void show_list(const char *debug, int counted, int nr,\n \t\tint subject_len;\n \n \t\tfprintf(stderr, \"%c%c%c \",\n-\t\t\t(flags & TREESAME) ? ' ' : 'T',\n-\t\t\t(flags & UNINTERESTING) ? 'U' : ' ',\n-\t\t\t(flags & COUNTED) ? 'C' : ' ');\n+\t\t\t(commit_flags & TREESAME) ? ' ' : 'T',\n+\t\t\t(commit_flags & UNINTERESTING) ? 'U' : ' ',\n+\t\t\t(commit_flags & COUNTED) ? 'C' : ' ');\n \t\tif (*commit_weight_at(&commit_weight, p->item))\n \t\t\tfprintf(stderr, \"%3d\", weight(p));\n \t\telse\n@@ -172,9 +172,9 @@ static struct commit_list *best_bisection(struct commit_list *list, int nr)\n \tbest = list;\n \tfor (p = list; p; p = p->next) {\n \t\tint distance;\n-\t\tunsigned flags = p->item->object.flags;\n+\t\tunsigned commit_flags = p->item->object.flags;\n \n-\t\tif (flags & TREESAME)\n+\t\tif (commit_flags & TREESAME)\n \t\t\tcontinue;\n \t\tdistance = weight(p);\n \t\tif (nr - distance < distance)\n@@ -213,9 +213,9 @@ static struct commit_list *best_bisection_sorted(struct commit_list *list, int n\n \n \tfor (p = list, cnt = 0; p; p = p->next) {\n \t\tint distance;\n-\t\tunsigned flags = p->item->object.flags;\n+\t\tunsigned commit_flags = p->item->object.flags;\n \n-\t\tif (flags & TREESAME)\n+\t\tif (commit_flags & TREESAME)\n \t\t\tcontinue;\n \t\tdistance = weight(p);\n \t\tif (nr - distance < distance)\n@@ -260,7 +260,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, int first_parent_only)\n+\t\t\t\t\t unsigned bisect_flags)\n {\n \tint n, counted;\n \tstruct commit_list *p;\n@@ -269,12 +269,12 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\n \n \tfor (n = 0, p = list; p; p = p->next) {\n \t\tstruct commit *commit = p->item;\n-\t\tunsigned flags = commit->object.flags;\n+\t\tunsigned commit_flags = commit->object.flags;\n \n \t\t*commit_weight_at(&commit_weight, p->item) = &weights[n++];\n-\t\tswitch (count_interesting_parents(commit, first_parent_only)) {\n+\t\tswitch (count_interesting_parents(commit, bisect_flags)) {\n \t\tcase 0:\n-\t\t\tif (!(flags & TREESAME)) {\n+\t\t\tif (!(commit_flags & TREESAME)) {\n \t\t\t\tweight_set(p, 1);\n \t\t\t\tcounted++;\n \t\t\t\tshow_list(\"bisection 2 count one\",\n@@ -315,13 +315,13 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\n \t\t\tcontinue;\n \t\tif (weight(p) != -2)\n \t\t\tcontinue;\n-\t\tif (first_parent_only)\n+\t\tif (bisect_flags & FIND_BISECTION_FIRST_PARENT_ONLY)\n \t\t\tBUG(\"shouldn't be calling count-distance in fp mode\");\n \t\tweight_set(p, count_distance(p));\n \t\tclear_distance(list);\n \n \t\t/* Does it happen to be at exactly half-way? */\n-\t\tif (!find_all && halfway(p, nr))\n+\t\tif (!(bisect_flags & FIND_BISECTION_ALL) && halfway(p, nr))\n \t\t\treturn p;\n \t\tcounted++;\n \t}\n@@ -331,14 +331,14 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\n \twhile (counted < nr) {\n \t\tfor (p = list; p; p = p->next) {\n \t\t\tstruct commit_list *q;\n-\t\t\tunsigned flags = p->item->object.flags;\n+\t\t\tunsigned commit_flags = p->item->object.flags;\n \n \t\t\tif (0 <= weight(p))\n \t\t\t\tcontinue;\n \n \t\t\tfor (q = p->item->parents;\n \t\t\t q;\n-\t\t\t q = first_parent_only ? NULL : q->next) {\n+\t\t\t q = bisect_flags & FIND_BISECTION_FIRST_PARENT_ONLY ? NULL : q->next) {\n \t\t\t\tif (q->item->object.flags & UNINTERESTING)\n \t\t\t\t\tcontinue;\n \t\t\t\tif (0 <= weight(q))\n@@ -352,7 +352,7 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\n \t\t\t * add one for p itself if p is to be counted,\n \t\t\t * otherwise inherit it from q directly.\n \t\t\t */\n-\t\t\tif (!(flags & TREESAME)) {\n+\t\t\tif (!(commit_flags & TREESAME)) {\n \t\t\t\tweight_set(p, weight(q)+1);\n \t\t\t\tcounted++;\n \t\t\t\tshow_list(\"bisection 2 count one\",\n@@ -362,21 +362,21 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\n \t\t\t\tweight_set(p, weight(q));\n \n \t\t\t/* Does it happen to be at exactly half-way? */\n-\t\t\tif (!find_all && halfway(p, nr))\n+\t\t\tif (!(bisect_flags & FIND_BISECTION_ALL) && halfway(p, nr))\n \t\t\t\treturn p;\n \t\t}\n \t}\n \n \tshow_list(\"bisection 2 counted all\", counted, nr, list);\n \n-\tif (!find_all)\n+\tif (!(bisect_flags & FIND_BISECTION_ALL))\n \t\treturn best_bisection(list, nr);\n \telse\n \t\treturn best_bisection_sorted(list, nr);\n }\n \n void find_bisection(struct commit_list **commit_list, int *reaches,\n-\t\t int *all, int find_all, int first_parent_only)\n+\t\t int *all, unsigned bisect_flags)\n {\n \tint nr, on_list;\n \tstruct commit_list *list, *p, *best, *next, *last;\n@@ -392,16 +392,16 @@ void find_bisection(struct commit_list **commit_list, int *reaches,\n \tfor (nr = on_list = 0, last = NULL, p = *commit_list;\n \t p;\n \t p = next) {\n-\t\tunsigned flags = p->item->object.flags;\n+\t\tunsigned commit_flags = p->item->object.flags;\n \n \t\tnext = p->next;\n-\t\tif (flags & UNINTERESTING) {\n+\t\tif (commit_flags & UNINTERESTING) {\n \t\t\tfree(p);\n \t\t\tcontinue;\n \t\t}\n \t\tp->next = last;\n \t\tlast = p;\n-\t\tif (!(flags & TREESAME))\n+\t\tif (!(commit_flags & TREESAME))\n \t\t\tnr++;\n \t\ton_list++;\n \t}\n@@ -412,9 +412,9 @@ 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, first_parent_only);\n+\tbest = do_find_bisection(list, nr, weights, bisect_flags);\n \tif (best) {\n-\t\tif (!find_all) {\n+\t\tif (!(bisect_flags & FIND_BISECTION_ALL)) {\n \t\t\tlist->item = best->item;\n \t\t\tfree_commit_list(list->next);\n \t\t\tbest = list;\n@@ -1004,23 +1004,30 @@ enum bisect_error bisect_next_all(struct repository *r, const char *prefix, int\n \tenum bisect_error res = BISECT_OK;\n \tstruct object_id *bisect_rev;\n \tchar *steps_msg;\n-\tint first_parent_only = read_first_parent_option();\n+\tunsigned bisect_flags = 0;\n \n \tread_bisect_terms(&term_bad, &term_good);\n \tif (read_bisect_refs())\n \t\tdie(_(\"reading bisect refs failed\"));\n \n+\tif (read_first_parent_option())\n+\t\tbisect_flags |= FIND_BISECTION_FIRST_PARENT_ONLY;\n+\n+\tif (skipped_revs.nr)\n+\t\tbisect_flags |= FIND_BISECTION_ALL;\n+\n \tres = check_good_are_ancestors_of_bad(r, prefix, no_checkout);\n \tif (res)\n \t\treturn res;\n \n \tbisect_rev_setup(r, &revs, prefix, \"%s\", \"^%s\", 1);\n-\trevs.first_parent_only = first_parent_only;\n+\n+\trevs.first_parent_only = !!(bisect_flags & FIND_BISECTION_FIRST_PARENT_ONLY);\n \trevs.limited = 1;\n \n \tbisect_common(&revs);\n \n-\tfind_bisection(&revs.commits, &reaches, &all, !!skipped_revs.nr, first_parent_only);\n+\tfind_bisection(&revs.commits, &reaches, &all, bisect_flags);\n \trevs.commits = managed_skipped(revs.commits, &tried);\n \n \tif (!revs.commits) {\ndiff --git a/bisect.h b/bisect.h\nindex a63af0505f..b673fd9ed2 100644\n--- a/bisect.h\n+++ b/bisect.h\n@@ -12,7 +12,7 @@ struct repository;\n * best commit, as chosen by `find_all`.\n */\n void find_bisection(struct commit_list **list, int *reaches, int *all,\n-\t\t int find_all, int first_parent_only);\n+\t\t unsigned bisect_flags);\n \n struct commit_list *filter_skipped(struct commit_list *list,\n \t\t\t\t struct commit_list **tried,\n@@ -23,6 +23,9 @@ struct commit_list *filter_skipped(struct commit_list *list,\n #define BISECT_SHOW_ALL\t\t(1<<0)\n #define REV_LIST_QUIET\t\t(1<<1)\n \n+#define FIND_BISECTION_ALL\t\t\t(1u<<0)\n+#define FIND_BISECTION_FIRST_PARENT_ONLY\t(1u<<1)\n+\n struct rev_list_info {\n \tstruct rev_info *revs;\n \tint flags;\ndiff --git a/builtin/rev-list.c b/builtin/rev-list.c\nindex d1a14596b2..25c6c3b38d 100644\n--- a/builtin/rev-list.c\n+++ b/builtin/rev-list.c\n@@ -637,8 +637,15 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)\n \n \tif (bisect_list) {\n \t\tint reaches, all;\n+\t\tunsigned bisect_flags = 0;\n \n-\t\tfind_bisection(&revs.commits, &reaches, &all, bisect_find_all, revs.first_parent_only);\n+\t\tif (bisect_find_all)\n+\t\t\tbisect_flags |= FIND_BISECTION_ALL;\n+\n+\t\tif (revs.first_parent_only)\n+\t\t\tbisect_flags |= FIND_BISECTION_FIRST_PARENT_ONLY;\n+\n+\t\tfind_bisection(&revs.commits, &reaches, &all, bisect_flags);\n \n \t\tif (bisect_show_vars)\n \t\t\treturn show_bisect_vars(&info, reaches, all);\n-- \n2.24.3 (Apple Git-128)\n\n"},{"id":"402621","messageId":"20200801175840.1877-5-alipman88@gmail.com","threadId":"53937","inReplyTo":"20200801175840.1877-1-alipman88@gmail.com","subject":"[PATCH v3 4/4] bisect: consistent style for git bisect run tests","fromName":"Aaron Lipman","fromEmail":"alipman88@gmail.com","sentAt":"2020-08-01T17:58:40Z","receivedAt":"2020-08-01T17:58:55Z","isPatch":true,"sender":{"key":"alipman88@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3922511?v=4"},"body":"Enforce consistent styling for tests on \"git bisect run\":\n- Use \"write_script\" to abstract away platform-specific details.\n- Favor current whitespace conventions.\n---\n t/t6030-bisect-porcelain.sh | 46 ++++++++++++++++++-------------------\n 1 file changed, 22 insertions(+), 24 deletions(-)\n\ndiff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh\nindex 51f5eb6ea3..63661c5e3d 100755\n--- a/t/t6030-bisect-porcelain.sh\n+++ b/t/t6030-bisect-porcelain.sh\n@@ -243,32 +243,30 @@ test_expect_success 'bisect skip: with commit both bad and skipped' '\n '\n \n # We want to automatically find the commit that\n-# introduced \"Another\" into hello.\n-test_expect_success \\\n- '\"git bisect run\" simple case' \\\n- 'echo \"#\"\\!\"/bin/sh\" > test_script.sh &&\n- echo \"grep Another hello > /dev/null\" >> test_script.sh &&\n- echo \"test \\$? -ne 0\" >> test_script.sh &&\n- chmod +x test_script.sh &&\n- git bisect start &&\n- git bisect good $HASH1 &&\n- git bisect bad $HASH4 &&\n- git bisect run ./test_script.sh > my_bisect_log.txt &&\n- grep \"$HASH3 is the first bad commit\" my_bisect_log.txt &&\n- git bisect reset'\n+# added \"Another\" into hello.\n+test_expect_success '\"git bisect run\" simple case' '\n+\twrite_script test_script.sh <<-\\EOF &&\n+\t! grep Another hello >/dev/null\n+\tEOF\n+\tgit bisect start &&\n+\tgit bisect good $HASH1 &&\n+\tgit bisect bad $HASH4 &&\n+\tgit bisect run ./test_script.sh >my_bisect_log.txt &&\n+\tgrep \"$HASH3 is the first bad commit\" my_bisect_log.txt &&\n+\tgit bisect reset\n+'\n \n # We want to automatically find the commit that\n-# introduced \"Ciao\" into hello.\n-test_expect_success \\\n- '\"git bisect run\" with more complex \"git bisect start\"' \\\n- 'echo \"#\"\\!\"/bin/sh\" > test_script.sh &&\n- echo \"grep Ciao hello > /dev/null\" >> test_script.sh &&\n- echo \"test \\$? -ne 0\" >> test_script.sh &&\n- chmod +x test_script.sh &&\n- git bisect start $HASH4 $HASH1 &&\n- git bisect run ./test_script.sh > my_bisect_log.txt &&\n- grep \"$HASH4 is the first bad commit\" my_bisect_log.txt &&\n- git bisect reset'\n+# added \"Ciao\" into hello.\n+test_expect_success '\"git bisect run\" with more complex \"git bisect start\"' '\n+\twrite_script test_script.sh <<-\\EOF &&\n+\t! grep Ciao hello >/dev/null\n+\tEOF\n+\tgit bisect start $HASH4 $HASH1 &&\n+\tgit bisect run ./test_script.sh >my_bisect_log.txt &&\n+\tgrep \"$HASH4 is the first bad commit\" my_bisect_log.txt &&\n+\tgit bisect reset\n+'\n \n # $HASH1 is good, $HASH5 is bad, we skip $HASH3\n # but $HASH4 is good,\n-- \n2.24.3 (Apple Git-128)\n\n"},{"id":"402623","messageId":"20200801175840.1877-3-alipman88@gmail.com","threadId":"53937","inReplyTo":"20200801175840.1877-1-alipman88@gmail.com","subject":"[PATCH v3 2/4] bisect: introduce first-parent flag","fromName":"Aaron Lipman","fromEmail":"alipman88@gmail.com","sentAt":"2020-08-01T17:58:38Z","receivedAt":"2020-08-01T17:58:55Z","isPatch":true,"sender":{"key":"alipman88@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3922511?v=4"},"body":"Upon seeing a merge commit when bisecting, this option may be used to\nfollow only the first parent.\n\nIn detecting regressions introduced through the merging of a branch, the\nmerge commit will be identified as introduction of the bug and its\nancestors will be ignored.\n\nThis option is particularly useful in avoiding false positives when a\nmerged branch contained broken or non-buildable commits, but the merge\nitself was OK.\n\nSigned-off-by: Aaron Lipman \n---\n Documentation/git-bisect.txt | 13 ++++++++++++-\n bisect.c | 10 +++++++++-\n builtin/bisect--helper.c | 14 ++++++++++----\n t/t6030-bisect-porcelain.sh | 18 ++++++++++++++++++\n 4 files changed, 49 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt\nindex 7586c5a843..0e993e4587 100644\n--- a/Documentation/git-bisect.txt\n+++ b/Documentation/git-bisect.txt\n@@ -17,7 +17,7 @@ The command takes various subcommands, and different options depending\n on the subcommand:\n \n git bisect start [--term-{old,good}= --term-{new,bad}=]\n-\t\t [--no-checkout] [ [...]] [--] [...]\n+\t\t [--no-checkout] [--first-parent] [ [...]] [--] [...]\n git bisect (bad|new|) []\n git bisect (good|old|) [...]\n git bisect terms [--term-good | --term-bad]\n@@ -365,6 +365,17 @@ does not require a checked out tree.\n +\n If the repository is bare, `--no-checkout` is assumed.\n \n+--first-parent::\n++\n+Follow only the first parent commit upon seeing a merge commit.\n++\n+In detecting regressions introduced through the merging of a branch, the merge\n+commit will be identified as introduction of the bug and its ancestors will be\n+ignored.\n++\n+This option is particularly useful in avoiding false positives when a merged\n+branch contained broken or non-buildable commits, but the merge itself was OK.\n+\n EXAMPLES\n --------\n \ndiff --git a/bisect.c b/bisect.c\nindex a11fdb1473..afbb5aea32 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -460,6 +460,7 @@ static GIT_PATH_FUNC(git_path_bisect_run, \"BISECT_RUN\")\n static GIT_PATH_FUNC(git_path_bisect_start, \"BISECT_START\")\n static GIT_PATH_FUNC(git_path_bisect_log, \"BISECT_LOG\")\n static GIT_PATH_FUNC(git_path_bisect_terms, \"BISECT_TERMS\")\n+static GIT_PATH_FUNC(git_path_bisect_first_parent, \"BISECT_FIRST_PARENT\")\n static GIT_PATH_FUNC(git_path_head_name, \"head-name\")\n \n static void read_bisect_paths(struct argv_array *array)\n@@ -981,6 +982,12 @@ void read_bisect_terms(const char **read_bad, const char **read_good)\n \tfclose(fp);\n }\n \n+static int read_first_parent_option(void)\n+{\n+\tconst char *filename = git_path_bisect_first_parent();\n+\treturn !access(filename, F_OK);\n+}\n+\n /*\n * We use the convention that return BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND (-10) means\n * the bisection process finished successfully.\n@@ -997,7 +1004,7 @@ enum bisect_error bisect_next_all(struct repository *r, const char *prefix, int\n \tenum bisect_error res = BISECT_OK;\n \tstruct object_id *bisect_rev;\n \tchar *steps_msg;\n-\tint first_parent_only = 0; /* TODO: pass --first-parent flag from git bisect start */\n+\tint first_parent_only = read_first_parent_option();\n \n \tread_bisect_terms(&term_bad, &term_good);\n \tif (read_bisect_refs())\n@@ -1141,6 +1148,7 @@ int bisect_clean_state(void)\n \tunlink_or_warn(git_path_bisect_names());\n \tunlink_or_warn(git_path_bisect_run());\n \tunlink_or_warn(git_path_bisect_terms());\n+\tunlink_or_warn(git_path_bisect_first_parent());\n \t/* Cleanup head-name if it got left by an old version of git-bisect */\n \tunlink_or_warn(git_path_head_name());\n \t/*\ndiff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\nindex ec4996282e..9273140f0b 100644\n--- a/builtin/bisect--helper.c\n+++ b/builtin/bisect--helper.c\n@@ -17,6 +17,7 @@ static GIT_PATH_FUNC(git_path_bisect_head, \"BISECT_HEAD\")\n static GIT_PATH_FUNC(git_path_bisect_log, \"BISECT_LOG\")\n static GIT_PATH_FUNC(git_path_head_name, \"head-name\")\n static GIT_PATH_FUNC(git_path_bisect_names, \"BISECT_NAMES\")\n+static GIT_PATH_FUNC(git_path_bisect_first_parent, \"BISECT_FIRST_PARENT\")\n \n static const char * const git_bisect_helper_usage[] = {\n \tN_(\"git bisect--helper --next-all [--no-checkout]\"),\n@@ -28,7 +29,7 @@ static const char * const git_bisect_helper_usage[] = {\n \tN_(\"git bisect--helper --bisect-next-check []\"),\n \tN_(\"git bisect--helper --bisect-terms [--term-good | --term-old | --term-bad | --term-new]\"),\n \tN_(\"git bisect--helper --bisect-start [--term-{old,good}= --term-{new,bad}=]\"\n-\t\t\t\t\t \"[--no-checkout] [ [...]] [--] [...]\"),\n+\t\t\t\t\t \" [--no-checkout] [--first-parent] [ [...]] [--] [...]\"),\n \tNULL\n };\n \n@@ -422,7 +423,7 @@ static int bisect_append_log_quoted(const char **argv)\n }\n \n static int bisect_start(struct bisect_terms *terms, int no_checkout,\n-\t\t\tconst char **argv, int argc)\n+\t\t\tint first_parent_only, const char **argv, int argc)\n {\n \tint i, has_double_dash = 0, must_write_terms = 0, bad_seen = 0;\n \tint flags, pathspec_pos, res = 0;\n@@ -577,6 +578,9 @@ static int bisect_start(struct bisect_terms *terms, int no_checkout,\n \t */\n \twrite_file(git_path_bisect_start(), \"%s\\n\", start_head.buf);\n \n+\tif (first_parent_only)\n+\t\twrite_file(git_path_bisect_first_parent(), \"\\n\");\n+\n \tif (no_checkout) {\n \t\tif (get_oid(start_head.buf, &oid) < 0) {\n \t\t\tres = error(_(\"invalid ref: '%s'\"), start_head.buf);\n@@ -632,7 +636,7 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\tBISECT_TERMS,\n \t\tBISECT_START\n \t} cmdmode = 0;\n-\tint no_checkout = 0, res = 0, nolog = 0;\n+\tint no_checkout = 0, first_parent_only = 0, res = 0, nolog = 0;\n \tstruct option options[] = {\n \t\tOPT_CMDMODE(0, \"next-all\", &cmdmode,\n \t\t\t N_(\"perform 'git bisect next'\"), NEXT_ALL),\n@@ -656,6 +660,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"start the bisect session\"), BISECT_START),\n \t\tOPT_BOOL(0, \"no-checkout\", &no_checkout,\n \t\t\t N_(\"update BISECT_HEAD instead of checking out the current commit\")),\n+\t\tOPT_BOOL(0, \"first-parent\", &first_parent_only,\n+\t\t\t N_(\"only trace the first parent of merge commits\")),\n \t\tOPT_BOOL(0, \"no-log\", &nolog,\n \t\t\t N_(\"no log for BISECT_WRITE\")),\n \t\tOPT_END()\n@@ -713,7 +719,7 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\tbreak;\n \tcase BISECT_START:\n \t\tset_terms(&terms, \"bad\", \"good\");\n-\t\tres = bisect_start(&terms, no_checkout, argv, argc);\n+\t\tres = bisect_start(&terms, no_checkout, first_parent_only, argv, argc);\n \t\tbreak;\n \tdefault:\n \t\treturn error(\"BUG: unknown subcommand '%d'\", cmdmode);\ndiff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh\nindex 36d9b2b2e4..51f5eb6ea3 100755\n--- a/t/t6030-bisect-porcelain.sh\n+++ b/t/t6030-bisect-porcelain.sh\n@@ -458,6 +458,24 @@ test_expect_success 'many merge bases creation' '\n \tgrep \"$SIDE_HASH5\" merge_bases.txt\n '\n \n+# We want to automatically find the merge that\n+# added \"line\" into hello.\n+test_expect_success '\"git bisect run --first-parent\" simple case' '\n+\tgit rev-list --first-parent $B_HASH ^$HASH4 >first_parent_chain.txt &&\n+\twrite_script test_script.sh <<-\\EOF &&\n+\tgrep $(git rev-parse HEAD) first_parent_chain.txt || exit -1\n+\t! grep line hello >/dev/null\n+\tEOF\n+ git bisect start --first-parent &&\n+ test_path_is_file \".git/BISECT_FIRST_PARENT\" &&\n+ git bisect good $HASH4 &&\n+ git bisect bad $B_HASH &&\n+ git bisect run ./test_script.sh >my_bisect_log.txt &&\n+ grep \"$B_HASH is the first bad commit\" my_bisect_log.txt &&\n+ git bisect reset &&\n+ test_path_is_missing .git/BISECT_FIRST_PARENT\n+'\n+\n test_expect_success 'good merge bases when good and bad are siblings' '\n \tgit bisect start \"$B_HASH\" \"$A_HASH\" > my_bisect_log.txt &&\n \ttest_i18ngrep \"merge base must be tested\" my_bisect_log.txt &&\n-- \n2.24.3 (Apple Git-128)\n\n"},{"id":"402624","messageId":"xmqq36562nki.fsf@gitster.c.googlers.com","threadId":"53937","inReplyTo":"20200801175840.1877-1-alipman88@gmail.com","subject":"Re: [PATCH v3 0/4] Introduce --first-parent flag for git bisect","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-08-01T19:06:21Z","receivedAt":"2020-08-01T19:06:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Aaron Lipman writes:\n\n>> I suspect that you would ask the same question to whoever added\n>> no_checkout to this thing, and I wouldn't be surprised if we end up\n>> removing both of these parameters (and parsing of the options inside\n>> cmd_bisect__helper()) after thinking about them, but anyway, let's\n>> read on.\n>\n> Hmm. Is there a preferred way to to add a --first-parent line to the\n> output of \"git bisect start --help\" via the parse-options API without\n> removing the --first-parent option from argv in the process?\n\nI'd rather not to see the code made ugly if the only reason why you\nhave duplicated command line parsing is to support \"git bisect start\n-h\" while cmd_bisect__helper() still exists. \n\nThat function is supposed to be a thin shim layer whose only reason\nof its existence is to serve as an interface with the scripted\nversion of \"git bisect\". When everything is migrated from the shell\nscript, cmd_bisect__helper() should disappear.\n\nIn its place, cmd_bisect() written in C would become a dispatcher\nthat only looks at the first token on the command line that comes\nafter [\"git\", \"bisect\"] and calls \"bisect_start()\", \"bisect_next()\",\netc. with the remainder of the command line with options such as\n\"--first-parent\" and \"--no-checkout\", which will be parsed by\nbisect_start() etc. parsing its argc/argv[] passed by cmd_bisect().\nIt is likely that cmd_bisect() would not have any call to\nparse_options(); instead cmd_start() etc. would call parse_options()\nwith their own option[] table.\n\nSo I am not sure if the change between v2 and v3 is going in the\nright direction.\n\n> As long as we're capturing the --first-parent option in\n> cmd_bisect__helper(), checking argv for a --first-parent flag in\n> bisect_start() is pointless.\n\nThis is going backwards, as far as I can tell. If anything, I'd\nrather see cmd_bisect__helper() get fixed so that it does not parse\n\"--first-parent\" (and similarly, \"--no-checkout\" that you imitated)\ninto first_parent_only (and no_checkout) variables and passed as\nparameters to bisect_start(). cmd_bisect__helper() should instead\njust let these flags (and there may be others) sit in argc/argv[]\nand have bisect_start() parse them, together with all other options\nbisect_start() already has its parser for.\n\nThanks.\n"},{"id":"402625","messageId":"20200801192717.17278-1-martin.agren@gmail.com","threadId":"53937","inReplyTo":"20200801175840.1877-5-alipman88@gmail.com","subject":"Re: [PATCH v3 4/4] bisect: consistent style for git bisect run tests","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2020-08-01T19:27:17Z","receivedAt":"2020-08-01T19:27:29Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"On Sat, 1 Aug 2020 at 20:03, Aaron Lipman wrote:\n> Subject: bisect: consistent style for git bisect run tests\n\nNit: this could be 't6030: modernize \"git bisect run\" tests' to tighten\nthe area a bit and to begin with a verb.\n\n> Enforce consistent styling for tests on \"git bisect run\":\n> - Use \"write_script\" to abstract away platform-specific details.\n> - Favor current whitespace conventions.\n> ---\n\nMissing sign-off.\n\nI think this should come earlier in the series, probably already as\npatch 1/4. Some reasons for that:\n\n * If this effort doesn't go any further (let's hope it does!), at\n least the initial cleanup can be merged, so that later endeavours can\n find a slightly cleaner table to start with.\n\n * Revisiting your other patches in `git log` after they've been merged,\n it will be obvious that they follow other tests, so one can focus on\n what they do and not on how they differ.\n\n * Let's first tidy up, then build shiny new things. ;-)\n\n> # We want to automatically find the commit that\n> -# introduced \"Another\" into hello.\n> -test_expect_success \\\n> - '\"git bisect run\" simple case' \\\n> - 'echo \"#\"\\!\"/bin/sh\" > test_script.sh &&\n[...]\n> +# added \"Another\" into hello.\n> +test_expect_success '\"git bisect run\" simple case' '\n> + write_script test_script.sh <<-\\EOF &&\n[...]\n\nAll of these look like correct, worthwhile modifications. The\ns/introduced/added/ changes aren't mentioned in the proposed log\nmessage. They have been discussed up-thread, but it might still be\ngood to add something like\n\n While at it, change \"introduced\" to \"added\" in the comments to make\n them read better.\n\nFWIW, I guess there's some minor added value in doing that change and\nsince you're doing other changes anyway, it might actually be worth the\nchurn.\n\nYou change two tests like this, but there are other \"git bisect run\"\ntests that look like they're in need of some modernizing. Looking for\n\"chmod\" I spotted the ones I'm tweaking below. Looking for \"git bisect\nrun\" you might find more that may contain some funny indentation. I'll\nleave it to you where to stop going down that rabbit-hole but if you do\nre-roll, it might make sense to squash in the diff below, maybe with\nsome modifications to the commit message. (Signed-off-by: Martin Ågren\n, FWIW.)\n\nMartin\n\n-- >8 --\n\ndiff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh\nindex 63661c5e3d..66c12bc665 100755\n--- a/t/t6030-bisect-porcelain.sh\n+++ b/t/t6030-bisect-porcelain.sh\n@@ -293,24 +293,17 @@ HASH6=\n test_expect_success 'bisect run & skip: cannot tell between 2' '\n \tadd_line_into_file \"6: Yet a line.\" hello &&\n \tHASH6=$(git rev-parse --verify HEAD) &&\n-\techo \"#\"\\!\"/bin/sh\" > test_script.sh &&\n-\techo \"sed -ne \\\\\\$p hello | grep Ciao > /dev/null && exit 125\" >> test_script.sh &&\n-\techo \"grep line hello > /dev/null\" >> test_script.sh &&\n-\techo \"test \\$? -ne 0\" >> test_script.sh &&\n-\tchmod +x test_script.sh &&\n+\twrite_script test_script.sh <<-\\EOF &&\n+\tsed -ne \\$p hello | grep Ciao >/dev/null && exit 125\n+\t! grep line hello >/dev/null\n+\tEOF\n \tgit bisect start $HASH6 $HASH1 &&\n-\tif git bisect run ./test_script.sh > my_bisect_log.txt\n-\tthen\n-\t\techo Oops, should have failed.\n-\t\tfalse\n-\telse\n-\t\ttest $? -eq 2 &&\n-\t\tgrep \"first bad commit could be any of\" my_bisect_log.txt &&\n-\t\t! grep $HASH3 my_bisect_log.txt &&\n-\t\t! grep $HASH6 my_bisect_log.txt &&\n-\t\tgrep $HASH4 my_bisect_log.txt &&\n-\t\tgrep $HASH5 my_bisect_log.txt\n-\tfi\n+\ttest_expect_code 2 git bisect run ./test_script.sh >my_bisect_log.txt &&\n+\tgrep \"first bad commit could be any of\" my_bisect_log.txt &&\n+\t! grep $HASH3 my_bisect_log.txt &&\n+\t! grep $HASH6 my_bisect_log.txt &&\n+\tgrep $HASH4 my_bisect_log.txt &&\n+\tgrep $HASH5 my_bisect_log.txt\n '\n \n HASH7=\n@@ -318,14 +311,13 @@ test_expect_success 'bisect run & skip: find first bad' '\n \tgit bisect reset &&\n \tadd_line_into_file \"7: Should be the last line.\" hello &&\n \tHASH7=$(git rev-parse --verify HEAD) &&\n-\techo \"#\"\\!\"/bin/sh\" > test_script.sh &&\n-\techo \"sed -ne \\\\\\$p hello | grep Ciao > /dev/null && exit 125\" >> test_script.sh &&\n-\techo \"sed -ne \\\\\\$p hello | grep day > /dev/null && exit 125\" >> test_script.sh &&\n-\techo \"grep Yet hello > /dev/null\" >> test_script.sh &&\n-\techo \"test \\$? -ne 0\" >> test_script.sh &&\n-\tchmod +x test_script.sh &&\n+\twrite_script test_script.sh <<-\\EOF &&\n+\tsed -ne \\$p hello | grep Ciao >/dev/null && exit 125\n+\tsed -ne \\$p hello | grep day >/dev/null && exit 125\n+\t! grep Yet hello >/dev/null\n+\tEOF\n \tgit bisect start $HASH7 $HASH1 &&\n-\tgit bisect run ./test_script.sh > my_bisect_log.txt &&\n+\tgit bisect run ./test_script.sh >my_bisect_log.txt &&\n \tgrep \"$HASH6 is the first bad commit\" my_bisect_log.txt\n '\n \n-- \n2.28.0.81.ge8ab941b67\n\n"},{"id":"402626","messageId":"CAN0heSqph3gW0-3bQRTTugxFjjztYp_EjwfCKQzJ1iFxdQKdug@mail.gmail.com","threadId":"53937","inReplyTo":"20200801175840.1877-4-alipman88@gmail.com","subject":"Re: [PATCH v3 3/4] bisect: combine args passed to find_bisection()","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2020-08-01T19:30:38Z","receivedAt":"2020-08-01T19:30:52Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"On Sat, 1 Aug 2020 at 20:01, Aaron Lipman wrote:\n> Also, rename existing the \"flags\" bitfield to \"commit_flags\", to\n> explicitly differentiate between the new \"bisect_flags\" bitfield.\n\ns/existing the/the existing/\n\nAlso, does \"separate between \" make grammatical sense? Should\nit be \"separate between \" or \"separate [it] from \"? Dunno..\n\nMartin\n"},{"id":"402854","messageId":"20200804220113.5909-1-alipman88@gmail.com","threadId":"53937","inReplyTo":"20200801175840.1877-1-alipman88@gmail.com","subject":"[PATCH v4 0/5] Introduce --first-parent flag for git bisect","fromName":"Aaron Lipman","fromEmail":"alipman88@gmail.com","sentAt":"2020-08-04T22:01:08Z","receivedAt":"2020-08-04T22:01:24Z","isPatch":true,"sender":{"key":"alipman88@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3922511?v=4"},"body":"OK, here's take 4! Responding to Junio's feedback, first:\n\n> That function [cmd_bisect__helper()] is supposed to be a thin shim\n> layer whose only reason of its existence is to serve as an interface\n> with the scripted version of \"git bisect\". When everything is\n> migrated from the shell script, cmd_bisect__helper() should disappear.\n> ...\n> This is going backwards, as far as I can tell. If anything, I'd\n> rather see cmd_bisect__helper() get fixed so that it does not parse\n> \"--first-parent\" (and similarly, \"--no-checkout\" that you imitated)\n> into first_parent_only (and no_checkout) variables and passed as\n> parameters to bisect_start().\n\nThanks for the explanation, Junio. (And for bearing with me while I\ngain familiarity with git's codebase.) Now that I've taken some time\nto examine how git-bisect.sh and cmd_bisect__helper() fit together,\nthe correct approach is much more clear.\n\nI've added a commit (3/5 in this iteration) that updates\ncmd_bisect__helper() to defer parsing the --no-checkout flag.\n\nAs no_checkout was also passed to bisect_next_all() [via implicitly\nchecking for the existence of .git/BISECT_HEAD when calling\nbisect_next() in git-bisect.sh], I've removed that parameter and\ninstead check for .git/BISECT_HEAD in bisect_next_all() to determine\nwhether no-checkout mode is on.\n\nThis means no-checkout mode can no longer be enabled by \"git\nbisect--helper --next-all --no-checkout\" - it can now only be turned\non when running \"git bisect start\". But this doesn't change git\nbisect's public API, so this should be OK.\n\nOther than the changes suggested by Martin (summarized below), the\nonly new change is removing the read_first_parent_option() wrapper\nfunction in favor of file_exists() from dir.h.\n\n---\n\nMartin, thanks for your suggestions - I've moved the commit updating\n\"git bisect run\" tests to 1/5, updated the commit message, and\nincluded the changes you provided. (I held off on additional\nindentation corrections, as it kinda felt like unnecessary code\nchurn/outside the scope of abstracting platform-specific details.)\n\n> Also, does \"separate between \" make grammatical sense?\n> Should it be \"separate between \" or \"separate [it] from\n> \"? Dunno..\n\nI think \"separate [it] from \" is correct, good catch!\n\n> (Signed-off-by: Martin Ågren , FWIW.)\n\nI'm still getting used to the conventions - should I add your name as\na signed-off-by tag, a thanks-to tag, or both?\n\nAaron Lipman (5):\n t6030: modernize \"git bisect run\" tests\n rev-list: allow bisect and first-parent flags\n cmd_bisect__helper: defer parsing no-checkout flag\n bisect: introduce first-parent flag\n bisect: combine args passed to find_bisection()\n\n Documentation/git-bisect.txt | 13 +++-\n Documentation/rev-list-options.txt | 7 +-\n bisect.c | 79 +++++++++++++---------\n bisect.h | 9 +--\n builtin/bisect--helper.c | 23 ++++---\n builtin/rev-list.c | 9 ++-\n git-bisect.sh | 2 +-\n revision.c | 3 -\n t/t6000-rev-list-misc.sh | 4 +-\n t/t6002-rev-list-bisect.sh | 45 +++++++++++++\n t/t6030-bisect-porcelain.sh | 104 ++++++++++++++++-------------\n 11 files changed, 195 insertions(+), 103 deletions(-)\n\n-- \n2.24.3 (Apple Git-128)\n\n"},{"id":"402855","messageId":"20200804220113.5909-2-alipman88@gmail.com","threadId":"53937","inReplyTo":"20200804220113.5909-1-alipman88@gmail.com","subject":"[PATCH v4 1/5] t6030: modernize \"git bisect run\" tests","fromName":"Aaron Lipman","fromEmail":"alipman88@gmail.com","sentAt":"2020-08-04T22:01:09Z","receivedAt":"2020-08-04T22:01:28Z","isPatch":true,"sender":{"key":"alipman88@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3922511?v=4"},"body":"Enforce consistent styling for tests on \"git bisect run\":\n- Use \"write_script\" to abstract away platform-specific details.\n- Favor current whitespace conventions.\n- While at it, change \"introduced\" to \"added\" in the comments to make\n them read better.\n\nThanks-to: Martin Ågren \nSigned-off-by: Aaron Lipman \n---\n t/t6030-bisect-porcelain.sh | 86 ++++++++++++++++---------------------\n 1 file changed, 38 insertions(+), 48 deletions(-)\n\ndiff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh\nindex 36d9b2b2e4..a66c4b89bc 100755\n--- a/t/t6030-bisect-porcelain.sh\n+++ b/t/t6030-bisect-porcelain.sh\n@@ -243,32 +243,30 @@ test_expect_success 'bisect skip: with commit both bad and skipped' '\n '\n \n # We want to automatically find the commit that\n-# introduced \"Another\" into hello.\n-test_expect_success \\\n- '\"git bisect run\" simple case' \\\n- 'echo \"#\"\\!\"/bin/sh\" > test_script.sh &&\n- echo \"grep Another hello > /dev/null\" >> test_script.sh &&\n- echo \"test \\$? -ne 0\" >> test_script.sh &&\n- chmod +x test_script.sh &&\n- git bisect start &&\n- git bisect good $HASH1 &&\n- git bisect bad $HASH4 &&\n- git bisect run ./test_script.sh > my_bisect_log.txt &&\n- grep \"$HASH3 is the first bad commit\" my_bisect_log.txt &&\n- git bisect reset'\n+# added \"Another\" into hello.\n+test_expect_success '\"git bisect run\" simple case' '\n+\twrite_script test_script.sh <<-\\EOF &&\n+\t! grep Another hello >/dev/null\n+\tEOF\n+\tgit bisect start &&\n+\tgit bisect good $HASH1 &&\n+\tgit bisect bad $HASH4 &&\n+\tgit bisect run ./test_script.sh >my_bisect_log.txt &&\n+\tgrep \"$HASH3 is the first bad commit\" my_bisect_log.txt &&\n+\tgit bisect reset\n+'\n \n # We want to automatically find the commit that\n-# introduced \"Ciao\" into hello.\n-test_expect_success \\\n- '\"git bisect run\" with more complex \"git bisect start\"' \\\n- 'echo \"#\"\\!\"/bin/sh\" > test_script.sh &&\n- echo \"grep Ciao hello > /dev/null\" >> test_script.sh &&\n- echo \"test \\$? -ne 0\" >> test_script.sh &&\n- chmod +x test_script.sh &&\n- git bisect start $HASH4 $HASH1 &&\n- git bisect run ./test_script.sh > my_bisect_log.txt &&\n- grep \"$HASH4 is the first bad commit\" my_bisect_log.txt &&\n- git bisect reset'\n+# added \"Ciao\" into hello.\n+test_expect_success '\"git bisect run\" with more complex \"git bisect start\"' '\n+\twrite_script test_script.sh <<-\\EOF &&\n+\t! grep Ciao hello >/dev/null\n+\tEOF\n+\tgit bisect start $HASH4 $HASH1 &&\n+\tgit bisect run ./test_script.sh >my_bisect_log.txt &&\n+\tgrep \"$HASH4 is the first bad commit\" my_bisect_log.txt &&\n+\tgit bisect reset\n+'\n \n # $HASH1 is good, $HASH5 is bad, we skip $HASH3\n # but $HASH4 is good,\n@@ -295,24 +293,17 @@ HASH6=\n test_expect_success 'bisect run & skip: cannot tell between 2' '\n \tadd_line_into_file \"6: Yet a line.\" hello &&\n \tHASH6=$(git rev-parse --verify HEAD) &&\n-\techo \"#\"\\!\"/bin/sh\" > test_script.sh &&\n-\techo \"sed -ne \\\\\\$p hello | grep Ciao > /dev/null && exit 125\" >> test_script.sh &&\n-\techo \"grep line hello > /dev/null\" >> test_script.sh &&\n-\techo \"test \\$? -ne 0\" >> test_script.sh &&\n-\tchmod +x test_script.sh &&\n+\twrite_script test_script.sh <<-\\EOF &&\n+\tsed -ne \\$p hello | grep Ciao >/dev/null && exit 125\n+\t! grep line hello >/dev/null\n+\tEOF\n \tgit bisect start $HASH6 $HASH1 &&\n-\tif git bisect run ./test_script.sh > my_bisect_log.txt\n-\tthen\n-\t\techo Oops, should have failed.\n-\t\tfalse\n-\telse\n-\t\ttest $? -eq 2 &&\n-\t\tgrep \"first bad commit could be any of\" my_bisect_log.txt &&\n-\t\t! grep $HASH3 my_bisect_log.txt &&\n-\t\t! grep $HASH6 my_bisect_log.txt &&\n-\t\tgrep $HASH4 my_bisect_log.txt &&\n-\t\tgrep $HASH5 my_bisect_log.txt\n-\tfi\n+\ttest_expect_code 2 git bisect run ./test_script.sh >my_bisect_log.txt &&\n+\tgrep \"first bad commit could be any of\" my_bisect_log.txt &&\n+\t! grep $HASH3 my_bisect_log.txt &&\n+\t! grep $HASH6 my_bisect_log.txt &&\n+\tgrep $HASH4 my_bisect_log.txt &&\n+\tgrep $HASH5 my_bisect_log.txt\n '\n \n HASH7=\n@@ -320,14 +311,13 @@ test_expect_success 'bisect run & skip: find first bad' '\n \tgit bisect reset &&\n \tadd_line_into_file \"7: Should be the last line.\" hello &&\n \tHASH7=$(git rev-parse --verify HEAD) &&\n-\techo \"#\"\\!\"/bin/sh\" > test_script.sh &&\n-\techo \"sed -ne \\\\\\$p hello | grep Ciao > /dev/null && exit 125\" >> test_script.sh &&\n-\techo \"sed -ne \\\\\\$p hello | grep day > /dev/null && exit 125\" >> test_script.sh &&\n-\techo \"grep Yet hello > /dev/null\" >> test_script.sh &&\n-\techo \"test \\$? -ne 0\" >> test_script.sh &&\n-\tchmod +x test_script.sh &&\n+\twrite_script test_script.sh <<-\\EOF &&\n+\tsed -ne \\$p hello | grep Ciao >/dev/null && exit 125\n+\tsed -ne \\$p hello | grep day >/dev/null && exit 125\n+\t! grep Yet hello >/dev/null\n+\tEOF\n \tgit bisect start $HASH7 $HASH1 &&\n-\tgit bisect run ./test_script.sh > my_bisect_log.txt &&\n+\tgit bisect run ./test_script.sh >my_bisect_log.txt &&\n \tgrep \"$HASH6 is the first bad commit\" my_bisect_log.txt\n '\n \n-- \n2.24.3 (Apple Git-128)\n\n"},{"id":"402857","messageId":"20200804220113.5909-3-alipman88@gmail.com","threadId":"53937","inReplyTo":"20200804220113.5909-1-alipman88@gmail.com","subject":"[PATCH v4 2/5] rev-list: allow bisect and first-parent flags","fromName":"Aaron Lipman","fromEmail":"alipman88@gmail.com","sentAt":"2020-08-04T22:01:10Z","receivedAt":"2020-08-04T22:01:36Z","isPatch":true,"sender":{"key":"alipman88@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3922511?v=4"},"body":"Add first_parent_only parameter to find_bisection(), removing the\nbarrier that prevented combining the --bisect and --first-parent flags\nwhen using git rev-list\n\nBased-on-patch-by: Tiago Botelho \nSigned-off-by: Aaron Lipman \n---\n Documentation/rev-list-options.txt | 7 ++---\n bisect.c | 28 ++++++++++++-------\n bisect.h | 2 +-\n builtin/rev-list.c | 2 +-\n revision.c | 3 --\n t/t6000-rev-list-misc.sh | 4 +--\n t/t6002-rev-list-bisect.sh | 45 ++++++++++++++++++++++++++++++\n 7 files changed, 70 insertions(+), 21 deletions(-)\n\ndiff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\nindex b01b2b6773..d3117ce51b 100644\n--- a/Documentation/rev-list-options.txt\n+++ b/Documentation/rev-list-options.txt\n@@ -128,8 +128,7 @@ parents) and `--max-parents=-1` (negative numbers denote no upper limit).\n \tbecause merges into a topic branch tend to be only about\n \tadjusting to updated upstream from time to time, and\n \tthis option allows you to ignore the individual commits\n-\tbrought in to your history by such a merge. Cannot be\n-\tcombined with --bisect.\n+\tbrought in to your history by such a merge.\n \n --not::\n \tReverses the meaning of the '{caret}' prefix (or lack thereof)\n@@ -207,7 +206,7 @@ ifndef::git-rev-list[]\n \tPretend as if the bad bisection ref `refs/bisect/bad`\n \twas listed and as if it was followed by `--not` and the good\n \tbisection refs `refs/bisect/good-*` on the command\n-\tline. Cannot be combined with --first-parent.\n+\tline.\n endif::git-rev-list[]\n \n --stdin::\n@@ -743,7 +742,7 @@ outputs 'midpoint', the output of the two commands\n would be of roughly the same length. Finding the change which\n introduces a regression is thus reduced to a binary search: repeatedly\n generate and test new 'midpoint's until the commit chain is of length\n-one. Cannot be combined with --first-parent.\n+one.\n \n --bisect-vars::\n \tThis calculates the same as `--bisect`, except that refs in\ndiff --git a/bisect.c b/bisect.c\nindex d5e830410f..a11fdb1473 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -88,15 +88,16 @@ static inline void weight_set(struct commit_list *elem, int weight)\n \t**commit_weight_at(&commit_weight, elem->item) = weight;\n }\n \n-static int count_interesting_parents(struct commit *commit)\n+static int count_interesting_parents(struct commit *commit, int first_parent_only)\n {\n \tstruct commit_list *p;\n \tint count;\n \n \tfor (count = 0, p = commit->parents; p; p = p->next) {\n-\t\tif (p->item->object.flags & UNINTERESTING)\n-\t\t\tcontinue;\n-\t\tcount++;\n+\t\tif (!(p->item->object.flags & UNINTERESTING))\n+\t\t\tcount++;\n+\t\tif (first_parent_only)\n+\t\t\tbreak;\n \t}\n \treturn count;\n }\n@@ -259,7 +260,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 first_parent_only)\n {\n \tint n, counted;\n \tstruct commit_list *p;\n@@ -271,7 +272,7 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\n \t\tunsigned flags = commit->object.flags;\n \n \t\t*commit_weight_at(&commit_weight, p->item) = &weights[n++];\n-\t\tswitch (count_interesting_parents(commit)) {\n+\t\tswitch (count_interesting_parents(commit, first_parent_only)) {\n \t\tcase 0:\n \t\t\tif (!(flags & TREESAME)) {\n \t\t\t\tweight_set(p, 1);\n@@ -314,6 +315,8 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\n \t\t\tcontinue;\n \t\tif (weight(p) != -2)\n \t\t\tcontinue;\n+\t\tif (first_parent_only)\n+\t\t\tBUG(\"shouldn't be calling count-distance in fp mode\");\n \t\tweight_set(p, count_distance(p));\n \t\tclear_distance(list);\n \n@@ -332,7 +335,10 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\n \n \t\t\tif (0 <= weight(p))\n \t\t\t\tcontinue;\n-\t\t\tfor (q = p->item->parents; q; q = q->next) {\n+\n+\t\t\tfor (q = p->item->parents;\n+\t\t\t q;\n+\t\t\t q = first_parent_only ? NULL : q->next) {\n \t\t\t\tif (q->item->object.flags & UNINTERESTING)\n \t\t\t\t\tcontinue;\n \t\t\t\tif (0 <= weight(q))\n@@ -370,7 +376,7 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\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 first_parent_only)\n {\n \tint nr, on_list;\n \tstruct commit_list *list, *p, *best, *next, *last;\n@@ -406,7 +412,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, first_parent_only);\n \tif (best) {\n \t\tif (!find_all) {\n \t\t\tlist->item = best->item;\n@@ -991,6 +997,7 @@ enum bisect_error bisect_next_all(struct repository *r, const char *prefix, int\n \tenum bisect_error res = BISECT_OK;\n \tstruct object_id *bisect_rev;\n \tchar *steps_msg;\n+\tint first_parent_only = 0; /* TODO: pass --first-parent flag from git bisect start */\n \n \tread_bisect_terms(&term_bad, &term_good);\n \tif (read_bisect_refs())\n@@ -1001,11 +1008,12 @@ enum bisect_error bisect_next_all(struct repository *r, const char *prefix, int\n \t\treturn res;\n \n \tbisect_rev_setup(r, &revs, prefix, \"%s\", \"^%s\", 1);\n+\trevs.first_parent_only = first_parent_only;\n \trevs.limited = 1;\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, first_parent_only);\n \trevs.commits = managed_skipped(revs.commits, &tried);\n \n \tif (!revs.commits) {\ndiff --git a/bisect.h b/bisect.h\nindex 8bad8d8391..a63af0505f 100644\n--- a/bisect.h\n+++ b/bisect.h\n@@ -12,7 +12,7 @@ struct repository;\n * best commit, as chosen by `find_all`.\n */\n void find_bisection(struct commit_list **list, int *reaches, int *all,\n-\t\t int find_all);\n+\t\t int find_all, int first_parent_only);\n \n struct commit_list *filter_skipped(struct commit_list *list,\n \t\t\t\t struct commit_list **tried,\ndiff --git a/builtin/rev-list.c b/builtin/rev-list.c\nindex f520111eda..d1a14596b2 100644\n--- a/builtin/rev-list.c\n+++ b/builtin/rev-list.c\n@@ -638,7 +638,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, revs.first_parent_only);\n \n \t\tif (bisect_show_vars)\n \t\t\treturn show_bisect_vars(&info, reaches, all);\ndiff --git a/revision.c b/revision.c\nindex 6de29cdf7a..e628d5ea56 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2891,9 +2891,6 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n \tif (!revs->reflog_info && revs->grep_filter.use_reflog_filter)\n \t\tdie(\"cannot use --grep-reflog without --walk-reflogs\");\n \n-\tif (revs->first_parent_only && revs->bisect)\n-\t\tdie(_(\"--first-parent is incompatible with --bisect\"));\n-\n \tif (revs->line_level_traverse &&\n \t (revs->diffopt.output_format & ~(DIFF_FORMAT_PATCH | DIFF_FORMAT_NO_OUTPUT)))\n \t\tdie(_(\"-L does not yet support diff formats besides -p and -s\"));\ndiff --git a/t/t6000-rev-list-misc.sh b/t/t6000-rev-list-misc.sh\nindex 3bb0e4ff8f..fc4d55dcb2 100755\n--- a/t/t6000-rev-list-misc.sh\n+++ b/t/t6000-rev-list-misc.sh\n@@ -128,8 +128,8 @@ test_expect_success 'rev-list can negate index objects' '\n \ttest_cmp expect actual\n '\n \n-test_expect_success '--bisect and --first-parent can not be combined' '\n-\ttest_must_fail git rev-list --bisect --first-parent HEAD\n+test_expect_success '--bisect and --first-parent can be combined' '\n+\tgit rev-list --bisect --first-parent HEAD\n '\n \n test_expect_success '--header shows a NUL after each commit' '\ndiff --git a/t/t6002-rev-list-bisect.sh b/t/t6002-rev-list-bisect.sh\nindex a661408038..7fc0f75ca5 100755\n--- a/t/t6002-rev-list-bisect.sh\n+++ b/t/t6002-rev-list-bisect.sh\n@@ -263,4 +263,49 @@ test_expect_success 'rev-parse --bisect can default to good/bad refs' '\n \ttest_cmp expect.sorted actual.sorted\n '\n \n+test_output_expect_success \"--bisect --first-parent\" 'git rev-list --bisect --first-parent E ^F' < expect.unsorted < expect &&\n+git rev-list --bisect-all --first-parent E ^F > actual &&\n+test_cmp actual expect\n+'\n+\n test_done\n-- \n2.24.3 (Apple Git-128)\n\n"},{"id":"402856","messageId":"20200804220113.5909-4-alipman88@gmail.com","threadId":"53937","inReplyTo":"20200804220113.5909-1-alipman88@gmail.com","subject":"[PATCH v4 3/5] cmd_bisect__helper: defer parsing no-checkout flag","fromName":"Aaron Lipman","fromEmail":"alipman88@gmail.com","sentAt":"2020-08-04T22:01:11Z","receivedAt":"2020-08-04T22:01:37Z","isPatch":true,"sender":{"key":"alipman88@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3922511?v=4"},"body":"cmd_bisect__helper() is intended as a temporary shim layer serving as an\ninterface for git-bisect.sh. This function and git-bisect.sh should\neventually be replaced by a C implementation, cmd_bisect(), serving as\nan entrypoint for all \"git bisect ...\" shell commands: cmd_bisect() will\nonly parse the first token following \"git bisect\", and dispatch the\nremaining args to the appropriate function [\"bisect_start()\",\n\"bisect_next()\", etc.].\n\nThus, cmd_bisect__helper() should not be responsible for parsing flags\nlike --no-checkout. Instead, let the --no-checkout flag remain in the\nargv array, so it may be evaluated alongside the other options already\nparsed by bisect_start().\n\nSigned-off-by: Aaron Lipman \n---\n bisect.c | 3 ++-\n bisect.h | 4 +---\n builtin/bisect--helper.c | 14 ++++++--------\n git-bisect.sh | 2 +-\n 4 files changed, 10 insertions(+), 13 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex a11fdb1473..950ff6f533 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -989,7 +989,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-enum bisect_error bisect_next_all(struct repository *r, const char *prefix, int no_checkout)\n+enum bisect_error bisect_next_all(struct repository *r, const char *prefix)\n {\n \tstruct rev_info revs;\n \tstruct commit_list *tried;\n@@ -997,6 +997,7 @@ enum bisect_error bisect_next_all(struct repository *r, const char *prefix, int\n \tenum bisect_error res = BISECT_OK;\n \tstruct object_id *bisect_rev;\n \tchar *steps_msg;\n+\tint no_checkout = ref_exists(\"BISECT_HEAD\");\n \tint first_parent_only = 0; /* TODO: pass --first-parent flag from git bisect start */\n \n \tread_bisect_terms(&term_bad, &term_good);\ndiff --git a/bisect.h b/bisect.h\nindex a63af0505f..7f30b94db3 100644\n--- a/bisect.h\n+++ b/bisect.h\n@@ -58,9 +58,7 @@ enum bisect_error {\n \tBISECT_INTERNAL_SUCCESS_MERGE_BASE = -11\n };\n \n-enum bisect_error bisect_next_all(struct repository *r,\n-\t\t const char *prefix,\n-\t\t int no_checkout);\n+enum bisect_error bisect_next_all(struct repository *r, const char *prefix);\n \n int estimate_bisect_steps(int all);\n \ndiff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\nindex 73f9324ad7..95a15b918f 100644\n--- a/builtin/bisect--helper.c\n+++ b/builtin/bisect--helper.c\n@@ -18,7 +18,7 @@ static GIT_PATH_FUNC(git_path_head_name, \"head-name\")\n static GIT_PATH_FUNC(git_path_bisect_names, \"BISECT_NAMES\")\n \n static const char * const git_bisect_helper_usage[] = {\n-\tN_(\"git bisect--helper --next-all [--no-checkout]\"),\n+\tN_(\"git bisect--helper --next-all\"),\n \tN_(\"git bisect--helper --write-terms \"),\n \tN_(\"git bisect--helper --bisect-clean-state\"),\n \tN_(\"git bisect--helper --bisect-reset []\"),\n@@ -420,9 +420,9 @@ static int bisect_append_log_quoted(const char **argv)\n \treturn res;\n }\n \n-static int bisect_start(struct bisect_terms *terms, int no_checkout,\n-\t\t\tconst char **argv, int argc)\n+static int bisect_start(struct bisect_terms *terms, const char **argv, int argc)\n {\n+\tint no_checkout = 0;\n \tint i, has_double_dash = 0, must_write_terms = 0, bad_seen = 0;\n \tint flags, pathspec_pos, res = 0;\n \tstruct string_list revs = STRING_LIST_INIT_DUP;\n@@ -631,7 +631,7 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\tBISECT_TERMS,\n \t\tBISECT_START\n \t} cmdmode = 0;\n-\tint no_checkout = 0, res = 0, nolog = 0;\n+\tint res = 0, nolog = 0;\n \tstruct option options[] = {\n \t\tOPT_CMDMODE(0, \"next-all\", &cmdmode,\n \t\t\t N_(\"perform 'git bisect next'\"), NEXT_ALL),\n@@ -653,8 +653,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"print out the bisect terms\"), BISECT_TERMS),\n \t\tOPT_CMDMODE(0, \"bisect-start\", &cmdmode,\n \t\t\t N_(\"start the bisect session\"), BISECT_START),\n-\t\tOPT_BOOL(0, \"no-checkout\", &no_checkout,\n-\t\t\t N_(\"update BISECT_HEAD instead of checking out the current commit\")),\n \t\tOPT_BOOL(0, \"no-log\", &nolog,\n \t\t\t N_(\"no log for BISECT_WRITE\")),\n \t\tOPT_END()\n@@ -670,7 +668,7 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \n \tswitch (cmdmode) {\n \tcase NEXT_ALL:\n-\t\tres = bisect_next_all(the_repository, prefix, no_checkout);\n+\t\tres = bisect_next_all(the_repository, prefix);\n \t\tbreak;\n \tcase WRITE_TERMS:\n \t\tif (argc != 2)\n@@ -712,7 +710,7 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\tbreak;\n \tcase BISECT_START:\n \t\tset_terms(&terms, \"bad\", \"good\");\n-\t\tres = bisect_start(&terms, no_checkout, argv, argc);\n+\t\tres = bisect_start(&terms, argv, argc);\n \t\tbreak;\n \tdefault:\n \t\treturn error(\"BUG: unknown subcommand '%d'\", cmdmode);\ndiff --git a/git-bisect.sh b/git-bisect.sh\nindex f03fbb18f0..c7580e51a0 100755\n--- a/git-bisect.sh\n+++ b/git-bisect.sh\n@@ -153,7 +153,7 @@ bisect_next() {\n \tgit bisect--helper --bisect-next-check $TERM_GOOD $TERM_BAD $TERM_GOOD|| exit\n \n \t# Perform all bisection computation, display and checkout\n-\tgit bisect--helper --next-all $(git rev-parse --verify -q BISECT_HEAD > /dev/null && echo --no-checkout)\n+\tgit bisect--helper --next-all\n \tres=$?\n \n \t# Check if we should exit because bisection is finished\n-- \n2.24.3 (Apple Git-128)\n\n"},{"id":"402858","messageId":"20200804220113.5909-5-alipman88@gmail.com","threadId":"53937","inReplyTo":"20200804220113.5909-1-alipman88@gmail.com","subject":"[PATCH v4 4/5] bisect: introduce first-parent flag","fromName":"Aaron Lipman","fromEmail":"alipman88@gmail.com","sentAt":"2020-08-04T22:01:12Z","receivedAt":"2020-08-04T22:01:39Z","isPatch":true,"sender":{"key":"alipman88@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3922511?v=4"},"body":"Upon seeing a merge commit when bisecting, this option may be used to\nfollow only the first parent.\n\nIn detecting regressions introduced through the merging of a branch, the\nmerge commit will be identified as introduction of the bug and its\nancestors will be ignored.\n\nThis option is particularly useful in avoiding false positives when a\nmerged branch contained broken or non-buildable commits, but the merge\nitself was OK.\n\nSigned-off-by: Aaron Lipman \n---\n Documentation/git-bisect.txt | 13 ++++++++++++-\n bisect.c | 5 ++++-\n builtin/bisect--helper.c | 9 ++++++++-\n t/t6030-bisect-porcelain.sh | 18 ++++++++++++++++++\n 4 files changed, 42 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt\nindex 7586c5a843..0e993e4587 100644\n--- a/Documentation/git-bisect.txt\n+++ b/Documentation/git-bisect.txt\n@@ -17,7 +17,7 @@ The command takes various subcommands, and different options depending\n on the subcommand:\n \n git bisect start [--term-{old,good}= --term-{new,bad}=]\n-\t\t [--no-checkout] [ [...]] [--] [...]\n+\t\t [--no-checkout] [--first-parent] [ [...]] [--] [...]\n git bisect (bad|new|) []\n git bisect (good|old|) [...]\n git bisect terms [--term-good | --term-bad]\n@@ -365,6 +365,17 @@ does not require a checked out tree.\n +\n If the repository is bare, `--no-checkout` is assumed.\n \n+--first-parent::\n++\n+Follow only the first parent commit upon seeing a merge commit.\n++\n+In detecting regressions introduced through the merging of a branch, the merge\n+commit will be identified as introduction of the bug and its ancestors will be\n+ignored.\n++\n+This option is particularly useful in avoiding false positives when a merged\n+branch contained broken or non-buildable commits, but the merge itself was OK.\n+\n EXAMPLES\n --------\n \ndiff --git a/bisect.c b/bisect.c\nindex 950ff6f533..bc4241b51f 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -15,6 +15,7 @@\n #include \"commit-slab.h\"\n #include \"commit-reach.h\"\n #include \"object-store.h\"\n+#include \"dir.h\"\n \n static struct oid_array good_revs;\n static struct oid_array skipped_revs;\n@@ -460,6 +461,7 @@ static GIT_PATH_FUNC(git_path_bisect_run, \"BISECT_RUN\")\n static GIT_PATH_FUNC(git_path_bisect_start, \"BISECT_START\")\n static GIT_PATH_FUNC(git_path_bisect_log, \"BISECT_LOG\")\n static GIT_PATH_FUNC(git_path_bisect_terms, \"BISECT_TERMS\")\n+static GIT_PATH_FUNC(git_path_bisect_first_parent, \"BISECT_FIRST_PARENT\")\n static GIT_PATH_FUNC(git_path_head_name, \"head-name\")\n \n static void read_bisect_paths(struct argv_array *array)\n@@ -998,7 +1000,7 @@ enum bisect_error bisect_next_all(struct repository *r, const char *prefix)\n \tstruct object_id *bisect_rev;\n \tchar *steps_msg;\n \tint no_checkout = ref_exists(\"BISECT_HEAD\");\n-\tint first_parent_only = 0; /* TODO: pass --first-parent flag from git bisect start */\n+\tint first_parent_only = file_exists(git_path_bisect_first_parent());\n \n \tread_bisect_terms(&term_bad, &term_good);\n \tif (read_bisect_refs())\n@@ -1142,6 +1144,7 @@ int bisect_clean_state(void)\n \tunlink_or_warn(git_path_bisect_names());\n \tunlink_or_warn(git_path_bisect_run());\n \tunlink_or_warn(git_path_bisect_terms());\n+\tunlink_or_warn(git_path_bisect_first_parent());\n \t/* Cleanup head-name if it got left by an old version of git-bisect */\n \tunlink_or_warn(git_path_head_name());\n \t/*\ndiff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\nindex 95a15b918f..a055147f19 100644\n--- a/builtin/bisect--helper.c\n+++ b/builtin/bisect--helper.c\n@@ -16,6 +16,7 @@ static GIT_PATH_FUNC(git_path_bisect_start, \"BISECT_START\")\n static GIT_PATH_FUNC(git_path_bisect_log, \"BISECT_LOG\")\n static GIT_PATH_FUNC(git_path_head_name, \"head-name\")\n static GIT_PATH_FUNC(git_path_bisect_names, \"BISECT_NAMES\")\n+static GIT_PATH_FUNC(git_path_bisect_first_parent, \"BISECT_FIRST_PARENT\")\n \n static const char * const git_bisect_helper_usage[] = {\n \tN_(\"git bisect--helper --next-all\"),\n@@ -27,7 +28,7 @@ static const char * const git_bisect_helper_usage[] = {\n \tN_(\"git bisect--helper --bisect-next-check []\"),\n \tN_(\"git bisect--helper --bisect-terms [--term-good | --term-old | --term-bad | --term-new]\"),\n \tN_(\"git bisect--helper --bisect-start [--term-{old,good}= --term-{new,bad}=]\"\n-\t\t\t\t\t \"[--no-checkout] [ [...]] [--] [...]\"),\n+\t\t\t\t\t \" [--no-checkout] [--first-parent] [ [...]] [--] [...]\"),\n \tNULL\n };\n \n@@ -423,6 +424,7 @@ static int bisect_append_log_quoted(const char **argv)\n static int bisect_start(struct bisect_terms *terms, const char **argv, int argc)\n {\n \tint no_checkout = 0;\n+\tint first_parent_only = 0;\n \tint i, has_double_dash = 0, must_write_terms = 0, bad_seen = 0;\n \tint flags, pathspec_pos, res = 0;\n \tstruct string_list revs = STRING_LIST_INIT_DUP;\n@@ -452,6 +454,8 @@ static int bisect_start(struct bisect_terms *terms, const char **argv, int argc)\n \t\t\tbreak;\n \t\t} else if (!strcmp(arg, \"--no-checkout\")) {\n \t\t\tno_checkout = 1;\n+\t\t} else if (!strcmp(arg, \"--first-parent\")) {\n+\t\t\tfirst_parent_only = 1;\n \t\t} else if (!strcmp(arg, \"--term-good\") ||\n \t\t\t !strcmp(arg, \"--term-old\")) {\n \t\t\ti++;\n@@ -576,6 +580,9 @@ static int bisect_start(struct bisect_terms *terms, const char **argv, int argc)\n \t */\n \twrite_file(git_path_bisect_start(), \"%s\\n\", start_head.buf);\n \n+\tif (first_parent_only)\n+\t\twrite_file(git_path_bisect_first_parent(), \"\\n\");\n+\n \tif (no_checkout) {\n \t\tif (get_oid(start_head.buf, &oid) < 0) {\n \t\t\tres = error(_(\"invalid ref: '%s'\"), start_head.buf);\ndiff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh\nindex a66c4b89bc..b886529e59 100755\n--- a/t/t6030-bisect-porcelain.sh\n+++ b/t/t6030-bisect-porcelain.sh\n@@ -448,6 +448,24 @@ test_expect_success 'many merge bases creation' '\n \tgrep \"$SIDE_HASH5\" merge_bases.txt\n '\n \n+# We want to automatically find the merge that\n+# added \"line\" into hello.\n+test_expect_success '\"git bisect run --first-parent\" simple case' '\n+\tgit rev-list --first-parent $B_HASH ^$HASH4 >first_parent_chain.txt &&\n+\twrite_script test_script.sh <<-\\EOF &&\n+\tgrep $(git rev-parse HEAD) first_parent_chain.txt || exit -1\n+\t! grep line hello >/dev/null\n+\tEOF\n+\tgit bisect start --first-parent &&\n+\ttest_path_is_file \".git/BISECT_FIRST_PARENT\" &&\n+\tgit bisect good $HASH4 &&\n+\tgit bisect bad $B_HASH &&\n+\tgit bisect run ./test_script.sh >my_bisect_log.txt &&\n+\tgrep \"$B_HASH is the first bad commit\" my_bisect_log.txt &&\n+\tgit bisect reset &&\n+\ttest_path_is_missing .git/BISECT_FIRST_PARENT\n+'\n+\n test_expect_success 'good merge bases when good and bad are siblings' '\n \tgit bisect start \"$B_HASH\" \"$A_HASH\" > my_bisect_log.txt &&\n \ttest_i18ngrep \"merge base must be tested\" my_bisect_log.txt &&\n-- \n2.24.3 (Apple Git-128)\n\n"},{"id":"402859","messageId":"20200804220113.5909-6-alipman88@gmail.com","threadId":"53937","inReplyTo":"20200804220113.5909-1-alipman88@gmail.com","subject":"[PATCH v4 5/5] bisect: combine args passed to find_bisection()","fromName":"Aaron Lipman","fromEmail":"alipman88@gmail.com","sentAt":"2020-08-04T22:01:13Z","receivedAt":"2020-08-04T22:01:40Z","isPatch":true,"sender":{"key":"alipman88@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3922511?v=4"},"body":"Now that find_bisection() accepts multiple boolean arguments, these may\nbe combined into a single unsigned integer in order to declutter some of\nthe code in bisect.c\n\nAlso, rename the existing \"flags\" bitfield to \"commit_flags\", to\nexplicitly differentiate it from the new \"bisect_flags\" bitfield.\n\nBased-on-patch-by: Harald Nordgren \nSigned-off-by: Aaron Lipman \n---\n bisect.c | 67 +++++++++++++++++++++++++---------------------\n bisect.h | 5 +++-\n builtin/rev-list.c | 9 ++++++-\n 3 files changed, 49 insertions(+), 32 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex bc4241b51f..1585fcc6ad 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -89,7 +89,7 @@ static inline void weight_set(struct commit_list *elem, int weight)\n \t**commit_weight_at(&commit_weight, elem->item) = weight;\n }\n \n-static int count_interesting_parents(struct commit *commit, int first_parent_only)\n+static int count_interesting_parents(struct commit *commit, unsigned bisect_flags)\n {\n \tstruct commit_list *p;\n \tint count;\n@@ -97,7 +97,7 @@ static int count_interesting_parents(struct commit *commit, int first_parent_onl\n \tfor (count = 0, p = commit->parents; p; p = p->next) {\n \t\tif (!(p->item->object.flags & UNINTERESTING))\n \t\t\tcount++;\n-\t\tif (first_parent_only)\n+\t\tif (bisect_flags & FIND_BISECTION_FIRST_PARENT_ONLY)\n \t\t\tbreak;\n \t}\n \treturn count;\n@@ -137,7 +137,7 @@ static void show_list(const char *debug, int counted, int nr,\n \tfor (p = list; p; p = p->next) {\n \t\tstruct commit_list *pp;\n \t\tstruct commit *commit = p->item;\n-\t\tunsigned flags = commit->object.flags;\n+\t\tunsigned commit_flags = commit->object.flags;\n \t\tenum object_type type;\n \t\tunsigned long size;\n \t\tchar *buf = read_object_file(&commit->object.oid, &type,\n@@ -146,9 +146,9 @@ static void show_list(const char *debug, int counted, int nr,\n \t\tint subject_len;\n \n \t\tfprintf(stderr, \"%c%c%c \",\n-\t\t\t(flags & TREESAME) ? ' ' : 'T',\n-\t\t\t(flags & UNINTERESTING) ? 'U' : ' ',\n-\t\t\t(flags & COUNTED) ? 'C' : ' ');\n+\t\t\t(commit_flags & TREESAME) ? ' ' : 'T',\n+\t\t\t(commit_flags & UNINTERESTING) ? 'U' : ' ',\n+\t\t\t(commit_flags & COUNTED) ? 'C' : ' ');\n \t\tif (*commit_weight_at(&commit_weight, p->item))\n \t\t\tfprintf(stderr, \"%3d\", weight(p));\n \t\telse\n@@ -173,9 +173,9 @@ static struct commit_list *best_bisection(struct commit_list *list, int nr)\n \tbest = list;\n \tfor (p = list; p; p = p->next) {\n \t\tint distance;\n-\t\tunsigned flags = p->item->object.flags;\n+\t\tunsigned commit_flags = p->item->object.flags;\n \n-\t\tif (flags & TREESAME)\n+\t\tif (commit_flags & TREESAME)\n \t\t\tcontinue;\n \t\tdistance = weight(p);\n \t\tif (nr - distance < distance)\n@@ -214,9 +214,9 @@ static struct commit_list *best_bisection_sorted(struct commit_list *list, int n\n \n \tfor (p = list, cnt = 0; p; p = p->next) {\n \t\tint distance;\n-\t\tunsigned flags = p->item->object.flags;\n+\t\tunsigned commit_flags = p->item->object.flags;\n \n-\t\tif (flags & TREESAME)\n+\t\tif (commit_flags & TREESAME)\n \t\t\tcontinue;\n \t\tdistance = weight(p);\n \t\tif (nr - distance < distance)\n@@ -261,7 +261,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, int first_parent_only)\n+\t\t\t\t\t unsigned bisect_flags)\n {\n \tint n, counted;\n \tstruct commit_list *p;\n@@ -270,12 +270,12 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\n \n \tfor (n = 0, p = list; p; p = p->next) {\n \t\tstruct commit *commit = p->item;\n-\t\tunsigned flags = commit->object.flags;\n+\t\tunsigned commit_flags = commit->object.flags;\n \n \t\t*commit_weight_at(&commit_weight, p->item) = &weights[n++];\n-\t\tswitch (count_interesting_parents(commit, first_parent_only)) {\n+\t\tswitch (count_interesting_parents(commit, bisect_flags)) {\n \t\tcase 0:\n-\t\t\tif (!(flags & TREESAME)) {\n+\t\t\tif (!(commit_flags & TREESAME)) {\n \t\t\t\tweight_set(p, 1);\n \t\t\t\tcounted++;\n \t\t\t\tshow_list(\"bisection 2 count one\",\n@@ -316,13 +316,13 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\n \t\t\tcontinue;\n \t\tif (weight(p) != -2)\n \t\t\tcontinue;\n-\t\tif (first_parent_only)\n+\t\tif (bisect_flags & FIND_BISECTION_FIRST_PARENT_ONLY)\n \t\t\tBUG(\"shouldn't be calling count-distance in fp mode\");\n \t\tweight_set(p, count_distance(p));\n \t\tclear_distance(list);\n \n \t\t/* Does it happen to be at exactly half-way? */\n-\t\tif (!find_all && halfway(p, nr))\n+\t\tif (!(bisect_flags & FIND_BISECTION_ALL) && halfway(p, nr))\n \t\t\treturn p;\n \t\tcounted++;\n \t}\n@@ -332,14 +332,14 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\n \twhile (counted < nr) {\n \t\tfor (p = list; p; p = p->next) {\n \t\t\tstruct commit_list *q;\n-\t\t\tunsigned flags = p->item->object.flags;\n+\t\t\tunsigned commit_flags = p->item->object.flags;\n \n \t\t\tif (0 <= weight(p))\n \t\t\t\tcontinue;\n \n \t\t\tfor (q = p->item->parents;\n \t\t\t q;\n-\t\t\t q = first_parent_only ? NULL : q->next) {\n+\t\t\t q = bisect_flags & FIND_BISECTION_FIRST_PARENT_ONLY ? NULL : q->next) {\n \t\t\t\tif (q->item->object.flags & UNINTERESTING)\n \t\t\t\t\tcontinue;\n \t\t\t\tif (0 <= weight(q))\n@@ -353,7 +353,7 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\n \t\t\t * add one for p itself if p is to be counted,\n \t\t\t * otherwise inherit it from q directly.\n \t\t\t */\n-\t\t\tif (!(flags & TREESAME)) {\n+\t\t\tif (!(commit_flags & TREESAME)) {\n \t\t\t\tweight_set(p, weight(q)+1);\n \t\t\t\tcounted++;\n \t\t\t\tshow_list(\"bisection 2 count one\",\n@@ -363,21 +363,21 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\n \t\t\t\tweight_set(p, weight(q));\n \n \t\t\t/* Does it happen to be at exactly half-way? */\n-\t\t\tif (!find_all && halfway(p, nr))\n+\t\t\tif (!(bisect_flags & FIND_BISECTION_ALL) && halfway(p, nr))\n \t\t\t\treturn p;\n \t\t}\n \t}\n \n \tshow_list(\"bisection 2 counted all\", counted, nr, list);\n \n-\tif (!find_all)\n+\tif (!(bisect_flags & FIND_BISECTION_ALL))\n \t\treturn best_bisection(list, nr);\n \telse\n \t\treturn best_bisection_sorted(list, nr);\n }\n \n void find_bisection(struct commit_list **commit_list, int *reaches,\n-\t\t int *all, int find_all, int first_parent_only)\n+\t\t int *all, unsigned bisect_flags)\n {\n \tint nr, on_list;\n \tstruct commit_list *list, *p, *best, *next, *last;\n@@ -393,16 +393,16 @@ void find_bisection(struct commit_list **commit_list, int *reaches,\n \tfor (nr = on_list = 0, last = NULL, p = *commit_list;\n \t p;\n \t p = next) {\n-\t\tunsigned flags = p->item->object.flags;\n+\t\tunsigned commit_flags = p->item->object.flags;\n \n \t\tnext = p->next;\n-\t\tif (flags & UNINTERESTING) {\n+\t\tif (commit_flags & UNINTERESTING) {\n \t\t\tfree(p);\n \t\t\tcontinue;\n \t\t}\n \t\tp->next = last;\n \t\tlast = p;\n-\t\tif (!(flags & TREESAME))\n+\t\tif (!(commit_flags & TREESAME))\n \t\t\tnr++;\n \t\ton_list++;\n \t}\n@@ -413,9 +413,9 @@ 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, first_parent_only);\n+\tbest = do_find_bisection(list, nr, weights, bisect_flags);\n \tif (best) {\n-\t\tif (!find_all) {\n+\t\tif (!(bisect_flags & FIND_BISECTION_ALL)) {\n \t\t\tlist->item = best->item;\n \t\t\tfree_commit_list(list->next);\n \t\t\tbest = list;\n@@ -1000,23 +1000,30 @@ enum bisect_error bisect_next_all(struct repository *r, const char *prefix)\n \tstruct object_id *bisect_rev;\n \tchar *steps_msg;\n \tint no_checkout = ref_exists(\"BISECT_HEAD\");\n-\tint first_parent_only = file_exists(git_path_bisect_first_parent());\n+\tunsigned bisect_flags = 0;\n \n \tread_bisect_terms(&term_bad, &term_good);\n \tif (read_bisect_refs())\n \t\tdie(_(\"reading bisect refs failed\"));\n \n+\tif (file_exists(git_path_bisect_first_parent()))\n+\t\tbisect_flags |= FIND_BISECTION_FIRST_PARENT_ONLY;\n+\n+\tif (skipped_revs.nr)\n+\t\tbisect_flags |= FIND_BISECTION_ALL;\n+\n \tres = check_good_are_ancestors_of_bad(r, prefix, no_checkout);\n \tif (res)\n \t\treturn res;\n \n \tbisect_rev_setup(r, &revs, prefix, \"%s\", \"^%s\", 1);\n-\trevs.first_parent_only = first_parent_only;\n+\n+\trevs.first_parent_only = !!(bisect_flags & FIND_BISECTION_FIRST_PARENT_ONLY);\n \trevs.limited = 1;\n \n \tbisect_common(&revs);\n \n-\tfind_bisection(&revs.commits, &reaches, &all, !!skipped_revs.nr, first_parent_only);\n+\tfind_bisection(&revs.commits, &reaches, &all, bisect_flags);\n \trevs.commits = managed_skipped(revs.commits, &tried);\n \n \tif (!revs.commits) {\ndiff --git a/bisect.h b/bisect.h\nindex 7f30b94db3..ec24ac2d7e 100644\n--- a/bisect.h\n+++ b/bisect.h\n@@ -12,7 +12,7 @@ struct repository;\n * best commit, as chosen by `find_all`.\n */\n void find_bisection(struct commit_list **list, int *reaches, int *all,\n-\t\t int find_all, int first_parent_only);\n+\t\t unsigned bisect_flags);\n \n struct commit_list *filter_skipped(struct commit_list *list,\n \t\t\t\t struct commit_list **tried,\n@@ -23,6 +23,9 @@ struct commit_list *filter_skipped(struct commit_list *list,\n #define BISECT_SHOW_ALL\t\t(1<<0)\n #define REV_LIST_QUIET\t\t(1<<1)\n \n+#define FIND_BISECTION_ALL\t\t\t(1u<<0)\n+#define FIND_BISECTION_FIRST_PARENT_ONLY\t(1u<<1)\n+\n struct rev_list_info {\n \tstruct rev_info *revs;\n \tint flags;\ndiff --git a/builtin/rev-list.c b/builtin/rev-list.c\nindex d1a14596b2..25c6c3b38d 100644\n--- a/builtin/rev-list.c\n+++ b/builtin/rev-list.c\n@@ -637,8 +637,15 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)\n \n \tif (bisect_list) {\n \t\tint reaches, all;\n+\t\tunsigned bisect_flags = 0;\n \n-\t\tfind_bisection(&revs.commits, &reaches, &all, bisect_find_all, revs.first_parent_only);\n+\t\tif (bisect_find_all)\n+\t\t\tbisect_flags |= FIND_BISECTION_ALL;\n+\n+\t\tif (revs.first_parent_only)\n+\t\t\tbisect_flags |= FIND_BISECTION_FIRST_PARENT_ONLY;\n+\n+\t\tfind_bisection(&revs.commits, &reaches, &all, bisect_flags);\n \n \t\tif (bisect_show_vars)\n \t\t\treturn show_bisect_vars(&info, reaches, all);\n-- \n2.24.3 (Apple Git-128)\n\n"},{"id":"402866","messageId":"xmqqtuxiqckg.fsf@gitster.c.googlers.com","threadId":"53937","inReplyTo":"20200804220113.5909-1-alipman88@gmail.com","subject":"Re: [PATCH v4 0/5] Introduce --first-parent flag for git bisect","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-08-04T22:19:11Z","receivedAt":"2020-08-04T22:19:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Aaron Lipman writes:\n\n> OK, here's take 4! Responding to Junio's feedback, first:\n>\n>> That function [cmd_bisect__helper()] is supposed to be a thin shim\n>> layer whose only reason of its existence is to serve as an interface\n>> with the scripted version of \"git bisect\". When everything is\n>> migrated from the shell script, cmd_bisect__helper() should disappear.\n>> ...\n>> This is going backwards, as far as I can tell. If anything, I'd\n>> rather see cmd_bisect__helper() get fixed so that it does not parse\n>> \"--first-parent\" (and similarly, \"--no-checkout\" that you imitated)\n>> into first_parent_only (and no_checkout) variables and passed as\n>> parameters to bisect_start().\n>\n> Thanks for the explanation, Junio. (And for bearing with me while I\n> gain familiarity with git's codebase.) Now that I've taken some time\n> to examine how git-bisect.sh and cmd_bisect__helper() fit together,\n> the correct approach is much more clear.\n\nI did notice that no_checkout variable in the top-level\ncmd_bisect__helper() was convenient to have in the current code\nstructure, and I didn't think how it should be fixed deeply enough,\nso for the purpose of this series, I do not mind if it is left\nuntouched. It's just that it was disturbing to see that addition of\n\"--first-parent\" wanted to add yet another variable to the\ntop-level, and that the new variable was totally unneeded.\n\n> As no_checkout was also passed to bisect_next_all() [via implicitly\n> checking for the existence of .git/BISECT_HEAD when calling\n> bisect_next() in git-bisect.sh], I've removed that parameter and\n> instead check for .git/BISECT_HEAD in bisect_next_all() to determine\n> whether no-checkout mode is on.\n>\n> This means no-checkout mode can no longer be enabled by \"git\n> bisect--helper --next-all --no-checkout\"\n\nUnlike \"start\", \"next-all\" is not really a command in the end-user's\nmind (\"git bisect next\" is, though). It merely is just an interim\n\"convenience\" interface for the remaining part of the scripted \"git\nbisect\" to call into. As long as the caller can do the same thing\nas before, it should be OK, I would think.\n"},{"id":"402875","messageId":"CAPig+cQumRSCQA3Et5=h7SD7zqMm_Z6LJVUTonkewR=hNR55Ug@mail.gmail.com","threadId":"53937","inReplyTo":"20200804220113.5909-3-alipman88@gmail.com","subject":"Re: [PATCH v4 2/5] rev-list: allow bisect and first-parent flags","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-08-05T00:38:18Z","receivedAt":"2020-08-05T00:38:32Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Aug 4, 2020 at 6:01 PM Aaron Lipman wrote:\n> Add first_parent_only parameter to find_bisection(), removing the\n> barrier that prevented combining the --bisect and --first-parent flags\n> when using git rev-list\n>\n> Signed-off-by: Aaron Lipman \n> ---\n> diff --git a/t/t6002-rev-list-bisect.sh b/t/t6002-rev-list-bisect.sh\n> @@ -263,4 +263,49 @@ test_expect_success 'rev-parse --bisect can default to good/bad refs' '\n> +test_output_expect_success \"--bisect --first-parent\" 'git rev-list --bisect --first-parent E ^F' < +test_expect_success \"--bisect-all --first-parent\" '\n> +cat > expect.unsorted < +$(git rev-parse E) (tag: E, dist=0)\n> +[...]\n> +$(git rev-parse e8) (tag: e8, dist=1)\n> +EOF\n> +\n> +# expect results to be ordered by distance (descending),\n> +# commit hash (ascending)\n> +sort -k4,4r -k1,1 expect.unsorted > expect &&\n> +git rev-list --bisect-all --first-parent E ^F > actual &&\n> +test_cmp actual expect\n> +'\n\nIndent this test body with TAB rather than having it sit at the left\nmargin. You can use <<-EOF for the here-doc to allow its content to be\nindented, as well.\n\nSwap the arguments to test_cmp.\n\nAlso, for style consistency, no whitespace following redirect:\n\n cat >expect.unsorted <<-EOF\n ...\n EOF\n ...\n sort ... >expect &&\n git rev-list ... >actual &&\n test_cmp expect actual\n"},{"id":"402893","messageId":"CAP8UFD1Z1V6+iPO2CMOqdHhUGkmTdvCK+TDNtoOcX8QvC4coNQ@mail.gmail.com","threadId":"53937","inReplyTo":"20200804220113.5909-1-alipman88@gmail.com","subject":"Re: [PATCH v4 0/5] Introduce --first-parent flag for git bisect","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2020-08-05T05:55:53Z","receivedAt":"2020-08-05T05:56:08Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Wed, Aug 5, 2020 at 12:04 AM Aaron Lipman wrote:\n>\n> OK, here's take 4! Responding to Junio's feedback, first:\n\n[...]\n\n> Martin, thanks for your suggestions\n\n[...]\n\nIt's better to have the people you are replying to as recipients of\nyour emails (in the \"To:\" field). I have added them into \"Cc:\".\n\n> > (Signed-off-by: Martin Ågren , FWIW.)\n>\n> I'm still getting used to the conventions - should I add your name as\n> a signed-off-by tag, a thanks-to tag, or both?\n\nWe often use the following trailers:\n\n- \"Helped-by:\" when someone helped you\n- \"Suggested-by:\" when someone suggested the main idea in the patch\n- \"Reported-by:\" when someone reported an issue fixed by the patch\n- \"Acked-by:\" when someone explicitly acked the patch\n- \"Reviewed-by:\" when someone explicitly gave their \"Reviewed-by:\"\n\nIf your patch is based on a patch from someone else, you can also keep\nthe \"Signed-off-by:\" and other trailers that the person already put in\nthe commit message. If you haven't made a lot of changes to a patch\ninitially from someone else you can also keep them as the author.\n\nThanks,\nChristian.\n"},{"id":"402894","messageId":"CAP8UFD1YAu8pCm4u0KscmJSV0bgd=hQg9F1LaLb=Wj_NHNshHg@mail.gmail.com","threadId":"53937","inReplyTo":"20200804220113.5909-2-alipman88@gmail.com","subject":"Re: [PATCH v4 1/5] t6030: modernize \"git bisect run\" tests","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2020-08-05T06:11:14Z","receivedAt":"2020-08-05T06:11:29Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Wed, Aug 5, 2020 at 12:03 AM Aaron Lipman wrote:\n>\n> Enforce consistent styling for tests on \"git bisect run\":\n> - Use \"write_script\" to abstract away platform-specific details.\n> - Favor current whitespace conventions.\n> - While at it, change \"introduced\" to \"added\" in the comments to make\n> them read better.\n>\n> Thanks-to: Martin Ågren \n\n'Helped-by:' and 'Suggested-by:' are much more often used than\n'Thanks-to:'. There are approximately 1182, 247 and 69 occurrences of\nthem respectively.\n\n> Signed-off-by: Aaron Lipman \n\nAnyway the patch looks good to me.\n\nThanks,\nChristian.\n"},{"id":"402895","messageId":"CAN0heSp6rGT1YFFVGzmf=dXQGfi-h9TQRTcKgAsHXyJqs2e7Jg@mail.gmail.com","threadId":"53937","inReplyTo":"20200804220113.5909-1-alipman88@gmail.com","subject":"Re: [PATCH v4 0/5] Introduce --first-parent flag for git bisect","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2020-08-05T06:18:05Z","receivedAt":"2020-08-05T06:18:20Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"Hi Aaron,\n\nBy the way, welcome to the list!\n\nOn Wed, 5 Aug 2020 at 00:02, Aaron Lipman wrote:\n>\n> Martin, thanks for your suggestions - I've moved the commit updating\n> \"git bisect run\" tests to 1/5, updated the commit message, and\n> included the changes you provided. (I held off on additional\n> indentation corrections, as it kinda felt like unnecessary code\n> churn/outside the scope of abstracting platform-specific details.)\n\nOk, great, I think it's fine to stop there. Those spots really are in a\nmuch better shape now, thanks.\n\n> > (Signed-off-by: Martin Ågren , FWIW.)\n>\n> I'm still getting used to the conventions - should I add your name as\n> a signed-off-by tag, a thanks-to tag, or both?\n\nWhat I meant was \"oh, and here's my Signed-off-by for the record\" so\nthat we wouldn't need a round-trip of \"cool, can I have your S-o-b on\nthat?\". Of course, I expressed that in such a lousy, compact way that\nwe needed a round-trip *anyway*. :-/\n\nAnd I'm not even sure we need a \"Signed-off-by\". I just posted the patch\nform of \"If you look for chmod, you'll find a few more spots where you\ncan do the exact same transformation, and you might want to look up\ntest_expect_code.\"\n\nI see you added \"Thanks-to\" and I would have been fine with no mention\nat all. I think that patch looks good now, thanks. No need to think\nabout the trailers for that patch now, I think you can safely focus on\nthe later patches from now on. ;-)\n\nMartin\n"},{"id":"403153","messageId":"xmqqmu36f9pv.fsf@gitster.c.googlers.com","threadId":"53937","inReplyTo":"CAP8UFD1Z1V6+iPO2CMOqdHhUGkmTdvCK+TDNtoOcX8QvC4coNQ@mail.gmail.com","subject":"Re: [PATCH v4 0/5] Introduce --first-parent flag for git bisect","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-08-07T21:05:16Z","receivedAt":"2020-08-07T21:05:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n> On Wed, Aug 5, 2020 at 12:04 AM Aaron Lipman wrote:\n>>\n>> OK, here's take 4! Responding to Junio's feedback, first:\n>\n> [...]\n>\n>> Martin, thanks for your suggestions\n>\n> [...]\n>\n> It's better to have the people you are replying to as recipients of\n> your emails (in the \"To:\" field). I have added them into \"Cc:\".\n>\n>> > (Signed-off-by: Martin Ågren , FWIW.)\n>>\n>> I'm still getting used to the conventions - should I add your name as\n>> a signed-off-by tag, a thanks-to tag, or both?\n>\n> We often use the following trailers:\n>\n> - \"Helped-by:\" when someone helped you\n> - \"Suggested-by:\" when someone suggested the main idea in the patch\n> - \"Reported-by:\" when someone reported an issue fixed by the patch\n> - \"Acked-by:\" when someone explicitly acked the patch\n> - \"Reviewed-by:\" when someone explicitly gave their \"Reviewed-by:\"\n>\n> If your patch is based on a patch from someone else, you can also keep\n> the \"Signed-off-by:\" and other trailers that the person already put in\n> the commit message. If you haven't made a lot of changes to a patch\n> initially from someone else you can also keep them as the author.\n\nOK. It appears that the patches themselves do not have fundamental\nissues, perhaps other than test style updates for [2/5]?\n\nAaron, Eric, can we have the hopefully final update to close this\ntopic, if that is the case?\n\nThanks.\n"},{"id":"403157","messageId":"CAPig+cT_1JNZOk7wZr3akdCmv7tn2RHieJd+A-usZ49jS0eCmA@mail.gmail.com","threadId":"53937","inReplyTo":"xmqqmu36f9pv.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v4 0/5] Introduce --first-parent flag for git bisect","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-08-07T21:17:43Z","receivedAt":"2020-08-07T21:17:57Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Aug 7, 2020 at 5:05 PM Junio C Hamano wrote:\n> OK. It appears that the patches themselves do not have fundamental\n> issues, perhaps other than test style updates for [2/5]?\n>\n> Aaron, Eric, can we have the hopefully final update to close this\n> topic, if that is the case?\n\nI have not personally reviewed any of these patches, nor have I been\nfollowing the topic. My one contribution to this thread[1] was merely\nto point out minor style violations in [2/5] which I noticed while\nquickly scrolling through the email. Consequently, I'm not in a\nposition to say whether or not there are any fundamental issues with\nthe changes made by any of these patches.\n\n[1]: https://lore.kernel.org/git/CAPig+cQumRSCQA3Et5=h7SD7zqMm_Z6LJVUTonkewR=hNR55Ug@mail.gmail.com/\n"},{"id":"403163","messageId":"20200807215838.19571-1-alipman88@gmail.com","threadId":"53937","inReplyTo":"20200804220113.5909-1-alipman88@gmail.com","subject":"[PATCH v5 0/5] Introduce --first-parent flag for git bisect","fromName":"Aaron Lipman","fromEmail":"alipman88@gmail.com","sentAt":"2020-08-07T21:58:33Z","receivedAt":"2020-08-07T21:58:48Z","isPatch":true,"sender":{"key":"alipman88@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3922511?v=4"},"body":"Thanks for catching those style violations, Eric. (And to everyone\nelse who reviewed!)\n\nJunio, here is the (hopefully) final update with Eric's style\ncorrections applied to 2/5.\n\nAaron Lipman (5):\n t6030: modernize \"git bisect run\" tests\n rev-list: allow bisect and first-parent flags\n cmd_bisect__helper: defer parsing no-checkout flag\n bisect: introduce first-parent flag\n bisect: combine args passed to find_bisection()\n\n Documentation/git-bisect.txt | 13 +++-\n Documentation/rev-list-options.txt | 7 +-\n bisect.c | 79 +++++++++++++---------\n bisect.h | 9 +--\n builtin/bisect--helper.c | 23 ++++---\n builtin/rev-list.c | 9 ++-\n git-bisect.sh | 2 +-\n revision.c | 3 -\n t/t6000-rev-list-misc.sh | 4 +-\n t/t6002-rev-list-bisect.sh | 45 +++++++++++++\n t/t6030-bisect-porcelain.sh | 104 ++++++++++++++++-------------\n 11 files changed, 195 insertions(+), 103 deletions(-)\n\n-- \n2.24.3 (Apple Git-128)\n\n"},{"id":"403164","messageId":"20200807215838.19571-2-alipman88@gmail.com","threadId":"53937","inReplyTo":"20200807215838.19571-1-alipman88@gmail.com","subject":"[PATCH v5 1/5] t6030: modernize \"git bisect run\" tests","fromName":"Aaron Lipman","fromEmail":"alipman88@gmail.com","sentAt":"2020-08-07T21:58:34Z","receivedAt":"2020-08-07T21:58:50Z","isPatch":true,"sender":{"key":"alipman88@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3922511?v=4"},"body":"Enforce consistent styling for tests on \"git bisect run\":\n- Use \"write_script\" to abstract away platform-specific details.\n- Favor current whitespace conventions.\n- While at it, change \"introduced\" to \"added\" in the comments to make\n them read better.\n\nHelped-by: Martin Ågren \nSigned-off-by: Aaron Lipman \n---\n t/t6030-bisect-porcelain.sh | 86 ++++++++++++++++---------------------\n 1 file changed, 38 insertions(+), 48 deletions(-)\n\ndiff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh\nindex 36d9b2b2e4..a66c4b89bc 100755\n--- a/t/t6030-bisect-porcelain.sh\n+++ b/t/t6030-bisect-porcelain.sh\n@@ -243,32 +243,30 @@ test_expect_success 'bisect skip: with commit both bad and skipped' '\n '\n \n # We want to automatically find the commit that\n-# introduced \"Another\" into hello.\n-test_expect_success \\\n- '\"git bisect run\" simple case' \\\n- 'echo \"#\"\\!\"/bin/sh\" > test_script.sh &&\n- echo \"grep Another hello > /dev/null\" >> test_script.sh &&\n- echo \"test \\$? -ne 0\" >> test_script.sh &&\n- chmod +x test_script.sh &&\n- git bisect start &&\n- git bisect good $HASH1 &&\n- git bisect bad $HASH4 &&\n- git bisect run ./test_script.sh > my_bisect_log.txt &&\n- grep \"$HASH3 is the first bad commit\" my_bisect_log.txt &&\n- git bisect reset'\n+# added \"Another\" into hello.\n+test_expect_success '\"git bisect run\" simple case' '\n+\twrite_script test_script.sh <<-\\EOF &&\n+\t! grep Another hello >/dev/null\n+\tEOF\n+\tgit bisect start &&\n+\tgit bisect good $HASH1 &&\n+\tgit bisect bad $HASH4 &&\n+\tgit bisect run ./test_script.sh >my_bisect_log.txt &&\n+\tgrep \"$HASH3 is the first bad commit\" my_bisect_log.txt &&\n+\tgit bisect reset\n+'\n \n # We want to automatically find the commit that\n-# introduced \"Ciao\" into hello.\n-test_expect_success \\\n- '\"git bisect run\" with more complex \"git bisect start\"' \\\n- 'echo \"#\"\\!\"/bin/sh\" > test_script.sh &&\n- echo \"grep Ciao hello > /dev/null\" >> test_script.sh &&\n- echo \"test \\$? -ne 0\" >> test_script.sh &&\n- chmod +x test_script.sh &&\n- git bisect start $HASH4 $HASH1 &&\n- git bisect run ./test_script.sh > my_bisect_log.txt &&\n- grep \"$HASH4 is the first bad commit\" my_bisect_log.txt &&\n- git bisect reset'\n+# added \"Ciao\" into hello.\n+test_expect_success '\"git bisect run\" with more complex \"git bisect start\"' '\n+\twrite_script test_script.sh <<-\\EOF &&\n+\t! grep Ciao hello >/dev/null\n+\tEOF\n+\tgit bisect start $HASH4 $HASH1 &&\n+\tgit bisect run ./test_script.sh >my_bisect_log.txt &&\n+\tgrep \"$HASH4 is the first bad commit\" my_bisect_log.txt &&\n+\tgit bisect reset\n+'\n \n # $HASH1 is good, $HASH5 is bad, we skip $HASH3\n # but $HASH4 is good,\n@@ -295,24 +293,17 @@ HASH6=\n test_expect_success 'bisect run & skip: cannot tell between 2' '\n \tadd_line_into_file \"6: Yet a line.\" hello &&\n \tHASH6=$(git rev-parse --verify HEAD) &&\n-\techo \"#\"\\!\"/bin/sh\" > test_script.sh &&\n-\techo \"sed -ne \\\\\\$p hello | grep Ciao > /dev/null && exit 125\" >> test_script.sh &&\n-\techo \"grep line hello > /dev/null\" >> test_script.sh &&\n-\techo \"test \\$? -ne 0\" >> test_script.sh &&\n-\tchmod +x test_script.sh &&\n+\twrite_script test_script.sh <<-\\EOF &&\n+\tsed -ne \\$p hello | grep Ciao >/dev/null && exit 125\n+\t! grep line hello >/dev/null\n+\tEOF\n \tgit bisect start $HASH6 $HASH1 &&\n-\tif git bisect run ./test_script.sh > my_bisect_log.txt\n-\tthen\n-\t\techo Oops, should have failed.\n-\t\tfalse\n-\telse\n-\t\ttest $? -eq 2 &&\n-\t\tgrep \"first bad commit could be any of\" my_bisect_log.txt &&\n-\t\t! grep $HASH3 my_bisect_log.txt &&\n-\t\t! grep $HASH6 my_bisect_log.txt &&\n-\t\tgrep $HASH4 my_bisect_log.txt &&\n-\t\tgrep $HASH5 my_bisect_log.txt\n-\tfi\n+\ttest_expect_code 2 git bisect run ./test_script.sh >my_bisect_log.txt &&\n+\tgrep \"first bad commit could be any of\" my_bisect_log.txt &&\n+\t! grep $HASH3 my_bisect_log.txt &&\n+\t! grep $HASH6 my_bisect_log.txt &&\n+\tgrep $HASH4 my_bisect_log.txt &&\n+\tgrep $HASH5 my_bisect_log.txt\n '\n \n HASH7=\n@@ -320,14 +311,13 @@ test_expect_success 'bisect run & skip: find first bad' '\n \tgit bisect reset &&\n \tadd_line_into_file \"7: Should be the last line.\" hello &&\n \tHASH7=$(git rev-parse --verify HEAD) &&\n-\techo \"#\"\\!\"/bin/sh\" > test_script.sh &&\n-\techo \"sed -ne \\\\\\$p hello | grep Ciao > /dev/null && exit 125\" >> test_script.sh &&\n-\techo \"sed -ne \\\\\\$p hello | grep day > /dev/null && exit 125\" >> test_script.sh &&\n-\techo \"grep Yet hello > /dev/null\" >> test_script.sh &&\n-\techo \"test \\$? -ne 0\" >> test_script.sh &&\n-\tchmod +x test_script.sh &&\n+\twrite_script test_script.sh <<-\\EOF &&\n+\tsed -ne \\$p hello | grep Ciao >/dev/null && exit 125\n+\tsed -ne \\$p hello | grep day >/dev/null && exit 125\n+\t! grep Yet hello >/dev/null\n+\tEOF\n \tgit bisect start $HASH7 $HASH1 &&\n-\tgit bisect run ./test_script.sh > my_bisect_log.txt &&\n+\tgit bisect run ./test_script.sh >my_bisect_log.txt &&\n \tgrep \"$HASH6 is the first bad commit\" my_bisect_log.txt\n '\n \n-- \n2.24.3 (Apple Git-128)\n\n"},{"id":"403165","messageId":"20200807215838.19571-3-alipman88@gmail.com","threadId":"53937","inReplyTo":"20200807215838.19571-1-alipman88@gmail.com","subject":"[PATCH v5 2/5] rev-list: allow bisect and first-parent flags","fromName":"Aaron Lipman","fromEmail":"alipman88@gmail.com","sentAt":"2020-08-07T21:58:35Z","receivedAt":"2020-08-07T21:58:51Z","isPatch":true,"sender":{"key":"alipman88@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3922511?v=4"},"body":"Add first_parent_only parameter to find_bisection(), removing the\nbarrier that prevented combining the --bisect and --first-parent flags\nwhen using git rev-list\n\nBased-on-patch-by: Tiago Botelho \nSigned-off-by: Aaron Lipman \n---\n Documentation/rev-list-options.txt | 7 ++---\n bisect.c | 28 ++++++++++++-------\n bisect.h | 2 +-\n builtin/rev-list.c | 2 +-\n revision.c | 3 --\n t/t6000-rev-list-misc.sh | 4 +--\n t/t6002-rev-list-bisect.sh | 45 ++++++++++++++++++++++++++++++\n 7 files changed, 70 insertions(+), 21 deletions(-)\n\ndiff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\nindex b01b2b6773..d3117ce51b 100644\n--- a/Documentation/rev-list-options.txt\n+++ b/Documentation/rev-list-options.txt\n@@ -128,8 +128,7 @@ parents) and `--max-parents=-1` (negative numbers denote no upper limit).\n \tbecause merges into a topic branch tend to be only about\n \tadjusting to updated upstream from time to time, and\n \tthis option allows you to ignore the individual commits\n-\tbrought in to your history by such a merge. Cannot be\n-\tcombined with --bisect.\n+\tbrought in to your history by such a merge.\n \n --not::\n \tReverses the meaning of the '{caret}' prefix (or lack thereof)\n@@ -207,7 +206,7 @@ ifndef::git-rev-list[]\n \tPretend as if the bad bisection ref `refs/bisect/bad`\n \twas listed and as if it was followed by `--not` and the good\n \tbisection refs `refs/bisect/good-*` on the command\n-\tline. Cannot be combined with --first-parent.\n+\tline.\n endif::git-rev-list[]\n \n --stdin::\n@@ -743,7 +742,7 @@ outputs 'midpoint', the output of the two commands\n would be of roughly the same length. Finding the change which\n introduces a regression is thus reduced to a binary search: repeatedly\n generate and test new 'midpoint's until the commit chain is of length\n-one. Cannot be combined with --first-parent.\n+one.\n \n --bisect-vars::\n \tThis calculates the same as `--bisect`, except that refs in\ndiff --git a/bisect.c b/bisect.c\nindex d5e830410f..a11fdb1473 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -88,15 +88,16 @@ static inline void weight_set(struct commit_list *elem, int weight)\n \t**commit_weight_at(&commit_weight, elem->item) = weight;\n }\n \n-static int count_interesting_parents(struct commit *commit)\n+static int count_interesting_parents(struct commit *commit, int first_parent_only)\n {\n \tstruct commit_list *p;\n \tint count;\n \n \tfor (count = 0, p = commit->parents; p; p = p->next) {\n-\t\tif (p->item->object.flags & UNINTERESTING)\n-\t\t\tcontinue;\n-\t\tcount++;\n+\t\tif (!(p->item->object.flags & UNINTERESTING))\n+\t\t\tcount++;\n+\t\tif (first_parent_only)\n+\t\t\tbreak;\n \t}\n \treturn count;\n }\n@@ -259,7 +260,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 first_parent_only)\n {\n \tint n, counted;\n \tstruct commit_list *p;\n@@ -271,7 +272,7 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\n \t\tunsigned flags = commit->object.flags;\n \n \t\t*commit_weight_at(&commit_weight, p->item) = &weights[n++];\n-\t\tswitch (count_interesting_parents(commit)) {\n+\t\tswitch (count_interesting_parents(commit, first_parent_only)) {\n \t\tcase 0:\n \t\t\tif (!(flags & TREESAME)) {\n \t\t\t\tweight_set(p, 1);\n@@ -314,6 +315,8 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\n \t\t\tcontinue;\n \t\tif (weight(p) != -2)\n \t\t\tcontinue;\n+\t\tif (first_parent_only)\n+\t\t\tBUG(\"shouldn't be calling count-distance in fp mode\");\n \t\tweight_set(p, count_distance(p));\n \t\tclear_distance(list);\n \n@@ -332,7 +335,10 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\n \n \t\t\tif (0 <= weight(p))\n \t\t\t\tcontinue;\n-\t\t\tfor (q = p->item->parents; q; q = q->next) {\n+\n+\t\t\tfor (q = p->item->parents;\n+\t\t\t q;\n+\t\t\t q = first_parent_only ? NULL : q->next) {\n \t\t\t\tif (q->item->object.flags & UNINTERESTING)\n \t\t\t\t\tcontinue;\n \t\t\t\tif (0 <= weight(q))\n@@ -370,7 +376,7 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\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 first_parent_only)\n {\n \tint nr, on_list;\n \tstruct commit_list *list, *p, *best, *next, *last;\n@@ -406,7 +412,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, first_parent_only);\n \tif (best) {\n \t\tif (!find_all) {\n \t\t\tlist->item = best->item;\n@@ -991,6 +997,7 @@ enum bisect_error bisect_next_all(struct repository *r, const char *prefix, int\n \tenum bisect_error res = BISECT_OK;\n \tstruct object_id *bisect_rev;\n \tchar *steps_msg;\n+\tint first_parent_only = 0; /* TODO: pass --first-parent flag from git bisect start */\n \n \tread_bisect_terms(&term_bad, &term_good);\n \tif (read_bisect_refs())\n@@ -1001,11 +1008,12 @@ enum bisect_error bisect_next_all(struct repository *r, const char *prefix, int\n \t\treturn res;\n \n \tbisect_rev_setup(r, &revs, prefix, \"%s\", \"^%s\", 1);\n+\trevs.first_parent_only = first_parent_only;\n \trevs.limited = 1;\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, first_parent_only);\n \trevs.commits = managed_skipped(revs.commits, &tried);\n \n \tif (!revs.commits) {\ndiff --git a/bisect.h b/bisect.h\nindex 8bad8d8391..a63af0505f 100644\n--- a/bisect.h\n+++ b/bisect.h\n@@ -12,7 +12,7 @@ struct repository;\n * best commit, as chosen by `find_all`.\n */\n void find_bisection(struct commit_list **list, int *reaches, int *all,\n-\t\t int find_all);\n+\t\t int find_all, int first_parent_only);\n \n struct commit_list *filter_skipped(struct commit_list *list,\n \t\t\t\t struct commit_list **tried,\ndiff --git a/builtin/rev-list.c b/builtin/rev-list.c\nindex f520111eda..d1a14596b2 100644\n--- a/builtin/rev-list.c\n+++ b/builtin/rev-list.c\n@@ -638,7 +638,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, revs.first_parent_only);\n \n \t\tif (bisect_show_vars)\n \t\t\treturn show_bisect_vars(&info, reaches, all);\ndiff --git a/revision.c b/revision.c\nindex 6de29cdf7a..e628d5ea56 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2891,9 +2891,6 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n \tif (!revs->reflog_info && revs->grep_filter.use_reflog_filter)\n \t\tdie(\"cannot use --grep-reflog without --walk-reflogs\");\n \n-\tif (revs->first_parent_only && revs->bisect)\n-\t\tdie(_(\"--first-parent is incompatible with --bisect\"));\n-\n \tif (revs->line_level_traverse &&\n \t (revs->diffopt.output_format & ~(DIFF_FORMAT_PATCH | DIFF_FORMAT_NO_OUTPUT)))\n \t\tdie(_(\"-L does not yet support diff formats besides -p and -s\"));\ndiff --git a/t/t6000-rev-list-misc.sh b/t/t6000-rev-list-misc.sh\nindex 3bb0e4ff8f..fc4d55dcb2 100755\n--- a/t/t6000-rev-list-misc.sh\n+++ b/t/t6000-rev-list-misc.sh\n@@ -128,8 +128,8 @@ test_expect_success 'rev-list can negate index objects' '\n \ttest_cmp expect actual\n '\n \n-test_expect_success '--bisect and --first-parent can not be combined' '\n-\ttest_must_fail git rev-list --bisect --first-parent HEAD\n+test_expect_success '--bisect and --first-parent can be combined' '\n+\tgit rev-list --bisect --first-parent HEAD\n '\n \n test_expect_success '--header shows a NUL after each commit' '\ndiff --git a/t/t6002-rev-list-bisect.sh b/t/t6002-rev-list-bisect.sh\nindex a661408038..b95a0212ad 100755\n--- a/t/t6002-rev-list-bisect.sh\n+++ b/t/t6002-rev-list-bisect.sh\n@@ -263,4 +263,49 @@ test_expect_success 'rev-parse --bisect can default to good/bad refs' '\n \ttest_cmp expect.sorted actual.sorted\n '\n \n+test_output_expect_success '--bisect --first-parent' 'git rev-list --bisect --first-parent E ^F' <expect.unsorted <<-EOF &&\n+\t$(git rev-parse E) (tag: E, dist=0)\n+\t$(git rev-parse e1) (tag: e1, dist=1)\n+\t$(git rev-parse e2) (tag: e2, dist=2)\n+\t$(git rev-parse e3) (tag: e3, dist=3)\n+\t$(git rev-parse e4) (tag: e4, dist=4)\n+\t$(git rev-parse e5) (tag: e5, dist=4)\n+\t$(git rev-parse e6) (tag: e6, dist=3)\n+\t$(git rev-parse e7) (tag: e7, dist=2)\n+\t$(git rev-parse e8) (tag: e8, dist=1)\n+\tEOF\n+\n+\t# expect results to be ordered by distance (descending),\n+\t# commit hash (ascending)\n+\tsort -k4,4r -k1,1 expect.unsorted >expect &&\n+\tgit rev-list --bisect-all --first-parent E ^F >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.24.3 (Apple Git-128)\n\n"},{"id":"403166","messageId":"20200807215838.19571-4-alipman88@gmail.com","threadId":"53937","inReplyTo":"20200807215838.19571-1-alipman88@gmail.com","subject":"[PATCH v5 3/5] cmd_bisect__helper: defer parsing no-checkout flag","fromName":"Aaron Lipman","fromEmail":"alipman88@gmail.com","sentAt":"2020-08-07T21:58:36Z","receivedAt":"2020-08-07T21:58:52Z","isPatch":true,"sender":{"key":"alipman88@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3922511?v=4"},"body":"cmd_bisect__helper() is intended as a temporary shim layer serving as an\ninterface for git-bisect.sh. This function and git-bisect.sh should\neventually be replaced by a C implementation, cmd_bisect(), serving as\nan entrypoint for all \"git bisect ...\" shell commands: cmd_bisect() will\nonly parse the first token following \"git bisect\", and dispatch the\nremaining args to the appropriate function [\"bisect_start()\",\n\"bisect_next()\", etc.].\n\nThus, cmd_bisect__helper() should not be responsible for parsing flags\nlike --no-checkout. Instead, let the --no-checkout flag remain in the\nargv array, so it may be evaluated alongside the other options already\nparsed by bisect_start().\n\nSigned-off-by: Aaron Lipman \n---\n bisect.c | 3 ++-\n bisect.h | 4 +---\n builtin/bisect--helper.c | 14 ++++++--------\n git-bisect.sh | 2 +-\n 4 files changed, 10 insertions(+), 13 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex a11fdb1473..950ff6f533 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -989,7 +989,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-enum bisect_error bisect_next_all(struct repository *r, const char *prefix, int no_checkout)\n+enum bisect_error bisect_next_all(struct repository *r, const char *prefix)\n {\n \tstruct rev_info revs;\n \tstruct commit_list *tried;\n@@ -997,6 +997,7 @@ enum bisect_error bisect_next_all(struct repository *r, const char *prefix, int\n \tenum bisect_error res = BISECT_OK;\n \tstruct object_id *bisect_rev;\n \tchar *steps_msg;\n+\tint no_checkout = ref_exists(\"BISECT_HEAD\");\n \tint first_parent_only = 0; /* TODO: pass --first-parent flag from git bisect start */\n \n \tread_bisect_terms(&term_bad, &term_good);\ndiff --git a/bisect.h b/bisect.h\nindex a63af0505f..7f30b94db3 100644\n--- a/bisect.h\n+++ b/bisect.h\n@@ -58,9 +58,7 @@ enum bisect_error {\n \tBISECT_INTERNAL_SUCCESS_MERGE_BASE = -11\n };\n \n-enum bisect_error bisect_next_all(struct repository *r,\n-\t\t const char *prefix,\n-\t\t int no_checkout);\n+enum bisect_error bisect_next_all(struct repository *r, const char *prefix);\n \n int estimate_bisect_steps(int all);\n \ndiff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\nindex 73f9324ad7..95a15b918f 100644\n--- a/builtin/bisect--helper.c\n+++ b/builtin/bisect--helper.c\n@@ -18,7 +18,7 @@ static GIT_PATH_FUNC(git_path_head_name, \"head-name\")\n static GIT_PATH_FUNC(git_path_bisect_names, \"BISECT_NAMES\")\n \n static const char * const git_bisect_helper_usage[] = {\n-\tN_(\"git bisect--helper --next-all [--no-checkout]\"),\n+\tN_(\"git bisect--helper --next-all\"),\n \tN_(\"git bisect--helper --write-terms \"),\n \tN_(\"git bisect--helper --bisect-clean-state\"),\n \tN_(\"git bisect--helper --bisect-reset []\"),\n@@ -420,9 +420,9 @@ static int bisect_append_log_quoted(const char **argv)\n \treturn res;\n }\n \n-static int bisect_start(struct bisect_terms *terms, int no_checkout,\n-\t\t\tconst char **argv, int argc)\n+static int bisect_start(struct bisect_terms *terms, const char **argv, int argc)\n {\n+\tint no_checkout = 0;\n \tint i, has_double_dash = 0, must_write_terms = 0, bad_seen = 0;\n \tint flags, pathspec_pos, res = 0;\n \tstruct string_list revs = STRING_LIST_INIT_DUP;\n@@ -631,7 +631,7 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\tBISECT_TERMS,\n \t\tBISECT_START\n \t} cmdmode = 0;\n-\tint no_checkout = 0, res = 0, nolog = 0;\n+\tint res = 0, nolog = 0;\n \tstruct option options[] = {\n \t\tOPT_CMDMODE(0, \"next-all\", &cmdmode,\n \t\t\t N_(\"perform 'git bisect next'\"), NEXT_ALL),\n@@ -653,8 +653,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"print out the bisect terms\"), BISECT_TERMS),\n \t\tOPT_CMDMODE(0, \"bisect-start\", &cmdmode,\n \t\t\t N_(\"start the bisect session\"), BISECT_START),\n-\t\tOPT_BOOL(0, \"no-checkout\", &no_checkout,\n-\t\t\t N_(\"update BISECT_HEAD instead of checking out the current commit\")),\n \t\tOPT_BOOL(0, \"no-log\", &nolog,\n \t\t\t N_(\"no log for BISECT_WRITE\")),\n \t\tOPT_END()\n@@ -670,7 +668,7 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \n \tswitch (cmdmode) {\n \tcase NEXT_ALL:\n-\t\tres = bisect_next_all(the_repository, prefix, no_checkout);\n+\t\tres = bisect_next_all(the_repository, prefix);\n \t\tbreak;\n \tcase WRITE_TERMS:\n \t\tif (argc != 2)\n@@ -712,7 +710,7 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\tbreak;\n \tcase BISECT_START:\n \t\tset_terms(&terms, \"bad\", \"good\");\n-\t\tres = bisect_start(&terms, no_checkout, argv, argc);\n+\t\tres = bisect_start(&terms, argv, argc);\n \t\tbreak;\n \tdefault:\n \t\treturn error(\"BUG: unknown subcommand '%d'\", cmdmode);\ndiff --git a/git-bisect.sh b/git-bisect.sh\nindex f03fbb18f0..c7580e51a0 100755\n--- a/git-bisect.sh\n+++ b/git-bisect.sh\n@@ -153,7 +153,7 @@ bisect_next() {\n \tgit bisect--helper --bisect-next-check $TERM_GOOD $TERM_BAD $TERM_GOOD|| exit\n \n \t# Perform all bisection computation, display and checkout\n-\tgit bisect--helper --next-all $(git rev-parse --verify -q BISECT_HEAD > /dev/null && echo --no-checkout)\n+\tgit bisect--helper --next-all\n \tres=$?\n \n \t# Check if we should exit because bisection is finished\n-- \n2.24.3 (Apple Git-128)\n\n"},{"id":"403167","messageId":"20200807215838.19571-6-alipman88@gmail.com","threadId":"53937","inReplyTo":"20200807215838.19571-1-alipman88@gmail.com","subject":"[PATCH v5 5/5] bisect: combine args passed to find_bisection()","fromName":"Aaron Lipman","fromEmail":"alipman88@gmail.com","sentAt":"2020-08-07T21:58:38Z","receivedAt":"2020-08-07T21:58:56Z","isPatch":true,"sender":{"key":"alipman88@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3922511?v=4"},"body":"Now that find_bisection() accepts multiple boolean arguments, these may\nbe combined into a single unsigned integer in order to declutter some of\nthe code in bisect.c\n\nAlso, rename the existing \"flags\" bitfield to \"commit_flags\", to\nexplicitly differentiate it from the new \"bisect_flags\" bitfield.\n\nBased-on-patch-by: Harald Nordgren \nSigned-off-by: Aaron Lipman \n---\n bisect.c | 67 +++++++++++++++++++++++++---------------------\n bisect.h | 5 +++-\n builtin/rev-list.c | 9 ++++++-\n 3 files changed, 49 insertions(+), 32 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex bc4241b51f..1585fcc6ad 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -89,7 +89,7 @@ static inline void weight_set(struct commit_list *elem, int weight)\n \t**commit_weight_at(&commit_weight, elem->item) = weight;\n }\n \n-static int count_interesting_parents(struct commit *commit, int first_parent_only)\n+static int count_interesting_parents(struct commit *commit, unsigned bisect_flags)\n {\n \tstruct commit_list *p;\n \tint count;\n@@ -97,7 +97,7 @@ static int count_interesting_parents(struct commit *commit, int first_parent_onl\n \tfor (count = 0, p = commit->parents; p; p = p->next) {\n \t\tif (!(p->item->object.flags & UNINTERESTING))\n \t\t\tcount++;\n-\t\tif (first_parent_only)\n+\t\tif (bisect_flags & FIND_BISECTION_FIRST_PARENT_ONLY)\n \t\t\tbreak;\n \t}\n \treturn count;\n@@ -137,7 +137,7 @@ static void show_list(const char *debug, int counted, int nr,\n \tfor (p = list; p; p = p->next) {\n \t\tstruct commit_list *pp;\n \t\tstruct commit *commit = p->item;\n-\t\tunsigned flags = commit->object.flags;\n+\t\tunsigned commit_flags = commit->object.flags;\n \t\tenum object_type type;\n \t\tunsigned long size;\n \t\tchar *buf = read_object_file(&commit->object.oid, &type,\n@@ -146,9 +146,9 @@ static void show_list(const char *debug, int counted, int nr,\n \t\tint subject_len;\n \n \t\tfprintf(stderr, \"%c%c%c \",\n-\t\t\t(flags & TREESAME) ? ' ' : 'T',\n-\t\t\t(flags & UNINTERESTING) ? 'U' : ' ',\n-\t\t\t(flags & COUNTED) ? 'C' : ' ');\n+\t\t\t(commit_flags & TREESAME) ? ' ' : 'T',\n+\t\t\t(commit_flags & UNINTERESTING) ? 'U' : ' ',\n+\t\t\t(commit_flags & COUNTED) ? 'C' : ' ');\n \t\tif (*commit_weight_at(&commit_weight, p->item))\n \t\t\tfprintf(stderr, \"%3d\", weight(p));\n \t\telse\n@@ -173,9 +173,9 @@ static struct commit_list *best_bisection(struct commit_list *list, int nr)\n \tbest = list;\n \tfor (p = list; p; p = p->next) {\n \t\tint distance;\n-\t\tunsigned flags = p->item->object.flags;\n+\t\tunsigned commit_flags = p->item->object.flags;\n \n-\t\tif (flags & TREESAME)\n+\t\tif (commit_flags & TREESAME)\n \t\t\tcontinue;\n \t\tdistance = weight(p);\n \t\tif (nr - distance < distance)\n@@ -214,9 +214,9 @@ static struct commit_list *best_bisection_sorted(struct commit_list *list, int n\n \n \tfor (p = list, cnt = 0; p; p = p->next) {\n \t\tint distance;\n-\t\tunsigned flags = p->item->object.flags;\n+\t\tunsigned commit_flags = p->item->object.flags;\n \n-\t\tif (flags & TREESAME)\n+\t\tif (commit_flags & TREESAME)\n \t\t\tcontinue;\n \t\tdistance = weight(p);\n \t\tif (nr - distance < distance)\n@@ -261,7 +261,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, int first_parent_only)\n+\t\t\t\t\t unsigned bisect_flags)\n {\n \tint n, counted;\n \tstruct commit_list *p;\n@@ -270,12 +270,12 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\n \n \tfor (n = 0, p = list; p; p = p->next) {\n \t\tstruct commit *commit = p->item;\n-\t\tunsigned flags = commit->object.flags;\n+\t\tunsigned commit_flags = commit->object.flags;\n \n \t\t*commit_weight_at(&commit_weight, p->item) = &weights[n++];\n-\t\tswitch (count_interesting_parents(commit, first_parent_only)) {\n+\t\tswitch (count_interesting_parents(commit, bisect_flags)) {\n \t\tcase 0:\n-\t\t\tif (!(flags & TREESAME)) {\n+\t\t\tif (!(commit_flags & TREESAME)) {\n \t\t\t\tweight_set(p, 1);\n \t\t\t\tcounted++;\n \t\t\t\tshow_list(\"bisection 2 count one\",\n@@ -316,13 +316,13 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\n \t\t\tcontinue;\n \t\tif (weight(p) != -2)\n \t\t\tcontinue;\n-\t\tif (first_parent_only)\n+\t\tif (bisect_flags & FIND_BISECTION_FIRST_PARENT_ONLY)\n \t\t\tBUG(\"shouldn't be calling count-distance in fp mode\");\n \t\tweight_set(p, count_distance(p));\n \t\tclear_distance(list);\n \n \t\t/* Does it happen to be at exactly half-way? */\n-\t\tif (!find_all && halfway(p, nr))\n+\t\tif (!(bisect_flags & FIND_BISECTION_ALL) && halfway(p, nr))\n \t\t\treturn p;\n \t\tcounted++;\n \t}\n@@ -332,14 +332,14 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\n \twhile (counted < nr) {\n \t\tfor (p = list; p; p = p->next) {\n \t\t\tstruct commit_list *q;\n-\t\t\tunsigned flags = p->item->object.flags;\n+\t\t\tunsigned commit_flags = p->item->object.flags;\n \n \t\t\tif (0 <= weight(p))\n \t\t\t\tcontinue;\n \n \t\t\tfor (q = p->item->parents;\n \t\t\t q;\n-\t\t\t q = first_parent_only ? NULL : q->next) {\n+\t\t\t q = bisect_flags & FIND_BISECTION_FIRST_PARENT_ONLY ? NULL : q->next) {\n \t\t\t\tif (q->item->object.flags & UNINTERESTING)\n \t\t\t\t\tcontinue;\n \t\t\t\tif (0 <= weight(q))\n@@ -353,7 +353,7 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\n \t\t\t * add one for p itself if p is to be counted,\n \t\t\t * otherwise inherit it from q directly.\n \t\t\t */\n-\t\t\tif (!(flags & TREESAME)) {\n+\t\t\tif (!(commit_flags & TREESAME)) {\n \t\t\t\tweight_set(p, weight(q)+1);\n \t\t\t\tcounted++;\n \t\t\t\tshow_list(\"bisection 2 count one\",\n@@ -363,21 +363,21 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\n \t\t\t\tweight_set(p, weight(q));\n \n \t\t\t/* Does it happen to be at exactly half-way? */\n-\t\t\tif (!find_all && halfway(p, nr))\n+\t\t\tif (!(bisect_flags & FIND_BISECTION_ALL) && halfway(p, nr))\n \t\t\t\treturn p;\n \t\t}\n \t}\n \n \tshow_list(\"bisection 2 counted all\", counted, nr, list);\n \n-\tif (!find_all)\n+\tif (!(bisect_flags & FIND_BISECTION_ALL))\n \t\treturn best_bisection(list, nr);\n \telse\n \t\treturn best_bisection_sorted(list, nr);\n }\n \n void find_bisection(struct commit_list **commit_list, int *reaches,\n-\t\t int *all, int find_all, int first_parent_only)\n+\t\t int *all, unsigned bisect_flags)\n {\n \tint nr, on_list;\n \tstruct commit_list *list, *p, *best, *next, *last;\n@@ -393,16 +393,16 @@ void find_bisection(struct commit_list **commit_list, int *reaches,\n \tfor (nr = on_list = 0, last = NULL, p = *commit_list;\n \t p;\n \t p = next) {\n-\t\tunsigned flags = p->item->object.flags;\n+\t\tunsigned commit_flags = p->item->object.flags;\n \n \t\tnext = p->next;\n-\t\tif (flags & UNINTERESTING) {\n+\t\tif (commit_flags & UNINTERESTING) {\n \t\t\tfree(p);\n \t\t\tcontinue;\n \t\t}\n \t\tp->next = last;\n \t\tlast = p;\n-\t\tif (!(flags & TREESAME))\n+\t\tif (!(commit_flags & TREESAME))\n \t\t\tnr++;\n \t\ton_list++;\n \t}\n@@ -413,9 +413,9 @@ 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, first_parent_only);\n+\tbest = do_find_bisection(list, nr, weights, bisect_flags);\n \tif (best) {\n-\t\tif (!find_all) {\n+\t\tif (!(bisect_flags & FIND_BISECTION_ALL)) {\n \t\t\tlist->item = best->item;\n \t\t\tfree_commit_list(list->next);\n \t\t\tbest = list;\n@@ -1000,23 +1000,30 @@ enum bisect_error bisect_next_all(struct repository *r, const char *prefix)\n \tstruct object_id *bisect_rev;\n \tchar *steps_msg;\n \tint no_checkout = ref_exists(\"BISECT_HEAD\");\n-\tint first_parent_only = file_exists(git_path_bisect_first_parent());\n+\tunsigned bisect_flags = 0;\n \n \tread_bisect_terms(&term_bad, &term_good);\n \tif (read_bisect_refs())\n \t\tdie(_(\"reading bisect refs failed\"));\n \n+\tif (file_exists(git_path_bisect_first_parent()))\n+\t\tbisect_flags |= FIND_BISECTION_FIRST_PARENT_ONLY;\n+\n+\tif (skipped_revs.nr)\n+\t\tbisect_flags |= FIND_BISECTION_ALL;\n+\n \tres = check_good_are_ancestors_of_bad(r, prefix, no_checkout);\n \tif (res)\n \t\treturn res;\n \n \tbisect_rev_setup(r, &revs, prefix, \"%s\", \"^%s\", 1);\n-\trevs.first_parent_only = first_parent_only;\n+\n+\trevs.first_parent_only = !!(bisect_flags & FIND_BISECTION_FIRST_PARENT_ONLY);\n \trevs.limited = 1;\n \n \tbisect_common(&revs);\n \n-\tfind_bisection(&revs.commits, &reaches, &all, !!skipped_revs.nr, first_parent_only);\n+\tfind_bisection(&revs.commits, &reaches, &all, bisect_flags);\n \trevs.commits = managed_skipped(revs.commits, &tried);\n \n \tif (!revs.commits) {\ndiff --git a/bisect.h b/bisect.h\nindex 7f30b94db3..ec24ac2d7e 100644\n--- a/bisect.h\n+++ b/bisect.h\n@@ -12,7 +12,7 @@ struct repository;\n * best commit, as chosen by `find_all`.\n */\n void find_bisection(struct commit_list **list, int *reaches, int *all,\n-\t\t int find_all, int first_parent_only);\n+\t\t unsigned bisect_flags);\n \n struct commit_list *filter_skipped(struct commit_list *list,\n \t\t\t\t struct commit_list **tried,\n@@ -23,6 +23,9 @@ struct commit_list *filter_skipped(struct commit_list *list,\n #define BISECT_SHOW_ALL\t\t(1<<0)\n #define REV_LIST_QUIET\t\t(1<<1)\n \n+#define FIND_BISECTION_ALL\t\t\t(1u<<0)\n+#define FIND_BISECTION_FIRST_PARENT_ONLY\t(1u<<1)\n+\n struct rev_list_info {\n \tstruct rev_info *revs;\n \tint flags;\ndiff --git a/builtin/rev-list.c b/builtin/rev-list.c\nindex d1a14596b2..25c6c3b38d 100644\n--- a/builtin/rev-list.c\n+++ b/builtin/rev-list.c\n@@ -637,8 +637,15 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)\n \n \tif (bisect_list) {\n \t\tint reaches, all;\n+\t\tunsigned bisect_flags = 0;\n \n-\t\tfind_bisection(&revs.commits, &reaches, &all, bisect_find_all, revs.first_parent_only);\n+\t\tif (bisect_find_all)\n+\t\t\tbisect_flags |= FIND_BISECTION_ALL;\n+\n+\t\tif (revs.first_parent_only)\n+\t\t\tbisect_flags |= FIND_BISECTION_FIRST_PARENT_ONLY;\n+\n+\t\tfind_bisection(&revs.commits, &reaches, &all, bisect_flags);\n \n \t\tif (bisect_show_vars)\n \t\t\treturn show_bisect_vars(&info, reaches, all);\n-- \n2.24.3 (Apple Git-128)\n\n"},{"id":"403168","messageId":"20200807215838.19571-5-alipman88@gmail.com","threadId":"53937","inReplyTo":"20200807215838.19571-1-alipman88@gmail.com","subject":"[PATCH v5 4/5] bisect: introduce first-parent flag","fromName":"Aaron Lipman","fromEmail":"alipman88@gmail.com","sentAt":"2020-08-07T21:58:37Z","receivedAt":"2020-08-07T21:58:57Z","isPatch":true,"sender":{"key":"alipman88@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3922511?v=4"},"body":"Upon seeing a merge commit when bisecting, this option may be used to\nfollow only the first parent.\n\nIn detecting regressions introduced through the merging of a branch, the\nmerge commit will be identified as introduction of the bug and its\nancestors will be ignored.\n\nThis option is particularly useful in avoiding false positives when a\nmerged branch contained broken or non-buildable commits, but the merge\nitself was OK.\n\nSigned-off-by: Aaron Lipman \n---\n Documentation/git-bisect.txt | 13 ++++++++++++-\n bisect.c | 5 ++++-\n builtin/bisect--helper.c | 9 ++++++++-\n t/t6030-bisect-porcelain.sh | 18 ++++++++++++++++++\n 4 files changed, 42 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt\nindex 7586c5a843..0e993e4587 100644\n--- a/Documentation/git-bisect.txt\n+++ b/Documentation/git-bisect.txt\n@@ -17,7 +17,7 @@ The command takes various subcommands, and different options depending\n on the subcommand:\n \n git bisect start [--term-{old,good}= --term-{new,bad}=]\n-\t\t [--no-checkout] [ [...]] [--] [...]\n+\t\t [--no-checkout] [--first-parent] [ [...]] [--] [...]\n git bisect (bad|new|) []\n git bisect (good|old|) [...]\n git bisect terms [--term-good | --term-bad]\n@@ -365,6 +365,17 @@ does not require a checked out tree.\n +\n If the repository is bare, `--no-checkout` is assumed.\n \n+--first-parent::\n++\n+Follow only the first parent commit upon seeing a merge commit.\n++\n+In detecting regressions introduced through the merging of a branch, the merge\n+commit will be identified as introduction of the bug and its ancestors will be\n+ignored.\n++\n+This option is particularly useful in avoiding false positives when a merged\n+branch contained broken or non-buildable commits, but the merge itself was OK.\n+\n EXAMPLES\n --------\n \ndiff --git a/bisect.c b/bisect.c\nindex 950ff6f533..bc4241b51f 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -15,6 +15,7 @@\n #include \"commit-slab.h\"\n #include \"commit-reach.h\"\n #include \"object-store.h\"\n+#include \"dir.h\"\n \n static struct oid_array good_revs;\n static struct oid_array skipped_revs;\n@@ -460,6 +461,7 @@ static GIT_PATH_FUNC(git_path_bisect_run, \"BISECT_RUN\")\n static GIT_PATH_FUNC(git_path_bisect_start, \"BISECT_START\")\n static GIT_PATH_FUNC(git_path_bisect_log, \"BISECT_LOG\")\n static GIT_PATH_FUNC(git_path_bisect_terms, \"BISECT_TERMS\")\n+static GIT_PATH_FUNC(git_path_bisect_first_parent, \"BISECT_FIRST_PARENT\")\n static GIT_PATH_FUNC(git_path_head_name, \"head-name\")\n \n static void read_bisect_paths(struct argv_array *array)\n@@ -998,7 +1000,7 @@ enum bisect_error bisect_next_all(struct repository *r, const char *prefix)\n \tstruct object_id *bisect_rev;\n \tchar *steps_msg;\n \tint no_checkout = ref_exists(\"BISECT_HEAD\");\n-\tint first_parent_only = 0; /* TODO: pass --first-parent flag from git bisect start */\n+\tint first_parent_only = file_exists(git_path_bisect_first_parent());\n \n \tread_bisect_terms(&term_bad, &term_good);\n \tif (read_bisect_refs())\n@@ -1142,6 +1144,7 @@ int bisect_clean_state(void)\n \tunlink_or_warn(git_path_bisect_names());\n \tunlink_or_warn(git_path_bisect_run());\n \tunlink_or_warn(git_path_bisect_terms());\n+\tunlink_or_warn(git_path_bisect_first_parent());\n \t/* Cleanup head-name if it got left by an old version of git-bisect */\n \tunlink_or_warn(git_path_head_name());\n \t/*\ndiff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\nindex 95a15b918f..a055147f19 100644\n--- a/builtin/bisect--helper.c\n+++ b/builtin/bisect--helper.c\n@@ -16,6 +16,7 @@ static GIT_PATH_FUNC(git_path_bisect_start, \"BISECT_START\")\n static GIT_PATH_FUNC(git_path_bisect_log, \"BISECT_LOG\")\n static GIT_PATH_FUNC(git_path_head_name, \"head-name\")\n static GIT_PATH_FUNC(git_path_bisect_names, \"BISECT_NAMES\")\n+static GIT_PATH_FUNC(git_path_bisect_first_parent, \"BISECT_FIRST_PARENT\")\n \n static const char * const git_bisect_helper_usage[] = {\n \tN_(\"git bisect--helper --next-all\"),\n@@ -27,7 +28,7 @@ static const char * const git_bisect_helper_usage[] = {\n \tN_(\"git bisect--helper --bisect-next-check []\"),\n \tN_(\"git bisect--helper --bisect-terms [--term-good | --term-old | --term-bad | --term-new]\"),\n \tN_(\"git bisect--helper --bisect-start [--term-{old,good}= --term-{new,bad}=]\"\n-\t\t\t\t\t \"[--no-checkout] [ [...]] [--] [...]\"),\n+\t\t\t\t\t \" [--no-checkout] [--first-parent] [ [...]] [--] [...]\"),\n \tNULL\n };\n \n@@ -423,6 +424,7 @@ static int bisect_append_log_quoted(const char **argv)\n static int bisect_start(struct bisect_terms *terms, const char **argv, int argc)\n {\n \tint no_checkout = 0;\n+\tint first_parent_only = 0;\n \tint i, has_double_dash = 0, must_write_terms = 0, bad_seen = 0;\n \tint flags, pathspec_pos, res = 0;\n \tstruct string_list revs = STRING_LIST_INIT_DUP;\n@@ -452,6 +454,8 @@ static int bisect_start(struct bisect_terms *terms, const char **argv, int argc)\n \t\t\tbreak;\n \t\t} else if (!strcmp(arg, \"--no-checkout\")) {\n \t\t\tno_checkout = 1;\n+\t\t} else if (!strcmp(arg, \"--first-parent\")) {\n+\t\t\tfirst_parent_only = 1;\n \t\t} else if (!strcmp(arg, \"--term-good\") ||\n \t\t\t !strcmp(arg, \"--term-old\")) {\n \t\t\ti++;\n@@ -576,6 +580,9 @@ static int bisect_start(struct bisect_terms *terms, const char **argv, int argc)\n \t */\n \twrite_file(git_path_bisect_start(), \"%s\\n\", start_head.buf);\n \n+\tif (first_parent_only)\n+\t\twrite_file(git_path_bisect_first_parent(), \"\\n\");\n+\n \tif (no_checkout) {\n \t\tif (get_oid(start_head.buf, &oid) < 0) {\n \t\t\tres = error(_(\"invalid ref: '%s'\"), start_head.buf);\ndiff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh\nindex a66c4b89bc..b886529e59 100755\n--- a/t/t6030-bisect-porcelain.sh\n+++ b/t/t6030-bisect-porcelain.sh\n@@ -448,6 +448,24 @@ test_expect_success 'many merge bases creation' '\n \tgrep \"$SIDE_HASH5\" merge_bases.txt\n '\n \n+# We want to automatically find the merge that\n+# added \"line\" into hello.\n+test_expect_success '\"git bisect run --first-parent\" simple case' '\n+\tgit rev-list --first-parent $B_HASH ^$HASH4 >first_parent_chain.txt &&\n+\twrite_script test_script.sh <<-\\EOF &&\n+\tgrep $(git rev-parse HEAD) first_parent_chain.txt || exit -1\n+\t! grep line hello >/dev/null\n+\tEOF\n+\tgit bisect start --first-parent &&\n+\ttest_path_is_file \".git/BISECT_FIRST_PARENT\" &&\n+\tgit bisect good $HASH4 &&\n+\tgit bisect bad $B_HASH &&\n+\tgit bisect run ./test_script.sh >my_bisect_log.txt &&\n+\tgrep \"$B_HASH is the first bad commit\" my_bisect_log.txt &&\n+\tgit bisect reset &&\n+\ttest_path_is_missing .git/BISECT_FIRST_PARENT\n+'\n+\n test_expect_success 'good merge bases when good and bad are siblings' '\n \tgit bisect start \"$B_HASH\" \"$A_HASH\" > my_bisect_log.txt &&\n \ttest_i18ngrep \"merge base must be tested\" my_bisect_log.txt &&\n-- \n2.24.3 (Apple Git-128)\n\n"},{"id":"403170","messageId":"xmqq364yf6tt.fsf@gitster.c.googlers.com","threadId":"53937","inReplyTo":"CAPig+cT_1JNZOk7wZr3akdCmv7tn2RHieJd+A-usZ49jS0eCmA@mail.gmail.com","subject":"Re: [PATCH v4 0/5] Introduce --first-parent flag for git bisect","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-08-07T22:07:42Z","receivedAt":"2020-08-07T22:07:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n> Consequently, I'm not in a\n> position to say whether or not there are any fundamental issues with\n> the changes made by any of these patches.\n\nSorry, that wasn't what I meant. A reroll would be done with an\nupdated test, so I was asking your eyeball once again on that.\n"},{"id":"403173","messageId":"CAPig+cTm8foBehQC3wVDN6Lx1SO8EGhApVbUfQ0iaBnDU7OxtA@mail.gmail.com","threadId":"53937","inReplyTo":"xmqq364yf6tt.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v4 0/5] Introduce --first-parent flag for git bisect","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-08-07T22:20:29Z","receivedAt":"2020-08-07T22:20:44Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Aug 7, 2020 at 6:07 PM Junio C Hamano wrote:\n> Eric Sunshine writes:\n> > Consequently, I'm not in a\n> > position to say whether or not there are any fundamental issues with\n> > the changes made by any of these patches.\n>\n> Sorry, that wasn't what I meant. A reroll would be done with an\n> updated test, so I was asking your eyeball once again on that.\n\nI eyeballed the test in the re-rolled [2/5] from a purely style\nperspective (not a content perspective), and it looks fine. I could\nsay (and could previously have said) something about the lost exit\ncodes from the $(git rev-parse ...) invocations within the here-doc in\norder to save Denton the trouble of eradicating them, but that seems\nrelatively minor. Re-thinking the unindented here-docs on the other\ntests, I wonder now if that they might be easier to read if indented\nwith \\-EOF (rather than using plain EOF), as well:\n\n test_output_expect_success '--first-parent' 'git rev-list ...' <<-\\EOF\n E\n ...\n e8\n EOF\n\nbut I doubt that such a minor style change is worth a re-roll (and I\nwouldn't ask Aaron to re-roll just for that).\n"}]}