[PATCH 0/3] Re: how to have --no-ff be the default for all branch

[PATCH 0/3] Re: how to have --no-ff be the default for all branch

[Jay Soffian]

2009/3/6 jean-luc malet <jeanluc.malet@gmail.com>:
> I would like that it is the default for all branch and that I use --ff
> when I want to do fast forward merge
> I know that I can set it up for one branch
> git config add branch.master.mergeoption --no-ff
> but I want it to be the default no just for one branch but for all branch
> git config add branch.*.mergeoption --no-ff
> don't work....

This series should do it for you. It teaches merge to support
merge.options, which is the default for all merge operations.

Jay Soffian (3):
  config: add git_config_option_string()
  builtin-merge: refactor to use git_config_option_string
  builtin-merge: add support for default merge options

 Documentation/git-merge.txt |   11 +++-
 builtin-merge.c             |   24 +++----
 cache.h                     |    2 +
 config.c                    |   39 ++++++++++++
 parse-options.c             |    2 +
 t/t7600-merge.sh            |  143 ++++++++++++++++++++++++++++++++++++-------
 6 files changed, 182 insertions(+), 39 deletions(-)

[PATCH 1/3] config: add git_config_option_string()

[Jay Soffian]

This patch teaches config a new function, git_config_option_string(),
which parses a string using parse-options.

This is useful for any command that allows command-line options to
appear in a config.

Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
Originally I had just factored this into its own function in
builtin-merge, but it seemed that it might be generally useful, so I
moved it to config.c

 cache.h         |    2 ++
 config.c        |   39 +++++++++++++++++++++++++++++++++++++++
 parse-options.c |    2 ++
 3 files changed, 43 insertions(+), 0 deletions(-)

diff --git a/cache.h b/cache.h
index 189151d..c6d3f05 100644
--- a/cache.h
+++ b/cache.h
@@ -854,6 +854,8 @@ extern unsigned long git_config_ulong(const char *, const char *);
 extern int git_config_bool_or_int(const char *, const char *, int *);
 extern int git_config_bool(const char *, const char *);
 extern int git_config_string(const char **, const char *, const char *);
+struct option;
+extern int git_config_option_string(const struct option *, int, const char *, const char *);
 extern int git_config_set(const char *, const char *);
 extern int git_config_set_multivar(const char *, const char *, const char *, int);
 extern int git_config_rename_section(const char *, const char *);
diff --git a/config.c b/config.c
index 0c8c76f..29cfd5b 100644
--- a/config.c
+++ b/config.c
@@ -7,6 +7,7 @@
  */
 #include "cache.h"
 #include "exec_cmd.h"
+#include "parse-options.h"
 
 #define MAXNAME (256)
 
@@ -353,6 +354,44 @@ int git_config_string(const char **dest, const char *var, const char *value)
 	return 0;
 }
 
+int git_config_option_string(const struct option *options, int flags,
+			     const char *var, const char *value)
+{
+	int argc, ret;
+	const char **argv;
+	char *buf;
+	struct parse_opt_ctx_t ctx;
+
+	if (!value)
+		return config_error_nonbool(var);
+
+	buf = xstrdup(value);
+	if ((argc = split_cmdline(buf, &argv)) < 0) {
+		free(buf);
+		return error("Malformed value for %s", var);
+	}
+	argv = xrealloc(argv, sizeof(*argv) * (argc + 2));
+	memmove(argv + 1, argv, sizeof(*argv) * (argc + 1));
+	argc++;
+
+	parse_options_start(&ctx, argc, argv, flags);
+	switch (parse_options_step(&ctx, options, NULL)) {
+	case PARSE_OPT_DONE:
+		ret = parse_options_end(&ctx);
+		break;
+	case PARSE_OPT_HELP: /* not supported in a config */
+	default: /* PARSE_OPT_UNKNOWN */
+		if (ctx.argv[0][1] == '-') {
+			ret = error("unknown option `%s'", ctx.argv[0] + 2);
+		} else {
+			ret = error("unknown switch `%c'", *ctx.opt);
+		}
+		break;
+	}
+	free(buf);
+	return ret;
+}
+
 static int git_default_core_config(const char *var, const char *value)
 {
 	/* This needs a better name */
diff --git a/parse-options.c b/parse-options.c
index 4c5d09d..7996b50 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -356,6 +356,8 @@ int parse_options(int argc, const char **argv, const struct option *options,
 int usage_with_options_internal(const char * const *usagestr,
 				const struct option *opts, int full)
 {
+	if (!usagestr)
+		return PARSE_OPT_HELP;
 	fprintf(stderr, "usage: %s\n", *usagestr++);
 	while (*usagestr && **usagestr)
 		fprintf(stderr, "   or: %s\n", *usagestr++);
-- 
1.6.2.rc2.332.g5d21b

[PATCH 2/3] builtin-merge: refactor to use git_config_option_string

[Jay Soffian]

This patch teaches builtin-merge to use git_config_option_string() for
parsing branch.<name>.mergeoptions

As a side-effect, setting branch.<name>.mergeoptions to "" is no longer
supported. The tests have been modified to reflect this fact, along with
some minor refactoring to support additional tests in the next patch.

Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
I think the side-effect is fine, since why would anyone have an empty
mergeoptions in their config? But if the list disagrees, it's trivial to
modify the preceeding patch to skip over an empty config w/o it being an
error.

Also, it seems like the with_config() utility function here might be
useful for other test scripts, so perhaps it should go into test-lib.sh?

 builtin-merge.c  |   14 +---------
 t/t7600-merge.sh |   74 +++++++++++++++++++++++++++++++++++++----------------
 2 files changed, 52 insertions(+), 36 deletions(-)

diff --git a/builtin-merge.c b/builtin-merge.c
index 6d2160d..504f2be 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -452,20 +452,8 @@ 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)
+		if (git_config_option_string(builtin_merge_options, 0, k, v))
 			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, builtin_merge_options,
-			      builtin_merge_usage, 0);
-		free(buf);
 	}
 
 	if (!strcmp(k, "merge.diffstat") || !strcmp(k, "merge.stat"))
diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
index e5b210b..9db8bb4 100755
--- a/t/t7600-merge.sh
+++ b/t/t7600-merge.sh
@@ -188,6 +188,37 @@ verify_no_mergehead() {
 	fi
 }
 
+with_config() {
+	names_to_unset=
+	retval=0
+	while test $# -gt 1
+	do
+		if test "$1" = "--"
+		then
+			shift
+			break
+		fi
+		if git config "$1" "$2"
+		then
+			names_to_unset="$names_to_unset $1"
+			shift
+			shift
+		else
+			retval=1
+			break
+		fi
+	done
+	if test $retval = 0
+	then
+		git "$@"
+		retval=$?
+	fi
+	for name in $names_to_unset
+	do
+		git config --unset "$name"
+	done
+	return $retval
+}
 
 test_expect_success 'setup' '
 	git add file &&
@@ -327,8 +358,8 @@ test_debug 'gitk --all'
 
 test_expect_success 'merge c1 with c2 (no-commit in config)' '
 	git reset --hard c1 &&
-	git config branch.master.mergeoptions "--no-commit" &&
-	git merge c2 &&
+	with_config branch.master.mergeoptions --no-commit -- \
+		merge c2 &&
 	verify_merge file result.1-5 &&
 	verify_head $c1 &&
 	verify_mergeheads $c2
@@ -338,8 +369,8 @@ test_debug 'gitk --all'
 
 test_expect_success 'merge c1 with c2 (squash in config)' '
 	git reset --hard c1 &&
-	git config branch.master.mergeoptions "--squash" &&
-	git merge c2 &&
+	with_config branch.master.mergeoptions --squash -- \
+		merge c2 &&
 	verify_merge file result.1-5 &&
 	verify_head $c1 &&
 	verify_no_mergehead &&
@@ -350,9 +381,9 @@ test_debug 'gitk --all'
 
 test_expect_success 'override config option -n with --summary' '
 	git reset --hard c1 &&
-	git config branch.master.mergeoptions "-n" &&
 	test_tick &&
-	git merge --summary c2 >diffstat.txt &&
+	with_config branch.master.mergeoptions -n -- \
+		merge --summary c2 >diffstat.txt &&
 	verify_merge file result.1-5 msg.1-5 &&
 	verify_parents $c1 $c2 &&
 	if ! grep "^ file |  *2 +-$" diffstat.txt
@@ -364,9 +395,9 @@ test_expect_success 'override config option -n with --summary' '
 
 test_expect_success 'override config option -n with --stat' '
 	git reset --hard c1 &&
-	git config branch.master.mergeoptions "-n" &&
 	test_tick &&
-	git merge --stat c2 >diffstat.txt &&
+	with_config branch.master.mergeoptions -n -- \
+		merge --stat c2 >diffstat.txt &&
 	verify_merge file result.1-5 msg.1-5 &&
 	verify_parents $c1 $c2 &&
 	if ! grep "^ file |  *2 +-$" diffstat.txt
@@ -380,9 +411,9 @@ test_debug 'gitk --all'
 
 test_expect_success 'override config option --stat' '
 	git reset --hard c1 &&
-	git config branch.master.mergeoptions "--stat" &&
 	test_tick &&
-	git merge -n c2 >diffstat.txt &&
+	with_config branch.master.mergeoptions --stat -- \
+		merge -n c2 >diffstat.txt &&
 	verify_merge file result.1-5 msg.1-5 &&
 	verify_parents $c1 $c2 &&
 	if grep "^ file |  *2 +-$" diffstat.txt
@@ -396,9 +427,9 @@ test_debug 'gitk --all'
 
 test_expect_success 'merge c1 with c2 (override --no-commit)' '
 	git reset --hard c1 &&
-	git config branch.master.mergeoptions "--no-commit" &&
 	test_tick &&
-	git merge --commit c2 &&
+	with_config branch.master.mergeoptions --no-commit -- \
+		merge --commit c2 &&
 	verify_merge file result.1-5 msg.1-5 &&
 	verify_parents $c1 $c2
 '
@@ -407,9 +438,9 @@ test_debug 'gitk --all'
 
 test_expect_success 'merge c1 with c2 (override --squash)' '
 	git reset --hard c1 &&
-	git config branch.master.mergeoptions "--squash" &&
 	test_tick &&
-	git merge --no-squash c2 &&
+	with_config branch.master.mergeoptions --squash -- \
+		merge --no-squash c2 &&
 	verify_merge file result.1-5 msg.1-5 &&
 	verify_parents $c1 $c2
 '
@@ -418,7 +449,6 @@ test_debug 'gitk --all'
 
 test_expect_success 'merge c0 with c1 (no-ff)' '
 	git reset --hard c0 &&
-	git config branch.master.mergeoptions "" &&
 	test_tick &&
 	git merge --no-ff c1 &&
 	verify_merge file result.1 &&
@@ -434,15 +464,16 @@ test_expect_success 'combining --squash and --no-ff is refused' '
 
 test_expect_success 'merge c0 with c1 (ff overrides no-ff)' '
 	git reset --hard c0 &&
-	git config branch.master.mergeoptions "--no-ff" &&
-	git merge --ff c1 &&
+	with_config branch.master.mergeoptions --no-ff -- \
+		merge --ff c1 &&
 	verify_merge file result.1 &&
 	verify_head $c1
 '
 
 test_expect_success 'merge log message' '
 	git reset --hard c0 &&
-	git merge --no-log c2 &&
+	with_config branch.master.mergeoptions --no-ff -- \
+		merge --no-log c2 &&
 	git show -s --pretty=format:%b HEAD >msg.act &&
 	verify_diff msg.nolog msg.act "[OOPS] bad merge log message" &&
 
