Merge branch 'jk/describe-omit-some-refs' into mk/describe-match-with-all
authorJunio C Hamano <gitster@pobox.com>
Wed, 20 Sep 2017 04:30:01 +0000 (13:30 +0900)
committerJunio C Hamano <gitster@pobox.com>
Wed, 20 Sep 2017 04:30:01 +0000 (13:30 +0900)
* jk/describe-omit-some-refs:
describe: fix matching to actually match all patterns

1  2 
builtin/describe.c
t/t6120-describe.sh
diff --combined builtin/describe.c
index e77163e909d07ec168d41d8b96e05afa73490848,ba4ebb4d1beed47b402e9a5da9ebaf83b206a9f9..3dc18364809c3c5f88b9f31d7877a58caad87755
@@@ -1,5 -1,4 +1,5 @@@
  #include "cache.h"
 +#include "config.h"
  #include "lockfile.h"
  #include "commit.h"
  #include "tag.h"
@@@ -10,7 -9,6 +10,7 @@@
  #include "diff.h"
  #include "hashmap.h"
  #include "argv-array.h"
 +#include "run-command.h"
  
  #define SEEN          (1u << 0)
  #define MAX_TAGS      (FLAG_BITS - 1)
@@@ -33,7 -31,7 +33,7 @@@ static int have_util
  static struct string_list patterns = STRING_LIST_INIT_NODUP;
  static struct string_list exclude_patterns = STRING_LIST_INIT_NODUP;
  static int always;
 -static const char *dirty;
 +static const char *suffix, *dirty, *broken;
  
  /* diff-index command arguments to check if working tree is dirty. */
  static const char *diff_index_args[] = {
  
  struct commit_name {
        struct hashmap_entry entry;
 -      unsigned char peeled[20];
 +      struct object_id peeled;
        struct tag *tag;
        unsigned prio:2; /* annotated tag = 2, tag = 1, head = 0 */
        unsigned name_checked:1;
 -      unsigned char sha1[20];
 +      struct object_id oid;
        char *path;
  };
  
  static const char *prio_names[] = {
 -      "head", "lightweight", "annotated",
 +      N_("head"), N_("lightweight"), N_("annotated"),
  };
  
 -static int commit_name_cmp(const struct commit_name *cn1,
 -              const struct commit_name *cn2, const void *peeled)
 +static int commit_name_cmp(const void *unused_cmp_data,
 +                         const void *entry,
 +                         const void *entry_or_key,
 +                         const void *peeled)
  {
 -      return hashcmp(cn1->peeled, peeled ? peeled : cn2->peeled);
 +      const struct commit_name *cn1 = entry;
 +      const struct commit_name *cn2 = entry_or_key;
 +
 +      return oidcmp(&cn1->peeled, peeled ? peeled : &cn2->peeled);
  }
  
 -static inline struct commit_name *find_commit_name(const unsigned char *peeled)
 +static inline struct commit_name *find_commit_name(const struct object_id *peeled)
  {
 -      return hashmap_get_from_hash(&names, sha1hash(peeled), peeled);
 +      return hashmap_get_from_hash(&names, sha1hash(peeled->hash), peeled->hash);
  }
  
  static int replace_name(struct commit_name *e,
                               int prio,
 -                             const unsigned char *sha1,
 +                             const struct object_id *oid,
                               struct tag **tag)
  {
        if (!e || e->prio < prio)
                struct tag *t;
  
                if (!e->tag) {
 -                      t = lookup_tag(e->sha1);
 +                      t = lookup_tag(&e->oid);
                        if (!t || parse_tag(t))
                                return 1;
                        e->tag = t;
                }
  
 -              t = lookup_tag(sha1);
 +              t = lookup_tag(oid);
                if (!t || parse_tag(t))
                        return 0;
                *tag = t;
  }
  
  static void add_to_known_names(const char *path,
 -                             const unsigned char *peeled,
 +                             const struct object_id *peeled,
                               int prio,
 -                             const unsigned char *sha1)
 +                             const struct object_id *oid)
  {
        struct commit_name *e = find_commit_name(peeled);
        struct tag *tag = NULL;
 -      if (replace_name(e, prio, sha1, &tag)) {
 +      if (replace_name(e, prio, oid, &tag)) {
                if (!e) {
                        e = xmalloc(sizeof(struct commit_name));
 -                      hashcpy(e->peeled, peeled);
 -                      hashmap_entry_init(e, sha1hash(peeled));
 +                      oidcpy(&e->peeled, peeled);
 +                      hashmap_entry_init(e, sha1hash(peeled->hash));
                        hashmap_add(&names, e);
                        e->path = NULL;
                }
                e->tag = tag;
                e->prio = prio;
                e->name_checked = 0;
 -              hashcpy(e->sha1, sha1);
 +              oidcpy(&e->oid, oid);
                free(e->path);
                e->path = xstrdup(path);
        }
@@@ -148,7 -141,7 +148,7 @@@ static int get_name(const char *path, c
                        return 0;
  
                for_each_string_list_item(item, &exclude_patterns) {
 -                      if (!wildmatch(item->string, path + 10, 0, NULL))
 +                      if (!wildmatch(item->string, path + 10, 0))
                                return 0;
                }
        }
         * pattern.
         */
        if (patterns.nr) {
+               int found = 0;
                struct string_list_item *item;
  
                if (!is_tag)
                        return 0;
  
                for_each_string_list_item(item, &patterns) {
-                       if (!wildmatch(item->string, path + 10, 0))
 -                      if (!wildmatch(item->string, path + 10, 0, NULL)) {
++                      if (!wildmatch(item->string, path + 10, 0)) {
+                               found = 1;
                                break;
+                       }
+               }
  
-                       /* If we get here, no pattern matched. */
+               if (!found)
                        return 0;
-               }
        }
  
        /* Is it annotated? */
        else
                prio = 0;
  
 -      add_to_known_names(all ? path + 5 : path + 10, peeled.hash, prio, oid->hash);
 +      add_to_known_names(all ? path + 5 : path + 10, &peeled, prio, oid);
        return 0;
  }
  
@@@ -251,7 -247,7 +254,7 @@@ static unsigned long finish_depth_compu
  static void display_name(struct commit_name *n)
  {
        if (n->prio == 2 && !n->tag) {
 -              n->tag = lookup_tag(n->sha1);
 +              n->tag = lookup_tag(&n->oid);
                if (!n->tag || parse_tag(n->tag))
                        die(_("annotated tag %s not available"), n->path);
        }
                printf("%s", n->path);
  }
  
 -static void show_suffix(int depth, const unsigned char *sha1)
 +static void show_suffix(int depth, const struct object_id *oid)
  {
 -      printf("-%d-g%s", depth, find_unique_abbrev(sha1, abbrev));
 +      printf("-%d-g%s", depth, find_unique_abbrev(oid->hash, abbrev));
  }
  
  static void describe(const char *arg, int last_one)
  {
 -      unsigned char sha1[20];
 +      struct object_id oid;
        struct commit *cmit, *gave_up_on = NULL;
        struct commit_list *list;
        struct commit_name *n;
        unsigned long seen_commits = 0;
        unsigned int unannotated_cnt = 0;
  
 -      if (get_sha1(arg, sha1))
 +      if (get_oid(arg, &oid))
                die(_("Not a valid object name %s"), arg);
 -      cmit = lookup_commit_reference(sha1);
 +      cmit = lookup_commit_reference(&oid);
        if (!cmit)
                die(_("%s is not a valid '%s' object"), arg, commit_type);
  
 -      n = find_commit_name(cmit->object.oid.hash);
 +      n = find_commit_name(&cmit->object.oid);
        if (n && (tags || all || n->prio == 2)) {
                /*
                 * Exact match to an existing ref.
                 */
                display_name(n);
                if (longformat)
 -                      show_suffix(0, n->tag ? n->tag->tagged->oid.hash : sha1);
 -              if (dirty)
 -                      printf("%s", dirty);
 +                      show_suffix(0, n->tag ? &n->tag->tagged->oid : &oid);
 +              if (suffix)
 +                      printf("%s", suffix);
                printf("\n");
                return;
        }
                struct commit *c;
                struct commit_name *n = hashmap_iter_first(&names, &iter);
                for (; n; n = hashmap_iter_next(&iter)) {
 -                      c = lookup_commit_reference_gently(n->peeled, 1);
 +                      c = lookup_commit_reference_gently(&n->peeled, 1);
                        if (c)
                                c->util = n;
                }
                struct object_id *oid = &cmit->object.oid;
                if (always) {
                        printf("%s", find_unique_abbrev(oid->hash, abbrev));
 -                      if (dirty)
 -                              printf("%s", dirty);
 +                      if (suffix)
 +                              printf("%s", suffix);
                        printf("\n");
                        return;
                }
        free_commit_list(list);
  
        if (debug) {
 +              static int label_width = -1;
 +              if (label_width < 0) {
 +                      int i, w;
 +                      for (i = 0; i < ARRAY_SIZE(prio_names); i++) {
 +                              w = strlen(_(prio_names[i]));
 +                              if (label_width < w)
 +                                      label_width = w;
 +                      }
 +              }
                for (cur_match = 0; cur_match < match_cnt; cur_match++) {
                        struct possible_tag *t = &all_matches[cur_match];
 -                      fprintf(stderr, " %-11s %8d %s\n",
 -                              prio_names[t->name->prio],
 +                      fprintf(stderr, " %-*s %8d %s\n",
 +                              label_width, _(prio_names[t->name->prio]),
                                t->depth, t->name->path);
                }
                fprintf(stderr, _("traversed %lu commits\n"), seen_commits);
  
        display_name(all_matches[0].name);
        if (abbrev)
 -              show_suffix(all_matches[0].depth, cmit->object.oid.hash);
 -      if (dirty)
 -              printf("%s", dirty);
 +              show_suffix(all_matches[0].depth, &cmit->object.oid);
 +      if (suffix)
 +              printf("%s", suffix);
        printf("\n");
  
        if (!last_one)
@@@ -461,9 -448,6 +464,9 @@@ int cmd_describe(int argc, const char *
                {OPTION_STRING, 0, "dirty",  &dirty, N_("mark"),
                        N_("append <mark> on dirty working tree (default: \"-dirty\")"),
                        PARSE_OPT_OPTARG, NULL, (intptr_t) "-dirty"},
 +              {OPTION_STRING, 0, "broken",  &broken, N_("mark"),
 +                      N_("append <mark> on broken working tree (default: \"-broken\")"),
 +                      PARSE_OPT_OPTARG, NULL, (intptr_t) "-broken"},
                OPT_END(),
        };
  
                return cmd_name_rev(args.argc, args.argv, prefix);
        }
  
 -      hashmap_init(&names, (hashmap_cmp_fn) commit_name_cmp, 0);
 +      hashmap_init(&names, commit_name_cmp, NULL, 0);
        for_each_rawref(get_name, NULL);
 -      if (!names.size && !always)
 +      if (!hashmap_get_size(&names) && !always)
                die(_("No names found, cannot describe anything."));
  
        if (argc == 0) {
 -              if (dirty) {
 +              if (broken) {
 +                      struct child_process cp = CHILD_PROCESS_INIT;
 +                      argv_array_pushv(&cp.args, diff_index_args);
 +                      cp.git_cmd = 1;
 +                      cp.no_stdin = 1;
 +                      cp.no_stdout = 1;
 +
 +                      if (!dirty)
 +                              dirty = "-dirty";
 +
 +                      switch (run_command(&cp)) {
 +                      case 0:
 +                              suffix = NULL;
 +                              break;
 +                      case 1:
 +                              suffix = dirty;
 +                              break;
 +                      default:
 +                              /* diff-index aborted abnormally */
 +                              suffix = broken;
 +                      }
 +              } else if (dirty) {
                        static struct lock_file index_lock;
                        int fd;
  
  
                        if (!cmd_diff_index(ARRAY_SIZE(diff_index_args) - 1,
                                            diff_index_args, prefix))
 -                              dirty = NULL;
 +                              suffix = NULL;
 +                      else
 +                              suffix = dirty;
                }
                describe("HEAD", 1);
        } else if (dirty) {
                die(_("--dirty is incompatible with commit-ishes"));
 +      } else if (broken) {
 +              die(_("--broken is incompatible with commit-ishes"));
        } else {
                while (argc-- > 0)
                        describe(*argv++, argc == 0);
diff --combined t/t6120-describe.sh
index aa74eb8f0d5dfb4f228b1e8c7b9dbbc0b88adb4d,38570f63bb8d55ce241f79bd8d8ff558d2ca5ecd..25110ea55d89656ab3fd4da525d4c94bc75840ac
@@@ -182,10 -182,14 +182,14 @@@ check_describe "test2-lightweight-*" --
  
  check_describe "test2-lightweight-*" --long --tags --match="test2-*" HEAD^
  
- check_describe "test1-lightweight-*" --long --tags --match="test1-*" --match="test2-*" HEAD^
+ check_describe "test2-lightweight-*" --long --tags --match="test1-*" --match="test2-*" HEAD^
  
  check_describe "test2-lightweight-*" --long --tags --match="test1-*" --no-match --match="test2-*" HEAD^
  
+ check_describe "test1-lightweight-*" --long --tags --match="test1-*" --match="test3-*" HEAD
+ check_describe "test1-lightweight-*" --long --tags --match="test3-*" --match="test1-*" HEAD
  test_expect_success 'name-rev with exact tags' '
        echo A >expect &&
        tag_object=$(git rev-parse refs/tags/A) &&
@@@ -233,24 -237,4 +237,24 @@@ test_expect_success 'describe --contain
        test_cmp expect actual
  '
  
 +test_expect_success 'setup and absorb a submodule' '
 +      test_create_repo sub1 &&
 +      test_commit -C sub1 initial &&
 +      git submodule add ./sub1 &&
 +      git submodule absorbgitdirs &&
 +      git commit -a -m "add submodule" &&
 +      git describe --dirty >expect &&
 +      git describe --broken >out &&
 +      test_cmp expect out
 +'
 +
 +test_expect_success 'describe chokes on severely broken submodules' '
 +      mv .git/modules/sub1/ .git/modules/sub_moved &&
 +      test_must_fail git describe --dirty
 +'
 +test_expect_success 'describe ignoring a borken submodule' '
 +      git describe --broken >out &&
 +      grep broken out
 +'
 +
  test_done