Well, it certainly serves *my* immediate needs and addresses the specific use case that I was originally working on. Though I think that what we've come up with would benefit the codebase in general if for no other reason than it lays some ground work for future features and is a bit more generic. I also wouldn't mind going a step further and working on the globbing feature in a separate series.
Here are some corrections to the previous patch:
This patch makes the branch specific options more flexible by laying
the ground work for supporting globs for the branch names.
At this time only the value '*' is supported which will set default
values for all branches. In the future this may be expanded to support
true globbing.
Signed-off-by: Michael Grubb <devel@xxxxxxxxxxxxx>
---
builtin/merge.c | 120 ++++++++++++++++++++++++++++++++++-------------------
t/t7600-merge.sh | 27 ++++++++----
2 files changed, 95 insertions(+), 52 deletions(-)
diff --git a/builtin/merge.c b/builtin/merge.c
index fa56205..9e6ec5f 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -31,20 +31,12 @@
#define DEFAULT_OCTOPUS (1<<1)
#define NO_FAST_FORWARD (1<<2)
#define NO_TRIVIAL (1<<3)
+#define MERGE_OPTIONS_INIT {NULL, NULL, NULL}
-/* This is for branch.<foo>. blocks
- * the vote member holds a value between
- * 0.0 and 1.0 which measures how closely
- * a branch name matches the key member.
- * where branch.*.mergeoptions would be 0.1 and
- * branch.<name>.mergeoptions would be 1.0
- * Also it is called vote because I couldn't come
- * up with a better name.
- */
struct merge_options_cb {
- char *key;
- char *value;
- float vote;
+ char *option;
+ char *spec;
+ char *name;
};
struct strategy {
@@ -518,30 +510,11 @@ cleanup:
strbuf_release(&bname);
}
-static void parse_git_merge_options(const char *k, const char *v,
- void *cb)
+static void free_merge_options_cb(struct merge_options_cb *data)
{
- struct merge_options_cb *merge_options = cb;
- int changed = 0;
-
- /* We only handle mergeoptions for now */
- if (suffixcmp(k, ".mergeoptions"))
- return;
-
- if (!prefixcmp(k, "branch.*") && merge_options->vote <= 0.1 ) {
- merge_options->vote = 0.1;
- changed = 1;
- } else if (branch && !prefixcmp(k, "branch.") &&
- !prefixcmp(k + 7, branch) &&
- merge_options->vote < 1.0) {
- merge_options->vote = 1.0;
- changed = 1;
- }
-
- if (changed) {
- merge_options->key = (char *)k;
- merge_options->value = (char *)v;
- }
+ free(data->option);
+ free(data->spec);
+ free(data->name);
}
static void apply_merge_options(struct merge_options_cb *opts)
@@ -550,26 +523,85 @@ static void apply_merge_options(struct merge_options_cb *opts)
int argc;
char *buf;
- if ( opts == NULL )
+ if (!opts)
return;
- buf = xstrdup(opts->value);
+ buf = xstrdup(opts->option);
argc = split_cmdline(buf, &argv);
if (argc < 0)
- die(_("Bad %s string: %s"),
- opts->key, split_cmdline_strerror(argc));
+ die(_("Bad branch.%s.%s string: %s"),
+ opts->spec, opts->name, split_cmdline_strerror(argc));
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);
+ builtin_merge_usage, 0);
free(buf);
}
+/*
+ * This function returns -1 if the first argument is less specific
+ * than the second, and 1 vice versa. Returns -1 if both arguments
+ * are the same.
+ */
+static int cmp_specificity(const char *a, const char *b)
+{
+ switch((!strcmp(a, "*") ? 2 : 0) |
+ (!strcmp(b, "*") ? 1 : 0)) {
+ case 3:
+ case 0:
+ return -1; /* later one wins if they are the same */
+ case 1:
+ return 1;
+ case 2:
+ return -1;
+ }
+ return 0; /* should never happen */
+}
+
+static void parse_git_merge_options(const char *k, const char *v,
+ void *cb)
+{
+ struct merge_options_cb *merge_options = cb;
+ char *spec, *eon; /* end-of-name */
+
+ k += 7; /* not interested in the leading "branch." */
+ eon = strrchr(k, '.');
+ if (!eon || strcmp(eon, ".mergeoptions"))
+ return;
+
+ /* k through eon is name or glob */
+ spec = xmemdupz(k, eon - k);
+ /*
+ * NEEDSWORK: for now we say "*" matches; we would need
+ * to turn the following into something like:
+ * if (has_wildcard(spec)
+ * ? !glob_matches(spec, branch)
+ * : strcmp(spec, branch)) {
+ * free(spec);
+ * return;
+ * }
+ */
+ if (strcmp(spec, "*") && strcmp(spec, branch)) {
+ free(spec);
+ return;
+ }
+
+ if (!merge_options->option ||
+ cmp_specificity(merge_options->spec, spec) < 0) {
+ free_merge_options_cb(merge_options);
+ merge_options->option = xstrdup(v);
+ merge_options->name = xstrdup(eon + 1);
+ merge_options->spec = spec;
+ return;
+ }
+ free(spec);
+}
+
static int git_merge_config(const char *k, const char *v, void *cb)
{
- if (cb != NULL && branch && !prefixcmp(k, "branch."))
+ if (branch && !prefixcmp(k, "branch."))
parse_git_merge_options(k, v, cb);
if (!strcmp(k, "merge.diffstat") || !strcmp(k, "merge.stat"))
@@ -1034,7 +1066,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
const char *head_arg;
int flag, head_invalid = 0, i;
int best_cnt = -1, merge_was_ok = 0, automerge_was_ok = 0;
- struct merge_options_cb merge_options = {NULL, NULL, 0.0};
+ struct merge_options_cb merge_options = MERGE_OPTIONS_INIT;
struct commit_list *common = NULL;
const char *best_strategy = NULL, *wt_strategy = NULL;
struct commit_list **remotes = &remoteheads;
@@ -1053,8 +1085,10 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
head_invalid = 1;
git_config(git_merge_config, &merge_options);
- if (merge_options.key != NULL && merge_options.value != NULL)
+ if (merge_options.option) {
apply_merge_options(&merge_options);
+ free_merge_options_cb(&merge_options);
+ }
/* for color.ui */
if (diff_use_color_default == -1)
diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
index 5b1f8e1..d7a60b2 100755
--- a/t/t7600-merge.sh
+++ b/t/t7600-merge.sh
@@ -417,37 +417,46 @@ test_debug 'git log --graph --decorate --oneline --all'
test_expect_success 'merge c0 with c1 (default no-ff)' '
git reset --hard c0 &&
- test_might_fail git config --unset branch.master.mergeoptions &&
- git config "branch.*.mergeoptions" "--no-ff" &&
+ cat >.git/config <<-\EOF &&
+ [branch "*"]
+ mergeoptions = --no-ff
+ EOF
test_tick &&
git merge c1 &&
git config --remove-section "branch.*" &&
verify_merge file result.1 &&
verify_parents $c0 $c1
'
-
test_debug 'git log --graph --decorate --oneline --all'
test_expect_success 'combine branch.*.mergeoptions with branch.x.mergeoptions' '
git reset --hard c0 &&
- test_might_fail git config --remove-section branch.master &&
- git config "branch.*.mergeoptions" "--no-ff" &&
- git config branch.master.mergeoptions "--ff" &&
+ cat >.git/config <<-\EOF &&
+ [branch "*"]
+ mergeoptions = --no-ff
+ [branch "master"]
+ mergeoptions = --ff
+ EOF
test_tick &&
git merge c1 &&
git config --remove-section "branch.*" &&
+ git config --remove-section "branch.master" &&
verify_merge file result.1 &&
verify_parents "$c0"
'
test_expect_success 'reverse branch.x.mergeoptions with branch.*.mergeoptions' '
git reset --hard c0 &&
- test_might_fail git config --remove-section branch.master &&
- git config branch.master.mergeoptions "--ff" &&
- git config "branch.*.mergeoptions" "--no-ff" &&
+ cat >.git/config <<-\EOF &&
+ [branch "master"]
+ mergeoptions = --ff
+ [branch "*"]
+ mergeoptions = --no-ff
+ EOF
test_tick &&
git merge c1 &&
git config --remove-section "branch.*" &&
+ git config --remove-section "branch.master" &&
verify_merge file result.1 &&
verify_parents "$c0"
'
--
1.7.5
On 5/3/11 11:58 PM, Jonathan Nieder wrote:
> Michael Grubb wrote:
>
>> I take this to mean that my patch is no longer needed/wanted?
>
> No, as usual it means that Junio was thinking and sent us his
> thoughts.
>
> Do you like it? If so, the way to move it forward would be to add
> tests and write a commit log message. If not, the way to move things
> forward would be to explain what use case it misses and work on a
> better patch that takes care of them.
>
> Hope that helps,
> Jonathan
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html