builtin-merge: give a proper error message for invalid strategies in config

'git merge -s foobar' diagnosed invalid "foobar" strategy and errored out
with a message, but foobar in pull.twohead or pull.octopus was just
silently ignored.  This makes invalid strategy both on the command line
and in the configuration file to trigger the same error.

Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin-merge.c b/builtin-merge.c
index e97c79e60cdb8227ad682a3927ae955005cbe1f3..0fd7985a112231ba164ed3ba408519d6ce732071 100644 (file)
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -77,6 +77,7 @@ static int option_parse_message(const struct option *opt,
 static struct strategy *get_strategy(const char *name)
 {
        int i;
 static struct strategy *get_strategy(const char *name)
 {
        int i;

+       struct strbuf err;

 
        if (!name)
                return NULL;
 
        if (!name)
                return NULL;


@@ -84,7 +85,13 @@ static struct strategy *get_strategy(const char *name)
        for (i = 0; i < ARRAY_SIZE(all_strategy); i++)
                if (!strcmp(name, all_strategy[i].name))
                        return &all_strategy[i];
        for (i = 0; i < ARRAY_SIZE(all_strategy); i++)
                if (!strcmp(name, all_strategy[i].name))
                        return &all_strategy[i];

-       return NULL;
+
+       strbuf_init(&err, 0);
+       for (i = 0; i < ARRAY_SIZE(all_strategy); i++)
+               strbuf_addf(&err, " %s", all_strategy[i].name);
+       fprintf(stderr, "Could not find merge strategy '%s'.\n", name);
+       fprintf(stderr, "Available strategies are:%s.\n", err.buf);
+       exit(1);

 }
 
 static void append_strategy(struct strategy *s)
 }
 
 static void append_strategy(struct strategy *s)


@@ -96,25 +103,10 @@ static void append_strategy(struct strategy *s)
 static int option_parse_strategy(const struct option *opt,
                                 const char *name, int unset)
 {
 static int option_parse_strategy(const struct option *opt,
                                 const char *name, int unset)
 {

-       int i;
-       struct strategy *s;
-

        if (unset)
                return 0;
 
        if (unset)
                return 0;
 

-       s = get_strategy(name);
-
-       if (s)
-               append_strategy(s);
-       else {
-               struct strbuf err;
-               strbuf_init(&err, 0);
-               for (i = 0; i < ARRAY_SIZE(all_strategy); i++)
-                       strbuf_addf(&err, " %s", all_strategy[i].name);
-               fprintf(stderr, "Could not find merge strategy '%s'.\n", name);
-               fprintf(stderr, "Available strategies are:%s.\n", err.buf);
-               exit(1);
-       }
+       append_strategy(get_strategy(name));

        return 0;
 }
 
        return 0;
 }
 


@@ -643,14 +635,9 @@ static void add_strategies(const char *string, unsigned attr)
 
        memset(&list, 0, sizeof(list));
        split_merge_strategies(string, &list, &list_nr, &list_alloc);
 
        memset(&list, 0, sizeof(list));
        split_merge_strategies(string, &list, &list_nr, &list_alloc);

-       if (list != NULL) {
-               for (i = 0; i < list_nr; i++) {
-                       struct strategy *s;
-
-                       s = get_strategy(list[i].name);
-                       if (s)
-                               append_strategy(s);
-               }
+       if (list) {
+               for (i = 0; i < list_nr; i++)
+                       append_strategy(get_strategy(list[i].name));

                return;
        }
        for (i = 0; i < ARRAY_SIZE(all_strategy); i++)
                return;
        }
        for (i = 0; i < ARRAY_SIZE(all_strategy); i++)

diff --git a/t/t7601-merge-pull-config.sh b/t/t7601-merge-pull-config.sh
index 95b4d71c5154a62bc2e483eecccada0079bf05f4..6b9f6388c7fc6f449621d73f1bb62b5fae29f223 100755 (executable)
--- a/t/t7601-merge-pull-config.sh
+++ b/t/t7601-merge-pull-config.sh
@@ -126,4 +126,16 @@ test_expect_success 'merge picks up the best result' '
        test $auto_count != $resolve_count
 '
 
        test $auto_count != $resolve_count
 '
 

+test_expect_success 'merge errors out on invalid strategy' '
+       git config pull.twohead "foobar" &&
+       git reset --hard c5 &&
+       test_must_fail git merge c6
+'
+
+test_expect_success 'merge errors out on invalid strategy' '
+       git config --unset-all pull.twohead &&
+       git reset --hard c5 &&
+       test_must_fail git merge -s "resolve recursive" c6
+'
+

 test_done
 test_done