git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 5/5] do not die on error of parsing fetchrecursesubmodules option

From
Heiko Voigt <hvoigt@hvoigt.net>
Date
Jun 5, 2014, 06:09 UTC
Message-ID
<20140605060946.GF23874@sandbox-ub>
In-Reply-To
<20140605060425.GA23874@sandbox-ub>

We should not die when reading the submodule config cache since the user might not be able to get out of that situation when the configuration is part of the history.

We should handle this condition later when the value is about to be used.

Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>
---
 builtin/fetch.c             |  1 +
 submodule-config.c          | 29 ++++++++++++++++++++++++++++-
 submodule-config.h          |  1 +
 submodule.c                 | 15 ---------------
 submodule.h                 |  2 +-
 t/t7410-submodule-config.sh | 35 +++++++++++++++++++++++++++++++++++
 6 files changed, 66 insertions(+), 17 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 55f457c..706326f 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -12,6 +12,7 @@
 #include "parse-options.h"
 #include "sigchain.h"
 #include "transport.h"
+#include "submodule-config.h"
 #include "submodule.h"
 #include "connected.h"
 #include "argv-array.h"
diff --git a/submodule-config.c b/submodule-config.c
index e445bf7..437fbdb 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -199,6 +199,30 @@ static struct submodule *lookup_or_create_by_name(struct submodule_cache *cache,
 	return submodule;
 }
 
