[PATCH v9 0/6] implement notes.mergeStrategy

[PATCH v9 0/6] implement notes.mergeStrategy

[Jacob Keller]

From: Jacob Keller <jacob.keller@gmail.com>

This series implements mergeStrategy configuration options which take
the same value as --strategy. This series does not change the allowed
refs to merge from or to. There is a known limitation that you cannot
merge from refs outside of refs/notes (precluding the use of such refs
as refs/tracking/origin/notes/ and so forth).

- Changes since v8 -
* drop the rewrite and merge option patches, since rewrite names are not
  really equivalent to merge names (ours/theirs is flipped)
* change docs on notes.<name>.mergeStrategy

This series does *not* deal with:
* changes to which refs can be merged, init_notes_check already prevents
  git-notes-merge into refs outside of refs/notes*
* use of rewrite names for merge strategies, including even concatenate

Hopefully a future contributor will have some time to look at making
re-write just use the notes merge instead of doing re-write by hand.
This would also potentially allow for manual merges. This series does
not begin down this road, since we do not want to limit what this future
author is allowed to do with regards to rewrite and merge strategy
names.

I think finally this series is good. It may be worth adding some
test_expect_failures around merging from refs/tracking/origin/notes if
we intend to ever allow notes merges from these sources.

Jacob Keller (6):
  notes: document cat_sort_uniq rewriteMode
  notes: extract enum notes_merge_strategy to notes-utils.h
  note: extract parse_notes_merge_strategy to notes-utils
  notes: add tests for --commit/--abort/--strategy exclusivity
  notes: add notes.mergeStrategy option to select default strategy
  notes: teach git-notes about notes.<name>.mergeStrategy option

 Documentation/config.txt              | 16 ++++++-
 Documentation/git-notes.txt           | 25 +++++++++--
 builtin/notes.c                       | 43 +++++++++++++------
 notes-merge.h                         | 10 ++---
 notes-utils.c                         | 18 ++++++++
 notes-utils.h                         |  9 ++++
 t/t3309-notes-merge-auto-resolve.sh   | 79 +++++++++++++++++++++++++++++++++++
 t/t3310-notes-merge-manual-resolve.sh | 12 ++++++
 8 files changed, 187 insertions(+), 25 deletions(-)

-- 
2.5.0.280.g4aaba03

[PATCH v9 1/6] notes: document cat_sort_uniq rewriteMode

[Jacob Keller]

From: Jacob Keller <jacob.keller@gmail.com>

Teach documentation about the cat_sort_uniq rewriteMode that got added
at the same time as the equivalent merge strategy.

Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
---
 Documentation/config.txt    | 4 ++--
 Documentation/git-notes.txt | 5 +++--
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 75ec02e8e90a..de67ad1fdedf 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1947,8 +1947,8 @@ notes.rewriteMode::
 	When copying notes during a rewrite (see the
 	"notes.rewrite.<command>" option), determines what to do if
 	the target commit already has a note.  Must be one of
-	`overwrite`, `concatenate`, or `ignore`.  Defaults to
-	`concatenate`.
+	`overwrite`, `concatenate`, `cat_sort_uniq`, or `ignore`.
+	Defaults to `concatenate`.
 +
 This setting can be overridden with the `GIT_NOTES_REWRITE_MODE`
 environment variable.
diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
index 851518d531b5..00c84be33ca9 100644
--- a/Documentation/git-notes.txt
+++ b/Documentation/git-notes.txt
@@ -331,7 +331,8 @@ environment variable.
 notes.rewriteMode::
 	When copying notes during a rewrite, what to do if the target
 	commit already has a note.  Must be one of `overwrite`,
-	`concatenate`, and `ignore`.  Defaults to `concatenate`.
+	`concatenate`, `cat_sort_uniq`, or `ignore`.  Defaults to
+	`concatenate`.
 +
 This setting can be overridden with the `GIT_NOTES_REWRITE_MODE`
 environment variable.
