Merge branch 'jt/clone-recursesub-ref-advise'

The interaction between "git clone --recurse-submodules" and
alternate object store was ill-designed.  The documentation and
code have been taught to make more clear recommendations when the
users see failures.

* jt/clone-recursesub-ref-advise:
  submodule--helper: advise on fatal alternate error
  Doc: explain submodule.alternateErrorStrategy

5 files changed, 19 insertions, 1 deletions

diff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt
index 6aaa360..d4e698c 100644
--- a/Documentation/config/advice.txt
+++ b/Documentation/config/advice.txt
@@ -107,4 +107,7 @@ advice.*::
 		editor input from the user.
 	nestedTag::
 		Advice shown if a user attempts to recursively tag a tag object.
+	submoduleAlternateErrorStrategyDie:
+		Advice shown when a submodule.alternateErrorStrategy option
+		configured to "die" causes a fatal error.
 --
diff --git a/Documentation/config/submodule.txt b/Documentation/config/submodule.txt
index 0a1293b..b331771 100644
--- a/Documentation/config/submodule.txt
+++ b/Documentation/config/submodule.txt
@@ -79,4 +79,6 @@ submodule.alternateLocation::
 submodule.alternateErrorStrategy::
 	Specifies how to treat errors with the alternates for a submodule
 	as computed via `submodule.alternateLocation`. Possible values are
-	`ignore`, `info`, `die`. Default is `die`.
+	`ignore`, `info`, `die`. Default is `die`. Note that if set to `ignore`
+	or `info`, and if there is an error with the computed alternate, the
+	clone proceeds as if no alternate was specified.
diff --git a/advice.c b/advice.c
index 3ee0ee2..249c60d 100644
--- a/advice.c
+++ b/advice.c
@@ -30,6 +30,7 @@ int advice_waiting_for_editor = 1;
 int advice_graft_file_deprecated = 1;
 int advice_checkout_ambiguous_remote_branch_name = 1;
 int advice_nested_tag = 1;
+int advice_submodule_alternate_error_strategy_die = 1;
 
 static int advice_use_color = -1;
 static char advice_colors[][COLOR_MAXLEN] = {
@@ -89,6 +90,7 @@ static struct {
 	{ "graftFileDeprecated", &advice_graft_file_deprecated },
 	{ "checkoutAmbiguousRemoteBranchName", &advice_checkout_ambiguous_remote_branch_name },
 	{ "nestedTag", &advice_nested_tag },
+	{ "submoduleAlternateErrorStrategyDie", &advice_submodule_alternate_error_strategy_die },
 
 	/* make this an alias for backward compatibility */
 	{ "pushNonFastForward", &advice_push_update_rejected }
diff --git a/advice.h b/advice.h
index d015404..b706780 100644
--- a/advice.h
+++ b/advice.h
@@ -30,6 +30,7 @@ extern int advice_waiting_for_editor;
 extern int advice_graft_file_deprecated;
 extern int advice_checkout_ambiguous_remote_branch_name;
 extern int advice_nested_tag;
+extern int advice_submodule_alternate_error_strategy_die;
 
 int git_default_advice_config(const char *var, const char *value);
 __attribute__((format (printf, 1, 2)))
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index a4f9f8d..c72931e 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -20,6 +20,7 @@
 #include "diff.h"
 #include "object-store.h"
 #include "dir.h"
+#include "advice.h"
 
 #define OPT_QUIET (1 << 0)
 #define OPT_CACHED (1 << 1)
@@ -1270,6 +1271,13 @@ struct submodule_alternate_setup {
 #define SUBMODULE_ALTERNATE_SETUP_INIT { NULL, \
 	SUBMODULE_ALTERNATE_ERROR_IGNORE, NULL }
 
+static const char alternate_error_advice[] = N_(
+"An alternate computed from a superproject's alternate is invalid.\n"
+"To allow Git to clone without an alternate in such a case, set\n"
+"submodule.alternateErrorStrategy to 'info' or, equivalently, clone with\n"
+"'--reference-if-able' instead of '--reference'."
+);
+
 static int add_possible_reference_from_superproject(
 		struct object_directory *odb, void *sas_cb)
 {
@@ -1301,6 +1309,8 @@ static int add_possible_reference_from_superproject(
 		} else {
 			switch (sas->error_mode) {
 			case SUBMODULE_ALTERNATE_ERROR_DIE:
+				if (advice_submodule_alternate_error_strategy_die)
+					advise(_(alternate_error_advice));
 				die(_("submodule '%s' cannot add alternate: %s"),
 				    sas->submodule_name, err.buf);
 			case SUBMODULE_ALTERNATE_ERROR_INFO: