submodule: allow erroneous values for the fetchRecurseSubmodules option

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>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 7910419c93275c58f4881202905ae1f24bfe8c91..faae548a28230ea17c4bd6188194238272138a43 100644 (file)
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -11,6 +11,7 @@
 #include "run-command.h"
 #include "parse-options.h"
 #include "sigchain.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 c64faf3d6f69a8c1ea1dfe8023d2aad6a18cc8c6..393de5357eb486d8fb9ee3be1d75c4b80771bd02 100644 (file)
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -204,6 +204,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)
 {
@@ -255,6 +279,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,
@@ -262,7 +288,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 5fe44ce4b7f0efab02a1aae20d7eee398e1a5fb8..9061e4ed386f417d1816a792664beadb46ed68e0 100644 (file)
--- 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 97355eb7ad82ac16c2b6317409e57615f75c5740..48225595ef9f40a0fa848497c39d3b621c81afbb 100644 (file)
--- 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 547219dcfa2758727d47402f74306d1b81fa0394..5507c3d9a098b1da016855c1fa4233ffc6e0a8cf 100644 (file)
--- 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/t7411-submodule-config.sh b/t/t7411-submodule-config.sh
index 7229978cd2ad6909e0fa8007966d8af73ccd70bb..fc97c3314e2d89e11b4a846457f5e183f4fc42c0 100755 (executable)
--- a/t/t7411-submodule-config.sh
+++ b/t/t7411-submodule-config.sh
@@ -115,4 +115,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