This will enable users to implement bisecting on first parents which can be useful for when the commits from a feature branch that we want to merge are not always tested. Signed-off-by: Tiago Botelho <tiagonbotelho@xxxxxxxxxxx> --- This patch refactors **only** the tests according to the suggestions made by Junio in v5 more specifically `https://public-inbox.org/git/xmqqa7rhi40f.fsf@xxxxxxxxxxxxxxxxxxxxxxxxx/`. The discussion between Junio, Christian and Johannes got pretty confusing in the sense that I am unsure if this is the actual problem I am supposed to solve in order to get the patch accepted, if it is not I will keep iterating on it until it is good enough to be merged bisect.c | 43 ++++++++++++++++++++++------------ bisect.h | 1 + builtin/rev-list.c | 3 +++ revision.c | 3 --- t/t6002-rev-list-bisect.sh | 58 ++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 90 insertions(+), 18 deletions(-) diff --git a/bisect.c b/bisect.c index 4eafc8262..cb80f29c5 100644 --- a/bisect.c +++ b/bisect.c @@ -82,15 +82,16 @@ static inline void weight_set(struct commit_list *elem, int weight) *((int*)(elem->item->util)) = weight; } -static int count_interesting_parents(struct commit *commit) +static int count_interesting_parents(struct commit *commit, unsigned bisect_flags) { struct commit_list *p; int count; for (count = 0, p = commit->parents; p; p = p->next) { - if (p->item->object.flags & UNINTERESTING) - continue; - count++; + if (!(p->item->object.flags & UNINTERESTING)) + count++; + if (bisect_flags & BISECT_FIRST_PARENT) + break; } return count; } @@ -117,10 +118,10 @@ static inline int halfway(struct commit_list *p, int nr) } #if !DEBUG_BISECT -#define show_list(a,b,c,d) do { ; } while (0) +#define show_list(a,b,c,d,e) do { ; } while (0) #else static void show_list(const char *debug, int counted, int nr, - struct commit_list *list) + struct commit_list *list, unsigned bisect_flags) { struct commit_list *p; @@ -146,10 +147,14 @@ static void show_list(const char *debug, int counted, int nr, else fprintf(stderr, "---"); fprintf(stderr, " %.*s", 8, oid_to_hex(&commit->object.oid)); - for (pp = commit->parents; pp; pp = pp->next) + for (pp = commit->parents; pp; pp = pp->next) { fprintf(stderr, " %.*s", 8, oid_to_hex(&pp->item->object.oid)); + if (bisect_flags & BISECT_FIRST_PARENT) + break; + } + subject_len = find_commit_subject(buf, &subject_start); if (subject_len) fprintf(stderr, " %.*s", subject_len, subject_start); @@ -267,13 +272,13 @@ static struct commit_list *do_find_bisection(struct commit_list *list, unsigned flags = commit->object.flags; p->item->util = &weights[n++]; - switch (count_interesting_parents(commit)) { + switch (count_interesting_parents(commit, bisect_flags)) { case 0: if (!(flags & TREESAME)) { weight_set(p, 1); counted++; show_list("bisection 2 count one", - counted, nr, list); + counted, nr, list, bisect_flags); } /* * otherwise, it is known not to reach any @@ -289,7 +294,7 @@ static struct commit_list *do_find_bisection(struct commit_list *list, } } - show_list("bisection 2 initialize", counted, nr, list); + show_list("bisection 2 initialize", counted, nr, list, bisect_flags); /* * If you have only one parent in the resulting set @@ -319,7 +324,7 @@ static struct commit_list *do_find_bisection(struct commit_list *list, counted++; } - show_list("bisection 2 count_distance", counted, nr, list); + show_list("bisection 2 count_distance", counted, nr, list, bisect_flags); while (counted < nr) { for (p = list; p; p = p->next) { @@ -329,6 +334,11 @@ static struct commit_list *do_find_bisection(struct commit_list *list, if (0 <= weight(p)) continue; for (q = p->item->parents; q; q = q->next) { + if ((bisect_flags & BISECT_FIRST_PARENT)) { + if (weight(q) < 0) + q = NULL; + break; + } if (q->item->object.flags & UNINTERESTING) continue; if (0 <= weight(q)) @@ -346,7 +356,7 @@ static struct commit_list *do_find_bisection(struct commit_list *list, weight_set(p, weight(q)+1); counted++; show_list("bisection 2 count one", - counted, nr, list); + counted, nr, list, bisect_flags); } else weight_set(p, weight(q)); @@ -357,7 +367,7 @@ static struct commit_list *do_find_bisection(struct commit_list *list, } } - show_list("bisection 2 counted all", counted, nr, list); + show_list("bisection 2 counted all", counted, nr, list, bisect_flags); if (!find_all) return best_bisection(list, nr); @@ -372,7 +382,7 @@ void find_bisection(struct commit_list **commit_list, int *reaches, struct commit_list *list, *p, *best, *next, *last; int *weights; - show_list("bisection 2 entry", 0, 0, *commit_list); + show_list("bisection 2 entry", 0, 0, *commit_list, bisect_flags); /* * Count the number of total and tree-changing items on the @@ -395,7 +405,7 @@ void find_bisection(struct commit_list **commit_list, int *reaches, on_list++; } list = last; - show_list("bisection 2 sorted", 0, nr, list); + show_list("bisection 2 sorted", 0, nr, list, bisect_flags); *all = nr; weights = xcalloc(on_list, sizeof(*weights)); @@ -962,6 +972,9 @@ int bisect_next_all(const char *prefix, int no_checkout) if (skipped_revs.nr) bisect_flags |= BISECT_FIND_ALL; + if (revs.first_parent_only) + bisect_flags |= BISECT_FIRST_PARENT; + find_bisection(&revs.commits, &reaches, &all, bisect_flags); revs.commits = managed_skipped(revs.commits, &tried); diff --git a/bisect.h b/bisect.h index 1d40a33ad..e445567c2 100644 --- a/bisect.h +++ b/bisect.h @@ -2,6 +2,7 @@ #define BISECT_H #define BISECT_FIND_ALL (1u<<0) +#define BISECT_FIRST_PARENT (1u<<1) /* * Find bisection. If something is found, `reaches` will be the number of diff --git a/builtin/rev-list.c b/builtin/rev-list.c index 8752f5bbe..66439e1b3 100644 --- a/builtin/rev-list.c +++ b/builtin/rev-list.c @@ -538,6 +538,9 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix) if (bisect_list) { int reaches, all; + if (revs.first_parent_only) + bisect_flags |= BISECT_FIRST_PARENT; + find_bisection(&revs.commits, &reaches, &all, bisect_flags); if (bisect_show_vars) diff --git a/revision.c b/revision.c index 4e0e193e5..b9d063805 100644 --- a/revision.c +++ b/revision.c @@ -2474,9 +2474,6 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s if (!revs->reflog_info && revs->grep_filter.use_reflog_filter) die("cannot use --grep-reflog without --walk-reflogs"); - if (revs->first_parent_only && revs->bisect) - die(_("--first-parent is incompatible with --bisect")); - if (revs->expand_tabs_in_log < 0) revs->expand_tabs_in_log = revs->expand_tabs_in_log_default; diff --git a/t/t6002-rev-list-bisect.sh b/t/t6002-rev-list-bisect.sh index a66140803..1bc297de5 100755 --- a/t/t6002-rev-list-bisect.sh +++ b/t/t6002-rev-list-bisect.sh @@ -263,4 +263,62 @@ test_expect_success 'rev-parse --bisect can default to good/bad refs' ' test_cmp expect.sorted actual.sorted ' +# We generate the following commit graph: +# +# B ------ C +# / \ +# A FX +# \ / +# D - CC - EX + +test_expect_success 'setup' ' + test_commit A && + test_commit B && + test_commit C && + git reset --hard A && + test_commit D && + test_commit CC && + test_commit EX && + test_merge FX C +' + +test_output_expect_success "--bisect --first-parent" 'git rev-list --bisect --first-parent FX ^A' <<EOF +$(git rev-parse CC) +EOF + +test_output_expect_success "--first-parent" 'git rev-list --first-parent FX ^A' <<EOF +$(git rev-parse FX) +$(git rev-parse EX) +$(git rev-parse CC) +$(git rev-parse D) +EOF + +test_output_expect_success "--bisect-vars --first-parent" 'git rev-list --bisect-vars --first-parent FX ^A' <<EOF +bisect_rev='$(git rev-parse CC)' +bisect_nr=1 +bisect_good=1 +bisect_bad=1 +bisect_all=4 +bisect_steps=1 +EOF + +test_expect_success "--bisect-all --first-parent" ' +cat >expect <<EOF && +$(git rev-parse CC) (dist=2) +$(git rev-parse EX) (dist=1) +$(git rev-parse D) (dist=1) +$(git rev-parse FX) (dist=0) +EOF + +# Make sure we have the same entries, nothing more, nothing less +git rev-list --bisect-all --first-parent FX ^A >actual && + sort actual >actual.sorted && + sort expect >expect.sorted && + test_cmp expect.sorted actual.sorted && + # Make sure the entries are sorted in the dist order + sed -e "s/.*(dist=\([1-9]*[0-9]\)).*/\1/" actual >actual.dists && + sort -r actual.dists >actual.dists.sorted && + test_cmp actual.dists.sorted actual.dists +' + test_done -- 2.16.3