Merge branch 'jk/remote-insteadof-cleanup'
authorJunio C Hamano <gitster@pobox.com>
Tue, 29 Jan 2019 20:47:55 +0000 (12:47 -0800)
committerJunio C Hamano <gitster@pobox.com>
Tue, 29 Jan 2019 20:47:55 +0000 (12:47 -0800)
Code clean-up.

* jk/remote-insteadof-cleanup:
remote: check config validity before creating rewrite struct

1  2 
remote.c
diff --combined remote.c
index 670dd448130d20325f5f9f1de8afd0f011535db2,571dab63f675eeb480bcd4cf465d8f54cc6ef998..9cc3b07d214a63271849243c5f823b8f87ad631b
+++ b/remote.c
@@@ -12,8 -12,6 +12,8 @@@
  #include "string-list.h"
  #include "mergesort.h"
  #include "argv-array.h"
 +#include "commit-reach.h"
 +#include "advice.h"
  
  enum map_direction { FROM_SRC, FROM_DST };
  
@@@ -337,14 -335,14 +337,14 @@@ static int handle_config(const char *ke
                if (!name)
                        return 0;
                if (!strcmp(subkey, "insteadof")) {
-                       rewrite = make_rewrite(&rewrites, name, namelen);
                        if (!value)
                                return config_error_nonbool(key);
+                       rewrite = make_rewrite(&rewrites, name, namelen);
                        add_instead_of(rewrite, xstrdup(value));
                } else if (!strcmp(subkey, "pushinsteadof")) {
-                       rewrite = make_rewrite(&rewrites_push, name, namelen);
                        if (!value)
                                return config_error_nonbool(key);
+                       rewrite = make_rewrite(&rewrites_push, name, namelen);
                        add_instead_of(rewrite, xstrdup(value));
                }
        }
                return 0;
        /* Handle remote.<name>.* variables */
        if (*name == '/') {
 -              warning("Config remote shorthand cannot begin with '/': %s",
 +              warning(_("config remote shorthand cannot begin with '/': %s"),
                        name);
                return 0;
        }
                if (!remote->receivepack)
                        remote->receivepack = v;
                else
 -                      error("more than one receivepack given, using the first");
 +                      error(_("more than one receivepack given, using the first"));
        } else if (!strcmp(subkey, "uploadpack")) {
                const char *v;
                if (git_config_string(&v, key, value))
                if (!remote->uploadpack)
                        remote->uploadpack = v;
                else
 -                      error("more than one uploadpack given, using the first");
 +                      error(_("more than one uploadpack given, using the first"));
        } else if (!strcmp(subkey, "tagopt")) {
                if (!strcmp(value, "--no-tags"))
                        remote->fetch_tags = -1;
@@@ -621,7 -619,7 +621,7 @@@ static void handle_duplicate(struct re
                         * FETCH_HEAD_IGNORE entries always appear at
                         * the end of the list.
                         */
 -                      die(_("Internal error"));
 +                      BUG("Internal error");
                }
        }
        free(ref2->peer_ref);