+static int parse_fetch_recurse(const char *opt, const char *arg,
+			       int die_on_error)
+{
+	switch (git_config_maybe_bool(opt, arg)) {
+	case 1:
+		return RECURSE_SUBMODULES_ON;
+	case 0:
+		return RECURSE_SUBMODULES_OFF;
+	default:
+		if (!strcmp(arg, "on-demand"))
+			return RECURSE_SUBMODULES_ON_DEMAND;
+
+		if (die_on_error)
+			die("bad %s argument: %s", opt, arg);
+		else
+			return RECURSE_SUBMODULES_ERROR;
+	}
+}
+
+int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg)
+{
+	return parse_fetch_recurse(opt, arg, 1);
+}
+
 static void warn_multiple_config(const unsigned char *commit_sha1,
 				 const char *name, const char *option)
 {
@@ -250,6 +274,8 @@ static int parse_config(const char *var, const char *value, void *data)
 		submodule->path = strbuf_detach(&path, NULL);
 		cache_put_path(me->cache, submodule);
 	} else if (!strcmp(item.buf, "fetchrecursesubmodules")) {
+		/* when parsing worktree configurations we can die early */
+		int die_on_error = is_null_sha1(me->gitmodules_sha1);
 		if (!me->overwrite &&
 		    submodule->fetch_recurse != RECURSE_SUBMODULES_NONE) {
 			warn_multiple_config(me->commit_sha1, submodule->name,
@@ -257,7 +283,8 @@ static int parse_config(const char *var, const char *value, void *data)
 			goto release_return;
 		}
 
-		submodule->fetch_recurse = parse_fetch_recurse_submodules_arg(var, value);
+		submodule->fetch_recurse = parse_fetch_recurse(var, value,
+								die_on_error);
 	} else if (!strcmp(item.buf, "ignore")) {
 		struct strbuf ignore = STRBUF_INIT;
 		if (!me->overwrite && submodule->ignore != NULL) {
diff --git a/submodule-config.h b/submodule-config.h
index 2083cb9..58afc83 100644
--- a/submodule-config.h
+++ b/submodule-config.h
@@ -18,6 +18,7 @@ struct submodule {
 	unsigned char gitmodules_sha1[20];
 };
 
+int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg);
 int parse_submodule_config_option(const char *var, const char *value);
 const struct submodule *submodule_from_name(const unsigned char *commit_sha1,
 		const char *name);
diff --git a/submodule.c b/submodule.c
index 188b4d2..75f502f 100644
--- a/submodule.c
+++ b/submodule.c
@@ -288,21 +288,6 @@ static void print_submodule_summary(struct rev_info *rev, FILE *f,
 	strbuf_release(&sb);
 }
 
-int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg)
-{
-	switch (git_config_maybe_bool(opt, arg)) {
-	case 1:
-		return RECURSE_SUBMODULES_ON;
-	case 0:
-		return RECURSE_SUBMODULES_OFF;
-	default:
-		if (!strcmp(arg, "on-demand"))
-			return RECURSE_SUBMODULES_ON_DEMAND;
-		/* TODO: remove the die for history parsing here */
-		die("bad %s argument: %s", opt, arg);
-	}
-}
-
 void show_submodule_summary(FILE *f, const char *path,
 		const char *line_prefix,
 		unsigned char one[20], unsigned char two[20],
diff --git a/submodule.h b/submodule.h
index 547219d..5507c3d 100644
--- a/submodule.h
+++ b/submodule.h
@@ -5,6 +5,7 @@ struct diff_options;
 struct argv_array;
 
 enum {
+	RECURSE_SUBMODULES_ERROR = -3,
 	RECURSE_SUBMODULES_NONE = -2,
 	RECURSE_SUBMODULES_ON_DEMAND = -1,
 	RECURSE_SUBMODULES_OFF = 0,
@@ -21,7 +22,6 @@ void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,
 int submodule_config(const char *var, const char *value, void *cb);
 void gitmodules_config(void);
 void handle_ignore_submodules_arg(struct diff_options *diffopt, const char *);
-int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg);
 void show_submodule_summary(FILE *f, const char *path,
 		const char *line_prefix,
 		unsigned char one[20], unsigned char two[20],
diff --git a/t/t7410-submodule-config.sh b/t/t7410-submodule-config.sh
index 1ec26c3..4a837af 100755
--- a/t/t7410-submodule-config.sh
+++ b/t/t7410-submodule-config.sh
@@ -103,4 +103,39 @@ test_expect_success 'reading of local configuration' '
 	)
 '
 
+cat >super/expect_fetchrecurse_die.err <<EOF
+fatal: bad submodule.submodule.fetchrecursesubmodules argument: blabla
+EOF
+
+test_expect_success 'local error in fetchrecursesubmodule dies early' '
+	(cd super &&
+		git config submodule.submodule.fetchrecursesubmodules blabla &&
+		test_must_fail test-submodule-config \
+			"" b \
+			"" submodule \
+				>actual.out 2>actual.err &&
+		touch expect_fetchrecurse_die.out &&
+		test_cmp expect_fetchrecurse_die.out actual.out  &&
+		test_cmp expect_fetchrecurse_die.err actual.err  &&
+		git config --unset submodule.submodule.fetchrecursesubmodules
+	)
+'
+
+test_expect_success 'error in history in fetchrecursesubmodule lets continue' '
+	(cd super &&
+		git config -f .gitmodules \
+			submodule.submodule.fetchrecursesubmodules blabla &&
+		git add .gitmodules &&
+		git config --unset -f .gitmodules \
+			submodule.submodule.fetchrecursesubmodules &&
+		git commit -m "add error in fetchrecursesubmodules" &&
+		test-submodule-config \
+			HEAD b \
+			HEAD submodule \
+				>actual &&
+		test_cmp expect_error actual  &&
+		git reset --hard HEAD^
+	)
+'
+
 test_done

-- 
2.0.0
Previous: Heiko VoigtNext: Junio C Hamano
Message 19 of 26 in “submodule config lookup API”
  1. 0/5 submodule config lookup APIHeiko Voigt, Jun 5, 2014
  2. 1/5 hashmap: add enum for hashmap free_entries optionHeiko Voigt, Jun 5, 2014
  3. Karsten BleesJun 6, 2014
  4. Heiko VoigtJun 10, 2014
  5. Karsten BleesJun 11, 2014
  6. Junio C HamanoJun 12, 2014
  7. Karsten BleesJun 17, 2014
  8. Heiko VoigtJun 17, 2014
  9. Junio C HamanoJun 17, 2014
  10. 2/5 implement submodule config cache for lookup of submodule namesHeiko Voigt, Jun 5, 2014
  11. W. Trevor KingJun 5, 2014
  12. Heiko VoigtJun 6, 2014
  13. Eric SunshineJun 8, 2014
  14. Heiko VoigtJun 10, 2014
  15. Junio C HamanoJun 12, 2014
  16. Heiko VoigtJun 13, 2014
  17. 3/5 extract functions for submodule config set and lookupHeiko Voigt, Jun 5, 2014
  18. 4/5 use new config API for worktree configurations of submodulesHeiko Voigt, Jun 5, 2014
  19. 5/5 do not die on error of parsing fetchrecursesubmodules optionHeiko Voigt, Jun 5, 2014
  20. Junio C HamanoJun 12, 2014
  21. Heiko VoigtJun 13, 2014
  22. Junio C HamanoJun 16, 2014
  23. Heiko VoigtJun 17, 2014
  24. Junio C HamanoJun 12, 2014
  25. Jens LehmannJun 13, 2014
  26. Junio C HamanoJun 13, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.