@@ -451,8 +482,8 @@ test_expect_success 'merge log message' '
 	verify_diff msg.log msg.act "[OOPS] bad merge log message" &&
 
 	git reset --hard HEAD^ &&
-	git config merge.log yes &&
-	git merge c3 &&
+	with_config merge.log yes -- \
+		merge c3 &&
 	git show -s --pretty=format:%b HEAD >msg.act &&
 	verify_diff msg.log msg.act "[OOPS] bad merge log message"
 '
@@ -461,7 +492,6 @@ test_debug 'gitk --all'
 
 test_expect_success 'merge c1 with c0, c2, c0, and c1' '
        git reset --hard c1 &&
-       git config branch.master.mergeoptions "" &&
        test_tick &&
        git merge c0 c2 c0 c1 &&
        verify_merge file result.1-5 &&
@@ -472,7 +502,6 @@ test_debug 'gitk --all'
 
 test_expect_success 'merge c1 with c0, c2, c0, and c1' '
        git reset --hard c1 &&
-       git config branch.master.mergeoptions "" &&
        test_tick &&
        git merge c0 c2 c0 c1 &&
        verify_merge file result.1-5 &&
@@ -483,7 +512,6 @@ test_debug 'gitk --all'
 
 test_expect_success 'merge c1 with c1 and c2' '
        git reset --hard c1 &&
-       git config branch.master.mergeoptions "" &&
        test_tick &&
        git merge c1 c2 &&
        verify_merge file result.1-5 &&
-- 
1.6.2.rc2.332.g5d21b

[PATCH 3/3] builtin-merge: add support for default merge options

[Jay Soffian]

This patch teaches merge a new setting, merge.options, which is
processed before any of the other merge configuration settings. It may
be used to establish a default which can then be overridden by more
specific branch.<name>.mergeoptions (or, obviously, command-line
switches).

Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
 Documentation/git-merge.txt |   11 +++++--
 builtin-merge.c             |   10 ++++++
 t/t7600-merge.sh            |   69 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 87 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index f7be584..3cb06e7 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -47,10 +47,15 @@ CONFIGURATION
 -------------
 include::merge-config.txt[]
 
+merge.options::
+	Sets default options for merging. The syntax and supported options are
+	equal to that of 'git-merge'. Arguments are split by spaces, and may be
+	quoted in the same way as alias.* config options.
+
 branch.<name>.mergeoptions::
-	Sets default options for merging into branch <name>. The syntax and
-	supported options are equal to that of 'git-merge', but option values
-	containing whitespace characters are currently not supported.
+	Sets default options for merging into branch <name>. This setting is
+	handled after `merge.options`, so it may be used to override any
+	settings appearing there. The syntax is identical to `merge.options`.
 
 HOW MERGE WORKS
 ---------------
diff --git a/builtin-merge.c b/builtin-merge.c
index 504f2be..1f124b3 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -447,6 +447,15 @@ cleanup:
 	strbuf_release(&bname);
 }
 
