From: Junio C Hamano <gitster@pobox.com>
Date: Mon, 11 Jul 2016 17:31:04 +0000 (-0700)
Subject: Merge branch 'sb/submodule-clone-retry'
X-Git-Tag: v2.10.0-rc0~140
X-Git-Url: https://git.lorimer.id.au/gitweb.git/diff_plain/bb2d8a817df91c68742e10ace2a791de176f7247?hp=-c

Merge branch 'sb/submodule-clone-retry'

"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
---

bb2d8a817df91c68742e10ace2a791de176f7247
diff --combined builtin/submodule--helper.c
index c7deb55785,8d01fdd1f0..b22352b6e1
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@@ -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;
@@@ -744,15 -774,39 +772,39 @@@
  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)
@@@ -836,17 -890,6 +888,17 @@@
  	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 b39ac106ec,f1919ca16f..7c62b53cc0
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@@ -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
@@@ -618,7 -647,7 +618,7 @@@
  			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)
@@@ -695,7 -724,7 +695,7 @@@
  		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" &&
@@@ -705,7 -734,7 +705,7 @@@
  			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"
@@@ -1103,7 -1126,7 +1103,7 @@@
  
  		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"