Re: [PATCH v5] Add default merge options for all branches

[Junio C Hamano <gitster@pobox.com>]

Thanks.

I think we still need to work on this a bit more, but I found an unrelated
and nastier issue before this patch can sanely be applied.

> +test_expect_success 'combine branch.master.mergeoptions with merge.ff' '
> +	git reset --hard c0 &&
> +	git config branch.master.mergeoptions --ff
> +	git config merge.ff false
> +	test_tick &&
> +	git merge c1 &&
> +	git config --remove-section "branch.master" &&
> +	git config --remove-section "merge" &&
> +	verify_merge file result.1 &&
> +	verify_parents "$c0"
> +'

If you insert an "exit" after this test and inspect the resulting commit,
you will see that it created a merge.

	Side note: I think verify_parents is buggy. It only makes sure
	that the earlier parents of HEAD match the commits given, and does
	not care if there actually are more parents.

In any case, your test exposed an ancient breakage ever since the
per-branch mergeoptions was introduced back when git-merge was a shell
script (aec7b36 (git-merge: add support for branch.<name>.mergeoptions,
2007-09-24).

I am not going to fix verify_parents tonight, as I have other git things
to do.

-- >8 --
Subject: [PATCH] merge: fix branch.<name>.mergeoptions

The parsing of the additional command line parameters supplied to
the branch.<name>.mergeoptions configuration variable was implemented
at the wrong stage.  If any merge-related variable came after we read
branch.<name>.mergeoptions, the earlier value was overwritten.

We should first read all the merge.* configuration, override them by
reading from branch.<name>.mergeoptions and then finally read from
the command line.

This patch should fix it, even though I now strongly suspect that
branch.<name>.mergeoptions that gives a single command line that
needs to be parsed was likely to be an ill-conceived idea to begin
with.  Sigh...

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-merge.c  |   39 +++++++++++++++++++++++++--------------
 t/t7600-merge.sh |   32 ++++++++++++++++++++++++++++++++
 2 files changed, 57 insertions(+), 14 deletions(-)

diff --git a/builtin-merge.c b/builtin-merge.c
index 3aaec7b..01389a3 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -54,6 +54,7 @@ static size_t use_strategies_nr, use_strategies_alloc;
 static const char **xopts;
 static size_t xopts_nr, xopts_alloc;
 static const char *branch;
+static char *branch_mergeoptions;
 static int verbosity;
 static int allow_rerere_auto;
 
@@ -474,25 +475,33 @@ cleanup:
 	strbuf_release(&bname);
 }
 
+static void parse_branch_merge_options(char *bmo)
+{
+	const char **argv;
+	int argc;
+	char *buf;
+
+	if (!bmo)
+		return;
+	argc = split_cmdline(bmo, &argv);
+	if (argc < 0)
+		die("Bad branch.%s.mergeoptions string", branch);
+	argv = xrealloc(argv, sizeof(*argv) * (argc + 2));
+	memmove(argv + 1, argv, sizeof(*argv) * (argc + 1));
+	argc++;
+	parse_options(argc, argv, NULL, builtin_merge_options,
+		      builtin_merge_usage, 0);
+	free(buf);
+}
+
 static int git_merge_config(const char *k, const char *v, void *cb)
 {
 	if (branch && !prefixcmp(k, "branch.") &&
 		!prefixcmp(k + 7, branch) &&
 		!strcmp(k + 7 + strlen(branch), ".mergeoptions")) {
-		const char **argv;
-		int argc;
-		char *buf;
-
-		buf = xstrdup(v);
-		argc = split_cmdline(buf, &argv);
-		if (argc < 0)
-			die("Bad branch.%s.mergeoptions string", branch);
-		argv = xrealloc(argv, sizeof(*argv) * (argc + 2));
-		memmove(argv + 1, argv, sizeof(*argv) * (argc + 1));
-		argc++;
-		parse_options(argc, argv, NULL, builtin_merge_options,
-			      builtin_merge_usage, 0);
-		free(buf);
+		free(branch_mergeoptions);
+		branch_mergeoptions = xstrdup(v);
+		return 0;
 	}
 
 	if (!strcmp(k, "merge.diffstat") || !strcmp(k, "merge.stat"))
@@ -918,6 +927,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 	if (diff_use_color_default == -1)
 		diff_use_color_default = git_use_color_default;
 
+	if (branch_mergeoptions)
+		parse_branch_merge_options(branch_mergeoptions);
 	argc = parse_options(argc, argv, prefix, builtin_merge_options,
 			builtin_merge_usage, 0);
 	if (verbosity < 0)
diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
index 57f6d2b..56c653d 100755
--- a/t/t7600-merge.sh
+++ b/t/t7600-merge.sh
@@ -372,6 +372,38 @@ test_expect_success 'merge c1 with c2 (no-commit in config)' '
 
 test_debug 'gitk --all'
 
+test_expect_success 'merge c1 with c2 (log in config)' '
+	git config branch.master.mergeoptions "" &&
+	git reset --hard c1 &&
+	git merge --log c2 &&
+	git show -s --pretty=tformat:%s%n%b >expect &&
+
+	git config branch.master.mergeoptions --log &&
+	git reset --hard c1 &&
+	git merge c2 &&
+	git show -s --pretty=tformat:%s%n%b >actual &&
+
+	test_cmp expect actual
+'
+
+test_expect_success 'merge c1 with c2 (log in config gets overridden)' '
+	(
+		git config --remove-section branch.master
+		git config --remove-section merge
+	)
+	git reset --hard c1 &&
+	git merge c2 &&
+	git show -s --pretty=tformat:%s%n%b >expect &&
+
+	git config branch.master.mergeoptions "--no-log" &&
+	git config merge.log true &&
+	git reset --hard c1 &&
+	git merge c2 &&
+	git show -s --pretty=tformat:%s%n%b >actual &&
+
+	test_cmp expect actual
+'
+
 test_expect_success 'merge c1 with c2 (squash in config)' '
 	git reset --hard c1 &&
 	git config branch.master.mergeoptions "--squash" &&
-- 
1.7.5.284.g84c3a8