Merge branch 'kn/ref-filter-branch-list' into maint
authorJunio C Hamano <gitster@pobox.com>
Wed, 12 Jul 2017 22:23:09 +0000 (15:23 -0700)
committerJunio C Hamano <gitster@pobox.com>
Wed, 12 Jul 2017 22:23:09 +0000 (15:23 -0700)
The rewrite of "git branch --list" using for-each-ref's internals
that happened in v2.13 regressed its handling of color.branch.local;
this has been fixed.

* kn/ref-filter-branch-list:
ref-filter.c: drop return from void function
branch: set remote color in ref-filter branch immediately
branch: use BRANCH_COLOR_LOCAL in ref-filter format
branch: only perform HEAD check for local branches

1  2 
builtin/branch.c
ref-filter.c
diff --combined builtin/branch.c
index 48a513a84dbf6cf2516a10cbcced267369bc747c,fd186fe3b86284dc479e9e8445478cf37f3a2349..a3bd2262b33672d7277553d77a0ac0e1830395d5
@@@ -33,7 -33,7 +33,7 @@@ static const char * const builtin_branc
  };
  
  static const char *head;
 -static unsigned char head_sha1[20];
 +static struct object_id head_oid;
  
  static int branch_use_color = -1;
  static char branch_colors[][COLOR_MAXLEN] = {
@@@ -118,13 -118,13 +118,13 @@@ static int branch_merged(int kind, cons
        if (kind == FILTER_REFS_BRANCHES) {
                struct branch *branch = branch_get(name);
                const char *upstream = branch_get_upstream(branch, NULL);
 -              unsigned char sha1[20];
 +              struct object_id oid;
  
                if (upstream &&
                    (reference_name = reference_name_to_free =
                     resolve_refdup(upstream, RESOLVE_REF_READING,
 -                                  sha1, NULL)) != NULL)
 -                      reference_rev = lookup_commit_reference(sha1);
 +                                  oid.hash, NULL)) != NULL)
 +                      reference_rev = lookup_commit_reference(oid.hash);
        }
        if (!reference_rev)
                reference_rev = head_rev;
  }
  
  static int check_branch_commit(const char *branchname, const char *refname,
 -                             const unsigned char *sha1, struct commit *head_rev,
 +                             const struct object_id *oid, struct commit *head_rev,
                               int kinds, int force)
  {
 -      struct commit *rev = lookup_commit_reference(sha1);
 +      struct commit *rev = lookup_commit_reference(oid->hash);
        if (!rev) {
                error(_("Couldn't look up commit object for '%s'"), refname);
                return -1;
@@@ -184,34 -184,31 +184,34 @@@ static int delete_branches(int argc, co
                           int quiet)
  {
        struct commit *head_rev = NULL;
 -      unsigned char sha1[20];
 +      struct object_id oid;
        char *name = NULL;
        const char *fmt;
        int i;
        int ret = 0;
        int remote_branch = 0;
        struct strbuf bname = STRBUF_INIT;
 +      unsigned allowed_interpret;
  
        switch (kinds) {
        case FILTER_REFS_REMOTES:
                fmt = "refs/remotes/%s";
                /* For subsequent UI messages */
                remote_branch = 1;
 +              allowed_interpret = INTERPRET_BRANCH_REMOTE;
  
                force = 1;
                break;
        case FILTER_REFS_BRANCHES:
                fmt = "refs/heads/%s";
 +              allowed_interpret = INTERPRET_BRANCH_LOCAL;
                break;
        default:
                die(_("cannot use -a with -d"));
        }
  
        if (!force) {
 -              head_rev = lookup_commit_reference(head_sha1);
 +              head_rev = lookup_commit_reference(head_oid.hash);
                if (!head_rev)
                        die(_("Couldn't look up commit object for HEAD"));
        }
                char *target = NULL;
                int flags = 0;
  
 -              strbuf_branchname(&bname, argv[i]);
 +              strbuf_branchname(&bname, argv[i], allowed_interpret);
                free(name);
                name = mkpathdup(fmt, bname.buf);
  
                                        RESOLVE_REF_READING
                                        | RESOLVE_REF_NO_RECURSE
                                        | RESOLVE_REF_ALLOW_BAD_NAME,
 -                                      sha1, &flags);
 +                                      oid.hash, &flags);
                if (!target) {
                        error(remote_branch
                              ? _("remote-tracking branch '%s' not found.")
                }
  
                if (!(flags & (REF_ISSYMREF|REF_ISBROKEN)) &&
 -                  check_branch_commit(bname.buf, name, sha1, head_rev, kinds,
 +                  check_branch_commit(bname.buf, name, &oid, head_rev, kinds,
                                        force)) {
                        ret = 1;
                        goto next;
                }
  
 -              if (delete_ref(name, is_null_sha1(sha1) ? NULL : sha1,
 +              if (delete_ref(NULL, name, is_null_oid(&oid) ? NULL : oid.hash,
                               REF_NODEREF)) {
                        error(remote_branch
                              ? _("Error deleting remote-tracking branch '%s'")
                               bname.buf,
                               (flags & REF_ISBROKEN) ? "broken"
                               : (flags & REF_ISSYMREF) ? target
 -                             : find_unique_abbrev(sha1, DEFAULT_ABBREV));
 +                             : find_unique_abbrev(oid.hash, DEFAULT_ABBREV));
                }
                delete_branch_config(bname.buf);
  
@@@ -334,8 -331,11 +334,11 @@@ static char *build_format(struct ref_fi
        struct strbuf local = STRBUF_INIT;
        struct strbuf remote = STRBUF_INIT;
  
-       strbuf_addf(&fmt, "%%(if)%%(HEAD)%%(then)* %s%%(else)  %%(end)",
-                   branch_get_color(BRANCH_COLOR_CURRENT));
+       strbuf_addf(&local, "%%(if)%%(HEAD)%%(then)* %s%%(else)  %s%%(end)",
+                   branch_get_color(BRANCH_COLOR_CURRENT),
+                   branch_get_color(BRANCH_COLOR_LOCAL));
+       strbuf_addf(&remote, "  %s",
+                   branch_get_color(BRANCH_COLOR_REMOTE));
  
        if (filter->verbose) {
                struct strbuf obname = STRBUF_INIT;
                else
                        strbuf_addf(&local, "%%(if)%%(upstream:track)%%(then)%%(upstream:track) %%(end)%%(contents:subject)");
  
-               strbuf_addf(&remote, "%s%%(align:%d,left)%s%%(refname:lstrip=2)%%(end)%s"
+               strbuf_addf(&remote, "%%(align:%d,left)%s%%(refname:lstrip=2)%%(end)%s"
                            "%%(if)%%(symref)%%(then) -> %%(symref:short)"
                            "%%(else) %s %%(contents:subject)%%(end)",
-                           branch_get_color(BRANCH_COLOR_REMOTE), maxwidth, quote_literal_for_format(remote_prefix),
+                           maxwidth, quote_literal_for_format(remote_prefix),
                            branch_get_color(BRANCH_COLOR_RESET), obname.buf);
                strbuf_release(&obname);
        } else {
                strbuf_addf(&local, "%%(refname:lstrip=2)%s%%(if)%%(symref)%%(then) -> %%(symref:short)%%(end)",
                            branch_get_color(BRANCH_COLOR_RESET));
-               strbuf_addf(&remote, "%s%s%%(refname:lstrip=2)%s%%(if)%%(symref)%%(then) -> %%(symref:short)%%(end)",
-                           branch_get_color(BRANCH_COLOR_REMOTE), quote_literal_for_format(remote_prefix),
+               strbuf_addf(&remote, "%s%%(refname:lstrip=2)%s%%(if)%%(symref)%%(then) -> %%(symref:short)%%(end)",
+                           quote_literal_for_format(remote_prefix),
                            branch_get_color(BRANCH_COLOR_RESET));
        }
  
@@@ -485,15 -485,14 +488,15 @@@ static void rename_branch(const char *o
  
        if (rename_ref(oldref.buf, newref.buf, logmsg.buf))
                die(_("Branch rename failed"));
 -      strbuf_release(&logmsg);
  
        if (recovery)
                warning(_("Renamed a misnamed branch '%s' away"), oldref.buf + 11);
  
 -      if (replace_each_worktree_head_symref(oldref.buf, newref.buf))
 +      if (replace_each_worktree_head_symref(oldref.buf, newref.buf, logmsg.buf))
                die(_("Branch renamed to %s, but HEAD is not updated!"), newname);
  
 +      strbuf_release(&logmsg);
 +
        strbuf_addf(&oldsection, "branch.%s", oldref.buf + 11);
        strbuf_release(&oldref);
        strbuf_addf(&newsection, "branch.%s", newref.buf + 11);
        strbuf_release(&newsection);
  }
  
 -static const char edit_description[] = "BRANCH_DESCRIPTION";
 +static GIT_PATH_FUNC(edit_description, "EDIT_DESCRIPTION")
  
  static int edit_branch_description(const char *branch_name)
  {
                      "  %s\n"
                      "Lines starting with '%c' will be stripped.\n"),
                    branch_name, comment_line_char);
 -      write_file_buf(git_path(edit_description), buf.buf, buf.len);
 +      write_file_buf(edit_description(), buf.buf, buf.len);
        strbuf_reset(&buf);
 -      if (launch_editor(git_path(edit_description), &buf, NULL)) {
 +      if (launch_editor(edit_description(), &buf, NULL)) {
                strbuf_release(&buf);
                return -1;
        }
@@@ -562,9 -561,7 +565,9 @@@ int cmd_branch(int argc, const char **a
                OPT_SET_INT('r', "remotes",     &filter.kind, N_("act on remote-tracking branches"),
                        FILTER_REFS_REMOTES),
                OPT_CONTAINS(&filter.with_commit, N_("print only branches that contain the commit")),
 +              OPT_NO_CONTAINS(&filter.no_commit, N_("print only branches that don't contain the commit")),
                OPT_WITH(&filter.with_commit, N_("print only branches that contain the commit")),
 +              OPT_WITHOUT(&filter.no_commit, N_("print only branches that don't contain the commit")),
                OPT__ABBREV(&filter.abbrev),
  
                OPT_GROUP(N_("Specific git-branch actions:")),
  
        track = git_branch_track;
  
 -      head = resolve_refdup("HEAD", 0, head_sha1, NULL);
 +      head = resolve_refdup("HEAD", 0, head_oid.hash, NULL);
        if (!head)
                die(_("Failed to resolve HEAD as a valid ref."));
        if (!strcmp(head, "HEAD"))
        if (!delete && !rename && !edit_description && !new_upstream && !unset_upstream && argc == 0)
                list = 1;
  
 -      if (filter.with_commit || filter.merge != REF_FILTER_MERGED_NONE || filter.points_at.nr)
 +      if (filter.with_commit || filter.merge != REF_FILTER_MERGED_NONE || filter.points_at.nr ||
 +          filter.no_commit)
                list = 1;
  
        if (!!delete + !!rename + !!new_upstream +
diff --combined ref-filter.c
index 2cc7b01277ca8565302eb1648c065ecbb1a25f47,238888e4de342c601b26650251a235ffeb3190f2..467c0279c9b6370b57080b5f1fcf20bdfd7e451e
@@@ -15,7 -15,6 +15,7 @@@
  #include "version.h"
  #include "trailer.h"
  #include "wt-status.h"
 +#include "commit-slab.h"
  
  static struct ref_msg {
        const char *gone;
@@@ -221,7 -220,7 +221,7 @@@ static void objectname_atom_parser(stru
  
  static void refname_atom_parser(struct used_atom *atom, const char *arg)
  {
-       return refname_atom_parser_internal(&atom->u.refname, arg, atom->name);
+       refname_atom_parser_internal(&atom->u.refname, arg, atom->name);
  }
  
  static align_type parse_align_position(const char *s)
@@@ -1257,18 -1256,12 +1257,18 @@@ char *get_head_description(void
                strbuf_addf(&desc, _("(no branch, bisect started on %s)"),
                            state.branch);
        else if (state.detached_from) {
 -              /* TRANSLATORS: make sure these match _("HEAD detached at ")
 -                 and _("HEAD detached from ") in wt-status.c */
                if (state.detached_at)
 +                      /*
 +                       * TRANSLATORS: make sure this matches "HEAD
 +                       * detached at " in wt-status.c
 +                       */
                        strbuf_addf(&desc, _("(HEAD detached at %s)"),
                                state.detached_from);
                else
 +                      /*
 +                       * TRANSLATORS: make sure this matches "HEAD
 +                       * detached from " in wt-status.c
 +                       */
                        strbuf_addf(&desc, _("(HEAD detached from %s)"),
                                state.detached_from);
        }
@@@ -1309,9 -1302,9 +1309,9 @@@ static void populate_value(struct ref_a
        ref->value = xcalloc(used_atom_cnt, sizeof(struct atom_value));
  
        if (need_symref && (ref->flag & REF_ISSYMREF) && !ref->symref) {
 -              unsigned char unused1[20];
 +              struct object_id unused1;
                ref->symref = resolve_refdup(ref->refname, RESOLVE_REF_READING,
 -                                           unused1, NULL);
 +                                           unused1.hash, NULL);
                if (!ref->symref)
                        ref->symref = "";
        }
@@@ -1477,23 -1470,10 +1477,23 @@@ static void get_ref_atom_value(struct r
        *v = &ref->value[atom];
  }
  
 +/*
 + * Unknown has to be "0" here, because that's the default value for
 + * contains_cache slab entries that have not yet been assigned.
 + */
  enum contains_result {
 -      CONTAINS_UNKNOWN = -1,
 -      CONTAINS_NO = 0,
 -      CONTAINS_YES = 1
 +      CONTAINS_UNKNOWN = 0,
 +      CONTAINS_NO,
 +      CONTAINS_YES
 +};
 +
 +define_commit_slab(contains_cache, enum contains_result);
 +
 +struct ref_filter_cbdata {
 +      struct ref_array *array;
 +      struct ref_filter *filter;
 +      struct contains_cache contains_cache;
 +      struct contains_cache no_contains_cache;
  };
  
  /*
@@@ -1524,24 -1504,24 +1524,24 @@@ static int in_commit_list(const struct 
   * Do not recurse to find out, though, but return -1 if inconclusive.
   */
  static enum contains_result contains_test(struct commit *candidate,
 -                          const struct commit_list *want)
 +                                        const struct commit_list *want,
 +                                        struct contains_cache *cache)
  {
 -      /* was it previously marked as containing a want commit? */
 -      if (candidate->object.flags & TMP_MARK)
 -              return 1;
 -      /* or marked as not possibly containing a want commit? */
 -      if (candidate->object.flags & UNINTERESTING)
 -              return 0;
 +      enum contains_result *cached = contains_cache_at(cache, candidate);
 +
 +      /* If we already have the answer cached, return that. */
 +      if (*cached)
 +              return *cached;
 +
        /* or are we it? */
        if (in_commit_list(want, candidate)) {
 -              candidate->object.flags |= TMP_MARK;
 -              return 1;
 +              *cached = CONTAINS_YES;
 +              return CONTAINS_YES;
        }
  
 -      if (parse_commit(candidate) < 0)
 -              return 0;
 -
 -      return -1;
 +      /* Otherwise, we don't know; prepare to recurse */
 +      parse_commit_or_die(candidate);
 +      return CONTAINS_UNKNOWN;
  }
  
  static void push_to_contains_stack(struct commit *candidate, struct contains_stack *contains_stack)
  }
  
  static enum contains_result contains_tag_algo(struct commit *candidate,
 -              const struct commit_list *want)
 +                                            const struct commit_list *want,
 +                                            struct contains_cache *cache)
  {
        struct contains_stack contains_stack = { 0, 0, NULL };
 -      int result = contains_test(candidate, want);
 +      enum contains_result result = contains_test(candidate, want, cache);
  
        if (result != CONTAINS_UNKNOWN)
                return result;
                struct commit_list *parents = entry->parents;
  
                if (!parents) {
 -                      commit->object.flags |= UNINTERESTING;
 +                      *contains_cache_at(cache, commit) = CONTAINS_NO;
                        contains_stack.nr--;
                }
                /*
                 * If we just popped the stack, parents->item has been marked,
 -               * therefore contains_test will return a meaningful 0 or 1.
 +               * therefore contains_test will return a meaningful yes/no.
                 */
 -              else switch (contains_test(parents->item, want)) {
 +              else switch (contains_test(parents->item, want, cache)) {
                case CONTAINS_YES:
 -                      commit->object.flags |= TMP_MARK;
 +                      *contains_cache_at(cache, commit) = CONTAINS_YES;
                        contains_stack.nr--;
                        break;
                case CONTAINS_NO:
                }
        }
        free(contains_stack.contains_stack);
 -      return contains_test(candidate, want);
 +      return contains_test(candidate, want, cache);
  }
  
 -static int commit_contains(struct ref_filter *filter, struct commit *commit)
 +static int commit_contains(struct ref_filter *filter, struct commit *commit,
 +                         struct commit_list *list, struct contains_cache *cache)
  {
        if (filter->with_commit_tag_algo)
 -              return contains_tag_algo(commit, filter->with_commit);
 -      return is_descendant_of(commit, filter->with_commit);
 +              return contains_tag_algo(commit, list, cache) == CONTAINS_YES;
 +      return is_descendant_of(commit, list);
  }
  
  /*
@@@ -1684,22 -1662,22 +1684,22 @@@ static int filter_pattern_match(struct 
   * the need to parse the object via parse_object(). peel_ref() might be a
   * more efficient alternative to obtain the pointee.
   */
 -static const unsigned char *match_points_at(struct sha1_array *points_at,
 -                                          const unsigned char *sha1,
 -                                          const char *refname)
 +static const struct object_id *match_points_at(struct oid_array *points_at,
 +                                             const struct object_id *oid,
 +                                             const char *refname)
  {
 -      const unsigned char *tagged_sha1 = NULL;
 +      const struct object_id *tagged_oid = NULL;
        struct object *obj;
  
 -      if (sha1_array_lookup(points_at, sha1) >= 0)
 -              return sha1;
 -      obj = parse_object(sha1);
 +      if (oid_array_lookup(points_at, oid) >= 0)
 +              return oid;
 +      obj = parse_object(oid->hash);
        if (!obj)
                die(_("malformed object at '%s'"), refname);
        if (obj->type == OBJ_TAG)
 -              tagged_sha1 = ((struct tag *)obj)->tagged->oid.hash;
 -      if (tagged_sha1 && sha1_array_lookup(points_at, tagged_sha1) >= 0)
 -              return tagged_sha1;
 +              tagged_oid = &((struct tag *)obj)->tagged->oid;
 +      if (tagged_oid && oid_array_lookup(points_at, tagged_oid) >= 0)
 +              return tagged_oid;
        return NULL;
  }
  
@@@ -1716,7 -1694,7 +1716,7 @@@ static struct ref_array_item *new_ref_a
        return ref;
  }
  
 -static int filter_ref_kind(struct ref_filter *filter, const char *refname)
 +static int ref_kind_from_refname(const char *refname)
  {
        unsigned int i;
  
                { "refs/tags/", FILTER_REFS_TAGS}
        };
  
 -      if (filter->kind == FILTER_REFS_BRANCHES ||
 -          filter->kind == FILTER_REFS_REMOTES ||
 -          filter->kind == FILTER_REFS_TAGS)
 -              return filter->kind;
 -      else if (!strcmp(refname, "HEAD"))
 +      if (!strcmp(refname, "HEAD"))
                return FILTER_REFS_DETACHED_HEAD;
  
        for (i = 0; i < ARRAY_SIZE(ref_kind); i++) {
        return FILTER_REFS_OTHERS;
  }
  
 +static int filter_ref_kind(struct ref_filter *filter, const char *refname)
 +{
 +      if (filter->kind == FILTER_REFS_BRANCHES ||
 +          filter->kind == FILTER_REFS_REMOTES ||
 +          filter->kind == FILTER_REFS_TAGS)
 +              return filter->kind;
 +      return ref_kind_from_refname(refname);
 +}
 +
  /*
   * A call-back given to for_each_ref().  Filter refs and keep them for
   * later object processing.
@@@ -1779,7 -1752,7 +1779,7 @@@ static int ref_filter_handler(const cha
        if (!filter_pattern_match(filter, refname))
                return 0;
  
 -      if (filter->points_at.nr && !match_points_at(&filter->points_at, oid->hash, refname))
 +      if (filter->points_at.nr && !match_points_at(&filter->points_at, oid, refname))
                return 0;
  
        /*
         * obtain the commit using the 'oid' available and discard all
         * non-commits early. The actual filtering is done later.
         */
 -      if (filter->merge_commit || filter->with_commit || filter->verbose) {
 +      if (filter->merge_commit || filter->with_commit || filter->no_commit || filter->verbose) {
                commit = lookup_commit_reference_gently(oid->hash, 1);
                if (!commit)
                        return 0;
 -              /* We perform the filtering for the '--contains' option */
 +              /* We perform the filtering for the '--contains' option... */
                if (filter->with_commit &&
 -                  !commit_contains(filter, commit))
 +                  !commit_contains(filter, commit, filter->with_commit, &ref_cbdata->contains_cache))
 +                      return 0;
 +              /* ...or for the `--no-contains' option */
 +              if (filter->no_commit &&
 +                  commit_contains(filter, commit, filter->no_commit, &ref_cbdata->no_contains_cache))
                        return 0;
        }
  
@@@ -1897,9 -1866,6 +1897,9 @@@ int filter_refs(struct ref_array *array
                broken = 1;
        filter->kind = type & FILTER_REFS_KIND_MASK;
  
 +      init_contains_cache(&ref_cbdata.contains_cache);
 +      init_contains_cache(&ref_cbdata.no_contains_cache);
 +
        /*  Simple per-ref filtering */
        if (!filter->kind)
                die("filter_refs: invalid type");
                        head_ref(ref_filter_handler, &ref_cbdata);
        }
  
 +      clear_contains_cache(&ref_cbdata.contains_cache);
 +      clear_contains_cache(&ref_cbdata.no_contains_cache);
  
        /*  Filters that need revision walking */
        if (filter->merge_commit)
@@@ -1958,7 -1922,8 +1958,7 @@@ static int cmp_ref_sorting(struct ref_s
        return (s->reverse) ? -cmp : cmp;
  }
  
 -static struct ref_sorting *ref_sorting;
 -static int compare_refs(const void *a_, const void *b_)
 +static int compare_refs(const void *a_, const void *b_, void *ref_sorting)
  {
        struct ref_array_item *a = *((struct ref_array_item **)a_);
        struct ref_array_item *b = *((struct ref_array_item **)b_);
  
  void ref_array_sort(struct ref_sorting *sorting, struct ref_array *array)
  {
 -      ref_sorting = sorting;
 -      QSORT(array->items, array->nr, compare_refs);
 +      QSORT_S(array->items, array->nr, compare_refs, sorting);
  }
  
  static void append_literal(const char *cp, const char *ep, struct ref_formatting_state *state)
@@@ -2046,16 -2012,6 +2046,16 @@@ void show_ref_array_item(struct ref_arr
        putchar('\n');
  }
  
 +void pretty_print_ref(const char *name, const unsigned char *sha1,
 +              const char *format)
 +{
 +      struct ref_array_item *ref_item;
 +      ref_item = new_ref_array_item(name, sha1, 0);
 +      ref_item->kind = ref_kind_from_refname(name);
 +      show_ref_array_item(ref_item, format, 0);
 +      free_array_item(ref_item);
 +}
 +
  /*  If no sorting option is given, use refname to sort as default */
  struct ref_sorting *ref_default_sorting(void)
  {
@@@ -2097,17 -2053,8 +2097,17 @@@ int parse_opt_merge_filter(const struc
  {
        struct ref_filter *rf = opt->value;
        unsigned char sha1[20];
 +      int no_merged = starts_with(opt->long_name, "no");
 +
 +      if (rf->merge) {
 +              if (no_merged) {
 +                      return opterror(opt, "is incompatible with --merged", 0);
 +              } else {
 +                      return opterror(opt, "is incompatible with --no-merged", 0);
 +              }
 +      }
  
 -      rf->merge = starts_with(opt->long_name, "no")
 +      rf->merge = no_merged
                ? REF_FILTER_MERGED_OMIT
                : REF_FILTER_MERGED_INCLUDE;