how to have --no-ff be the default for all branch

PATCH In review Jay Soffian 3 participants 15 messages Jun 15, 2016 Documentation

lore

Share & tools

lore

Jay Soffian

author

2016-06-15 22:46 UTC #0 lore raw

2009/3/6 jean-luc malet <[email protected]>:

Quoted 7 lines

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(-)

Jay Soffian

author

2016-06-15 22:46 UTC #1 lore raw

PATCH 1/3 config: add git_config_option_string()

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 <[email protected]>

---
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(-)

cache.h +2 -0

index 189151d..c6d3f05 100644
@@ -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 *);

config.c +39 -0

index 0c8c76f..29cfd5b 100644
@@ -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 */

parse-options.c +2 -0

index 4c5d09d..7996b50 100644
@@ -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++);

Signature · 1 line

1.6.2.rc2.332.g5d21b

Jay Soffian

author

2016-06-15 22:46 UTC #2 lore raw

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

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 <[email protected]>

---
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(-)

builtin-merge.c +1 ~~-13~~

index 6d2160d..504f2be 100644
@@ -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"))

t/t7600-merge.sh +51 ~~-23~~

169 lines not on this page · raw message

Signature · 1 line

1.6.2.rc2.332.g5d21b

Jay Soffian

author

2016-06-15 22:46 UTC #3 lore raw

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

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 <[email protected]>

---
 Documentation/git-merge.txt |   11 +++++--
 builtin-merge.c             |   10 ++++++
 t/t7600-merge.sh            |   69 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 87 insertions(+), 3 deletions(-)

Documentation/git-merge.txt +8 -3

index f7be584..3cb06e7 100644
@@ -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
 ---------------

builtin-merge.c +10 -0

index 504f2be..1f124b3 100644
@@ -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 */

t/t7600-merge.sh +69 -0

87 lines not on this page · raw message

Signature · 1 line

1.6.2.rc2.332.g5d21b

Junio C Hamano

2016-06-15 22:46 UTC #4 lore raw

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

builtin-merge.c int cmd_merge(int argc, const char **argv, const char *prefix) · line 851

Quoted 6 lines · Jay Soffian

builtin-merge.c +1 -0

@@ -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?

Jay Soffian

author

2016-06-15 22:46 UTC #5 lore raw

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

Quoted 4 lines · Junio C Hamano

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.)

Quoted 1 line

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

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

Quoted 3 lines

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.

Jay Soffian

author

2016-06-15 22:46 UTC #6 lore raw

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

Signed-off-by Jay Soffian <[email protected]>

---

Quoted 4 lines · Junio C Hamano

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(-)

Documentation/git-merge.txt +9 -3

index f7be584..5d80a78 100644
@@ -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
 ---------------

builtin-merge.c +18 -4

index 504f2be..d4dc4fe 100644
@@ -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)

t/t7600-merge.sh +69 -0

87 lines not on this page · raw message

Signature · 1 line

1.6.2.rc2.332.g5d21b

Junio C Hamano

2016-06-15 22:46 UTC #7 lore raw

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

In reply to Jay Soffian

Quoted 10 lines · Jay Soffian

Quoted 4 lines · Junio C Hamano
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.

Jay Soffian

author

2016-06-15 22:46 UTC #8 lore raw

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

Quoted 11 lines · Junio C Hamano

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.

Quoted 2 lines

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

Okay, you've convinced me.

Junio C Hamano

2016-06-15 22:46 UTC #9 lore raw

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

Quoted 8 lines · Jay Soffian

Quoted 7 lines · Junio C Hamano
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.

Quoted 7 lines

Quoted 4 lines
[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.

jean-luc malet

2016-06-15 22:46 UTC #10 lore raw

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

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

Quoted 23 lines · Junio C Hamano

Quoted 8 lines · Jay Soffian
Quoted 7 lines · Junio C Hamano
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.
Quoted 7 lines
Quoted 4 lines
[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.

Signature · 9 lines

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

jean-luc malet

2016-06-15 22:48 UTC #11 lore raw

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

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

Quoted 52 lines · 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
Quoted 15 lines · Junio C Hamano
Quoted 13 lines · Jay Soffian
Quoted 11 lines · Junio C Hamano
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
Quoted 8 lines
Quoted 2 lines
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

Signature · 9 lines

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

Jay Soffian

author

2016-06-15 22:48 UTC #12 lore raw

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

Quoted 5 lines · 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

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.

Quoted 67 lines · jean-luc malet

Quoted 41 lines
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
Quoted 15 lines · Junio C Hamano
Quoted 13 lines · Jay Soffian
Quoted 11 lines · Junio C Hamano
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
Quoted 8 lines
Quoted 2 lines
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.
Quoted 11 lines
--
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

jean-luc malet

2016-06-15 22:48 UTC #13 lore raw

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

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

Quoted 81 lines · Jay Soffian

Quoted 5 lines · 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
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.
Quoted 67 lines · jean-luc malet
Quoted 41 lines
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
Quoted 15 lines · Junio C Hamano
Quoted 13 lines · Jay Soffian
Quoted 11 lines · Junio C Hamano
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
Quoted 8 lines
Quoted 2 lines
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.
Quoted 11 lines
--
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

Signature · 9 lines

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

Jay Soffian

author

2016-06-15 22:46 UTC #14 lore raw

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

In reply to Junio C Hamano

Quoted 3 lines · Junio C Hamano

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.