Merge branch 'mm/fetch-show-error-message-on-unadvertised-object'
authorJunio C Hamano <gitster@pobox.com>
Tue, 14 Mar 2017 22:23:18 +0000 (15:23 -0700)
committerJunio C Hamano <gitster@pobox.com>
Tue, 14 Mar 2017 22:23:18 +0000 (15:23 -0700)
"git fetch" that requests a commit by object name, when the other
side does not allow such an request, failed without much
explanation.

* mm/fetch-show-error-message-on-unadvertised-object:
fetch-pack: add specific error for fetching an unadvertised object
fetch_refs_via_pack: call report_unmatched_refs
fetch-pack: move code to report unmatched refs to a function

1  2 
fetch-pack.c
remote.h
t/t5516-fetch-push.sh
transport.c
diff --combined fetch-pack.c
index e0f5d5ce875acad79b5ddcfc4e24b6ff8bc33df9,f12bfcdbb12c0bde9271ba45e5d8b6941439efc1..d07d85ce302d39abc8c017c377d4230d729fdeb4
@@@ -35,7 -35,6 +35,7 @@@ static const char *alternate_shallow_fi
  #define COMMON_REF    (1U << 2)
  #define SEEN          (1U << 3)
  #define POPPED                (1U << 4)
 +#define ALTERNATE     (1U << 5)
  
  static int marked;
  
