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>

builtin-merge.c

@@ -77,14 +77,21 @@ static int option_parse_message(const struct option *opt,
 static struct strategy *get_strategy(const char *name)
 {
 	int i;
+	struct strbuf err;
 
 	if (!name)
 		return NULL;
 
 	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)
@@ -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)
 {
-	int i;
-	struct strategy *s;
-
 	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;
 }
 
@@ -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);
-	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++)

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