When ran with '--merges-only', git bisect will only look at merge commits -- commits with 2 or more parents or the initial commit. Signed-off-by: Harald Nordgren <haraldnordgren@xxxxxxxxx> --- Notes: Proof of concept of a feature that I have wanted in Git for a while. In my daily work my company uses GitHub, which creates lots of merge commits. In general, tests are only ran on the tip of a branch before merging, so the different commits within a merge commit are allowed not to be buildable. Therefore 'git bisect' often doesn't work since it will give lots of false positives for anything that is not a merge commit. If we could have a feature to only bisect merge commits then it would be easier to pinpoint which merge causes any particular issue. After that, a bisect could be done within the merge to pinpoint futher. As a follow-up to this patch -- we could potentially create a feature that automatically continues into regular bisect within the bad merge commit after completed '--merges-only' bisection. bisect.c | 87 ++++++++++++++++++++++++++---- bisect.h | 4 +- builtin/bisect--helper.c | 28 +++++++++- builtin/rev-list.c | 2 +- git-bisect.sh | 5 ++ t/t6070-bisect-merge-commits.sh | 116 ++++++++++++++++++++++++++++++++++++++++ 6 files changed, 228 insertions(+), 14 deletions(-) create mode 100755 t/t6070-bisect-merge-commits.sh diff --git a/bisect.c b/bisect.c index a579b5088..e8a2f77c7 100644 --- a/bisect.c +++ b/bisect.c @@ -254,7 +254,7 @@ static struct commit_list *best_bisection_sorted(struct commit_list *list, int n */ static struct commit_list *do_find_bisection(struct commit_list *list, int nr, int *weights, - int find_all) + int find_all, int only_merge_commits) { int n, counted; struct commit_list *p; @@ -266,6 +266,13 @@ static struct commit_list *do_find_bisection(struct commit_list *list, unsigned flags = commit->object.flags; p->item->util = &weights[n++]; + + if (only_merge_commits) { + weight_set(p, -2); + counted++; + continue; + } + switch (count_interesting_parents(commit)) { case 0: if (!(flags & TREESAME)) { @@ -305,11 +312,17 @@ static struct commit_list *do_find_bisection(struct commit_list *list, * way, and then fill the blanks using cheaper algorithm. */ for (p = list; p; p = p->next) { + int distance; if (p->item->object.flags & UNINTERESTING) continue; if (weight(p) != -2) continue; - weight_set(p, count_distance(p)); + + if (only_merge_commits) + distance = count_distance(p) - 1; + else + distance = count_distance(p); + weight_set(p, distance); clear_distance(list); /* Does it happen to be at exactly half-way? */ @@ -330,11 +343,17 @@ static struct commit_list *do_find_bisection(struct commit_list *list, for (q = p->item->parents; q; q = q->next) { if (q->item->object.flags & UNINTERESTING) continue; + if (!q->item->util) + break; if (0 <= weight(q)) break; } if (!q) continue; + if (!q->item->util) { + counted++; + continue; + } /* * weight for p is unknown but q is known. @@ -364,8 +383,43 @@ static struct commit_list *do_find_bisection(struct commit_list *list, return best_bisection_sorted(list, nr); } +int merge_commit_or_root(const struct commit c) +{ + if (!c.parents) + return 1; + + return !!c.parents->next; +} + +void filter_non_merge_commits(struct commit_list **commit_list) +{ + struct commit_list *list1 = *commit_list; + struct commit_list *list2 = NULL; + *commit_list = NULL; + + for ( ; list1; list1 = list1->next) { + if (merge_commit_or_root(*list1->item)) { + list2 = list1; + list1 = list1->next; + list2->next = NULL; + list2->item->parents = NULL; + *commit_list = list2; + break; + } + } + + for ( ; list1; list1 = list1->next) { + list2->next = NULL; + if (merge_commit_or_root(*list1->item)) { + list2->next = list1; + list2 = list2->next; + list2->item->parents = list1; + } + } +} + void find_bisection(struct commit_list **commit_list, int *reaches, - int *all, int find_all) + int *all, int find_all, int only_merge_commits) { int nr, on_list; struct commit_list *list, *p, *best, *next, *last; @@ -373,6 +427,10 @@ void find_bisection(struct commit_list **commit_list, int *reaches, show_list("bisection 2 entry", 0, 0, *commit_list); + if (only_merge_commits) { + filter_non_merge_commits(commit_list); + } + /* * Count the number of total and tree-changing items on the * list, while reversing the list. @@ -400,7 +458,7 @@ void find_bisection(struct commit_list **commit_list, int *reaches, weights = xcalloc(on_list, sizeof(*weights)); /* Do the real work of finding bisection commit. */ - best = do_find_bisection(list, nr, weights, find_all); + best = do_find_bisection(list, nr, weights, find_all, only_merge_commits); if (best) { if (!find_all) { list->item = best->item; @@ -878,7 +936,7 @@ static void check_good_are_ancestors_of_bad(const char *prefix, int no_checkout) /* * This does "git diff-tree --pretty COMMIT" without one fork+exec. */ -static void show_diff_tree(const char *prefix, struct commit *commit) +static void show_diff_tree(const char *prefix, struct commit *commit, int only_merge_commits) { struct rev_info opt; @@ -893,6 +951,11 @@ static void show_diff_tree(const char *prefix, struct commit *commit) opt.use_terminator = 0; opt.commit_format = CMIT_FMT_DEFAULT; + if (only_merge_commits) { + opt.ignore_merges = 0; + opt.combine_merges = 1; + } + /* diff-tree init */ if (!opt.diffopt.output_format) opt.diffopt.output_format = DIFF_FORMAT_RAW; @@ -938,7 +1001,7 @@ void read_bisect_terms(const char **read_bad, const char **read_good) * If no_checkout is non-zero, the bisection process does not * checkout the trial commit but instead simply updates BISECT_HEAD. */ -int bisect_next_all(const char *prefix, int no_checkout) +int bisect_next_all(const char *prefix, int no_checkout, int only_merge_commits) { struct rev_info revs; struct commit_list *tried; @@ -957,7 +1020,7 @@ int bisect_next_all(const char *prefix, int no_checkout) bisect_common(&revs); - find_bisection(&revs.commits, &reaches, &all, !!skipped_revs.nr); + find_bisection(&revs.commits, &reaches, &all, !!skipped_revs.nr, only_merge_commits); revs.commits = managed_skipped(revs.commits, &tried); if (!revs.commits) { @@ -983,10 +1046,14 @@ int bisect_next_all(const char *prefix, int no_checkout) bisect_rev = &revs.commits->item->object.oid; if (!oidcmp(bisect_rev, current_bad_oid)) { + char *format_string = NULL; exit_if_skipped_commits(tried, current_bad_oid); - printf("%s is the first %s commit\n", oid_to_hex(bisect_rev), - term_bad); - show_diff_tree(prefix, revs.commits->item); + if (only_merge_commits) + format_string = "%s is the first %s merge\n"; + else + format_string = "%s is the first %s commit\n"; + printf(format_string, oid_to_hex(bisect_rev), term_bad); + show_diff_tree(prefix, revs.commits->item, only_merge_commits); /* This means the bisection process succeeded. */ exit(10); } diff --git a/bisect.h b/bisect.h index a5d9248a4..664ada180 100644 --- a/bisect.h +++ b/bisect.h @@ -9,7 +9,7 @@ * best commit, as chosen by `find_all`. */ extern void find_bisection(struct commit_list **list, int *reaches, int *all, - int find_all); + int find_all, int only_merge_commits); extern struct commit_list *filter_skipped(struct commit_list *list, struct commit_list **tried, @@ -28,7 +28,7 @@ struct rev_list_info { const char *header_prefix; }; -extern int bisect_next_all(const char *prefix, int no_checkout); +extern int bisect_next_all(const char *prefix, int no_checkout, int only_merge_commits); extern int estimate_bisect_steps(int all); diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c index 4b5fadcbe..9d7a1dd23 100644 --- a/builtin/bisect--helper.c +++ b/builtin/bisect--helper.c @@ -106,6 +106,32 @@ static void check_expected_revs(const char **revs, int rev_nr) } } +static GIT_PATH_FUNC(git_path_bisect_merges_only, "MERGES_ONLY_BISECT") + +static int merges_only(void) +{ + const char *filename = git_path_bisect_merges_only(); + struct stat st; + struct strbuf str = STRBUF_INIT; + FILE *fp; + int res = 0; + + if (stat(filename, &st) || !S_ISREG(st.st_mode)) + return 0; + + fp = fopen_or_warn(filename, "r"); + if (!fp) + return 0; + + if (strbuf_getline_lf(&str, fp) != EOF) + res = atoi(str.buf); + + strbuf_release(&str); + fclose(fp); + + return res; +} + int cmd_bisect__helper(int argc, const char **argv, const char *prefix) { enum { @@ -137,7 +163,7 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix) switch (cmdmode) { case NEXT_ALL: - return bisect_next_all(prefix, no_checkout); + return bisect_next_all(prefix, no_checkout, merges_only()); case WRITE_TERMS: if (argc != 2) return error(_("--write-terms requires two arguments")); diff --git a/builtin/rev-list.c b/builtin/rev-list.c index fadd3ec14..cacffe6c6 100644 --- a/builtin/rev-list.c +++ b/builtin/rev-list.c @@ -538,7 +538,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix) if (bisect_list) { int reaches, all; - find_bisection(&revs.commits, &reaches, &all, bisect_find_all); + find_bisection(&revs.commits, &reaches, &all, bisect_find_all, 0); if (bisect_show_vars) return show_bisect_vars(&info, reaches, all); diff --git a/git-bisect.sh b/git-bisect.sh index 54cbfecc5..730c983c5 100755 --- a/git-bisect.sh +++ b/git-bisect.sh @@ -82,6 +82,7 @@ bisect_start() { bad_seen=0 eval='' must_write_terms=0 + merges_only=0 revs='' if test "z$(git rev-parse --is-bare-repository)" != zfalse then @@ -96,6 +97,9 @@ bisect_start() { shift break ;; + --merges-only) + merges_only=1 + shift ;; --no-checkout) mode=--no-checkout shift ;; @@ -212,6 +216,7 @@ bisect_start() { git bisect--helper --write-terms "$TERM_BAD" "$TERM_GOOD" || exit fi && echo "git bisect start$orig_args" >>"$GIT_DIR/BISECT_LOG" || exit + echo "$merges_only" >"$GIT_DIR/MERGES_ONLY_BISECT" || exit # # Check if we can proceed to the next bisect state. # diff --git a/t/t6070-bisect-merge-commits.sh b/t/t6070-bisect-merge-commits.sh new file mode 100755 index 000000000..edfae0cfd --- /dev/null +++ b/t/t6070-bisect-merge-commits.sh @@ -0,0 +1,116 @@ +#!/bin/sh +# +# Copyright (c) 2018 Harald Nordgren +# +test_description='Tests git bisect merge commit functionality' + +exec </dev/null + +. ./test-lib.sh + +add_line_into_file() +{ + _line=$1 + _file=$2 + + if [ -f "$_file" ]; then + echo "$_line" >> $_file || return $? + MSG="Add <$_line> into <$_file>." + else + echo "$_line" > $_file || return $? + git add $_file || return $? + MSG="Create file <$_file> with <$_line> inside." + fi + + test_tick + git commit --quiet -m "$MSG" $_file +} + +create_merge_commit() +{ + _branch=$1 + _file=$2 + + git checkout -b $_branch && + add_line_into_file "hello" $_file && + git checkout master && + git merge $_branch --no-edit --no-ff +} + +HASH1= +HASH2= +HASH3= +HASH4= +HASH5= +HASH6= + +test_expect_success 'set up basic repo with 3 files and 3 merge commits' ' + add_line_into_file "hello" hello && + HASH1=$(git rev-parse --verify HEAD) && + + add_line_into_file "hello" hello && + git checkout -b branch1 && + add_line_into_file "hello" hello1 && + git checkout master && + git checkout -b branch2 && + add_line_into_file "hello" hello2 && + git checkout master && + git checkout -b branch3 && + add_line_into_file "hello" hello3 && + git checkout master && + git merge branch1 --no-edit --no-ff && + HASH2=$(git rev-parse --verify HEAD) && + + add_line_into_file "hello" hello && + add_line_into_file "hello" hello && + git merge branch2 --no-edit --no-ff && + git merge branch3 --no-edit --no-ff && + HASH3=$(git rev-parse --verify HEAD) && + + add_line_into_file "hello" hello && + HASH4=$(git rev-parse --verify HEAD) && + + create_merge_commit branch4 hello4 && + create_merge_commit branch5 hello5 && + create_merge_commit branch6 hello6 && + create_merge_commit branch7 hello7 && + create_merge_commit branch8 hello8 && + create_merge_commit branch9 hello9 && + create_merge_commit branch10 hello10 && + create_merge_commit branch11 hello11 && + create_merge_commit branch12 hello12 && + create_merge_commit branch13 hello13 && + create_merge_commit branch14 hello14 && + create_merge_commit branch15 hello15 && + create_merge_commit branch16 hello16 && + HASH5=$(git rev-parse --verify HEAD) && + + create_merge_commit branch17 hello17 && + create_merge_commit branch18 hello18 && + create_merge_commit branch19 hello19 && + create_merge_commit branch20 hello20 && + HASH6=$(git rev-parse --verify HEAD) +' + +test_expect_success 'bisect skip: successful result' ' + test_when_finished git bisect reset && + git bisect reset && + git bisect start $HASH4 $HASH1 --merges-only && + git bisect good && + git bisect good && + git bisect bad > my_bisect_log.txt && + grep "$HASH3 is the first bad merge" my_bisect_log.txt +' + +test_expect_success 'bisect skip: successful result' ' + test_when_finished git bisect reset && + git bisect reset && + git bisect start $HASH6 $HASH2 --merges-only && + git bisect good && + git bisect good && + git bisect bad && + git bisect bad > my_bisect_log.txt && + grep "$HASH5 is the first bad merge" my_bisect_log.txt +' + +test_done -- 2.14.3 (Apple Git-98)