+static int git_merge_config_default(const char *k, const char *v, void *cb)
+{
+	if (!strcmp(k, "merge.options")) {
+		if (git_config_option_string(builtin_merge_options, 0, k, v))
+			die("Bad merge.options string");
+	}
+	return 0;
+}
+
 static int git_merge_config(const char *k, const char *v, void *cb)
 {
 	if (branch && !prefixcmp(k, "branch.") &&
@@ -838,6 +847,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 	if (is_null_sha1(head))
 		head_invalid = 1;
 
+	git_config(git_merge_config_default, NULL);
 	git_config(git_merge_config, NULL);
 
 	/* for color.ui */
diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
index 9db8bb4..aaecdab 100755
--- a/t/t7600-merge.sh
+++ b/t/t7600-merge.sh
@@ -367,6 +367,16 @@ test_expect_success 'merge c1 with c2 (no-commit in config)' '
 
 test_debug 'gitk --all'
 
+test_expect_success 'merge c1 with c2 (no-commit in merge.options)' '
+	git reset --hard c1 &&
+	with_config merge.options --no-commit -- merge c2 &&
+	verify_merge file result.1-5 &&
+	verify_head $c1 &&
+	verify_mergeheads $c2
+'
+
+test_debug 'gitk --all'
+
 test_expect_success 'merge c1 with c2 (squash in config)' '
 	git reset --hard c1 &&
 	with_config branch.master.mergeoptions --squash -- \
@@ -379,6 +389,17 @@ test_expect_success 'merge c1 with c2 (squash in config)' '
 
 test_debug 'gitk --all'
 
+test_expect_success 'merge c1 with c2 (squash in merge.options)' '
+	git reset --hard c1 &&
+	with_config merge.options --squash -- merge c2 &&
+	verify_merge file result.1-5 &&
+	verify_head $c1 &&
+	verify_no_mergehead &&
+	verify_diff squash.1-5 .git/SQUASH_MSG "[OOPS] bad squash message"
+'
+
+test_debug 'gitk --all'
+
 test_expect_success 'override config option -n with --summary' '
 	git reset --hard c1 &&
 	test_tick &&
@@ -425,6 +446,54 @@ test_expect_success 'override config option --stat' '
 
 test_debug 'gitk --all'
 
+test_expect_success 'override merge.options -n with branch mergeoptions --summary' '
+	git reset --hard c1 &&
+	test_tick &&
+	with_config merge.options -n branch.master.mergeoptions --summary -- \
+		merge c2 >diffstat.txt &&
+	verify_merge file result.1-5 msg.1-5 &&
+	verify_parents $c1 $c2 &&
+	if ! grep "^ file |  *2 +-$" diffstat.txt
+	then
+		echo "[OOPS] diffstat was not generated with --summary"
+		false
+	fi
+'
+
+test_debug 'gitk --all'
+
+test_expect_success 'override merge.options -n with branch mergeoptions --stat' '
+	git reset --hard c1 &&
+	test_tick &&
+	with_config merge.options -n branch.master.mergeoptions --stat -- \
+		merge c2 >diffstat.txt &&
+	verify_merge file result.1-5 msg.1-5 &&
+	verify_parents $c1 $c2 &&
+	if ! grep "^ file |  *2 +-$" diffstat.txt
+	then
+		echo "[OOPS] diffstat was not generated with --stat"
+		false
+	fi
+'
+
+test_debug 'gitk --all'
+
+test_expect_success 'override merge.options --stat' '
+	git reset --hard c1 &&
+	test_tick &&
+	with_config merge.options --stat branch.master.mergeoptions -n -- \
+		merge c2 >diffstat.txt &&
+	verify_merge file result.1-5 msg.1-5 &&
+	verify_parents $c1 $c2 &&
+	if grep "^ file |  *2 +-$" diffstat.txt
+	then
+		echo "[OOPS] diffstat was generated"
+		false
+	fi
+'
+
+test_debug 'gitk --all'
+
 test_expect_success 'merge c1 with c2 (override --no-commit)' '
 	git reset --hard c1 &&
 	test_tick &&
-- 
1.6.2.rc2.332.g5d21b

Re: [PATCH 3/3] builtin-merge: add support for default merge options

[Junio C Hamano]

Jay Soffian <jaysoffian@gmail.com> writes:

> @@ -838,6 +847,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
>  	if (is_null_sha1(head))
>  		head_invalid = 1;
>  
> +	git_config(git_merge_config_default, NULL);
>  	git_config(git_merge_config, NULL);


The placement of this comes before parse_options(), just like the part
that slurps "branch.*.mergeoptions", so it can be overridden by the
command line just like "branch.*.mergeoptions" can, which is good.

When you are on branch "frotz", your config have both merge.options and
branch.frotz.mergeoptions, and you give some other options from the
command line, how should they interact?  I'd expect the branch.*.options
to take effect, ignoring merge.options entirely.

I think the right way to structure this is to change the code in
git_merge_config() that accepts "branch.*.mergeoptions" to just store a
xstrdup() pointer away, add a similar thing in the same function for the
new "merge.options" variable.  Get rid of your git_merge_config_default
function that forces git_config() to iterate over the same config file one
more time.  And after the config parser returns, run the parse_options
only once.

In other words, the overall code structure would look like this:

static char *options_from_config;
static int options_from_config_taken_from_branch_config;

static int git_merge_config(...)
{
	if (branch && !prefixcmp(k, "branch.") ... ) {
		/*
                 * We may have found merge.options first;
		 * free it and override it with the value of
                 * branch.*.mergeoptions for the current branch
                 * we just found.
                 */
        	free(options_from_config);
               	options_from_config_taken_from_branch_config = 1;
               	options_from_config = xstrdup(value);
		return 0;
	}
        if (!strcmp(k, "merge.options")) {
		/*
                 * Do not override branch.*.mergeoptions for the
                 * current branch if we already found one.
                 */
               	if (!options_from_config_taken_from_branch_config)
                	options_from_config = xstrdup(value);
		return 0;
	}
        ...
}

int cmd_merge(...)
{
	...
        git_config(git_merge_config, NULL);
        if (options_from_config)
		/*
                 * There is a "prime" options given in
                 * the configuration file.  Parse it.
                 */
                git_config_option_string(builtin_merge_options, ...,
                			options_from_config);
	...
        argc = parse_options(argc, argv, builtin_merge_options,...);
	...
}

If for some reason you would want to have cumulative options across
branch.*.merge, merge.options and the command line, then you would instead
keep two separate strings, and call git_config_option_string() for both of
them, before processing the real command line options.

Hmm?

Re: [PATCH 3/3] builtin-merge: add support for default merge options

[Jay Soffian]

On Fri, Mar 6, 2009 at 5:46 PM, Junio C Hamano <gitster@pobox.com> wrote:

> When you are on branch "frotz", your config have both merge.options and
> branch.frotz.mergeoptions, and you give some other options from the
> command line, how should they interact?  I'd expect the branch.*.options
> to take effect, ignoring merge.options entirely.

Really? I didn't think that would be consistent with the fact that the
the command line options override branch.*.options, but don't replace
them. So I specifically coded it such that there are three separate
layers all merged together. (Which is also how I documented it in the
man page.)

> If for some reason you would want to have cumulative options across

Which I do, or I wouldn't have coded it that way. :-)

> branch.*.merge, merge.options and the command line, then you would instead
> keep two separate strings, and call git_config_option_string() for both of
> them, before processing the real command line options.

Ah, right that would be better.

j.

[PATCH v2 3/3] builtin-merge: add support for default merge options

[Jay Soffian]

This patch teaches merge a new setting, merge.options, which is
processed before any of the other merge configuration settings. It may
be used to establish a default which can then be overridden by more
specific branch.<name>.mergeoptions (or, obviously, command-line
switches).

Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
On Fri, Mar 6, 2009 at 5:46 PM, Junio C Hamano <gitster@pobox.com> wrote:
> If for some reason you would want to have cumulative options across
> branch.*.merge, merge.options and the command line, then you would instead
> keep two separate strings, and call git_config_option_string() for both of
> them, before processing the real command line options.

Which is what this version does. I also made the explanation of this behavior
in the man page more explicit.

 Documentation/git-merge.txt |   12 +++++--
 builtin-merge.c             |   22 +++++++++++--
 t/t7600-merge.sh            |   69 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 96 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index f7be584..5d80a78 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -47,10 +47,16 @@ CONFIGURATION
 -------------
 include::merge-config.txt[]
 
+merge.options::
+	Sets default options for merging. The syntax and supported options are
+	equal to that of 'git-merge'. Arguments are split by spaces, and may be
+	quoted in the same way as alias.* config options.
+
 branch.<name>.mergeoptions::
-	Sets default options for merging into branch <name>. The syntax and
-	supported options are equal to that of 'git-merge', but option values
-	containing whitespace characters are currently not supported.
+	Sets default options for merging into branch <name>. This setting is
+	handled after and is cumulative to `merge.options`. So it may override,
+	but does replace, any settings appearing there. The syntax is identical
+	to `merge.options`.
 
 HOW MERGE WORKS
 ---------------
diff --git a/builtin-merge.c b/builtin-merge.c
index 504f2be..d4dc4fe 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -50,6 +50,8 @@ static unsigned char head[20], stash[20];
 static struct strategy **use_strategies;
 static size_t use_strategies_nr, use_strategies_alloc;
 static const char *branch;
+static const char *branch_option_string = NULL;
+static const char *default_option_string = NULL;
 static int verbosity;
 
 static struct strategy all_strategy[] = {
@@ -451,10 +453,8 @@ 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")) {
-		if (git_config_option_string(builtin_merge_options, 0, k, v))
-			die("Bad branch.%s.mergeoptions string", branch);
-	}
+		!strcmp(k + 7 + strlen(branch), ".mergeoptions"))
+			return git_config_string(&branch_option_string, k, v);
 
 	if (!strcmp(k, "merge.diffstat") || !strcmp(k, "merge.stat"))
 		show_diffstat = git_config_bool(k, v);
@@ -462,6 +462,8 @@ static int git_merge_config(const char *k, const char *v, void *cb)
 		return git_config_string(&pull_twohead, k, v);
 	else if (!strcmp(k, "pull.octopus"))
 		return git_config_string(&pull_octopus, k, v);
+	else if (!strcmp(k, "merge.options"))
+		return git_config_string(&default_option_string, k, v);
 	else if (!strcmp(k, "merge.log") || !strcmp(k, "merge.summary"))
 		option_log = git_config_bool(k, v);
 	return git_diff_ui_config(k, v, cb);
@@ -839,6 +841,18 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		head_invalid = 1;
 
 	git_config(git_merge_config, NULL);
+	if (default_option_string) {
+		if (git_config_option_string(builtin_merge_options, 0,
+		    "merge.options", default_option_string))
+			die("Bad merge.options string");
+	}
+	if (branch_option_string) {
+		strbuf_addf(&buf, "branch.%s.mergeoptions", branch);
+		if (git_config_option_string(builtin_merge_options, 0,
+		     buf.buf, branch_option_string))
+			die("Bad %s string", buf.buf);
+		strbuf_reset(&buf);
+	}
 
 	/* for color.ui */
 	if (diff_use_color_default == -1)
diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
index 9db8bb4..aaecdab 100755
--- a/t/t7600-merge.sh
+++ b/t/t7600-merge.sh
@@ -367,6 +367,16 @@ test_expect_success 'merge c1 with c2 (no-commit in config)' '
 
 test_debug 'gitk --all'
 
+test_expect_success 'merge c1 with c2 (no-commit in merge.options)' '
+	git reset --hard c1 &&
+	with_config merge.options --no-commit -- merge c2 &&
+	verify_merge file result.1-5 &&
+	verify_head $c1 &&
+	verify_mergeheads $c2
+'
+
+test_debug 'gitk --all'
+
 test_expect_success 'merge c1 with c2 (squash in config)' '
 	git reset --hard c1 &&
 	with_config branch.master.mergeoptions --squash -- \
@@ -379,6 +389,17 @@ test_expect_success 'merge c1 with c2 (squash in config)' '
 
 test_debug 'gitk --all'
 
+test_expect_success 'merge c1 with c2 (squash in merge.options)' '
+	git reset --hard c1 &&
+	with_config merge.options --squash -- merge c2 &&
+	verify_merge file result.1-5 &&
+	verify_head $c1 &&
+	verify_no_mergehead &&
+	verify_diff squash.1-5 .git/SQUASH_MSG "[OOPS] bad squash message"
+'
+
+test_debug 'gitk --all'
+
 test_expect_success 'override config option -n with --summary' '
 	git reset --hard c1 &&
 	test_tick &&
@@ -425,6 +446,54 @@ test_expect_success 'override config option --stat' '
 
 test_debug 'gitk --all'
 
+test_expect_success 'override merge.options -n with branch mergeoptions --summary' '
+	git reset --hard c1 &&
+	test_tick &&
+	with_config merge.options -n branch.master.mergeoptions --summary -- \
+		merge c2 >diffstat.txt &&
+	verify_merge file result.1-5 msg.1-5 &&
+	verify_parents $c1 $c2 &&
+	if ! grep "^ file |  *2 +-$" diffstat.txt
+	then
+		echo "[OOPS] diffstat was not generated with --summary"
+		false
+	fi
+'
+
+test_debug 'gitk --all'
+
+test_expect_success 'override merge.options -n with branch mergeoptions --stat' '
+	git reset --hard c1 &&
+	test_tick &&
+	with_config merge.options -n branch.master.mergeoptions --stat -- \
+		merge c2 >diffstat.txt &&
+	verify_merge file result.1-5 msg.1-5 &&
+	verify_parents $c1 $c2 &&
+	if ! grep "^ file |  *2 +-$" diffstat.txt
+	then
+		echo "[OOPS] diffstat was not generated with --stat"
+		false
+	fi
+'
+
+test_debug 'gitk --all'
+
+test_expect_success 'override merge.options --stat' '
+	git reset --hard c1 &&
+	test_tick &&
+	with_config merge.options --stat branch.master.mergeoptions -n -- \
+		merge c2 >diffstat.txt &&
+	verify_merge file result.1-5 msg.1-5 &&
+	verify_parents $c1 $c2 &&
+	if grep "^ file |  *2 +-$" diffstat.txt
+	then
+		echo "[OOPS] diffstat was generated"
+		false
+	fi
+'
+
+test_debug 'gitk --all'
+
 test_expect_success 'merge c1 with c2 (override --no-commit)' '
 	git reset --hard c1 &&
 	test_tick &&
-- 
1.6.2.rc2.332.g5d21b

Re: [PATCH 3/3] builtin-merge: add support for default merge options

[Junio C Hamano]

Jay Soffian <jaysoffian@gmail.com> writes:

> On Fri, Mar 6, 2009 at 5:46 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
>> When you are on branch "frotz", your config have both merge.options and
>> branch.frotz.mergeoptions, and you give some other options from the
>> command line, how should they interact?  I'd expect the branch.*.options
>> to take effect, ignoring merge.options entirely.
>
> Really? I didn't think that would be consistent with the fact that the
> the command line options override branch.*.options, but don't replace
> them.

I think cumulative option in configuration is bad in practice, but I'll
explain why after talking about something else.

I think it would be much better if you did not introduce a new
configuration merge.options which is not consistent with everything else
to begin with.

Instead, if your addition was literally to allow saying things like this,
it would be much easier to understand.

	[branch "*"]
        	mergeoptions = ...
                remote = origin
                rebase = true
	[branch "frotz"]
        	mergeoptions = ; nothing
                rebase = false
	[branch "nitfol"]
        	remote = xyzzy

When on branch 'nitfol', because there is an overriding "remote" defined,
we would not look at branch.*.remote and fetch from xyzzy instead of
fetching from the default origin.  Because there is no "rebase" defined
for that branch, we would use branch.*.rebase=true from the fall-back
default.

When on branch 'frotz', because you have an explicit mergeoptions that
says "I do not want any", it would override whatever is defined for the
corresponding fall-back default branch.*.mergeoptions.

Having explained that I think branch.*.mergeoptions is syntactically nicer
and more extensible as the UI to the end user, let's discuss the
"cumulative" aspect.  In the following, I'll keep using branch.*.$option,
but you can read it as if I said merge.options and the discussion is the
same.

There are two reasons why you as an end user specify a concrete value
(e.g. "empty") for a concrete branch name (e.g. branch.frotz.mergeoptions).
One is because you know the current value set to the fall-back default
(e.g. branch.*.mergeoptions) is not suitable for this particular branch.
Another is because you know you may want to change the fall-back default
sometime in the future, and you do not want that to affect your setting
you are making for this particular branch today.

For the purpose of the first reason above, if you allowed cumulative
option, the end user needs to inspect branch.*.$option and come up with a
countermanding set of options to set to branch.frotz.$option.  If there is
no cumulative option, the end user does not have to worry about what
branch.*.$option says.  Non-cumulative is simply easier to understand.

For the purpose of the second reason above, when the user has to update
branch.frotz.$option because some external situation changed (e.g. the
user used to be an e-mail contributor, but now gained "push privilege";
the user became the primary maintainer; etc.), the same argument on
maintenance burden as above holds.  To update branch.*.$option, you need
to inspect every branch.$specific.$option (or lack thereof) as well in
either way.

So overall, cumulative configuration tend to be more cumbersome for the
end user to manage.

You cannot draw a direct analogy with the command line options, which is
used as a single-shot override by nature.  The user knows what usually
happens when he says "git pull" while on branch 'frotz' without options,
and countermanding specific aspects (but not necessarily others) of the
operation for this single invocation.  Because the configuration values
are set so that the user can set-and-forget the exact syntax to invoke
each feature, cumulativeness between configured default and command line
override makes more sense.

Re: [PATCH 3/3] builtin-merge: add support for default merge options

[Jay Soffian]

On Fri, Mar 6, 2009 at 7:58 PM, Junio C Hamano <gitster@pobox.com> wrote:
> I think it would be much better if you did not introduce a new
> configuration merge.options which is not consistent with everything else
> to begin with.
>
> Instead, if your addition was literally to allow saying things like this,
> it would be much easier to understand.
>
>        [branch "*"]
>                mergeoptions = ...
>                remote = origin
>                rebase = true

Okay. That's probably a more invasive patch, because a lot more code
paths look at [branch ...], but I'll look into it.

> So overall, cumulative configuration tend to be more cumbersome for the
> end user to manage.

Okay, you've convinced me.

j.

Re: [PATCH 3/3] builtin-merge: add support for default merge options

[Junio C Hamano]

Jay Soffian <jaysoffian@gmail.com> writes:

> On Fri, Mar 6, 2009 at 7:58 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> I think it would be much better if you did not introduce a new
>> configuration merge.options which is not consistent with everything else
>> to begin with.
>>
>> Instead, if your addition was literally to allow saying things like this,
>> it would be much easier to understand.
>>
>>        [branch "*"]
>>                mergeoptions = ...
>>                remote = origin
>>                rebase = true
>
> Okay. That's probably a more invasive patch, because a lot more code
> paths look at [branch ...], but I'll look into it.

Note that you do not have to cover branch.*.remote and other things in the
same patch.  The first one could just handle branch.*.mergeoptions and you
can let later patches to implement the fallbacks for other variables.

Re: [PATCH 3/3] builtin-merge: add support for default merge options

[Jay Soffian]

On Sat, Mar 7, 2009 at 2:18 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Note that you do not have to cover branch.*.remote and other things in the
> same patch.  The first one could just handle branch.*.mergeoptions and you
> can let later patches to implement the fallbacks for other variables.

Oh, indeed, but I think I do need to provide all the patches at once.
Otherwise it will just be very confusing and I anticipate a user
asking "how come I can use branch.*.mergeoptions, but none of the
other branch.*.foo settings work?"

And to be honest, that concern extends beyond branch.*. "How come git
foo allows defaults, but git bar does not?" seems like a valid
question, and while "because no one implemented defaults for foo and
bar" is a valid answer, it isn't a very satisfying one (to me).

This thread is what inspired me to start the "what's so evil about
overriding builtins with aliases" thread (please don't address that
here, I got it from the other thread). But I still wonder if we should
provide defaults consistently instead of piecemeal. For example:

