Merge branch 'sb/submodule-clone-retry'
authorJunio C Hamano <gitster@pobox.com>
Mon, 11 Jul 2016 17:31:04 +0000 (10:31 -0700)
committerJunio C Hamano <gitster@pobox.com>
Mon, 11 Jul 2016 17:31:04 +0000 (10:31 -0700)
"git submodule update" that drives many "git clone" could
eventually hit flaky servers/network conditions on one of the
submodules; the command learned to retry the attempt.

* sb/submodule-clone-retry:
submodule update: continue when a clone fails
submodule--helper: initial clone learns retry logic

1  2 
builtin/submodule--helper.c
git-submodule.sh
index c7deb55785ddbf32c378c83c1dc3ed4cacdf26db,8d01fdd1f0fa8029ba1703b80790262a38a0b441..b22352b6e1e4c40d1a6e182f5c12abf6351f71fb
@@@ -287,8 -287,10 +287,8 @@@ static int module_list(int argc, const 
        argc = parse_options(argc, argv, prefix, module_list_options,
                             git_submodule_helper_usage, 0);
  
 -      if (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0) {
 -              printf("#unmatched\n");
 +      if (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0)
                return 1;
 -      }
  
        for (i = 0; i < list.nr; i++) {
                const struct cache_entry *ce = list.entries[i];
@@@ -590,10 -592,14 +590,14 @@@ struct submodule_update_clone 
  
        /* If we want to stop as fast as possible and return an error */
        unsigned quickstop : 1;
+       /* failed clones to be retried again */
+       const struct cache_entry **failed_clones;
+       int failed_clones_nr, failed_clones_alloc;
  };
  #define SUBMODULE_UPDATE_CLONE_INIT {0, MODULE_LIST_INIT, 0, \
        SUBMODULE_UPDATE_STRATEGY_INIT, 0, -1, NULL, NULL, NULL, NULL, \
-       STRING_LIST_INIT_DUP, 0}
+       STRING_LIST_INIT_DUP, 0, NULL, 0, 0}
  
  
  static void next_submodule_warn_missing(struct submodule_update_clone *suc,
@@@ -718,23 -724,47 +722,47 @@@ cleanup
  static int update_clone_get_next_task(struct child_process *child,
                                      struct strbuf *err,
                                      void *suc_cb,
-                                     void **void_task_cb)
+                                     void **idx_task_cb)
  {
        struct submodule_update_clone *suc = suc_cb;
+       const struct cache_entry *ce;
+       int index;
  
        for (; suc->current < suc->list.nr; suc->current++) {
-               const struct cache_entry *ce = suc->list.entries[suc->current];
+               ce = suc->list.entries[suc->current];
                if (prepare_to_clone_next_submodule(ce, child, suc, err)) {
+                       int *p = xmalloc(sizeof(*p));
+                       *p = suc->current;
+                       *idx_task_cb = p;
                        suc->current++;
                        return 1;
                }
        }
+       /*
+        * The loop above tried cloning each submodule once, now try the
+        * stragglers again, which we can imagine as an extension of the
+        * entry list.
+        */
+       index = suc->current - suc->list.nr;
+       if (index < suc->failed_clones_nr) {
+               int *p;
+               ce = suc->failed_clones[index];
+               if (!prepare_to_clone_next_submodule(ce, child, suc, err))
+                       die("BUG: ce was a submodule before?");
+               p = xmalloc(sizeof(*p));
+               *p = suc->current;
+               *idx_task_cb = p;
+               suc->current ++;
+               return 1;
+       }
        return 0;
  }
  
  static int update_clone_start_failure(struct strbuf *err,
                                      void *suc_cb,
-                                     void *void_task_cb)
+                                     void *idx_task_cb)
  {
        struct submodule_update_clone *suc = suc_cb;
        suc->quickstop = 1;
  static int update_clone_task_finished(int result,
                                      struct strbuf *err,
                                      void *suc_cb,
-                                     void *void_task_cb)
+                                     void *idx_task_cb)
  {
+       const struct cache_entry *ce;
        struct submodule_update_clone *suc = suc_cb;
  
+       int *idxP = *(int**)idx_task_cb;
+       int idx = *idxP;
+       free(idxP);
        if (!result)
                return 0;
  
-       suc->quickstop = 1;
-       return 1;
+       if (idx < suc->list.nr) {
+               ce  = suc->list.entries[idx];
+               strbuf_addf(err, _("Failed to clone '%s'. Retry scheduled"),
+                           ce->name);
+               strbuf_addch(err, '\n');
+               ALLOC_GROW(suc->failed_clones,
+                          suc->failed_clones_nr + 1,
+                          suc->failed_clones_alloc);
+               suc->failed_clones[suc->failed_clones_nr++] = ce;
+               return 0;
+       } else {
+               idx = suc->current - suc->list.nr;
+               ce  = suc->failed_clones[idx];
+               strbuf_addf(err, _("Failed to clone '%s' a second time, aborting"),
+                           ce->name);
+               strbuf_addch(err, '\n');
+               suc->quickstop = 1;
+               return 1;
+       }
+       return 0;
  }
  
  static int update_clone(int argc, const char **argv, const char *prefix)
        return 0;
  }
  
 +static int resolve_relative_path(int argc, const char **argv, const char *prefix)
 +{
 +      struct strbuf sb = STRBUF_INIT;
 +      if (argc != 3)
 +              die("submodule--helper relative_path takes exactly 2 arguments, got %d", argc);
 +
 +      printf("%s", relative_path(argv[1], argv[2], &sb));
 +      strbuf_release(&sb);
 +      return 0;
 +}
 +
  struct cmd_struct {
        const char *cmd;
        int (*fn)(int, const char **, const char *);
@@@ -857,7 -900,6 +909,7 @@@ static struct cmd_struct commands[] = 
        {"name", module_name},
        {"clone", module_clone},
        {"update-clone", update_clone},
 +      {"relative-path", resolve_relative_path},
        {"resolve-relative-url", resolve_relative_url},
        {"resolve-relative-url-test", resolve_relative_url_test},
        {"init", module_init}
diff --combined git-submodule.sh
index b39ac106ec33112b1dbd2bc5cad5767ce73bdc44,f1919ca16fdeec18d66c4b04485fdf802e119b14..7c62b53cc0dfcf47770a8f30a59f625c8657c948
@@@ -46,6 -46,41 +46,6 @@@ prefix
  custom_name=
  depth=
  
 -# Resolve a path to be relative to another path.  This is intended for
 -# converting submodule paths when git-submodule is run in a subdirectory
 -# and only handles paths where the directory separator is '/'.
 -#
 -# The output is the first argument as a path relative to the second argument,
 -# which defaults to $wt_prefix if it is omitted.
 -relative_path ()
 -{
 -      local target curdir result
 -      target=$1
 -      curdir=${2-$wt_prefix}
 -      curdir=${curdir%/}
 -      result=
 -
 -      while test -n "$curdir"
 -      do
 -              case "$target" in
 -              "$curdir/"*)
 -                      target=${target#"$curdir"/}
 -                      break
 -                      ;;
 -              esac
 -
 -              result="${result}../"
 -              if test "$curdir" = "${curdir%/*}"
 -              then
 -                      curdir=
 -              else
 -                      curdir="${curdir%/*}"
 -              fi
 -      done
 -
 -      echo "$result$target"
 -}
 -
  die_if_unmatched ()
  {
        if test "$1" = "#unmatched"
@@@ -310,23 -345,20 +310,23 @@@ cmd_foreach(
        # command in the subshell (and a recursive call to this function)
        exec 3<&0
  
 -      git submodule--helper list --prefix "$wt_prefix"|
 +      {
 +              git submodule--helper list --prefix "$wt_prefix" ||
 +              echo "#unmatched"
 +      } |
        while read mode sha1 stage sm_path
        do
                die_if_unmatched "$mode"
                if test -e "$sm_path"/.git
                then
 -                      displaypath=$(relative_path "$prefix$sm_path")
 +                      displaypath=$(git submodule--helper relative-path "$prefix$sm_path" "$wt_prefix")
                        say "$(eval_gettext "Entering '\$displaypath'")"
                        name=$(git submodule--helper name "$sm_path")
                        (
                                prefix="$prefix$sm_path/"
                                sanitize_submodule_env
                                cd "$sm_path" &&
 -                              sm_path=$(relative_path "$sm_path") &&
 +                              sm_path=$(git submodule--helper relative-path "$sm_path" "$wt_prefix") &&
                                # we make $path available to scripts ...
                                path=$sm_path &&
                                if test $# -eq 1
@@@ -421,16 -453,13 +421,16 @@@ cmd_deinit(
                die "$(eval_gettext "Use '--all' if you really want to deinitialize all submodules")"
        fi
  
 -      git submodule--helper list --prefix "$wt_prefix" "$@" |
 +      {
 +              git submodule--helper list --prefix "$wt_prefix" "$@" ||
 +              echo "#unmatched"
 +      } |
        while read mode sha1 stage sm_path
        do
                die_if_unmatched "$mode"
                name=$(git submodule--helper name "$sm_path") || exit
  
 -              displaypath=$(relative_path "$sm_path")
 +              displaypath=$(git submodule--helper relative-path "$sm_path" "$wt_prefix")
  
                # Remove the submodule work tree (unless the user already did it)
                if test -d "$sm_path"
@@@ -601,7 -630,7 +601,7 @@@ cmd_update(
                        fi
                fi
  
 -              displaypath=$(relative_path "$prefix$sm_path")
 +              displaypath=$(git submodule--helper relative-path "$prefix$sm_path" "$wt_prefix")
  
                if test $just_cloned -eq 1
                then
                        if test -z "$nofetch"
                        then
                                # Fetch remote before determining tracking $sha1
 -                              (sanitize_submodule_env; cd "$sm_path" && git-fetch) ||
 +                              fetch_in_submodule "$sm_path" ||
                                die "$(eval_gettext "Unable to fetch in submodule path '\$sm_path'")"
                        fi
                        remote_name=$(sanitize_submodule_env; cd "$sm_path" && get_default_remote)
                if test -n "$recursive"
                then
                        (
 -                              prefix=$(relative_path "$prefix$sm_path/")
 +                              prefix=$(git submodule--helper relative-path "$prefix$sm_path/" "$wt_prefix")
                                wt_prefix=
                                sanitize_submodule_env
                                cd "$sm_path" &&
                        if test $res -gt 0
                        then
                                die_msg="$(eval_gettext "Failed to recurse into submodule path '\$displaypath'")"
-                               if test $res -eq 1
+                               if test $res -ne 2
                                then
                                        err="${err};$die_msg"
                                        continue
@@@ -879,7 -908,7 +879,7 @@@ cmd_summary() 
                ! GIT_DIR="$name/.git" git-rev-parse -q --verify $sha1_dst^0 >/dev/null &&
                missing_dst=t
  
 -              display_name=$(relative_path "$name")
 +              display_name=$(git submodule--helper relative-path "$name" "$wt_prefix")
  
                total_commits=
                case "$missing_src,$missing_dst" in
@@@ -991,16 -1020,13 +991,16 @@@ cmd_status(
                shift
        done
  
 -      git submodule--helper list --prefix "$wt_prefix" "$@" |
 +      {
 +              git submodule--helper list --prefix "$wt_prefix" "$@" ||
 +              echo "#unmatched"
 +      } |
        while read mode sha1 stage sm_path
        do
                die_if_unmatched "$mode"
                name=$(git submodule--helper name "$sm_path") || exit
                url=$(git config submodule."$name".url)
 -              displaypath=$(relative_path "$prefix$sm_path")
 +              displaypath=$(git submodule--helper relative-path "$prefix$sm_path" "$wt_prefix")
                if test "$stage" = U
                then
                        say "U$sha1 $displaypath"
@@@ -1072,10 -1098,7 +1072,10 @@@ cmd_sync(
                esac
        done
        cd_to_toplevel
 -      git submodule--helper list --prefix "$wt_prefix" "$@" |
 +      {
 +              git submodule--helper list --prefix "$wt_prefix" "$@" ||
 +              echo "#unmatched"
 +      } |
        while read mode sha1 stage sm_path
        do
                die_if_unmatched "$mode"
  
                if git config "submodule.$name.url" >/dev/null 2>/dev/null
                then
 -                      displaypath=$(relative_path "$prefix$sm_path")
 +                      displaypath=$(git submodule--helper relative-path "$prefix$sm_path" "$wt_prefix")
                        say "$(eval_gettext "Synchronizing submodule url for '\$displaypath'")"
                        git config submodule."$name".url "$super_config_url"