@matheusbsbs / matheus-bagliano / commits / 8376a70441

branch: clean up commit flags after merge-filter walk

When we run `branch --merged`, we use prepare_revision_walk with the merge-filter marked as UNINTERESTING. Any branch tips that are marked UNINTERESTING after it returns must be ancestors of that commit. As we iterate through the list of refs to show, we check item->commit->object.flags to see whether it was marked. This interacts badly with --verbose, which will do a separate walk to find the ahead/behind information for each branch. There are two bad things that can happen: 1. The ahead/behind walk may get the wrong results, because it can see a bogus UNINTERESTING flag leftover from the merge-filter walk. 2. We may omit some branches if their tips are involved in the ahead/behind traversal of a branch shown earlier. The ahead/behind walk carefully cleans up its commit flags, meaning it may also erase the UNINTERESTING flag that we expect to check later. We can solve this by moving the merge-filter state for each ref into its "struct ref_item" as soon as we finish the merge-filter walk. That fixes (2). Then we are free to clear the commit flags we used in the walk, fixing (1). Note that we actually do away with the matches_merge_filter helper entirely here, and inline it between the revision walk and the flag-clearing. This ensures that nobody accidentally calls it at the wrong time (it is only safe to check in that instant between the setting and clearing of the global flag). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Sep 18, 2014 at 06:49 UTC 8376a704419813c9af1d056cbd36b9ff6744c8bc

2 files changed +48 -14

builtin/branch.c

+19 -14

@@ -280,6 +280,7 @@ struct ref_item {

280 char *dest;

281 unsigned int kind, width;

282 struct commit *commit;

283 + int ignore;

284 };

285

286 struct ref_list {

@@ -385,6 +386,7 @@ static int append_ref(const char *refname, const unsigned char *sha1, int flags,

386 newitem->commit = commit;

387 newitem->width = utf8_strwidth(refname);

388 newitem->dest = resolve_symref(orig_refname, prefix);

389 + newitem->ignore = 0;

390 /* adjust for "remotes/" */

391 if (newitem->kind == REF_REMOTE_BRANCH &&

392 ref_list->kinds != REF_REMOTE_BRANCH)

@@ -484,17 +486,6 @@ static void fill_tracking_info(struct strbuf *stat, const char *branch_name,

486 free(ref);

487 }

488

487 -static int matches_merge_filter(struct commit *commit)

488 -{

489 - int is_merged;

490 -

491 - if (merge_filter == NO_FILTER)

492 - return 1;

493 -

494 - is_merged = !!(commit->object.flags & UNINTERESTING);

495 - return (is_merged == (merge_filter == SHOW_MERGED));

496 -}

497 -

489 static void add_verbose_info(struct strbuf *out, struct ref_item *item,

490 int verbose, int abbrev)

491 {

@@ -522,10 +513,9 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose,

513 {

514 char c;

515 int color;

525 - struct commit *commit = item->commit;

516 struct strbuf out = STRBUF_INIT, name = STRBUF_INIT;

517

528 - if (!matches_merge_filter(commit))

518 + if (item->ignore)

519 return;

520

521 switch (item->kind) {

@@ -575,7 +565,7 @@ static int calc_maxwidth(struct ref_list *refs)

565 {

566 int i, w = 0;

567 for (i = 0; i < refs->index; i++) {

578 - if (!matches_merge_filter(refs->list[i].commit))

568 + if (refs->list[i].ignore)

569 continue;

570 if (refs->list[i].width > w)

571 w = refs->list[i].width;

@@ -618,6 +608,7 @@ static void show_detached(struct ref_list *ref_list)

608 item.kind = REF_LOCAL_BRANCH;

609 item.dest = NULL;

610 item.commit = head_commit;

611 + item.ignore = 0;

612 if (item.width > ref_list->maxwidth)

613 ref_list->maxwidth = item.width;

614 print_ref_item(&item, ref_list->maxwidth, ref_list->verbose, ref_list->abbrev, 1, "");

@@ -656,6 +647,20 @@ static int print_ref_list(int kinds, int detached, int verbose, int abbrev, stru

647

648 if (prepare_revision_walk(&ref_list.revs))

649 die(_("revision walk setup failed"));

650 +

651 + for (i = 0; i < ref_list.index; i++) {

652 + struct ref_item *item = &ref_list.list[i];

653 + struct commit *commit = item->commit;

654 + int is_merged = !!(commit->object.flags & UNINTERESTING);

655 + item->ignore = is_merged != (merge_filter == SHOW_MERGED);

656 + }

657 +

658 + for (i = 0; i < ref_list.index; i++) {

659 + struct ref_item *item = &ref_list.list[i];

660 + clear_commit_marks(item->commit, ALL_REV_FLAGS);

661 + }

662 + clear_commit_marks(filter, ALL_REV_FLAGS);

663 +

664 if (verbose)

665 ref_list.maxwidth = calc_maxwidth(&ref_list);

666 }

t/t3201-branch-contains.sh

+29

@@ -130,4 +130,33 @@ test_expect_success 'implicit --list conflicts with modification options' '

130

131 '

132

133 +# We want to set up a case where the walk for the tracking info

134 +# of one branch crosses the tip of another branch (and make sure

135 +# that the latter walk does not mess up our flag to see if it was

136 +# merged).

137 +#

138 +# Here "topic" tracks "master" with one extra commit, and "zzz" points to the

139 +# same tip as master The name "zzz" must come alphabetically after "topic"

140 +# as we process them in that order.

141 +test_expect_success 'branch --merged with --verbose' '

142 + git branch --track topic master &&

143 + git branch zzz topic &&

144 + git checkout topic &&

145 + test_commit foo &&

146 + git branch --merged topic >actual &&

147 + cat >expect <<-\EOF &&

148 + master

149 + * topic

150 + zzz

151 + EOF

152 + test_cmp expect actual &&

153 + git branch --verbose --merged topic >actual &&

154 + cat >expect <<-\EOF &&

155 + master c77a0a9 second on master

156 + * topic 2c939f4 [ahead 1] foo

157 + zzz c77a0a9 second on master

158 + EOF

159 + test_cmp expect actual

160 +'

161 +

162 test_done