[defaults]
   merge = --no-ff

But I suppose that will be objected to as confusing/complicated/likely
to have side-effects.

j.

Re: [PATCH 3/3] builtin-merge: add support for default merge options

[jean-luc malet]

as a user,
1) I looked how to override defaults, since merge seems to use --ff as
default, I looked if there was any global config to override that.
nothing found... so go back to google
2) I found on the web that we can set it using
branch.branchname.mergeoptions so I said "what if I use a branch named
*, the config file was correct and git didn't insult me so.... but not
the expected result...

since the commands have allready defaults builtin, why not storing
thoses defaults in the config file? like this it's easier to update
them or change the way git behave....
JLM

On Sat, Mar 7, 2009 at 8:18 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Jay Soffian <jaysoffian@gmail.com> writes:
>
>> On Fri, Mar 6, 2009 at 7:58 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>> I think it would be much better if you did not introduce a new
>>> configuration merge.options which is not consistent with everything else
>>> to begin with.
>>>
>>> Instead, if your addition was literally to allow saying things like this,
>>> it would be much easier to understand.
>>>
>>>        [branch "*"]
>>>                mergeoptions = ...
>>>                remote = origin
>>>                rebase = true
>>
>> Okay. That's probably a more invasive patch, because a lot more code
>> paths look at [branch ...], but I'll look into it.
>
> Note that you do not have to cover branch.*.remote and other things in the
> same patch.  The first one could just handle branch.*.mergeoptions and you
> can let later patches to implement the fallbacks for other variables.
>
>



