Merge branch 'ms/packet-err-check' into jt/fetch-v2-sideband
authorJunio C Hamano <gitster@pobox.com>
Mon, 14 Jan 2019 19:16:04 +0000 (11:16 -0800)
committerJunio C Hamano <gitster@pobox.com>
Mon, 14 Jan 2019 19:16:04 +0000 (11:16 -0800)
* ms/packet-err-check:
pack-protocol.txt: accept error packets in any context
Use packet_reader instead of packet_read_line

1  2 
fetch-pack.c
t/t5703-upload-pack-ref-in-want.sh
transport.c
diff --combined fetch-pack.c
index dd6700bda9240f2278e663f9147df1c0808d2efc,3f9626dbf70ad126d2ae430d58094987d05886b6..ee0847e6ae19b88d345b40600d9b3a9b87af892e
@@@ -135,38 -135,42 +135,42 @@@ enum ack_type 
        ACK_ready
  };
  
- static void consume_shallow_list(struct fetch_pack_args *args, int fd)
+ static void consume_shallow_list(struct fetch_pack_args *args,
+                                struct packet_reader *reader)
  {
        if (args->stateless_rpc && args->deepen) {
                /* If we sent a depth we will get back "duplicate"
                 * shallow and unshallow commands every time there
                 * is a block of have lines exchanged.
                 */
-               char *line;
-               while ((line = packet_read_line(fd, NULL))) {
-                       if (starts_with(line, "shallow "))
+               while (packet_reader_read(reader) == PACKET_READ_NORMAL) {
+                       if (starts_with(reader->line, "shallow "))
                                continue;
-                       if (starts_with(line, "unshallow "))
+                       if (starts_with(reader->line, "unshallow "))
                                continue;
                        die(_("git fetch-pack: expected shallow list"));
                }
+               if (reader->status != PACKET_READ_FLUSH)
+                       die(_("git fetch-pack: expected a flush packet after shallow list"));
        }
  }
  
- static enum ack_type get_ack(int fd, struct object_id *result_oid)
+ static enum ack_type get_ack(struct packet_reader *reader,
+                            struct object_id *result_oid)
  {
        int len;
-       char *line = packet_read_line(fd, &len);
        const char *arg;
  
-       if (!line)
+       if (packet_reader_read(reader) != PACKET_READ_NORMAL)
                die(_("git fetch-pack: expected ACK/NAK, got a flush packet"));
-       if (!strcmp(line, "NAK"))
+       len = reader->pktlen;
+       if (!strcmp(reader->line, "NAK"))
                return NAK;
-       if (skip_prefix(line, "ACK ", &arg)) {
+       if (skip_prefix(reader->line, "ACK ", &arg)) {
                if (!get_oid_hex(arg, result_oid)) {
                        arg += 40;
-                       len -= arg - line;
+                       len -= arg - reader->line;
                        if (len < 1)
                                return ACK;
                        if (strstr(arg, "continue"))
                        return ACK;
                }
        }
-       if (skip_prefix(line, "ERR ", &arg))
-               die(_("remote error: %s"), arg);
-       die(_("git fetch-pack: expected ACK/NAK, got '%s'"), line);
+       die(_("git fetch-pack: expected ACK/NAK, got '%s'"), reader->line);
  }
  
  static void send_request(struct fetch_pack_args *args,
@@@ -248,10 -250,15 +250,15 @@@ static int find_common(struct fetch_neg
        int got_ready = 0;
        struct strbuf req_buf = STRBUF_INIT;
        size_t state_len = 0;
+       struct packet_reader reader;
  
        if (args->stateless_rpc && multi_ack == 1)
                die(_("--stateless-rpc requires multi_ack_detailed"));
  
+       packet_reader_init(&reader, fd[0], NULL, 0,
+                          PACKET_READ_CHOMP_NEWLINE |
+                          PACKET_READ_DIE_ON_ERR_PACKET);
        if (!args->no_dependents) {
                mark_tips(negotiator, args->negotiation_tips);
                for_each_cached_alternate(negotiator, insert_one_alternate_object);
        state_len = req_buf.len;
  
        if (args->deepen) {
-               char *line;
                const char *arg;
                struct object_id oid;
  
                send_request(args, fd[1], &req_buf);
-               while ((line = packet_read_line(fd[0], NULL))) {
-                       if (skip_prefix(line, "shallow ", &arg)) {
+               while (packet_reader_read(&reader) == PACKET_READ_NORMAL) {
+                       if (skip_prefix(reader.line, "shallow ", &arg)) {
                                if (get_oid_hex(arg, &oid))
-                                       die(_("invalid shallow line: %s"), line);
+                                       die(_("invalid shallow line: %s"), reader.line);
                                register_shallow(the_repository, &oid);
                                continue;
                        }
-                       if (skip_prefix(line, "unshallow ", &arg)) {
+                       if (skip_prefix(reader.line, "unshallow ", &arg)) {
                                if (get_oid_hex(arg, &oid))
-                                       die(_("invalid unshallow line: %s"), line);
+                                       die(_("invalid unshallow line: %s"), reader.line);
                                if (!lookup_object(the_repository, oid.hash))
-                                       die(_("object not found: %s"), line);
+                                       die(_("object not found: %s"), reader.line);
                                /* make sure that it is parsed as shallow */
                                if (!parse_object(the_repository, &oid))
-                                       die(_("error in object: %s"), line);
+                                       die(_("error in object: %s"), reader.line);
                                if (unregister_shallow(&oid))
-                                       die(_("no shallow found: %s"), line);
+                                       die(_("no shallow found: %s"), reader.line);
                                continue;
                        }
-                       die(_("expected shallow/unshallow, got %s"), line);
+                       die(_("expected shallow/unshallow, got %s"), reader.line);
                }
        } else if (!args->stateless_rpc)
                send_request(args, fd[1], &req_buf);
                        if (!args->stateless_rpc && count == INITIAL_FLUSH)
                                continue;
  
-                       consume_shallow_list(args, fd[0]);
+                       consume_shallow_list(args, &reader);
                        do {
-                               ack = get_ack(fd[0], result_oid);
+                               ack = get_ack(&reader, result_oid);
                                if (ack)
                                        print_verbose(args, _("got %s %d %s"), "ack",
                                                      ack, oid_to_hex(result_oid));
@@@ -469,9 -475,9 +475,9 @@@ done
        strbuf_release(&req_buf);
  
        if (!got_ready || !no_done)
-               consume_shallow_list(args, fd[0]);
+               consume_shallow_list(args, &reader);
        while (flushes || multi_ack) {
-               int ack = get_ack(fd[0], result_oid);
+               int ack = get_ack(&reader, result_oid);
                if (ack) {
                        print_verbose(args, _("got %s (%d) %s"), "ack",
                                      ack, oid_to_hex(result_oid));
@@@ -636,6 -642,23 +642,6 @@@ struct loose_object_iter 
        struct ref *refs;
  };
  
 -/*
 - *  If the number of refs is not larger than the number of loose objects,
 - *  this function stops inserting.
 - */
 -static int add_loose_objects_to_set(const struct object_id *oid,
 -                                  const char *path,
 -                                  void *data)
 -{
 -      struct loose_object_iter *iter = data;
 -      oidset_insert(iter->loose_object_set, oid);
 -      if (iter->refs == NULL)
 -              return 1;
 -
 -      iter->refs = iter->refs->next;
 -      return 0;
 -}
 -
  /*
   * Mark recent commits available locally and reachable from a local ref as
   * COMPLETE. If args->no_dependents is false, also mark COMPLETE remote refs as
@@@ -653,14 -676,30 +659,14 @@@ static void mark_complete_and_common_re
        struct ref *ref;
        int old_save_commit_buffer = save_commit_buffer;
        timestamp_t cutoff = 0;
 -      struct oidset loose_oid_set = OIDSET_INIT;
 -      int use_oidset = 0;
 -      struct loose_object_iter iter = {&loose_oid_set, *refs};
 -
 -      /* Enumerate all loose objects or know refs are not so many. */
 -      use_oidset = !for_each_loose_object(add_loose_objects_to_set,
 -                                          &iter, 0);
  
        save_commit_buffer = 0;
  
        for (ref = *refs; ref; ref = ref->next) {
                struct object *o;
 -              unsigned int flags = OBJECT_INFO_QUICK;
  
 -              if (use_oidset &&
 -                  !oidset_contains(&loose_oid_set, &ref->old_oid)) {
 -                      /*
 -                       * I know this does not exist in the loose form,
 -                       * so check if it exists in a non-loose form.
 -                       */
 -                      flags |= OBJECT_INFO_IGNORE_LOOSE;
 -              }
 -
 -              if (!has_object_file_with_flags(&ref->old_oid, flags))
 +              if (!has_object_file_with_flags(&ref->old_oid,
 +                                              OBJECT_INFO_QUICK))
                        continue;
                o = parse_object(the_repository, &ref->old_oid);
                if (!o)
                }
        }
  
 -      oidset_clear(&loose_oid_set);
 -
        if (!args->deepen) {
                for_each_ref(mark_complete_oid, NULL);
                for_each_cached_alternate(NULL, mark_alternate_complete);
@@@ -1316,7 -1357,8 +1322,8 @@@ static struct ref *do_fetch_pack_v2(str
        struct fetch_negotiator negotiator;
        fetch_negotiator_init(&negotiator, negotiation_algorithm);
        packet_reader_init(&reader, fd[0], NULL, 0,
-                          PACKET_READ_CHOMP_NEWLINE);
+                          PACKET_READ_CHOMP_NEWLINE |
+                          PACKET_READ_DIE_ON_ERR_PACKET);
  
        while (state != FETCH_DONE) {
                switch (state) {
index 7053899cb5a0cbd5cca10cb4029b70df866dbff8,d2a9d0c127d6ce43b0a23fae94df84cdddc72469..f87b2f6df329975e243ab9c00b510ee4cc588d0b
@@@ -208,7 -208,7 +208,7 @@@ test_expect_success 'server is initiall
        cp -r "$LOCAL_PRISTINE" local &&
        inconsistency master 1234567890123456789012345678901234567890 &&
        test_must_fail git -C local fetch 2>err &&
-       test_i18ngrep "ERR upload-pack: not our ref" err
 -      grep "fatal: remote error: upload-pack: not our ref" err
++      test_i18ngrep "fatal: remote error: upload-pack: not our ref" err
  '
  
  test_expect_success 'server is initially ahead - ref in want' '
@@@ -254,7 -254,7 +254,7 @@@ test_expect_success 'server loses a re
        echo "s/master/raster/" >"$HTTPD_ROOT_PATH/one-time-sed" &&
        test_must_fail git -C local fetch 2>err &&
  
-       test_i18ngrep "ERR unknown ref refs/heads/raster" err
 -      grep "fatal: remote error: unknown ref refs/heads/raster" err
++      test_i18ngrep "fatal: remote error: unknown ref refs/heads/raster" err
  '
  
  stop_httpd
diff --combined transport.c
index 99678153c13a3ca442088595273fbcd6caacda29,12db4251c15c088bdad4601418cb59e8e2f03b6d..e078812897eddefcd5a55a7d474dd4a7a89434fc
@@@ -154,7 -154,7 +154,7 @@@ static int fetch_refs_from_bundle(struc
                               int nr_heads, struct ref **to_fetch)
  {
        struct bundle_transport_data *data = transport->data;
 -      return unbundle(&data->header, data->fd,
 +      return unbundle(the_repository, &data->header, data->fd,
                        transport->progress ? BUNDLE_VERBOSE : 0);
  }
  
@@@ -273,7 -273,8 +273,8 @@@ static struct ref *handshake(struct tra
  
        packet_reader_init(&reader, data->fd[0], NULL, 0,
                           PACKET_READ_CHOMP_NEWLINE |
-                          PACKET_READ_GENTLE_ON_EOF);
+                          PACKET_READ_GENTLE_ON_EOF |
+                          PACKET_READ_DIE_ON_ERR_PACKET);
  
        data->version = discover_version(&reader);
        switch (data->version) {
@@@ -1105,8 -1106,7 +1106,8 @@@ static int run_pre_push_hook(struct tra
        return ret;
  }
  
 -int transport_push(struct transport *transport,
 +int transport_push(struct repository *r,
 +                 struct transport *transport,
                   struct refspec *rs, int flags,
                   unsigned int *reject_reasons)
  {
                                        oid_array_append(&commits,
                                                          &ref->new_oid);
  
 -                      if (!push_unpushed_submodules(the_repository,
 +                      if (!push_unpushed_submodules(r,
                                                      &commits,
                                                      transport->remote,
                                                      rs,
                                        oid_array_append(&commits,
                                                          &ref->new_oid);
  
 -                      if (find_unpushed_submodules(the_repository,
 +                      if (find_unpushed_submodules(r,
                                                     &commits,
                                                     transport->remote->name,
                                                     &needs_pushing)) {
@@@ -1434,7 -1434,7 +1435,7 @@@ struct alternate_refs_data 
        void *data;
  };
  
 -static int refs_from_alternate_cb(struct alternate_object_database *e,
 +static int refs_from_alternate_cb(struct object_directory *e,
                                  void *data)
  {
        struct strbuf path = STRBUF_INIT;