{"thread":{"id":"37560","subject":"git branch --merged and git branch --verbose do not combine","startedAt":"2014-09-14T07:52:04Z","lastAt":"2014-09-18T10:49:43Z","messageCount":5,"participants":["David Kastrup","Junio C Hamano","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"249389","messageId":"87fvfutza3.fsf@fencepost.gnu.org","threadId":"37560","inReplyTo":null,"subject":"git branch --merged and git branch --verbose do not combine","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2014-09-14T07:52:04Z","receivedAt":"2014-09-14T07:52:04Z","isPatch":false,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"\ndak@lola:/usr/local/tmp/lilypond$ ../git/git branch --merged --verbose\nfatal: malformed object name --verbose\ndak@lola:/usr/local/tmp/lilypond$ ../git/git branch --verbose --merged\n* (detached from 5b2267a) 1c23f39 Merge branch 'issue4097' into HEAD\n  issue346                8cf033e Issue 346: monochord issues\ndak@lola:/usr/local/tmp/lilypond$ ../git/git branch --merged\n* (detached from 5b2267a)\n  issue346\n  issue3855\n  issue4030\n  issue4083\n  issue4092\n  issue4096\n  issue4097\n  removelyr\n  test-master\n  test-staging\n\nSo depending on the order of --verbose and --merged, one gets an\nincomplete list (basically ignoring --merged) or an error message.\n\nSince both behavior and naming of the --verbose option is more or less\northogonal to other listing options, it would make sense to allow it to\ncombine with them.\n\n-- \nDavid Kastrup\n"},{"id":"249440","messageId":"xmqq38bs92sg.fsf@gitster.dls.corp.google.com","threadId":"37560","inReplyTo":"87fvfutza3.fsf@fencepost.gnu.org","subject":"Re: git branch --merged and git branch --verbose do not combine","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-09-15T18:06:23Z","receivedAt":"2014-09-15T18:06:23Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Kastrup <dak@gnu.org> writes:\n\n> dak@lola:/usr/local/tmp/lilypond$ ../git/git branch --merged --verbose\n> fatal: malformed object name --verbose\n\nOnly at the very end of the command line if you omit something that\nis required, Git helps by defaulting the missing rev to HEAD.  You\ncan be a bit more explicit in the middle, i.e. instead of asking\n\"Which are branches that already has been merged in --verbose?\", you\ncan ask \"branch --merged HEAD --verbose\", meaning \"What are branhes\nthat already has been merged in HEAD, please give me a verbose\nanswer?\" perhaps?\n"},{"id":"249448","messageId":"87a960r9d2.fsf@fencepost.gnu.org","threadId":"37560","inReplyTo":"xmqq38bs92sg.fsf@gitster.dls.corp.google.com","subject":"Re: git branch --merged and git branch --verbose do not combine","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2014-09-15T19:07:05Z","receivedAt":"2014-09-15T19:07:05Z","isPatch":false,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> David Kastrup <dak@gnu.org> writes:\n>\n>> dak@lola:/usr/local/tmp/lilypond$ ../git/git branch --merged --verbose\n>> fatal: malformed object name --verbose\n>\n> Only at the very end of the command line if you omit something that\n> is required, Git helps by defaulting the missing rev to HEAD.  You\n> can be a bit more explicit in the middle, i.e. instead of asking\n> \"Which are branches that already has been merged in --verbose?\", you\n> can ask \"branch --merged HEAD --verbose\", meaning \"What are branhes\n> that already has been merged in HEAD, please give me a verbose\n> answer?\" perhaps?\n\nThis gives the same result as\n\ngit branch --verbose --merged\n\nnamely _only_ listing the current branch verbosely.  Use of --verbose\nkills any effect of --merged: instead of branches merged to the named\nbranch (or to the default of HEAD), _only_ the named branch (or the\ndefault of HEAD) gets listed.\n\nSo no.\n\n-- \nDavid Kastrup\n"},{"id":"249458","messageId":"xmqqwq944ov0.fsf@gitster.dls.corp.google.com","threadId":"37560","inReplyTo":"87a960r9d2.fsf@fencepost.gnu.org","subject":"Re: git branch --merged and git branch --verbose do not combine","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-09-15T20:20:51Z","receivedAt":"2014-09-15T20:20:51Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Kastrup <dak@gnu.org> writes:\n\n> This gives the same result as\n>\n> git branch --verbose --merged\n>\n> namely _only_ listing the current branch verbosely.\n\nHmph.  Then that is a different issue.  As I never use --merged\nmyself, even though I use \"git branch [--verbose] --no-merged pu\"\nquite often to check the list of topics that I received but not yet\nmerged anywhere in my tree, I wouldn't be entirely surprised that\nthe combination does not work, but with a quick glance of the code,\nespecially how matches_merge_filter() uses \"--[no-]merged\", I cannot\noffhand imagine how --no-merged would work and --merged break.\n"},{"id":"249630","messageId":"20140918104943.GA13481@peff.net","threadId":"37560","inReplyTo":"xmqqwq944ov0.fsf@gitster.dls.corp.google.com","subject":"[PATCH] branch: clean up commit flags after merge-filter walk","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-09-18T10:49:43Z","receivedAt":"2014-09-18T10:49:43Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Sep 15, 2014 at 01:20:51PM -0700, Junio C Hamano wrote:\n\n> David Kastrup <dak@gnu.org> writes:\n> \n> > This gives the same result as\n> >\n> > git branch --verbose --merged\n> >\n> > namely _only_ listing the current branch verbosely.\n> \n> Hmph.  Then that is a different issue.  As I never use --merged\n> myself, even though I use \"git branch [--verbose] --no-merged pu\"\n> quite often to check the list of topics that I received but not yet\n> merged anywhere in my tree, I wouldn't be entirely surprised that\n> the combination does not work, but with a quick glance of the code,\n> especially how matches_merge_filter() uses \"--[no-]merged\", I cannot\n> offhand imagine how --no-merged would work and --merged break.\n\nUgh. The bug is caused by a failure to clean up the global flag state. I\nam really tempted to try converting the revision walker to keep its\nflags in a commit_slab so that this sort of thing can never bite us\nagain.\n\nBut here is the minimal fix.\n\n-- >8 --\nSubject: branch: clean up commit flags after merge-filter walk\n\nWhen we run `branch --merged`, we use prepare_revision_walk\nwith the merge-filter marked as UNINTERESTING. Any branch\ntips that are marked UNINTERESTING after it returns must be\nancestors of that commit. As we iterate through the list of\nrefs to show, we check item->commit->object.flags to see\nwhether it was marked.\n\nThis interacts badly with --verbose, which will do a\nseparate walk to find the ahead/behind information for each\nbranch. There are two bad things that can happen:\n\n  1. The ahead/behind walk may get the wrong results,\n     because it can see a bogus UNINTERESTING flag leftover\n     from the merge-filter walk.\n\n  2. We may omit some branches if their tips are involved in\n     the ahead/behind traversal of a branch shown earlier.\n     The ahead/behind walk carefully cleans up its commit\n     flags, meaning it may also erase the UNINTERESTING\n     flag that we expect to check later.\n\nWe can solve this by moving the merge-filter state for each\nref into its \"struct ref_item\" as soon as we finish the\nmerge-filter walk. That fixes (2). Then we are free to clear\nthe commit flags we used in the walk, fixing (1).\n\nNote that we actually do away with the matches_merge_filter\nhelper entirely here, and inline it between the revision\nwalk and the flag-clearing. This ensures that nobody\naccidentally calls it at the wrong time (it is only safe to\ncheck in that instant between the setting and clearing of\nthe global flag).\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n builtin/branch.c           | 33 +++++++++++++++++++--------------\n t/t3201-branch-contains.sh | 29 +++++++++++++++++++++++++++++\n 2 files changed, 48 insertions(+), 14 deletions(-)\n\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex ced422b..9e4666f 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -280,6 +280,7 @@ struct ref_item {\n \tchar *dest;\n \tunsigned int kind, width;\n \tstruct commit *commit;\n+\tint ignore;\n };\n \n struct ref_list {\n@@ -385,6 +386,7 @@ static int append_ref(const char *refname, const unsigned char *sha1, int flags,\n \tnewitem->commit = commit;\n \tnewitem->width = utf8_strwidth(refname);\n \tnewitem->dest = resolve_symref(orig_refname, prefix);\n+\tnewitem->ignore = 0;\n \t/* adjust for \"remotes/\" */\n \tif (newitem->kind == REF_REMOTE_BRANCH &&\n \t    ref_list->kinds != REF_REMOTE_BRANCH)\n@@ -484,17 +486,6 @@ static void fill_tracking_info(struct strbuf *stat, const char *branch_name,\n \tfree(ref);\n }\n \n-static int matches_merge_filter(struct commit *commit)\n-{\n-\tint is_merged;\n-\n-\tif (merge_filter == NO_FILTER)\n-\t\treturn 1;\n-\n-\tis_merged = !!(commit->object.flags & UNINTERESTING);\n-\treturn (is_merged == (merge_filter == SHOW_MERGED));\n-}\n-\n static void add_verbose_info(struct strbuf *out, struct ref_item *item,\n \t\t\t     int verbose, int abbrev)\n {\n@@ -522,10 +513,9 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose,\n {\n \tchar c;\n \tint color;\n-\tstruct commit *commit = item->commit;\n \tstruct strbuf out = STRBUF_INIT, name = STRBUF_INIT;\n \n-\tif (!matches_merge_filter(commit))\n+\tif (item->ignore)\n \t\treturn;\n \n \tswitch (item->kind) {\n@@ -575,7 +565,7 @@ static int calc_maxwidth(struct ref_list *refs)\n {\n \tint i, w = 0;\n \tfor (i = 0; i < refs->index; i++) {\n-\t\tif (!matches_merge_filter(refs->list[i].commit))\n+\t\tif (refs->list[i].ignore)\n \t\t\tcontinue;\n \t\tif (refs->list[i].width > w)\n \t\t\tw = refs->list[i].width;\n@@ -618,6 +608,7 @@ static void show_detached(struct ref_list *ref_list)\n \t\titem.kind = REF_LOCAL_BRANCH;\n \t\titem.dest = NULL;\n \t\titem.commit = head_commit;\n+\t\titem.ignore = 0;\n \t\tif (item.width > ref_list->maxwidth)\n \t\t\tref_list->maxwidth = item.width;\n \t\tprint_ref_item(&item, ref_list->maxwidth, ref_list->verbose, ref_list->abbrev, 1, \"\");\n@@ -656,6 +647,20 @@ static int print_ref_list(int kinds, int detached, int verbose, int abbrev, stru\n \n \t\tif (prepare_revision_walk(&ref_list.revs))\n \t\t\tdie(_(\"revision walk setup failed\"));\n+\n+\t\tfor (i = 0; i < ref_list.index; i++) {\n+\t\t\tstruct ref_item *item = &ref_list.list[i];\n+\t\t\tstruct commit *commit = item->commit;\n+\t\t\tint is_merged = !!(commit->object.flags & UNINTERESTING);\n+\t\t\titem->ignore = is_merged != (merge_filter == SHOW_MERGED);\n+\t\t}\n+\n+\t\tfor (i = 0; i < ref_list.index; i++) {\n+\t\t\tstruct ref_item *item = &ref_list.list[i];\n+\t\t\tclear_commit_marks(item->commit, ALL_REV_FLAGS);\n+\t\t}\n+\t\tclear_commit_marks(filter, ALL_REV_FLAGS);\n+\n \t\tif (verbose)\n \t\t\tref_list.maxwidth = calc_maxwidth(&ref_list);\n \t}\ndiff --git a/t/t3201-branch-contains.sh b/t/t3201-branch-contains.sh\nindex 141b061..912a663 100755\n--- a/t/t3201-branch-contains.sh\n+++ b/t/t3201-branch-contains.sh\n@@ -130,4 +130,33 @@ test_expect_success 'implicit --list conflicts with modification options' '\n \n '\n \n+# We want to set up a case where the walk for the tracking info\n+# of one branch crosses the tip of another branch (and make sure\n+# that the latter walk does not mess up our flag to see if it was\n+# merged).\n+#\n+# Here \"topic\" tracks \"master\" with one extra commit, and \"zzz\" points to the\n+# same tip as master The name \"zzz\" must come alphabetically after \"topic\"\n+# as we process them in that order.\n+test_expect_success 'branch --merged with --verbose' '\n+\tgit branch --track topic master &&\n+\tgit branch zzz topic &&\n+\tgit checkout topic &&\n+\ttest_commit foo &&\n+\tgit branch --merged topic >actual &&\n+\tcat >expect <<-\\EOF &&\n+\t  master\n+\t* topic\n+\t  zzz\n+\tEOF\n+\ttest_cmp expect actual &&\n+\tgit branch --verbose --merged topic >actual &&\n+\tcat >expect <<-\\EOF &&\n+\t  master c77a0a9 second on master\n+\t* topic  2c939f4 [ahead 1] foo\n+\t  zzz    c77a0a9 second on master\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.1.0.486.g83bb229\n"}]}