-- 
KISS! (Keep It Simple, Stupid!)
(garde le simple, imbécile!)
"mais qu'est-ce que tu m'as pondu comme usine à gaz? fait des choses
simples et qui marchent, espèce d'imbécile!"
-----------------------------
"Si vous pensez que vous êtes trop petit pour changer quoique ce soit,
essayez donc de dormir avec un moustique dans votre chambre." Betty
Reese
http://www.grainesdechangement.com/citations.htm

Re: [PATCH 3/3] builtin-merge: add support for default merge options

[jean-luc malet]

Hi!
I would like to know the status about this patch...
I think this is a useful feature to have a default configuration
settable for all branch.<name>.* options
thanks and regards
JLM

On Sat, Mar 7, 2009 at 8:31 PM, jean-luc malet <jeanluc.malet@gmail.com> wrote:
> as a user,
> 1) I looked how to override defaults, since merge seems to use --ff as
> default, I looked if there was any global config to override that.
> nothing found... so go back to google
> 2) I found on the web that we can set it using
> branch.branchname.mergeoptions so I said "what if I use a branch named
> *, the config file was correct and git didn't insult me so.... but not
> the expected result...
>
> since the commands have allready defaults builtin, why not storing
> thoses defaults in the config file? like this it's easier to update
> them or change the way git behave....
> JLM
>
> On Sat, Mar 7, 2009 at 8:18 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Jay Soffian <jaysoffian@gmail.com> writes:
>>
>>> On Fri, Mar 6, 2009 at 7:58 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>>> I think it would be much better if you did not introduce a new
>>>> configuration merge.options which is not consistent with everything else
>>>> to begin with.
>>>>
>>>> Instead, if your addition was literally to allow saying things like this,
>>>> it would be much easier to understand.
>>>>
>>>>        [branch "*"]
>>>>                mergeoptions = ...
>>>>                remote = origin
>>>>                rebase = true
>>>
>>> Okay. That's probably a more invasive patch, because a lot more code
>>> paths look at [branch ...], but I'll look into it.
>>
>> Note that you do not have to cover branch.*.remote and other things in the
>> same patch.  The first one could just handle branch.*.mergeoptions and you
>> can let later patches to implement the fallbacks for other variables.
>>
>>
>
>
>
> --
> KISS! (Keep It Simple, Stupid!)
> (garde le simple, imbécile!)
> "mais qu'est-ce que tu m'as pondu comme usine à gaz? fait des choses
> simples et qui marchent, espèce d'imbécile!"
> -----------------------------
> "Si vous pensez que vous êtes trop petit pour changer quoique ce soit,
> essayez donc de dormir avec un moustique dans votre chambre." Betty
> Reese
> http://www.grainesdechangement.com/citations.htm
>