@@@ -68,41 -67,6 +68,41 @@@ static inline void print_verbose(const 
        fputc('\n', stderr);
  }
  
 +struct alternate_object_cache {
 +      struct object **items;
 +      size_t nr, alloc;
 +};
 +
 +static void cache_one_alternate(const char *refname,
 +                              const struct object_id *oid,
 +                              void *vcache)
 +{
 +      struct alternate_object_cache *cache = vcache;
 +      struct object *obj = parse_object(oid->hash);
 +
 +      if (!obj || (obj->flags & ALTERNATE))
 +              return;
 +
 +      obj->flags |= ALTERNATE;
 +      ALLOC_GROW(cache->items, cache->nr + 1, cache->alloc);
 +      cache->items[cache->nr++] = obj;
 +}
 +
 +static void for_each_cached_alternate(void (*cb)(struct object *))
 +{
 +      static int initialized;
 +      static struct alternate_object_cache cache;
 +      size_t i;
 +
 +      if (!initialized) {
 +              for_each_alternate_ref(cache_one_alternate, &cache);
 +              initialized = 1;
 +      }
 +
 +      for (i = 0; i < cache.nr; i++)
 +              cb(cache.items[i]);
 +}
 +
  static void rev_list_push(struct commit *commit, int mark)
  {
        if (!(commit->object.flags & mark)) {
@@@ -289,9 -253,9 +289,9 @@@ static void send_request(struct fetch_p
                write_or_die(fd, buf->buf, buf->len);
  }
  
 -static void insert_one_alternate_ref(const struct ref *ref, void *unused)
 +static void insert_one_alternate_object(struct object *obj)
  {
 -      rev_list_insert_ref(NULL, ref->old_oid.hash);
 +      rev_list_insert_ref(NULL, obj->oid.hash);
  }
  
  #define INITIAL_FLUSH 16
@@@ -334,7 -298,7 +334,7 @@@ static int find_common(struct fetch_pac
        marked = 1;
  
        for_each_ref(rev_list_insert_ref_oid, NULL);
 -      for_each_alternate_ref(insert_one_alternate_ref, NULL);
 +      for_each_cached_alternate(insert_one_alternate_object);
  
        fetching = 0;
        for ( ; refs ; refs = refs->next) {
@@@ -614,7 -578,7 +614,7 @@@ static void filter_refs(struct fetch_pa
                                        break; /* definitely do not have it */
                                else if (cmp == 0) {
                                        keep = 1; /* definitely have it */
-                                       sought[i]->matched = 1;
+                                       sought[i]->match_status = REF_MATCHED;
                                }
                                i++;
                        }
        }
  
        /* Append unmatched requests to the list */
-       if ((allow_unadvertised_object_request &
-           (ALLOW_TIP_SHA1 | ALLOW_REACHABLE_SHA1))) {
-               for (i = 0; i < nr_sought; i++) {
-                       unsigned char sha1[20];
+       for (i = 0; i < nr_sought; i++) {
+               unsigned char sha1[20];
  
-                       ref = sought[i];
-                       if (ref->matched)
-                               continue;
-                       if (get_sha1_hex(ref->name, sha1) ||
-                           ref->name[40] != '\0' ||
-                           hashcmp(sha1, ref->old_oid.hash))
-                               continue;
+               ref = sought[i];
+               if (ref->match_status != REF_NOT_MATCHED)
+                       continue;
+               if (get_sha1_hex(ref->name, sha1) ||
+                   ref->name[40] != '\0' ||
+                   hashcmp(sha1, ref->old_oid.hash))
+                       continue;
  
-                       ref->matched = 1;
+               if ((allow_unadvertised_object_request &
+                   (ALLOW_TIP_SHA1 | ALLOW_REACHABLE_SHA1))) {
+                       ref->match_status = REF_MATCHED;
                        *newtail = copy_ref(ref);
                        newtail = &(*newtail)->next;
+               } else {
+                       ref->match_status = REF_UNADVERTISED_NOT_ALLOWED;
                }
        }
        *refs = newlist;
  }
  
 -static void mark_alternate_complete(const struct ref *ref, void *unused)
 +static void mark_alternate_complete(struct object *obj)
  {
 -      mark_complete(ref->old_oid.hash);
 +      mark_complete(obj->oid.hash);
  }
  
  static int everything_local(struct fetch_pack_args *args,
  
        if (!args->deepen) {
                for_each_ref(mark_complete_oid, NULL);
 -              for_each_alternate_ref(mark_alternate_complete, NULL);
 +              for_each_cached_alternate(mark_alternate_complete);
                commit_list_sort_by_date(&complete);
                if (cutoff)
                        mark_recent_complete_commits(args, cutoff);
@@@ -1130,3 -1096,26 +1132,26 @@@ struct ref *fetch_pack(struct fetch_pac
        clear_shallow_info(&si);
        return ref_cpy;
  }
+ int report_unmatched_refs(struct ref **sought, int nr_sought)
+ {
+       int i, ret = 0;
+       for (i = 0; i < nr_sought; i++) {
+               if (!sought[i])
+                       continue;
+               switch (sought[i]->match_status) {
+               case REF_MATCHED:
+                       continue;
+               case REF_NOT_MATCHED:
+                       error(_("no such remote ref %s"), sought[i]->name);
+                       break;
+               case REF_UNADVERTISED_NOT_ALLOWED:
+                       error(_("Server does not allow request for unadvertised object %s"),
+                             sought[i]->name);
+                       break;
+               }
+               ret = 1;
+       }
+       return ret;
+ }
diff --combined remote.h
index a5bbbe0ef965c5e62be50e8ee0c03794e393cc8c,0b9d8c45895262a751f3f7f01a192f75eb007503..dd8c5175776baabbac2bd735e21bc859fb62a111
+++ b/remote.h
@@@ -15,7 -15,7 +15,7 @@@ struct remote 
        struct hashmap_entry ent;  /* must be first */
  
        const char *name;
 -      int origin;
 +      int origin, configured_in_repo;
  
        const char *foreign_vcs;
  
@@@ -60,7 -60,7 +60,7 @@@
  
  struct remote *remote_get(const char *name);
  struct remote *pushremote_get(const char *name);
 -int remote_is_configured(struct remote *remote);
 +int remote_is_configured(struct remote *remote, int in_repo);
  
  typedef int each_remote_fn(struct remote *remote, void *priv);
  int for_each_remote(each_remote_fn fn, void *priv);
@@@ -89,8 -89,13 +89,13 @@@ struct ref 
                force:1,
                forced_update:1,
                expect_old_sha1:1,
-               deletion:1,
-               matched:1;
+               deletion:1;
+       enum {
+               REF_NOT_MATCHED = 0, /* initial value */
+               REF_MATCHED,
+               REF_UNADVERTISED_NOT_ALLOWED
+       } match_status;
  
        /*
         * Order is important here, as we write to FETCH_HEAD
diff --combined t/t5516-fetch-push.sh
index 0fc5a7c596b5d5e01ecb43f81d3d6eaafe611b7b,78f3b8ef22f919e98bb668686aa265c3f2688f74..177897ea0b1e00cc4ec0f0e43510411ab19f1681
@@@ -1004,7 -1004,7 +1004,7 @@@ test_expect_success 'push --porcelain' 
  test_expect_success 'push --porcelain bad url' '
        mk_empty testrepo &&
        test_must_fail git push >.git/bar --porcelain asdfasdfasd refs/heads/master:refs/remotes/origin/master &&
 -      test_must_fail grep -q Done .git/bar
 +      ! grep -q Done .git/bar
  '
  
  test_expect_success 'push --porcelain rejected' '
@@@ -1098,7 -1098,8 +1098,8 @@@ test_expect_success 'fetch exact SHA1' 
                test_must_fail git cat-file -t $the_commit &&
  
                # fetching the hidden object should fail by default
-               test_must_fail git fetch -v ../testrepo $the_commit:refs/heads/copy &&
+               test_must_fail git fetch -v ../testrepo $the_commit:refs/heads/copy 2>err &&
+               test_i18ngrep "Server does not allow request for unadvertised object" err &&
                test_must_fail git rev-parse --verify refs/heads/copy &&
  
                # the server side can allow it to succeed
diff --combined transport.c
index 5828e06afd670fc9c563da8bf233c34778872e9c,c377907d4fdb99709c8164ee8708b34f7e16fd61..ea1feac9dcd83dac05cf0e2c35cf5f7a022810aa
@@@ -204,6 -204,7 +204,7 @@@ static struct ref *get_refs_via_connect
  static int fetch_refs_via_pack(struct transport *transport,
                               int nr_heads, struct ref **to_fetch)
  {
+       int ret = 0;
        struct git_transport_data *data = transport->data;
        struct ref *refs;
        char *dest = xstrdup(transport->url);
                          &transport->pack_lockfile);
        close(data->fd[0]);
        close(data->fd[1]);
-       if (finish_connect(data->conn)) {
-               free_refs(refs);
-               refs = NULL;
-       }
+       if (finish_connect(data->conn))
+               ret = -1;
        data->conn = NULL;
        data->got_remote_heads = 0;
        data->options.self_contained_and_connected =
                args.self_contained_and_connected;
  
+       if (refs == NULL)
+               ret = -1;
+       if (report_unmatched_refs(to_fetch, nr_heads))
+               ret = -1;
        free_refs(refs_tmp);
        free_refs(refs);
        free(dest);
-       return (refs ? 0 : -1);
+       return ret;
  }
  
  static int push_had_errors(struct ref *ref)
@@@ -299,7 -303,7 +303,7 @@@ void transport_update_tracking_ref(stru
                if (verbose)
                        fprintf(stderr, "updating local tracking ref '%s'\n", rs.dst);
                if (ref->deletion) {
 -                      delete_ref(rs.dst, NULL, 0);
 +                      delete_ref(NULL, rs.dst, NULL, 0);
                } else
                        update_ref("update by push", rs.dst,
                                        ref->new_oid.hash, NULL, 0, 0);
@@@ -664,89 -668,21 +668,89 @@@ static const struct string_list *protoc
        return enabled ? &allowed : NULL;
  }
  
 -int is_transport_allowed(const char *type)
 +enum protocol_allow_config {
 +      PROTOCOL_ALLOW_NEVER = 0,
 +      PROTOCOL_ALLOW_USER_ONLY,
 +      PROTOCOL_ALLOW_ALWAYS
 +};
 +
 +static enum protocol_allow_config parse_protocol_config(const char *key,
 +                                                      const char *value)
  {
 -      const struct string_list *allowed = protocol_whitelist();
 -      return !allowed || string_list_has_string(allowed, type);
 +      if (!strcasecmp(value, "always"))
 +              return PROTOCOL_ALLOW_ALWAYS;
 +      else if (!strcasecmp(value, "never"))
 +              return PROTOCOL_ALLOW_NEVER;
 +      else if (!strcasecmp(value, "user"))
 +              return PROTOCOL_ALLOW_USER_ONLY;
 +
 +      die("unknown value for config '%s': %s", key, value);
  }
  
 -void transport_check_allowed(const char *type)
 +static enum protocol_allow_config get_protocol_config(const char *type)
  {
 -      if (!is_transport_allowed(type))
 -              die("transport '%s' not allowed", type);
 +      char *key = xstrfmt("protocol.%s.allow", type);
 +      char *value;
 +
 +      /* first check the per-protocol config */
 +      if (!git_config_get_string(key, &value)) {
 +              enum protocol_allow_config ret =
 +                      parse_protocol_config(key, value);
 +              free(key);
 +              free(value);
 +              return ret;
 +      }
 +      free(key);
 +
 +      /* if defined, fallback to user-defined default for unknown protocols */
 +      if (!git_config_get_string("protocol.allow", &value)) {
 +              enum protocol_allow_config ret =
 +                      parse_protocol_config("protocol.allow", value);
 +              free(value);
 +              return ret;
 +      }
 +
 +      /* fallback to built-in defaults */
 +      /* known safe */
 +      if (!strcmp(type, "http") ||
 +          !strcmp(type, "https") ||
 +          !strcmp(type, "git") ||
 +          !strcmp(type, "ssh") ||
 +          !strcmp(type, "file"))
 +              return PROTOCOL_ALLOW_ALWAYS;
 +
 +      /* known scary; err on the side of caution */
 +      if (!strcmp(type, "ext"))
 +              return PROTOCOL_ALLOW_NEVER;
 +
 +      /* unknown; by default let them be used only directly by the user */
 +      return PROTOCOL_ALLOW_USER_ONLY;
  }
  
 -int transport_restrict_protocols(void)
 +int is_transport_allowed(const char *type, int from_user)
  {
 -      return !!protocol_whitelist();
 +      const struct string_list *whitelist = protocol_whitelist();
 +      if (whitelist)
 +              return string_list_has_string(whitelist, type);
 +
 +      switch (get_protocol_config(type)) {
 +      case PROTOCOL_ALLOW_ALWAYS:
 +              return 1;
 +      case PROTOCOL_ALLOW_NEVER:
 +              return 0;
 +      case PROTOCOL_ALLOW_USER_ONLY:
 +              if (from_user < 0)
 +                      from_user = git_env_bool("GIT_PROTOCOL_FROM_USER", 1);
 +              return from_user;
 +      }
 +
 +      die("BUG: invalid protocol_allow_config type");
 +}
 +
 +void transport_check_allowed(const char *type)
 +{
 +      if (!is_transport_allowed(type, -1))
 +              die("transport '%s' not allowed", type);
  }
  
  struct transport *transport_get(struct remote *remote, const char *url)
@@@ -1015,9 -951,7 +1019,9 @@@ int transport_push(struct transport *tr
                        if (run_pre_push_hook(transport, remote_refs))
                                return -1;
  
 -              if ((flags & TRANSPORT_RECURSE_SUBMODULES_ON_DEMAND) && !is_bare_repository()) {
 +              if ((flags & (TRANSPORT_RECURSE_SUBMODULES_ON_DEMAND |
 +                            TRANSPORT_RECURSE_SUBMODULES_ONLY)) &&
 +                  !is_bare_repository()) {
                        struct ref *ref = remote_refs;
                        struct sha1_array commits = SHA1_ARRAY_INIT;
  
                }
  
                if (((flags & TRANSPORT_RECURSE_SUBMODULES_CHECK) ||
 -                   ((flags & TRANSPORT_RECURSE_SUBMODULES_ON_DEMAND) &&
 +                   ((flags & (TRANSPORT_RECURSE_SUBMODULES_ON_DEMAND |
 +                              TRANSPORT_RECURSE_SUBMODULES_ONLY)) &&
                      !pretend)) && !is_bare_repository()) {
                        struct ref *ref = remote_refs;
                        struct string_list needs_pushing = STRING_LIST_INIT_DUP;
                        sha1_array_clear(&commits);
                }
  
 -              push_ret = transport->push_refs(transport, remote_refs, flags);
 +              if (!(flags & TRANSPORT_RECURSE_SUBMODULES_ONLY))
 +                      push_ret = transport->push_refs(transport, remote_refs, flags);
 +              else
 +                      push_ret = 0;
                err = push_had_errors(remote_refs);
                ret = push_ret | err;
  
                if (flags & TRANSPORT_PUSH_SET_UPSTREAM)
                        set_upstreams(transport, remote_refs, pretend);
  
 -              if (!(flags & TRANSPORT_PUSH_DRY_RUN)) {
 +              if (!(flags & (TRANSPORT_PUSH_DRY_RUN |
 +                             TRANSPORT_RECURSE_SUBMODULES_ONLY))) {
                        struct ref *ref;
                        for (ref = remote_refs; ref; ref = ref->next)
                                transport_update_tracking_ref(transport->remote, ref, verbose);
@@@ -1206,42 -1135,6 +1210,42 @@@ literal_copy
        return xstrdup(url);
  }
  
 +static void read_alternate_refs(const char *path,
 +                              alternate_ref_fn *cb,
 +                              void *data)
 +{
 +      struct child_process cmd = CHILD_PROCESS_INIT;
 +      struct strbuf line = STRBUF_INIT;
 +      FILE *fh;
 +
 +      cmd.git_cmd = 1;
 +      argv_array_pushf(&cmd.args, "--git-dir=%s", path);
 +      argv_array_push(&cmd.args, "for-each-ref");
 +      argv_array_push(&cmd.args, "--format=%(objectname) %(refname)");
 +      cmd.env = local_repo_env;
 +      cmd.out = -1;
 +
 +      if (start_command(&cmd))
 +              return;
 +
 +      fh = xfdopen(cmd.out, "r");
 +      while (strbuf_getline_lf(&line, fh) != EOF) {
 +              struct object_id oid;
 +
 +              if (get_oid_hex(line.buf, &oid) ||
 +                  line.buf[GIT_SHA1_HEXSZ] != ' ') {
 +                      warning("invalid line while parsing alternate refs: %s",
 +                              line.buf);
 +                      break;
 +              }
 +
 +              cb(line.buf + GIT_SHA1_HEXSZ + 1, &oid, data);
 +      }
 +
 +      fclose(fh);
 +      finish_command(&cmd);
 +}
 +
  struct alternate_refs_data {
        alternate_ref_fn *fn;
        void *data;
  static int refs_from_alternate_cb(struct alternate_object_database *e,
                                  void *data)
  {
 -      char *other;
 -      size_t len;
 -      struct remote *remote;
 -      struct transport *transport;
 -      const struct ref *extra;
 +      struct strbuf path = STRBUF_INIT;
 +      size_t base_len;
        struct alternate_refs_data *cb = data;
  
 -      other = xstrdup(real_path(e->path));
 -      len = strlen(other);
 -
 -      while (other[len-1] == '/')
 -              other[--len] = '\0';
 -      if (len < 8 || memcmp(other + len - 8, "/objects", 8))
 +      if (!strbuf_realpath(&path, e->path, 0))
                goto out;
 +      if (!strbuf_strip_suffix(&path, "/objects"))
 +              goto out;
 +      base_len = path.len;
 +
        /* Is this a git repository with refs? */
 -      memcpy(other + len - 8, "/refs", 6);
 -      if (!is_directory(other))
 +      strbuf_addstr(&path, "/refs");
 +      if (!is_directory(path.buf))
                goto out;
 -      other[len - 8] = '\0';
 -      remote = remote_get(other);
 -      transport = transport_get(remote, other);
 -      for (extra = transport_get_remote_refs(transport);
 -           extra;
 -           extra = extra->next)
 -              cb->fn(extra, cb->data);
 -      transport_disconnect(transport);
 +      strbuf_setlen(&path, base_len);
 +
 +      read_alternate_refs(path.buf, cb->fn, cb->data);
 +
  out:
 -      free(other);
 +      strbuf_release(&path);
        return 0;
  }