Merge branch 'jk/remote-pushremote-config-reading'
authorJunio C Hamano <gitster@pobox.com>
Fri, 14 Mar 2014 21:26:04 +0000 (14:26 -0700)
committerJunio C Hamano <gitster@pobox.com>
Fri, 14 Mar 2014 21:26:05 +0000 (14:26 -0700)
"git push" did not pay attention to branch.*.pushremote if it is
defined earlier than remote.pushdefault; the order of these two
variables in the configuration file should not matter, but it did by
mistake.

* jk/remote-pushremote-config-reading:
remote: handle pushremote config in any order

1  2 
remote.c
t/t5516-fetch-push.sh
diff --combined remote.c
index e41251ee77af1426dd26d4b13ee61a39cffccdeb,c0e95e2eb6dfb92f7c41d219809ad337569912ae..5f63d55056e7f1087766374a20a23e794bf3530a
+++ b/remote.c
@@@ -49,6 -49,7 +49,7 @@@ static int branches_nr
  
  static struct branch *current_branch;
  static const char *default_remote_name;
+ static const char *branch_pushremote_name;
  static const char *pushremote_name;
  static int explicit_default_remote_name;
  
@@@ -76,7 -77,7 +77,7 @@@ static const char *alias_url(const cha
                if (!r->rewrite[i])
                        continue;
                for (j = 0; j < r->rewrite[i]->instead_of_nr; j++) {
 -                      if (!prefixcmp(url, r->rewrite[i]->instead_of[j].s) &&
 +                      if (starts_with(url, r->rewrite[i]->instead_of[j].s) &&
                            (!longest ||
                             longest->len < r->rewrite[i]->instead_of[j].len)) {
                                longest = &(r->rewrite[i]->instead_of[j]);
@@@ -239,13 -240,13 +240,13 @@@ static void read_remotes_file(struct re
                int value_list;
                char *s, *p;
  
 -              if (!prefixcmp(buffer, "URL:")) {
 +              if (starts_with(buffer, "URL:")) {
                        value_list = 0;
                        s = buffer + 4;
 -              } else if (!prefixcmp(buffer, "Push:")) {
 +              } else if (starts_with(buffer, "Push:")) {
                        value_list = 1;
                        s = buffer + 5;
 -              } else if (!prefixcmp(buffer, "Pull:")) {
 +              } else if (starts_with(buffer, "Pull:")) {
                        value_list = 2;
                        s = buffer + 5;
                } else
@@@ -337,7 -338,7 +338,7 @@@ static int handle_config(const char *ke
        const char *subkey;
        struct remote *remote;
        struct branch *branch;
 -      if (!prefixcmp(key, "branch.")) {
 +      if (starts_with(key, "branch.")) {
                name = key + 7;
                subkey = strrchr(name, '.');
                if (!subkey)
                        }
                } else if (!strcmp(subkey, ".pushremote")) {
                        if (branch == current_branch)
-                               if (git_config_string(&pushremote_name, key, value))
+                               if (git_config_string(&branch_pushremote_name, key, value))
                                        return -1;
                } else if (!strcmp(subkey, ".merge")) {
                        if (!value)
                }
                return 0;
        }
 -      if (!prefixcmp(key, "url.")) {
 +      if (starts_with(key, "url.")) {
                struct rewrite *rewrite;
                name = key + 4;
                subkey = strrchr(name, '.');
                }
        }
  
 -      if (prefixcmp(key,  "remote."))
 +      if (!starts_with(key,  "remote."))
                return 0;
        name = key + 7;
  
@@@ -487,11 -488,15 +488,15 @@@ static void read_config(void
        current_branch = NULL;
        head_ref = resolve_ref_unsafe("HEAD", sha1, 0, &flag);
        if (head_ref && (flag & REF_ISSYMREF) &&
 -          !prefixcmp(head_ref, "refs/heads/")) {
 +          starts_with(head_ref, "refs/heads/")) {
                current_branch =
                        make_branch(head_ref + strlen("refs/heads/"), 0);
        }
        git_config(handle_config, NULL);
+       if (branch_pushremote_name) {
+               free((char *)pushremote_name);
+               pushremote_name = branch_pushremote_name;
+       }
        alias_all_urls();
  }
  
@@@ -745,66 -750,35 +750,66 @@@ int for_each_remote(each_remote_fn fn, 
        return result;
  }
  
 -void ref_remove_duplicates(struct ref *ref_map)
 +static void handle_duplicate(struct ref *ref1, struct ref *ref2)
 +{
 +      if (strcmp(ref1->name, ref2->name)) {
 +              if (ref1->fetch_head_status != FETCH_HEAD_IGNORE &&
 +                  ref2->fetch_head_status != FETCH_HEAD_IGNORE) {
 +                      die(_("Cannot fetch both %s and %s to %s"),
 +                          ref1->name, ref2->name, ref2->peer_ref->name);
 +              } else if (ref1->fetch_head_status != FETCH_HEAD_IGNORE &&
 +                         ref2->fetch_head_status == FETCH_HEAD_IGNORE) {
 +                      warning(_("%s usually tracks %s, not %s"),
 +                              ref2->peer_ref->name, ref2->name, ref1->name);
 +              } else if (ref1->fetch_head_status == FETCH_HEAD_IGNORE &&
 +                         ref2->fetch_head_status == FETCH_HEAD_IGNORE) {
 +                      die(_("%s tracks both %s and %s"),
 +                          ref2->peer_ref->name, ref1->name, ref2->name);
 +              } else {
 +                      /*
 +                       * This last possibility doesn't occur because
 +                       * FETCH_HEAD_IGNORE entries always appear at
 +                       * the end of the list.
 +                       */
 +                      die(_("Internal error"));
 +              }
 +      }
 +      free(ref2->peer_ref);
 +      free(ref2);
 +}
 +
 +struct ref *ref_remove_duplicates(struct ref *ref_map)
  {
        struct string_list refs = STRING_LIST_INIT_NODUP;
 -      struct string_list_item *item = NULL;
 -      struct ref *prev = NULL, *next = NULL;
 -      for (; ref_map; prev = ref_map, ref_map = next) {
 -              next = ref_map->next;
 -              if (!ref_map->peer_ref)
 -                      continue;
 +      struct ref *retval = NULL;
 +      struct ref **p = &retval;
  
 -              item = string_list_lookup(&refs, ref_map->peer_ref->name);
 -              if (item) {
 -                      if (strcmp(((struct ref *)item->util)->name,
 -                                 ref_map->name))
 -                              die("%s tracks both %s and %s",
 -                                  ref_map->peer_ref->name,
 -                                  ((struct ref *)item->util)->name,
 -                                  ref_map->name);
 -                      prev->next = ref_map->next;
 -                      free(ref_map->peer_ref);
 -                      free(ref_map);
 -                      ref_map = prev; /* skip this; we freed it */
 -                      continue;
 -              }
 +      while (ref_map) {
 +              struct ref *ref = ref_map;
 +
 +              ref_map = ref_map->next;
 +              ref->next = NULL;
  
 -              item = string_list_insert(&refs, ref_map->peer_ref->name);
 -              item->util = ref_map;
 +              if (!ref->peer_ref) {
 +                      *p = ref;
 +                      p = &ref->next;
 +              } else {
 +                      struct string_list_item *item =
 +                              string_list_insert(&refs, ref->peer_ref->name);
 +
 +                      if (item->util) {
 +                              /* Entry already existed */
 +                              handle_duplicate((struct ref *)item->util, ref);
 +                      } else {
 +                              *p = ref;
 +                              p = &ref->next;
 +                              item->util = ref;
 +                      }
 +              }
        }
 +
        string_list_clear(&refs, 0);
 +      return retval;
  }
  
  int remote_has_url(struct remote *remote, const char *url)
@@@ -852,12 -826,10 +857,12 @@@ static int match_name_with_pattern(cons
        return ret;
  }
  
 -static int query_refspecs(struct refspec *refs, int ref_count, struct refspec *query)
 +int query_refspecs(struct refspec *refs, int ref_count, struct refspec *query)
  {
        int i;
        int find_src = !query->src;
 +      const char *needle = find_src ? query->dst : query->src;
 +      char **result = find_src ? &query->src : &query->dst;
  
        if (find_src && !query->dst)
                return error("query_refspecs: need either src or dst");
                struct refspec *refspec = &refs[i];
                const char *key = find_src ? refspec->dst : refspec->src;
                const char *value = find_src ? refspec->src : refspec->dst;
 -              const char *needle = find_src ? query->dst : query->src;
 -              char **result = find_src ? &query->src : &query->dst;
  
                if (!refspec->dst)
                        continue;
@@@ -986,9 -960,9 +991,9 @@@ void sort_ref_list(struct ref **l, int 
        *l = llist_mergesort(*l, ref_list_get_next, ref_list_set_next, cmp);
  }
  
 -static int count_refspec_match(const char *pattern,
 -                             struct ref *refs,
 -                             struct ref **matched_ref)
 +int count_refspec_match(const char *pattern,
 +                      struct ref *refs,
 +                      struct ref **matched_ref)
  {
        int patlen = strlen(pattern);
        struct ref *matched_weak = NULL;
                char *name = refs->name;
                int namelen = strlen(name);
  
 -              if (!refname_match(pattern, name, ref_rev_parse_rules))
 +              if (!refname_match(pattern, name))
                        continue;
  
                /* A match is "weak" if it is with refs outside
                 */
                if (namelen != patlen &&
                    patlen != namelen - 5 &&
 -                  prefixcmp(name, "refs/heads/") &&
 -                  prefixcmp(name, "refs/tags/")) {
 +                  !starts_with(name, "refs/heads/") &&
 +                  !starts_with(name, "refs/tags/")) {
                        /* We want to catch the case where only weak
                         * matches are found and there are multiple
                         * matches, and where more than one strong
@@@ -1085,9 -1059,9 +1090,9 @@@ static char *guess_ref(const char *name
        if (!r)
                return NULL;
  
 -      if (!prefixcmp(r, "refs/heads/"))
 +      if (starts_with(r, "refs/heads/"))
                strbuf_addstr(&buf, "refs/heads/");
 -      else if (!prefixcmp(r, "refs/tags/"))
 +      else if (starts_with(r, "refs/tags/"))
                strbuf_addstr(&buf, "refs/tags/");
        else
                return NULL;
@@@ -1135,7 -1109,7 +1140,7 @@@ static int match_explicit(struct ref *s
                dst_value = resolve_ref_unsafe(matched_src->name, sha1, 1, &flag);
                if (!dst_value ||
                    ((flag & REF_ISSYMREF) &&
 -                   prefixcmp(dst_value, "refs/heads/")))
 +                   !starts_with(dst_value, "refs/heads/")))
                        die("%s cannot be resolved to branch.",
                            matched_src->name);
        }
@@@ -1224,7 -1198,7 +1229,7 @@@ static char *get_ref_match(const struc
                 * including refs outside refs/heads/ hierarchy, but
                 * that does not make much sense these days.
                 */
 -              if (!send_mirror && prefixcmp(ref->name, "refs/heads/"))
 +              if (!send_mirror && !starts_with(ref->name, "refs/heads/"))
                        return NULL;
                name = xstrdup(ref->name);
        }
@@@ -1279,7 -1253,7 +1284,7 @@@ static void add_missing_tags(struct re
                        add_to_tips(&sent_tips, ref->peer_ref->new_sha1);
                else
                        add_to_tips(&sent_tips, ref->old_sha1);
 -              if (!prefixcmp(ref->name, "refs/tags/"))
 +              if (starts_with(ref->name, "refs/tags/"))
                        string_list_append(&dst_tag, ref->name);
        }
        clear_commit_marks_many(sent_tips.nr, sent_tips.tip, TMP_MARK);
  
        /* Collect tags they do not have. */
        for (ref = src; ref; ref = ref->next) {
 -              if (prefixcmp(ref->name, "refs/tags/"))
 +              if (!starts_with(ref->name, "refs/tags/"))
                        continue; /* not a tag */
                if (string_list_has_string(&dst_tag, ref->name))
                        continue; /* they already have it */
@@@ -1512,7 -1486,7 +1517,7 @@@ void set_ref_status_for_push(struct re
                 */
  
                else if (!ref->deletion && !is_null_sha1(ref->old_sha1)) {
 -                      if (!prefixcmp(ref->name, "refs/tags/"))
 +                      if (starts_with(ref->name, "refs/tags/"))
                                reject_reason = REF_STATUS_REJECT_ALREADY_EXISTS;
                        else if (!has_sha1_file(ref->old_sha1))
                                reject_reason = REF_STATUS_REJECT_FETCH_FIRST;
@@@ -1571,7 -1545,7 +1576,7 @@@ int branch_merge_matches(struct branch 
  {
        if (!branch || i < 0 || i >= branch->merge_nr)
                return 0;
 -      return refname_match(branch->merge[i]->src, refname, ref_fetch_rules);
 +      return refname_match(branch->merge[i]->src, refname);
  }
  
  static int ignore_symref_update(const char *refname)
        return (flag & REF_ISSYMREF);
  }
  
 +/*
 + * Create and return a list of (struct ref) consisting of copies of
 + * each remote_ref that matches refspec.  refspec must be a pattern.
 + * Fill in the copies' peer_ref to describe the local tracking refs to
 + * which they map.  Omit any references that would map to an existing
 + * local symbolic ref.
 + */
  static struct ref *get_expanded_map(const struct ref *remote_refs,
                                    const struct refspec *refspec)
  {
        struct ref *ret = NULL;
        struct ref **tail = &ret;
  
 -      char *expn_name;
 -
        for (ref = remote_refs; ref; ref = ref->next) {
 +              char *expn_name = NULL;
 +
                if (strchr(ref->name, '^'))
                        continue; /* a dereference item */
                if (match_name_with_pattern(refspec->src, ref->name,
                        struct ref *cpy = copy_ref(ref);
  
                        cpy->peer_ref = alloc_ref(expn_name);
 -                      free(expn_name);
                        if (refspec->force)
                                cpy->peer_ref->force = 1;
                        *tail = cpy;
                        tail = &cpy->next;
                }
 +              free(expn_name);
        }
  
        return ret;
@@@ -1624,7 -1591,7 +1629,7 @@@ static const struct ref *find_ref_by_na
  {
        const struct ref *ref;
        for (ref = refs; ref; ref = ref->next) {
 -              if (refname_match(name, ref->name, ref_fetch_rules))
 +              if (refname_match(name, ref->name))
                        return ref;
        }
        return NULL;
@@@ -1645,12 -1612,12 +1650,12 @@@ static struct ref *get_local_ref(const 
        if (!name || name[0] == '\0')
                return NULL;
  
 -      if (!prefixcmp(name, "refs/"))
 +      if (starts_with(name, "refs/"))
                return alloc_ref(name);
  
 -      if (!prefixcmp(name, "heads/") ||
 -          !prefixcmp(name, "tags/") ||
 -          !prefixcmp(name, "remotes/"))
 +      if (starts_with(name, "heads/") ||
 +          starts_with(name, "tags/") ||
 +          starts_with(name, "remotes/"))
                return alloc_ref_with_prefix("refs/", 5, name);
  
        return alloc_ref_with_prefix("refs/heads/", 11, name);
@@@ -1685,7 -1652,7 +1690,7 @@@ int get_fetch_map(const struct ref *rem
  
        for (rmp = &ref_map; *rmp; ) {
                if ((*rmp)->peer_ref) {
 -                      if (prefixcmp((*rmp)->peer_ref->name, "refs/") ||
 +                      if (!starts_with((*rmp)->peer_ref->name, "refs/") ||
                            check_refname_format((*rmp)->peer_ref->name, 0)) {
                                struct ref *ignore = *rmp;
                                error("* Ignoring funny ref '%s' locally",
@@@ -1969,7 -1936,7 +1974,7 @@@ struct ref *guess_remote_head(const str
        /* Look for another ref that points there */
        for (r = refs; r; r = r->next) {
                if (r != head &&
 -                  !prefixcmp(r->name, "refs/heads/") &&
 +                  starts_with(r->name, "refs/heads/") &&
                    !hashcmp(r->old_sha1, head->old_sha1)) {
                        *tail = copy_ref(r);
                        tail = &((*tail)->next);
@@@ -2121,7 -2088,7 +2126,7 @@@ static void apply_cas(struct push_cas_o
        /* Find an explicit --<option>=<name>[:<value>] entry */
        for (i = 0; i < cas->nr; i++) {
                struct push_cas *entry = &cas->entry[i];
 -              if (!refname_match(entry->refname, ref->name, ref_rev_parse_rules))
 +              if (!refname_match(entry->refname, ref->name))
                        continue;
                ref->expect_old_sha1 = 1;
                if (!entry->use_tracking)
diff --combined t/t5516-fetch-push.sh
index 926e7f6b979d4d1c69ecc9d280cd6de365627012,084a8cbd91ae7de5de2ea3440a98950adae183d6..67e0ab346204b437c8e3585d948c2c61858e1928
@@@ -536,6 -536,19 +536,19 @@@ test_expect_success 'push with config b
        check_push_result down_repo $the_commit heads/master
  '
  
+ test_expect_success 'branch.*.pushremote config order is irrelevant' '
+       mk_test one_repo heads/master &&
+       mk_test two_repo heads/master &&
+       test_config remote.one.url one_repo &&
+       test_config remote.two.url two_repo &&
+       test_config branch.master.pushremote two_repo &&
+       test_config remote.pushdefault one_repo &&
+       test_config push.default matching &&
+       git push &&
+       check_push_result one_repo $the_first_commit heads/master &&
+       check_push_result two_repo $the_commit heads/master
+ '
  test_expect_success 'push with dry-run' '
  
        mk_test testrepo heads/master &&
@@@ -1126,81 -1139,6 +1139,81 @@@ test_expect_success 'fetch follows tag
        test_cmp expect actual
  '
  
 +test_expect_success 'pushing a specific ref applies remote.$name.push as refmap' '
 +      mk_test testrepo heads/master &&
 +      rm -fr src dst &&
 +      git init src &&
 +      git init --bare dst &&
 +      (
 +              cd src &&
 +              git pull ../testrepo master &&
 +              git branch next &&
 +              git config remote.dst.url ../dst &&
 +              git config remote.dst.push "+refs/heads/*:refs/remotes/src/*" &&
 +              git push dst master &&
 +              git show-ref refs/heads/master |
 +              sed -e "s|refs/heads/|refs/remotes/src/|" >../dst/expect
 +      ) &&
 +      (
 +              cd dst &&
 +              test_must_fail git show-ref refs/heads/next &&
 +              test_must_fail git show-ref refs/heads/master &&
 +              git show-ref refs/remotes/src/master >actual
 +      ) &&
 +      test_cmp dst/expect dst/actual
 +'
 +
 +test_expect_success 'with no remote.$name.push, it is not used as refmap' '
 +      mk_test testrepo heads/master &&
 +      rm -fr src dst &&
 +      git init src &&
 +      git init --bare dst &&
 +      (
 +              cd src &&
 +              git pull ../testrepo master &&
 +              git branch next &&
 +              git config remote.dst.url ../dst &&
 +              git config push.default matching &&
 +              git push dst master &&
 +              git show-ref refs/heads/master >../dst/expect
 +      ) &&
 +      (
 +              cd dst &&
 +              test_must_fail git show-ref refs/heads/next &&
 +              git show-ref refs/heads/master >actual
 +      ) &&
 +      test_cmp dst/expect dst/actual
 +'
 +
 +test_expect_success 'with no remote.$name.push, upstream mapping is used' '
 +      mk_test testrepo heads/master &&
 +      rm -fr src dst &&
 +      git init src &&
 +      git init --bare dst &&
 +      (
 +              cd src &&
 +              git pull ../testrepo master &&
 +              git branch next &&
 +              git config remote.dst.url ../dst &&
 +              git config remote.dst.fetch "+refs/heads/*:refs/remotes/dst/*" &&
 +              git config push.default upstream &&
 +
 +              git config branch.master.merge refs/heads/trunk &&
 +              git config branch.master.remote dst &&
 +
 +              git push dst master &&
 +              git show-ref refs/heads/master |
 +              sed -e "s|refs/heads/master|refs/heads/trunk|" >../dst/expect
 +      ) &&
 +      (
 +              cd dst &&
 +              test_must_fail git show-ref refs/heads/master &&
 +              test_must_fail git show-ref refs/heads/next &&
 +              git show-ref refs/heads/trunk >actual
 +      ) &&
 +      test_cmp dst/expect dst/actual
 +'
 +
  test_expect_success 'push does not follow tags by default' '
        mk_test testrepo heads/master &&
        rm -fr src dst &&