-- 
KISS! (Keep It Simple, Stupid!)
(garde le simple, imbécile!)
"mais qu'est-ce que tu m'as pondu comme usine à gaz? fait des choses
simples et qui marchent, espèce d'imbécile!"
-----------------------------
"Si vous pensez que vous êtes trop petit pour changer quoique ce soit,
essayez donc de dormir avec un moustique dans votre chambre." Betty
Reese
http://www.grainesdechangement.com/citations.htm

Re: [PATCH 3/3] builtin-merge: add support for default merge options

[Jay Soffian]

On Fri, Mar 19, 2010 at 10:19 AM, jean-luc malet
<jeanluc.malet@gmail.com> wrote:
> Hi!
> I would like to know the status about this patch...
> I think this is a useful feature to have a default configuration
> settable for all branch.<name>.* options
> thanks and regards

I lost interest in it apparently. I see it's still marked in my TODO,
but I haven't had any personal time to spend on git lately.

j.

> On Sat, Mar 7, 2009 at 8:31 PM, jean-luc malet <jeanluc.malet@gmail.com> wrote:
>> as a user,
>> 1) I looked how to override defaults, since merge seems to use --ff as
>> default, I looked if there was any global config to override that.
>> nothing found... so go back to google
>> 2) I found on the web that we can set it using
>> branch.branchname.mergeoptions so I said "what if I use a branch named
>> *, the config file was correct and git didn't insult me so.... but not
>> the expected result...
>>
>> since the commands have allready defaults builtin, why not storing
>> thoses defaults in the config file? like this it's easier to update
>> them or change the way git behave....
>> JLM
>>
>> On Sat, Mar 7, 2009 at 8:18 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Jay Soffian <jaysoffian@gmail.com> writes:
>>>
>>>> On Fri, Mar 6, 2009 at 7:58 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>>>> I think it would be much better if you did not introduce a new
>>>>> configuration merge.options which is not consistent with everything else
>>>>> to begin with.
>>>>>
>>>>> Instead, if your addition was literally to allow saying things like this,
>>>>> it would be much easier to understand.
>>>>>
>>>>>        [branch "*"]
>>>>>                mergeoptions = ...
>>>>>                remote = origin
>>>>>                rebase = true
>>>>
>>>> Okay. That's probably a more invasive patch, because a lot more code
>>>> paths look at [branch ...], but I'll look into it.
>>>
>>> Note that you do not have to cover branch.*.remote and other things in the
>>> same patch.  The first one could just handle branch.*.mergeoptions and you
>>> can let later patches to implement the fallbacks for other variables.
>>>
>>>
>>
>>
>>
>> --
>> KISS! (Keep It Simple, Stupid!)
>> (garde le simple, imbécile!)
>> "mais qu'est-ce que tu m'as pondu comme usine à gaz? fait des choses
>> simples et qui marchent, espèce d'imbécile!"
>> -----------------------------
>> "Si vous pensez que vous êtes trop petit pour changer quoique ce soit,
>> essayez donc de dormir avec un moustique dans votre chambre." Betty
>> Reese
>> http://www.grainesdechangement.com/citations.htm
>>
>
>
>
> --
> KISS! (Keep It Simple, Stupid!)
> (garde le simple, imbécile!)
> "mais qu'est-ce que tu m'as pondu comme usine à gaz? fait des choses
> simples et qui marchent, espèce d'imbécile!"
> -----------------------------
> "Si vous pensez que vous êtes trop petit pour changer quoique ce soit,
> essayez donc de dormir avec un moustique dans votre chambre." Betty
> Reese
> http://www.grainesdechangement.com/citations.htm
>

