Merge branch 'jc/transport-do-not-use-connect-twice-in-fetch'
authorJunio C Hamano <gitster@pobox.com>
Mon, 9 Sep 2013 21:50:37 +0000 (14:50 -0700)
committerJunio C Hamano <gitster@pobox.com>
Mon, 9 Sep 2013 21:50:37 +0000 (14:50 -0700)
The auto-tag-following code in "git fetch" tries to reuse the same
transport twice when the serving end does not cooperate and does
not give tags that point to commits that are asked for as part of
the primary transfer. Unfortunately, Git-aware transport helper
interface is not designed to be used more than once, hence this
does not work over smart-http transfer.

* jc/transport-do-not-use-connect-twice-in-fetch:
builtin/fetch.c: Fix a sparse warning
fetch: work around "transport-take-over" hack
fetch: refactor code that fetches leftover tags
fetch: refactor code that prepares a transport
fetch: rename file-scope global "transport" to "gtransport"
t5802: add test for connect helper

1  2 
builtin/fetch.c
transport.c
transport.h
diff --combined builtin/fetch.c
index 593653955201995227c062d3878aafc09b424940,564705555b897bde0a1a2c3fad3c1f1b39903b53..9e654efa3bb725edd91f6eb7059b263668e622bc
@@@ -30,17 -30,14 +30,18 @@@ enum 
        TAGS_SET = 2
  };
  
 -static int all, append, dry_run, force, keep, multiple, prune, update_head_ok, verbosity;
 +static int fetch_prune_config = -1; /* unspecified */
 +static int prune = -1; /* unspecified */
 +#define PRUNE_BY_DEFAULT 0 /* do we prune by default? */
 +
 +static int all, append, dry_run, force, keep, multiple, update_head_ok, verbosity;
  static int progress = -1, recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
  static int tags = TAGS_DEFAULT, unshallow;
  static const char *depth;
  static const char *upload_pack;
  static struct strbuf default_rla = STRBUF_INIT;
- static struct transport *transport;
+ static struct transport *gtransport;
+ static struct transport *gsecondary;
  static const char *submodule_prefix = "";
  static const char *recurse_submodules_default;
  