@@ -368,7 +369,7 @@ does not match any refs is silently ignored.
 'GIT_NOTES_REWRITE_MODE'::
 	When copying notes during a rewrite, what to do if the target
 	commit already has a note.
-	Must be one of `overwrite`, `concatenate`, and `ignore`.
+	Must be one of `overwrite`, `concatenate`, `cat_sort_uniq`, or `ignore`.
 	This overrides the `core.rewriteMode` setting.
 
 'GIT_NOTES_REWRITE_REF'::
-- 
2.5.0.280.g4aaba03

[PATCH v9 2/6] notes: extract enum notes_merge_strategy to notes-utils.h

[Jacob Keller]

From: Jacob Keller <jacob.keller@gmail.com>

A future patch will extract parsing of the --strategy string into a
helper function in notes.c and will require the enumeration definition.

Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
---
 notes-merge.h | 10 +++-------
 notes-utils.h |  8 ++++++++
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/notes-merge.h b/notes-merge.h
index 1d01f6aacf54..0d890563b5f4 100644
--- a/notes-merge.h
+++ b/notes-merge.h
@@ -1,6 +1,8 @@
 #ifndef NOTES_MERGE_H
 #define NOTES_MERGE_H
 
+#include "notes-utils.h"
+
 #define NOTES_MERGE_WORKTREE "NOTES_MERGE_WORKTREE"
 
 enum notes_merge_verbosity {
@@ -13,13 +15,7 @@ struct notes_merge_options {
 	const char *remote_ref;
 	struct strbuf commit_msg;
 	int verbosity;
-	enum {
-		NOTES_MERGE_RESOLVE_MANUAL = 0,
-		NOTES_MERGE_RESOLVE_OURS,
-		NOTES_MERGE_RESOLVE_THEIRS,
-		NOTES_MERGE_RESOLVE_UNION,
-		NOTES_MERGE_RESOLVE_CAT_SORT_UNIQ
-	} strategy;
+	enum notes_merge_strategy strategy;
 	unsigned has_worktree:1;
 };
 
diff --git a/notes-utils.h b/notes-utils.h
index 890ddb33e13a..db5811e3f718 100644
--- a/notes-utils.h
+++ b/notes-utils.h
@@ -19,6 +19,14 @@ void create_notes_commit(struct notes_tree *t, struct commit_list *parents,
 
 void commit_notes(struct notes_tree *t, const char *msg);
 
+enum notes_merge_strategy {
+		NOTES_MERGE_RESOLVE_MANUAL = 0,
+		NOTES_MERGE_RESOLVE_OURS,
+		NOTES_MERGE_RESOLVE_THEIRS,
+		NOTES_MERGE_RESOLVE_UNION,
+		NOTES_MERGE_RESOLVE_CAT_SORT_UNIQ
+};
+
 struct notes_rewrite_cfg {
 	struct notes_tree **trees;
 	const char *cmd;
-- 
2.5.0.280.g4aaba03

[PATCH v9 3/6] note: extract parse_notes_merge_strategy to notes-utils

[Jacob Keller]

From: Jacob Keller <jacob.keller@gmail.com>

Combining rewrite and notes-merge functionality has been left as an
exercise for a future contributor.

Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
---
 builtin/notes.c | 12 +-----------
 notes-utils.c   | 18 ++++++++++++++++++
 notes-utils.h   |  1 +
 3 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/builtin/notes.c b/builtin/notes.c
index 042348082709..0e7aba98f4d7 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -797,17 +797,7 @@ static int merge(int argc, const char **argv, const char *prefix)
 	o.remote_ref = remote_ref.buf;
 
 	if (strategy) {
-		if (!strcmp(strategy, "manual"))
-			o.strategy = NOTES_MERGE_RESOLVE_MANUAL;
-		else if (!strcmp(strategy, "ours"))
-			o.strategy = NOTES_MERGE_RESOLVE_OURS;
-		else if (!strcmp(strategy, "theirs"))
-			o.strategy = NOTES_MERGE_RESOLVE_THEIRS;
-		else if (!strcmp(strategy, "union"))
-			o.strategy = NOTES_MERGE_RESOLVE_UNION;
-		else if (!strcmp(strategy, "cat_sort_uniq"))
-			o.strategy = NOTES_MERGE_RESOLVE_CAT_SORT_UNIQ;
-		else {
+		if (parse_notes_merge_strategy(strategy, &o.strategy)) {
 			error("Unknown -s/--strategy: %s", strategy);
 			usage_with_options(git_notes_merge_usage, options);
 		}
diff --git a/notes-utils.c b/notes-utils.c
index ccbf0737a34e..299e34bccc58 100644
--- a/notes-utils.c
+++ b/notes-utils.c
@@ -54,6 +54,24 @@ void commit_notes(struct notes_tree *t, const char *msg)
 	strbuf_release(&buf);
 }
 
+int parse_notes_merge_strategy(const char *v, enum notes_merge_strategy *s)
+{
+	if (!strcmp(v, "manual"))
+		*s = NOTES_MERGE_RESOLVE_MANUAL;
+	else if (!strcmp(v, "ours"))
+		*s = NOTES_MERGE_RESOLVE_OURS;
+	else if (!strcmp(v, "theirs"))
+		*s = NOTES_MERGE_RESOLVE_THEIRS;
+	else if (!strcmp(v, "union"))
+		*s = NOTES_MERGE_RESOLVE_UNION;
+	else if (!strcmp(v, "cat_sort_uniq"))
+		*s = NOTES_MERGE_RESOLVE_CAT_SORT_UNIQ;
+	else
+		return -1;
+
+	return 0;
+}
+
 static combine_notes_fn parse_combine_notes_fn(const char *v)
 {
 	if (!strcasecmp(v, "overwrite"))
diff --git a/notes-utils.h b/notes-utils.h
index db5811e3f718..fa538e1d9502 100644
--- a/notes-utils.h
+++ b/notes-utils.h
@@ -37,6 +37,7 @@ struct notes_rewrite_cfg {
 	int mode_from_env;
 };
 
+int parse_notes_merge_strategy(const char *v, enum notes_merge_strategy *s);
 struct notes_rewrite_cfg *init_copy_notes_for_rewrite(const char *cmd);
 int copy_note_for_rewrite(struct notes_rewrite_cfg *c,
 			  const unsigned char *from_obj, const unsigned char *to_obj);
-- 
2.5.0.280.g4aaba03

[PATCH v9 4/6] notes: add tests for --commit/--abort/--strategy exclusivity

[Jacob Keller]

From: Jacob Keller <jacob.keller@gmail.com>

Add new tests to ensure that --commit, --abort, and --strategy are
mutually exclusive.

Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
---
 t/t3310-notes-merge-manual-resolve.sh | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/t/t3310-notes-merge-manual-resolve.sh b/t/t3310-notes-merge-manual-resolve.sh
index 195bb97f859d..d5572121da69 100755
--- a/t/t3310-notes-merge-manual-resolve.sh
+++ b/t/t3310-notes-merge-manual-resolve.sh
@@ -314,6 +314,18 @@ y and z notes on 1st commit
 
 EOF
 
+test_expect_success 'do not allow mixing --commit and --abort' '
+	test_must_fail git notes merge --commit --abort
+'
+
+test_expect_success 'do not allow mixing --commit and --strategy' '
+	test_must_fail git notes merge --commit --strategy theirs
+'
+
+test_expect_success 'do not allow mixing --abort and --strategy' '
+	test_must_fail git notes merge --abort --strategy theirs
+'
+
 test_expect_success 'finalize conflicting merge (z => m)' '
 	# Resolve conflicts and finalize merge
 	cat >.git/NOTES_MERGE_WORKTREE/$commit_sha1 <<EOF &&
-- 
2.5.0.280.g4aaba03

[PATCH v9 5/6] notes: add notes.mergeStrategy option to select default strategy

[Jacob Keller]

From: Jacob Keller <jacob.keller@gmail.com>

Teach git-notes about "notes.mergeStrategy" to select a general strategy
for all notes merges. This enables a user to always get expected merge
strategy such as "cat_sort_uniq" without having to pass the "-s" option
manually.

Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
---
 Documentation/config.txt            |  6 ++++++
 Documentation/git-notes.txt         | 14 ++++++++++++-
 builtin/notes.c                     | 19 ++++++++++++++++--
 t/t3309-notes-merge-auto-resolve.sh | 40 +++++++++++++++++++++++++++++++++++++
 4 files changed, 76 insertions(+), 3 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index de67ad1fdedf..2ff3ed64a4d4 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1919,6 +1919,12 @@ mergetool.writeToTemp::
 mergetool.prompt::
 	Prompt before each invocation of the merge resolution program.
 
+notes.mergeStrategy::
+	Which merge strategy to choose by default when resolving notes
+	conflicts.  Must be one of `manual`, `ours`, `theirs`, `union`, or
+	`cat_sort_uniq`.  Defaults to `manual`.  See "NOTES MERGE STRATEGIES"
+	section of linkgit:git-notes[1] for more information on each strategy.
+
 notes.displayRef::
 	The (fully qualified) refname from which to show notes when
 	showing commit messages.  The value of this variable can be set
diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
index 00c84be33ca9..71453d4a700f 100644
--- a/Documentation/git-notes.txt
+++ b/Documentation/git-notes.txt
@@ -101,7 +101,7 @@ merge::
 	any) into the current notes ref (called "local").
 +
 If conflicts arise and a strategy for automatically resolving
-conflicting notes (see the -s/--strategy option) is not given,
+conflicting notes (see the "NOTES MERGE STRATEGIES" section) is not given,
 the "manual" resolver is used. This resolver checks out the
 conflicting notes in a special worktree (`.git/NOTES_MERGE_WORKTREE`),
 and instructs the user to manually resolve the conflicts there.
@@ -183,6 +183,7 @@ OPTIONS
 	When merging notes, resolve notes conflicts using the given
 	strategy. The following strategies are recognized: "manual"
 	(default), "ours", "theirs", "union" and "cat_sort_uniq".
+	This option overrides the "notes.mergeStrategy" configuration setting.
 	See the "NOTES MERGE STRATEGIES" section below for more
 	information on each notes merge strategy.
 
@@ -247,6 +248,9 @@ When done, the user can either finalize the merge with
 'git notes merge --commit', or abort the merge with
 'git notes merge --abort'.
 
+Users may select an automated merge strategy from among the following using
+either -s/--strategy option or configuring notes.mergeStrategy accordingly:
+
 "ours" automatically resolves conflicting notes in favor of the local
 version (i.e. the current notes ref).
 
@@ -310,6 +314,14 @@ core.notesRef::
 	This setting can be overridden through the environment and
 	command line.
 
+notes.mergeStrategy::
+	Which merge strategy to choose by default when resolving notes
+	conflicts.  Must be one of `manual`, `ours`, `theirs`, `union`, or
+	`cat_sort_uniq`.  Defaults to `manual`.  See "NOTES MERGE STRATEGIES"
+	section above for more information on each strategy.
++
+This setting can be overridden by passing the `--strategy` option.
+
 notes.displayRef::
 	Which ref (or refs, if a glob or specified more than once), in
 	addition to the default set by `core.notesRef` or
diff --git a/builtin/notes.c b/builtin/notes.c
index 0e7aba98f4d7..91f7a6547b0f 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -738,6 +738,19 @@ static int merge_commit(struct notes_merge_options *o)
 	return ret;
 }
 
+static int git_config_get_notes_strategy(const char *key,
+					 enum notes_merge_strategy *strategy)
+{
+	const char *value;
+
+	if (git_config_get_string_const(key, &value))
+		return 1;
+	if (parse_notes_merge_strategy(value, strategy))
+		git_die_config(key, "unknown notes merge strategy %s", value);
+
+	return 0;
+}
+
 static int merge(int argc, const char **argv, const char *prefix)
 {
 	struct strbuf remote_ref = STRBUF_INIT, msg = STRBUF_INIT;
@@ -796,15 +809,17 @@ static int merge(int argc, const char **argv, const char *prefix)
 	expand_notes_ref(&remote_ref);
 	o.remote_ref = remote_ref.buf;
 
+	t = init_notes_check("merge");
+
 	if (strategy) {
 		if (parse_notes_merge_strategy(strategy, &o.strategy)) {
 			error("Unknown -s/--strategy: %s", strategy);
 			usage_with_options(git_notes_merge_usage, options);
 		}
+	} else {
+		git_config_get_notes_strategy("notes.mergeStrategy", &o.strategy);
 	}
 
-	t = init_notes_check("merge");
-
 	strbuf_addf(&msg, "notes: Merged notes from %s into %s",
 		    remote_ref.buf, default_notes_ref());
 	strbuf_add(&(o.commit_msg), msg.buf + 7, msg.len - 7); /* skip "notes: " */
diff --git a/t/t3309-notes-merge-auto-resolve.sh b/t/t3309-notes-merge-auto-resolve.sh
index 461fd84755d7..1cd047f8d75f 100755
--- a/t/t3309-notes-merge-auto-resolve.sh
+++ b/t/t3309-notes-merge-auto-resolve.sh
@@ -298,6 +298,13 @@ test_expect_success 'merge z into y with invalid strategy => Fail/No changes' '
 	verify_notes y y
 '
 
+test_expect_success 'merge z into y with invalid configuration option => Fail/No changes' '
+	git config core.notesRef refs/notes/y &&
+	test_must_fail git -c notes.mergeStrategy="foo" notes merge z &&
+	# Verify no changes (y)
+	verify_notes y y
+'
+
 cat <<EOF | sort >expect_notes_ours
 68b8630d25516028bed862719855b3d6768d7833 $commit_sha15
 5de7ea7ad4f47e7ff91989fb82234634730f75df $commit_sha14
@@ -365,6 +372,17 @@ test_expect_success 'reset to pre-merge state (y)' '
 	verify_notes y y
 '
 
+test_expect_success 'merge z into y with "ours" configuration option => Non-conflicting 3-way merge' '
+	git -c notes.mergeStrategy="ours" notes merge z &&
+	verify_notes y ours
+'
+
+test_expect_success 'reset to pre-merge state (y)' '
+	git update-ref refs/notes/y refs/notes/y^1 &&
+	# Verify pre-merge state
+	verify_notes y y
+'
+
 cat <<EOF | sort >expect_notes_theirs
 9b4b2c61f0615412da3c10f98ff85b57c04ec765 $commit_sha15
 5de7ea7ad4f47e7ff91989fb82234634730f75df $commit_sha14
@@ -432,6 +450,17 @@ test_expect_success 'reset to pre-merge state (y)' '
 	verify_notes y y
 '
 
+test_expect_success 'merge z into y with "theirs" strategy overriding configuration option "ours" => Non-conflicting 3-way merge' '
+	git -c notes.mergeStrategy="ours" notes merge --strategy=theirs z &&
+	verify_notes y theirs
+'
+
+test_expect_success 'reset to pre-merge state (y)' '
+	git update-ref refs/notes/y refs/notes/y^1 &&
+	# Verify pre-merge state
+	verify_notes y y
+'
+
 cat <<EOF | sort >expect_notes_union
 7c4e546efd0fe939f876beb262ece02797880b54 $commit_sha15
 5de7ea7ad4f47e7ff91989fb82234634730f75df $commit_sha14
@@ -644,4 +673,15 @@ test_expect_success 'merge y into z with "cat_sort_uniq" strategy => Non-conflic
 	verify_notes z cat_sort_uniq
 '
 
+test_expect_success 'reset to pre-merge state (z)' '
+	git update-ref refs/notes/z refs/notes/z^1 &&
+	# Verify pre-merge state
+	verify_notes z z
+'
+
+test_expect_success 'merge y into z with "cat_sort_uniq" strategy configuration option => Non-conflicting 3-way merge' '
+	git -c notes.mergeStrategy="cat_sort_uniq" notes merge y &&
+	verify_notes z cat_sort_uniq
+'
+
 test_done
-- 
2.5.0.280.g4aaba03

[PATCH v9 6/6] notes: teach git-notes about notes.<name>.mergeStrategy option

[Jacob Keller]

From: Jacob Keller <jacob.keller@gmail.com>

Teach notes about a new "notes.<name>.mergeStrategy" option for
configuring the notes merge strategy when merging into
refs/notes/<name>. This option allows for the selection of merge
strategy for particular notes refs, rather than all notes ref merges, as
user may not want cat_sort_uniq for all refs, but only some. Note that
the <name> is the local reference we are merging into, not the remote
ref we merged from. The assumption is that users will mostly want to
configure separate local ref merge strategies rather than strategies
depending on which remote ref they merge from.

notes.<name>.mergeStrategy overrides the general behavior as it is more
specific.

Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
---
 Documentation/config.txt            |  6 ++++++
 Documentation/git-notes.txt         |  6 ++++++
 builtin/notes.c                     | 14 ++++++++++++-
 t/t3309-notes-merge-auto-resolve.sh | 39 +++++++++++++++++++++++++++++++++++++
 4 files changed, 64 insertions(+), 1 deletion(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 2ff3ed64a4d4..f60a9101df31 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1925,6 +1925,12 @@ notes.mergeStrategy::
 	`cat_sort_uniq`.  Defaults to `manual`.  See "NOTES MERGE STRATEGIES"
 	section of linkgit:git-notes[1] for more information on each strategy.
 
+notes.<name>.mergeStrategy::
+	Which merge strategy to choose when doing a notes merge into
+	refs/notes/<name>.  This overrides the more general
+	"notes.mergeStrategy".  See the "NOTES MERGE STRATEGIES" section in
+	linkgit:git-notes[1] for more information on the available strategies.
+
 notes.displayRef::
 	The (fully qualified) refname from which to show notes when
 	showing commit messages.  The value of this variable can be set
diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
index 71453d4a700f..a9a916f360ec 100644
--- a/Documentation/git-notes.txt
+++ b/Documentation/git-notes.txt
@@ -322,6 +322,12 @@ notes.mergeStrategy::
 +
 This setting can be overridden by passing the `--strategy` option.
 
+notes.<name>.mergeStrategy::
+	Which merge strategy to choose when doing a notes merge into
+	refs/notes/<name>.  This overrides the more general
+	"notes.mergeStrategy".  See the "NOTES MERGE STRATEGIES" section above
+	for more information on each available strategy.
+
 notes.displayRef::
 	Which ref (or refs, if a glob or specified more than once), in
 	addition to the default set by `core.notesRef` or
diff --git a/builtin/notes.c b/builtin/notes.c
index 91f7a6547b0f..3608c6478528 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -817,7 +817,19 @@ static int merge(int argc, const char **argv, const char *prefix)
 			usage_with_options(git_notes_merge_usage, options);
 		}
 	} else {
-		git_config_get_notes_strategy("notes.mergeStrategy", &o.strategy);
+		struct strbuf merge_key = STRBUF_INIT;
+		const char *short_ref = NULL;
+
+		if (!skip_prefix(o.local_ref, "refs/notes/", &short_ref))
+			die("BUG: local ref %s is outside of refs/notes/",
+			    o.local_ref);
+
+		strbuf_addf(&merge_key, "notes.%s.mergeStrategy", short_ref);
+
+		if (git_config_get_notes_strategy(merge_key.buf, &o.strategy))
+			git_config_get_notes_strategy("notes.mergeStrategy", &o.strategy);
+
+		strbuf_release(&merge_key);
 	}
 
 	strbuf_addf(&msg, "notes: Merged notes from %s into %s",
diff --git a/t/t3309-notes-merge-auto-resolve.sh b/t/t3309-notes-merge-auto-resolve.sh
index 1cd047f8d75f..14c2adf970d7 100755
--- a/t/t3309-notes-merge-auto-resolve.sh
+++ b/t/t3309-notes-merge-auto-resolve.sh
@@ -383,6 +383,17 @@ test_expect_success 'reset to pre-merge state (y)' '
 	verify_notes y y
 '
 
+test_expect_success 'merge z into y with "ours" per-ref configuration option => Non-conflicting 3-way merge' '
+	git -c notes.y.mergeStrategy="ours" notes merge z &&
+	verify_notes y ours
+'
+
+test_expect_success 'reset to pre-merge state (y)' '
+	git update-ref refs/notes/y refs/notes/y^1 &&
+	# Verify pre-merge state
+	verify_notes y y
+'
+
 cat <<EOF | sort >expect_notes_theirs
 9b4b2c61f0615412da3c10f98ff85b57c04ec765 $commit_sha15
 5de7ea7ad4f47e7ff91989fb82234634730f75df $commit_sha14
@@ -534,6 +545,34 @@ test_expect_success 'reset to pre-merge state (y)' '
 	verify_notes y y
 '
 
+test_expect_success 'merge z into y with "union" strategy overriding per-ref configuration => Non-conflicting 3-way merge' '
+	git -c notes.y.mergeStrategy="theirs" notes merge --strategy=union z &&
+	verify_notes y union
+'
+
+test_expect_success 'reset to pre-merge state (y)' '
+	git update-ref refs/notes/y refs/notes/y^1 &&
+	# Verify pre-merge state
+	verify_notes y y
+'
+
+test_expect_success 'merge z into y with "union" per-ref overriding general configuration => Non-conflicting 3-way merge' '
+	git -c notes.y.mergeStrategy="union" -c notes.mergeStrategy="theirs" notes merge z &&
+	verify_notes y union
+'
+
+test_expect_success 'reset to pre-merge state (y)' '
+	git update-ref refs/notes/y refs/notes/y^1 &&
+	# Verify pre-merge state
+	verify_notes y y
+'
+
+test_expect_success 'merge z into y with "manual" per-ref only checks specific ref configuration => Conflicting 3-way merge' '
+	test_must_fail git -c notes.z.mergeStrategy="union" notes merge z &&
+	git notes merge --abort &&
+	verify_notes y y
+'
+
 cat <<EOF | sort >expect_notes_union2
 d682107b8bf7a7aea1e537a8d5cb6a12b60135f1 $commit_sha15
 5de7ea7ad4f47e7ff91989fb82234634730f75df $commit_sha14
-- 
2.5.0.280.g4aaba03

Re: [PATCH v9 0/6] implement notes.mergeStrategy

[Junio C Hamano]

I just said v8 needs reroll in the latest "What's cooking"; after a
quick read, this round looks very reasonable, as you said in your
cover letter.  Will replace what has been queued.

Thanks.

Re: [PATCH v9 3/6] note: extract parse_notes_merge_strategy to notes-utils

[Junio C Hamano]

Jacob Keller <jacob.e.keller@intel.com> writes:

> From: Jacob Keller <jacob.keller@gmail.com>
>
> Combining rewrite and notes-merge functionality has been left as an
> exercise for a future contributor.

This comment is probably unnecessary; we haven't even established if
such a combination is desirable AFAICT in the discussion.

The patch itself looks very straightforward and obviously good.

Thanks.

>
> Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
> ---
>  builtin/notes.c | 12 +-----------
>  notes-utils.c   | 18 ++++++++++++++++++
>  notes-utils.h   |  1 +
>  3 files changed, 20 insertions(+), 11 deletions(-)
>
> diff --git a/builtin/notes.c b/builtin/notes.c
> index 042348082709..0e7aba98f4d7 100644
> --- a/builtin/notes.c
> +++ b/builtin/notes.c
> @@ -797,17 +797,7 @@ static int merge(int argc, const char **argv, const char *prefix)
>  	o.remote_ref = remote_ref.buf;
>  
>  	if (strategy) {
> -		if (!strcmp(strategy, "manual"))
> -			o.strategy = NOTES_MERGE_RESOLVE_MANUAL;
> -		else if (!strcmp(strategy, "ours"))
> -			o.strategy = NOTES_MERGE_RESOLVE_OURS;
> -		else if (!strcmp(strategy, "theirs"))
> -			o.strategy = NOTES_MERGE_RESOLVE_THEIRS;
> -		else if (!strcmp(strategy, "union"))
> -			o.strategy = NOTES_MERGE_RESOLVE_UNION;
> -		else if (!strcmp(strategy, "cat_sort_uniq"))
> -			o.strategy = NOTES_MERGE_RESOLVE_CAT_SORT_UNIQ;
> -		else {
> +		if (parse_notes_merge_strategy(strategy, &o.strategy)) {
>  			error("Unknown -s/--strategy: %s", strategy);
>  			usage_with_options(git_notes_merge_usage, options);
>  		}
> diff --git a/notes-utils.c b/notes-utils.c
> index ccbf0737a34e..299e34bccc58 100644
> --- a/notes-utils.c
> +++ b/notes-utils.c
> @@ -54,6 +54,24 @@ void commit_notes(struct notes_tree *t, const char *msg)
>  	strbuf_release(&buf);
>  }
>  
> +int parse_notes_merge_strategy(const char *v, enum notes_merge_strategy *s)
> +{
> +	if (!strcmp(v, "manual"))
> +		*s = NOTES_MERGE_RESOLVE_MANUAL;
> +	else if (!strcmp(v, "ours"))
> +		*s = NOTES_MERGE_RESOLVE_OURS;
> +	else if (!strcmp(v, "theirs"))
> +		*s = NOTES_MERGE_RESOLVE_THEIRS;
> +	else if (!strcmp(v, "union"))
> +		*s = NOTES_MERGE_RESOLVE_UNION;
> +	else if (!strcmp(v, "cat_sort_uniq"))
> +		*s = NOTES_MERGE_RESOLVE_CAT_SORT_UNIQ;
> +	else
> +		return -1;
> +
> +	return 0;
> +}
> +
>  static combine_notes_fn parse_combine_notes_fn(const char *v)
>  {
>  	if (!strcasecmp(v, "overwrite"))
> diff --git a/notes-utils.h b/notes-utils.h
> index db5811e3f718..fa538e1d9502 100644
> --- a/notes-utils.h
> +++ b/notes-utils.h
> @@ -37,6 +37,7 @@ struct notes_rewrite_cfg {
>  	int mode_from_env;
>  };
>  
> +int parse_notes_merge_strategy(const char *v, enum notes_merge_strategy *s);
>  struct notes_rewrite_cfg *init_copy_notes_for_rewrite(const char *cmd);
>  int copy_note_for_rewrite(struct notes_rewrite_cfg *c,
>  			  const unsigned char *from_obj, const unsigned char *to_obj);

Re: [PATCH v9 3/6] note: extract parse_notes_merge_strategy to notes-utils

[Jacob Keller]

On Mon, Aug 17, 2015 at 3:38 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Jacob Keller <jacob.e.keller@intel.com> writes:
>
>> From: Jacob Keller <jacob.keller@gmail.com>
>>
>> Combining rewrite and notes-merge functionality has been left as an
>> exercise for a future contributor.
>
> This comment is probably unnecessary; we haven't even established if
> such a combination is desirable AFAICT in the discussion.
>
> The patch itself looks very straightforward and obviously good.
>
> Thanks.
>

Feel free to fix that up if you want. Probably easier than me
respinning the whole series.

Regards,
Jake