Re: [PATCH 3/3] builtin-merge: add support for default merge options

[jean-luc malet]

Is there someone that can review this patch and maybe include it into
next GIT release?
I think it is really usefull especially when used in company
environment where default policies have to be set
thanks and regards
JLM

On Fri, Mar 19, 2010 at 3:54 PM, Jay Soffian <jaysoffian@gmail.com> wrote:
> On Fri, Mar 19, 2010 at 10:19 AM, jean-luc malet
> <jeanluc.malet@gmail.com> wrote:
>> Hi!
>> I would like to know the status about this patch...
>> I think this is a useful feature to have a default configuration
>> settable for all branch.<name>.* options
>> thanks and regards
>
> I lost interest in it apparently. I see it's still marked in my TODO,
> but I haven't had any personal time to spend on git lately.
>
> j.
>
>> On Sat, Mar 7, 2009 at 8:31 PM, jean-luc malet <jeanluc.malet@gmail.com> wrote:
>>> as a user,
>>> 1) I looked how to override defaults, since merge seems to use --ff as
>>> default, I looked if there was any global config to override that.
>>> nothing found... so go back to google
>>> 2) I found on the web that we can set it using
>>> branch.branchname.mergeoptions so I said "what if I use a branch named
>>> *, the config file was correct and git didn't insult me so.... but not
>>> the expected result...
>>>
>>> since the commands have allready defaults builtin, why not storing
>>> thoses defaults in the config file? like this it's easier to update
>>> them or change the way git behave....
>>> JLM
>>>
>>> On Sat, Mar 7, 2009 at 8:18 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>>> Jay Soffian <jaysoffian@gmail.com> writes:
>>>>
>>>>> On Fri, Mar 6, 2009 at 7:58 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>>>>> I think it would be much better if you did not introduce a new
>>>>>> configuration merge.options which is not consistent with everything else
>>>>>> to begin with.
>>>>>>
>>>>>> Instead, if your addition was literally to allow saying things like this,
>>>>>> it would be much easier to understand.
>>>>>>
>>>>>>        [branch "*"]
>>>>>>                mergeoptions = ...
>>>>>>                remote = origin
>>>>>>                rebase = true
>>>>>
>>>>> Okay. That's probably a more invasive patch, because a lot more code
>>>>> paths look at [branch ...], but I'll look into it.
>>>>
>>>> Note that you do not have to cover branch.*.remote and other things in the
>>>> same patch.  The first one could just handle branch.*.mergeoptions and you
>>>> can let later patches to implement the fallbacks for other variables.
>>>>
>>>>
>>>
>>>
>>>
>>> --
>>> KISS! (Keep It Simple, Stupid!)
>>> (garde le simple, imbécile!)
>>> "mais qu'est-ce que tu m'as pondu comme usine à gaz? fait des choses
>>> simples et qui marchent, espèce d'imbécile!"
>>> -----------------------------
>>> "Si vous pensez que vous êtes trop petit pour changer quoique ce soit,
>>> essayez donc de dormir avec un moustique dans votre chambre." Betty
>>> Reese
>>> http://www.grainesdechangement.com/citations.htm
>>>
>>
>>
>>
>> --
>> KISS! (Keep It Simple, Stupid!)
>> (garde le simple, imbécile!)
>> "mais qu'est-ce que tu m'as pondu comme usine à gaz? fait des choses
>> simples et qui marchent, espèce d'imbécile!"
>> -----------------------------
>> "Si vous pensez que vous êtes trop petit pour changer quoique ce soit,
>> essayez donc de dormir avec un moustique dans votre chambre." Betty
>> Reese
>> http://www.grainesdechangement.com/citations.htm
>>
>



-- 
KISS! (Keep It Simple, Stupid!)
(garde le simple, imbécile!)
"mais qu'est-ce que tu m'as pondu comme usine à gaz? fait des choses
simples et qui marchent, espèce d'imbécile!"
-----------------------------
"Si vous pensez que vous êtes trop petit pour changer quoique ce soit,
essayez donc de dormir avec un moustique dans votre chambre." Betty
Reese
http://www.grainesdechangement.com/citations.htm