@@@ -58,39 -55,30 +59,39 @@@ static int option_parse_recurse_submodu
        return 0;
  }
  
 +static int git_fetch_config(const char *k, const char *v, void *cb)
 +{
 +      if (!strcmp(k, "fetch.prune")) {
 +              fetch_prune_config = git_config_bool(k, v);
 +              return 0;
 +      }
 +      return 0;
 +}
 +
  static struct option builtin_fetch_options[] = {
        OPT__VERBOSITY(&verbosity),
 -      OPT_BOOLEAN(0, "all", &all,
 -                  N_("fetch from all remotes")),
 -      OPT_BOOLEAN('a', "append", &append,
 -                  N_("append to .git/FETCH_HEAD instead of overwriting")),
 +      OPT_BOOL(0, "all", &all,
 +               N_("fetch from all remotes")),
 +      OPT_BOOL('a', "append", &append,
 +               N_("append to .git/FETCH_HEAD instead of overwriting")),
        OPT_STRING(0, "upload-pack", &upload_pack, N_("path"),
                   N_("path to upload pack on remote end")),
        OPT__FORCE(&force, N_("force overwrite of local branch")),
 -      OPT_BOOLEAN('m', "multiple", &multiple,
 -                  N_("fetch from multiple remotes")),
 +      OPT_BOOL('m', "multiple", &multiple,
 +               N_("fetch from multiple remotes")),
        OPT_SET_INT('t', "tags", &tags,
                    N_("fetch all tags and associated objects"), TAGS_SET),
        OPT_SET_INT('n', NULL, &tags,
                    N_("do not fetch all tags (--no-tags)"), TAGS_UNSET),
 -      OPT_BOOLEAN('p', "prune", &prune,
 -                  N_("prune remote-tracking branches no longer on remote")),
 +      OPT_BOOL('p', "prune", &prune,
 +               N_("prune remote-tracking branches no longer on remote")),
        { OPTION_CALLBACK, 0, "recurse-submodules", NULL, N_("on-demand"),
                    N_("control recursive fetching of submodules"),
                    PARSE_OPT_OPTARG, option_parse_recurse_submodules },
 -      OPT_BOOLEAN(0, "dry-run", &dry_run,
 -                  N_("dry run")),
 -      OPT_BOOLEAN('k', "keep", &keep, N_("keep downloaded pack")),
 -      OPT_BOOLEAN('u', "update-head-ok", &update_head_ok,
 +      OPT_BOOL(0, "dry-run", &dry_run,
 +               N_("dry run")),
 +      OPT_BOOL('k', "keep", &keep, N_("keep downloaded pack")),
 +      OPT_BOOL('u', "update-head-ok", &update_head_ok,
                    N_("allow updating of HEAD ref")),
        OPT_BOOL(0, "progress", &progress, N_("force progress reporting")),
        OPT_STRING(0, "depth", &depth, N_("depth"),
  
  static void unlock_pack(void)
  {
-       if (transport)
-               transport_unlock_pack(transport);
+       if (gtransport)
+               transport_unlock_pack(gtransport);
+       if (gsecondary)
+               transport_unlock_pack(gsecondary);
  }
  
  static void unlock_pack_on_signal(int signo)
@@@ -733,6 -723,48 +736,48 @@@ static int truncate_fetch_head(void
        return 0;
  }
  
+ static void set_option(struct transport *transport, const char *name, const char *value)
+ {
+       int r = transport_set_option(transport, name, value);
+       if (r < 0)
+               die(_("Option \"%s\" value \"%s\" is not valid for %s"),
+                   name, value, transport->url);
+       if (r > 0)
+               warning(_("Option \"%s\" is ignored for %s\n"),
+                       name, transport->url);
+ }
+ static struct transport *prepare_transport(struct remote *remote)
+ {
+       struct transport *transport;
+       transport = transport_get(remote, NULL);
+       transport_set_verbosity(transport, verbosity, progress);
+       if (upload_pack)
+               set_option(transport, TRANS_OPT_UPLOADPACK, upload_pack);
+       if (keep)
+               set_option(transport, TRANS_OPT_KEEP, "yes");
+       if (depth)
+               set_option(transport, TRANS_OPT_DEPTH, depth);
+       return transport;
+ }
+ static void backfill_tags(struct transport *transport, struct ref *ref_map)
+ {
+       if (transport->cannot_reuse) {
+               gsecondary = prepare_transport(transport->remote);
+               transport = gsecondary;
+       }
+       transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, NULL);
+       transport_set_option(transport, TRANS_OPT_DEPTH, "0");
+       fetch_refs(transport, ref_map);
+       if (gsecondary) {
+               transport_disconnect(gsecondary);
+               gsecondary = NULL;
+       }
+ }
  static int do_fetch(struct transport *transport,
                    struct refspec *refs, int ref_count)
  {
                goto cleanup;
        }
        if (prune) {
 -              /* If --tags was specified, pretend the user gave us the canonical tags refspec */
 +              /*
 +               * If --tags was specified, pretend that the user gave us
 +               * the canonical tags refspec
 +               */
                if (tags == TAGS_SET) {
                        const char *tags_str = "refs/tags/*:refs/tags/*";
                        struct refspec *tags_refspec, *refspec;
                struct ref **tail = &ref_map;
                ref_map = NULL;
                find_non_local_tags(transport, &ref_map, &tail);
-               if (ref_map) {
-                       transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, NULL);
-                       transport_set_option(transport, TRANS_OPT_DEPTH, "0");
-                       fetch_refs(transport, ref_map);
-               }
+               if (ref_map)
+                       backfill_tags(transport, ref_map);
                free_refs(ref_map);
        }
  
        return retcode;
  }
  
- static void set_option(const char *name, const char *value)
- {
-       int r = transport_set_option(transport, name, value);
-       if (r < 0)
-               die(_("Option \"%s\" value \"%s\" is not valid for %s"),
-                       name, value, transport->url);
-       if (r > 0)
-               warning(_("Option \"%s\" is ignored for %s\n"),
-                       name, transport->url);
- }
  static int get_one_remote_for_fetch(struct remote *remote, void *priv)
  {
        struct string_list *list = priv;
@@@ -898,7 -913,7 +929,7 @@@ static void add_options_to_argv(struct 
  {
        if (dry_run)
                argv_array_push(argv, "--dry-run");
 -      if (prune)
 +      if (prune > 0)
                argv_array_push(argv, "--prune");
        if (update_head_ok)
                argv_array_push(argv, "--update-head-ok");
@@@ -965,26 -980,7 +996,18 @@@ static int fetch_one(struct remote *rem
                die(_("No remote repository specified.  Please, specify either a URL or a\n"
                    "remote name from which new revisions should be fetched."));
  
-       transport = transport_get(remote, NULL);
+       gtransport = prepare_transport(remote);
 +
 +      if (prune < 0) {
 +              /* no command line request */
-               if (0 <= transport->remote->prune)
-                       prune = transport->remote->prune;
++              if (0 <= gtransport->remote->prune)
++                      prune = gtransport->remote->prune;
 +              else if (0 <= fetch_prune_config)
 +                      prune = fetch_prune_config;
 +              else
 +                      prune = PRUNE_BY_DEFAULT;
 +      }
 +
-       transport_set_verbosity(transport, verbosity, progress);
-       if (upload_pack)
-               set_option(TRANS_OPT_UPLOADPACK, upload_pack);
-       if (keep)
-               set_option(TRANS_OPT_KEEP, "yes");
-       if (depth)
-               set_option(TRANS_OPT_DEPTH, depth);
        if (argc > 0) {
                int j = 0;
                refs = xcalloc(argc + 1, sizeof(const char *));
        sigchain_push_common(unlock_pack_on_signal);
        atexit(unlock_pack);
        refspec = parse_fetch_refspec(ref_nr, refs);
-       exit_code = do_fetch(transport, refspec, ref_nr);
+       exit_code = do_fetch(gtransport, refspec, ref_nr);
        free_refspec(ref_nr, refspec);
-       transport_disconnect(transport);
-       transport = NULL;
+       transport_disconnect(gtransport);
+       gtransport = NULL;
        return exit_code;
  }
  
@@@ -1034,8 -1030,6 +1057,8 @@@ int cmd_fetch(int argc, const char **ar
        for (i = 1; i < argc; i++)
                strbuf_addf(&default_rla, " %s", argv[i]);
  
 +      git_config(git_fetch_config, NULL);
 +
        argc = parse_options(argc, argv, prefix,
                             builtin_fetch_options, builtin_fetch_usage, 0);
  
diff --combined transport.c
index b321d6a49d40c78612284557f7ef6d7822a45c8f,de255880a41248857990380d7622512fd18160b0..7202b7777d804b75eb2ec54f666e81173690678c
@@@ -3,8 -3,6 +3,8 @@@
  #include "run-command.h"
  #include "pkt-line.h"
  #include "fetch-pack.h"
 +#include "remote.h"
 +#include "connect.h"
  #include "send-pack.h"
  #include "walker.h"
  #include "bundle.h"
@@@ -709,10 -707,6 +709,10 @@@ static int print_one_push_status(struc
                print_ref_status('!', "[rejected]", ref, ref->peer_ref,
                                                 "needs force", porcelain);
                break;
 +      case REF_STATUS_REJECT_STALE:
 +              print_ref_status('!', "[rejected]", ref, ref->peer_ref,
 +                                               "stale info", porcelain);
 +              break;
        case REF_STATUS_REMOTE_REJECT:
                print_ref_status('!', "[remote rejected]", ref,
                                                 ref->deletion ? NULL : ref->peer_ref,
@@@ -881,6 -875,8 +881,8 @@@ void transport_take_over(struct transpo
        transport->push_refs = git_transport_push;
        transport->disconnect = disconnect_git;
        transport->smart_options = &(data->options);
+       transport->cannot_reuse = 1;
  }
  
  static int is_local(const char *url)
@@@ -1082,7 -1078,6 +1084,7 @@@ static int run_pre_push_hook(struct tra
        for (r = remote_refs; r; r = r->next) {
                if (!r->peer_ref) continue;
                if (r->status == REF_STATUS_REJECT_NONFASTFORWARD) continue;
 +              if (r->status == REF_STATUS_REJECT_STALE) continue;
                if (r->status == REF_STATUS_UPTODATE) continue;
  
                strbuf_reset(&buf);
@@@ -1147,12 -1142,6 +1149,12 @@@ int transport_push(struct transport *tr
                        return -1;
                }
  
 +              if (transport->smart_options &&
 +                  transport->smart_options->cas &&
 +                  !is_empty_cas(transport->smart_options->cas))
 +                      apply_push_cas(transport->smart_options->cas,
 +                                     transport->remote, remote_refs);
 +
                set_ref_status_for_push(remote_refs,
                        flags & TRANSPORT_PUSH_MIRROR,
                        flags & TRANSPORT_PUSH_FORCE);
diff --combined transport.h
index 10f7556001c98c9da533c959b1e1c7b3ba8b2ca0,96e0ede89f6c98efe988425aecd2418538b202e5..8f96bed775e720043658169b46f70e192cf76b41
@@@ -2,7 -2,6 +2,7 @@@
  #define TRANSPORT_H
  
  #include "cache.h"
 +#include "run-command.h"
  #include "remote.h"
  
  struct git_transport_options {
@@@ -14,7 -13,6 +14,7 @@@
        int depth;
        const char *uploadpack;
        const char *receivepack;
 +      struct push_cas_option *cas;
  };
  
  struct transport {
         */
        unsigned got_remote_refs : 1;
  
+       /*
+        * Transports that call take-over destroys the data specific to
+        * the transport type while doing so, and cannot be reused.
+        */
+       unsigned cannot_reuse : 1;
        /**
         * Returns 0 if successful, positive if the option is not
         * recognized or is inapplicable, and negative if the option
@@@ -128,9 -132,6 +134,9 @@@ struct transport *transport_get(struct 
  /* Transfer the data as a thin pack if not null */
  #define TRANS_OPT_THIN "thin"
  
 +/* Check the current value of the remote ref */
 +#define TRANS_OPT_CAS "cas"
 +
  /* Keep the pack that was transferred if not null */
  #define TRANS_OPT_KEEP "keep"