@@@ -681,7 -679,7 +681,7 @@@ static int match_name_with_pattern(cons
        size_t namelen;
        int ret;
        if (!kstar)
 -              die("Key '%s' of pattern had no '*'", key);
 +              die(_("key '%s' of pattern had no '*'"), key);
        klen = kstar - key;
        ksuffixlen = strlen(kstar + 1);
        namelen = strlen(name);
                struct strbuf sb = STRBUF_INIT;
                const char *vstar = strchr(value, '*');
                if (!vstar)
 -                      die("Value '%s' of pattern has no '*'", value);
 +                      die(_("value '%s' of pattern has no '*'"), value);
                strbuf_add(&sb, value, vstar - value);
                strbuf_add(&sb, name + klen, namelen - klen - ksuffixlen);
                strbuf_addstr(&sb, vstar + 1);
@@@ -708,7 -706,7 +708,7 @@@ static void query_refspecs_multiple(str
        int find_src = !query->src;
  
        if (find_src && !query->dst)
 -              error("query_refspecs_multiple: need either src or dst");
 +              BUG("query_refspecs_multiple: need either src or dst");
  
        for (i = 0; i < rs->nr; i++) {
                struct refspec_item *refspec = &rs->items[i];
@@@ -736,7 -734,7 +736,7 @@@ int query_refspecs(struct refspec *rs, 
        char **result = find_src ? &query->src : &query->dst;
  
        if (find_src && !query->dst)
 -              return error("query_refspecs: need either src or dst");
 +              BUG("query_refspecs: need either src or dst");
  
        for (i = 0; i < rs->nr; i++) {
                struct refspec_item *refspec = &rs->items[i];
@@@ -969,13 -967,12 +969,13 @@@ static char *guess_ref(const char *name
        if (!r)
                return NULL;
  
 -      if (starts_with(r, "refs/heads/"))
 +      if (starts_with(r, "refs/heads/")) {
                strbuf_addstr(&buf, "refs/heads/");
 -      else if (starts_with(r, "refs/tags/"))
 +      } else if (starts_with(r, "refs/tags/")) {
                strbuf_addstr(&buf, "refs/tags/");
 -      else
 +      } else {
                return NULL;
 +      }
  
        strbuf_addstr(&buf, name);
        return strbuf_detach(&buf, NULL);
@@@ -997,68 -994,12 +997,68 @@@ static int match_explicit_lhs(struct re
                 * way to delete 'other' ref at the remote end.
                 */
                if (try_explicit_object_name(rs->src, match) < 0)
 -                      return error("src refspec %s does not match any.", rs->src);
 +                      return error(_("src refspec %s does not match any"), rs->src);
                if (allocated_match)
                        *allocated_match = 1;
                return 0;
        default:
 -              return error("src refspec %s matches more than one.", rs->src);
 +              return error(_("src refspec %s matches more than one"), rs->src);
 +      }
 +}
 +
 +static void show_push_unqualified_ref_name_error(const char *dst_value,
 +                                               const char *matched_src_name)
 +{
 +      struct object_id oid;
 +      enum object_type type;
 +
 +      /*
 +       * TRANSLATORS: "matches '%s'%" is the <dst> part of "git push
 +       * <remote> <src>:<dst>" push, and "being pushed ('%s')" is
 +       * the <src>.
 +       */
 +      error(_("The destination you provided is not a full refname (i.e.,\n"
 +              "starting with \"refs/\"). We tried to guess what you meant by:\n"
 +              "\n"
 +              "- Looking for a ref that matches '%s' on the remote side.\n"
 +              "- Checking if the <src> being pushed ('%s')\n"
 +              "  is a ref in \"refs/{heads,tags}/\". If so we add a corresponding\n"
 +              "  refs/{heads,tags}/ prefix on the remote side.\n"
 +              "\n"
 +              "Neither worked, so we gave up. You must fully qualify the ref."),
 +            dst_value, matched_src_name);
 +
 +      if (!advice_push_unqualified_ref_name)
 +              return;
 +
 +      if (get_oid(matched_src_name, &oid))
 +              BUG("'%s' is not a valid object, "
 +                  "match_explicit_lhs() should catch this!",
 +                  matched_src_name);
 +      type = oid_object_info(the_repository, &oid, NULL);
 +      if (type == OBJ_COMMIT) {
 +              advise(_("The <src> part of the refspec is a commit object.\n"
 +                       "Did you mean to create a new branch by pushing to\n"
 +                       "'%s:refs/heads/%s'?"),
 +                     matched_src_name, dst_value);
 +      } else if (type == OBJ_TAG) {
 +              advise(_("The <src> part of the refspec is a tag object.\n"
 +                       "Did you mean to create a new tag by pushing to\n"
 +                       "'%s:refs/tags/%s'?"),
 +                     matched_src_name, dst_value);
 +      } else if (type == OBJ_TREE) {
 +              advise(_("The <src> part of the refspec is a tree object.\n"
 +                       "Did you mean to tag a new tree by pushing to\n"
 +                       "'%s:refs/tags/%s'?"),
 +                     matched_src_name, dst_value);
 +      } else if (type == OBJ_BLOB) {
 +              advise(_("The <src> part of the refspec is a blob object.\n"
 +                       "Did you mean to tag a new blob by pushing to\n"
 +                       "'%s:refs/tags/%s'?"),
 +                     matched_src_name, dst_value);
 +      } else {
 +              BUG("'%s' should be commit/tag/tree/blob, is '%d'",
 +                  matched_src_name, type);
        }
  }
  
@@@ -1088,7 -1029,7 +1088,7 @@@ static int match_explicit(struct ref *s
                if (!dst_value ||
                    ((flag & REF_ISSYMREF) &&
                     !starts_with(dst_value, "refs/heads/")))
 -                      die("%s cannot be resolved to branch.",
 +                      die(_("%s cannot be resolved to branch"),
                            matched_src->name);
        }
  
        case 1:
                break;
        case 0:
 -              if (starts_with(dst_value, "refs/"))
 +              if (starts_with(dst_value, "refs/")) {
                        matched_dst = make_linked_ref(dst_value, dst_tail);
 -              else if (is_null_oid(&matched_src->new_oid))
 -                      error("unable to delete '%s': remote ref does not exist",
 +              } else if (is_null_oid(&matched_src->new_oid)) {
 +                      error(_("unable to delete '%s': remote ref does not exist"),
                              dst_value);
 -              else if ((dst_guess = guess_ref(dst_value, matched_src))) {
 +              else if ((dst_guess = guess_ref(dst_value, matched_src))) {
                        matched_dst = make_linked_ref(dst_guess, dst_tail);
                        free(dst_guess);
 -              } else
 -                      error("unable to push to unqualified destination: %s\n"
 -                            "The destination refspec neither matches an "
 -                            "existing ref on the remote nor\n"
 -                            "begins with refs/, and we are unable to "
 -                            "guess a prefix based on the source ref.",
 -                            dst_value);
 +              } else {
 +                      show_push_unqualified_ref_name_error(dst_value,
 +                                                           matched_src->name);
 +              }
                break;
        default:
                matched_dst = NULL;
 -              error("dst refspec %s matches more than one.",
 +              error(_("dst refspec %s matches more than one"),
                      dst_value);
                break;
        }
        if (!matched_dst)
                return -1;
        if (matched_dst->peer_ref)
 -              return error("dst ref %s receives from more than one src.",
 -                    matched_dst->name);
 +              return error(_("dst ref %s receives from more than one src"),
 +                           matched_dst->name);
        else {
                matched_dst->peer_ref = allocated_src ?
                                        matched_src :
@@@ -1260,36 -1204,9 +1260,36 @@@ static void add_missing_tags(struct re
         * sent to the other side.
         */
        if (sent_tips.nr) {
 +              const int reachable_flag = 1;
 +              struct commit_list *found_commits;
 +              struct commit **src_commits;
 +              int nr_src_commits = 0, alloc_src_commits = 16;
 +              ALLOC_ARRAY(src_commits, alloc_src_commits);
 +
                for_each_string_list_item(item, &src_tag) {
                        struct ref *ref = item->util;
 +                      struct commit *commit;
 +
 +                      if (is_null_oid(&ref->new_oid))
 +                              continue;
 +                      commit = lookup_commit_reference_gently(the_repository,
 +                                                              &ref->new_oid,
 +                                                              1);
 +                      if (!commit)
 +                              /* not pushing a commit, which is not an error */
 +                              continue;
 +
 +                      ALLOC_GROW(src_commits, nr_src_commits + 1, alloc_src_commits);
 +                      src_commits[nr_src_commits++] = commit;
 +              }
 +
 +              found_commits = get_reachable_subset(sent_tips.tip, sent_tips.nr,
 +                                                   src_commits, nr_src_commits,
 +                                                   reachable_flag);
 +
 +              for_each_string_list_item(item, &src_tag) {
                        struct ref *dst_ref;
 +                      struct ref *ref = item->util;
                        struct commit *commit;
  
                        if (is_null_oid(&ref->new_oid))
                         * Is this tag, which they do not have, reachable from
                         * any of the commits we are sending?
                         */
 -                      if (!in_merge_bases_many(commit, sent_tips.nr, sent_tips.tip))
 +                      if (!(commit->object.flags & reachable_flag))
                                continue;
  
                        /* Add it in */
                        oidcpy(&dst_ref->new_oid, &ref->new_oid);
                        dst_ref->peer_ref = copy_ref(ref);
                }
 +
 +              clear_commit_marks_many(nr_src_commits, src_commits, reachable_flag);
 +              free(src_commits);
 +              free_commit_list(found_commits);
        }
 +
        string_list_clear(&src_tag, 0);
        free(sent_tips.tip);
  }
@@@ -1476,7 -1388,7 +1476,7 @@@ void set_ref_status_for_push(struct re
  
                ref->deletion = is_null_oid(&ref->new_oid);
                if (!ref->deletion &&
 -                      !oidcmp(&ref->old_oid, &ref->new_oid)) {
 +                      oideq(&ref->old_oid, &ref->new_oid)) {
                        ref->status = REF_STATUS_UPTODATE;
                        continue;
                }
                 * branch.
                 */
                if (ref->expect_old_sha1) {
 -                      if (oidcmp(&ref->old_oid, &ref->old_oid_expect))
 +                      if (!oideq(&ref->old_oid, &ref->old_oid_expect))
                                reject_reason = REF_STATUS_REJECT_STALE;
                        else
                                /* If the ref isn't stale then force the update. */
@@@ -1837,7 -1749,7 +1837,7 @@@ int get_fetch_map(const struct ref *rem
                        ref_map = get_remote_ref(remote_refs, name);
                }
                if (!missing_ok && !ref_map)
 -                      die("Couldn't find remote ref %s", name);
 +                      die(_("couldn't find remote ref %s"), name);
                if (ref_map) {
                        ref_map->peer_ref = get_local_ref(refspec->dst);
                        if (ref_map->peer_ref && refspec->force)
                        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",
 +                              error(_("* Ignoring funny ref '%s' locally"),
                                      (*rmp)->peer_ref->name);
                                *rmp = (*rmp)->next;
                                free(ignore->peer_ref);
@@@ -1879,6 -1791,55 +1879,6 @@@ int resolve_remote_symref(struct ref *r
        return 1;
  }
  
 -static void unmark_and_free(struct commit_list *list, unsigned int mark)
 -{
 -      while (list) {
 -              struct commit *commit = pop_commit(&list);
 -              commit->object.flags &= ~mark;
 -      }
 -}
 -
 -int ref_newer(const struct object_id *new_oid, const struct object_id *old_oid)
 -{
 -      struct object *o;
 -      struct commit *old_commit, *new_commit;
 -      struct commit_list *list, *used;
 -      int found = 0;
 -
 -      /*
 -       * Both new_commit and old_commit must be commit-ish and new_commit is descendant of
 -       * old_commit.  Otherwise we require --force.
 -       */
 -      o = deref_tag(the_repository, parse_object(the_repository, old_oid),
 -                    NULL, 0);
 -      if (!o || o->type != OBJ_COMMIT)
 -              return 0;
 -      old_commit = (struct commit *) o;
 -
 -      o = deref_tag(the_repository, parse_object(the_repository, new_oid),
 -                    NULL, 0);
 -      if (!o || o->type != OBJ_COMMIT)
 -              return 0;
 -      new_commit = (struct commit *) o;
 -
 -      if (parse_commit(new_commit) < 0)
 -              return 0;
 -
 -      used = list = NULL;
 -      commit_list_insert(new_commit, &list);
 -      while (list) {
 -              new_commit = pop_most_recent_commit(&list, TMP_MARK);
 -              commit_list_insert(new_commit, &used);
 -              if (new_commit == old_commit) {
 -                      found = 1;
 -                      break;
 -              }
 -      }
 -      unmark_and_free(list, TMP_MARK);
 -      unmark_and_free(used, TMP_MARK);
 -      return found;
 -}
 -
  /*
   * Lookup the upstream branch for the given branch and if present, optionally
   * compute the commit ahead/behind values for the pair.
@@@ -1942,10 -1903,10 +1942,10 @@@ int stat_tracking_info(struct branch *b
                         oid_to_hex(&theirs->object.oid));
        argv_array_push(&argv, "--");
  
 -      init_revisions(&revs, NULL);
 +      repo_init_revisions(the_repository, &revs, NULL);
        setup_revisions(argv.argc, argv.argv, &revs, NULL);
        if (prepare_revision_walk(&revs))
 -              die("revision walk setup failed");
 +              die(_("revision walk setup failed"));
  
        /* ... and count the commits on each side. */
        while (1) {
@@@ -2088,7 -2049,7 +2088,7 @@@ struct ref *guess_remote_head(const str
        /* If refs/heads/master could be right, it is. */
        if (!all) {
                r = find_ref_by_name(refs, "refs/heads/master");
 -              if (r && !oidcmp(&r->old_oid, &head->old_oid))
 +              if (r && oideq(&r->old_oid, &head->old_oid))
                        return copy_ref(r);
        }
  
        for (r = refs; r; r = r->next) {
                if (r != head &&
                    starts_with(r->name, "refs/heads/") &&
 -                  !oidcmp(&r->old_oid, &head->old_oid)) {
 +                  oideq(&r->old_oid, &head->old_oid)) {
                        *tail = copy_ref(r);
                        tail = &((*tail)->next);
                        if (!all)
@@@ -2218,8 -2179,7 +2218,8 @@@ static int parse_push_cas_option(struc
        else if (!colon[1])
                oidclr(&entry->expect);
        else if (get_oid(colon + 1, &entry->expect))
 -              return error("cannot parse expected object name '%s'", colon + 1);
 +              return error(_("cannot parse expected object name '%s'"),
 +                           colon + 1);
        return 0;
  }