git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 0/4] notes mergestrategy configuration option
@ 2015-08-14 17:50 Jacob Keller
  2015-08-14 17:50 ` [PATCH v5 1/4] notes: document cat_sort_uniq rewriteMode Jacob Keller
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Jacob Keller @ 2015-08-14 17:50 UTC (permalink / raw)
  To: git
  Cc: Johan Herland, Michael Haggerty, Eric Sunshine, Junio C Hamano,
	Jacob Keller

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

I have tried to cover all of the comments since the last sending of
this series. I am not 100% sure if I got everything, so please feel free
to respond again if I missed something.

This series implements the notes.mergestrategy option, as well as a
notes.<ref>.mergestrategy option which can be used to customize what
strategy is used to merge into a particular ref. I chose to use the
local ref instead of the remote reference, as it is most likely that you
will merge into a given ref with the same strategy rather than always
using the same strategy to merge from a ref.

The notes.<ref>.mergestrategy option uses the short name under
refs/notes/ and does not allow arbitrary references. Today git-notes
does not allow *any* arbitrary ref as a notes ref for either merge or
any other operation, (see init_notes_check!). Due to this, we can force
the config to use the shorthand name instead of the full name.

In the future we may wish to allow non-write operations to reference an
arbitrary ref (such as refs/remote-notes/* or refs/tracking/*/notes/)
but this is not today supported, and in anycase merge is a destructive
write operation so it should always require refs/notes/ references.


Changes since v4
* renamed option to mergestrategy for both per-ref and general option
  for consistency and to keep the notes.merge and notes.<ref>.merge
  available as keys for future notes enhancements

* use shortrefs for notes.<ref>.mergestrategy

Jacob Keller (4):
  notes: document cat_sort_uniq rewriteMode
  notes: add tests for --commit/--abort/--strategy exclusivity
  notes: add notes.mergestrategy option to select default strategy
  notes: teach git-notes about notes.<ref>.mergestrategy option

 Documentation/config.txt              | 18 ++++++++--
 Documentation/git-notes.txt           | 23 ++++++++++--
 builtin/notes.c                       | 46 +++++++++++++++++-------
 notes-merge.h                         | 16 +++++----
 t/t3309-notes-merge-auto-resolve.sh   | 68 +++++++++++++++++++++++++++++++++++
 t/t3310-notes-merge-manual-resolve.sh | 12 +++++++
 6 files changed, 159 insertions(+), 24 deletions(-)

-- 
2.5.0.280.g4aaba03

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v5 1/4] notes: document cat_sort_uniq rewriteMode
  2015-08-14 17:50 [PATCH v5 0/4] notes mergestrategy configuration option Jacob Keller
@ 2015-08-14 17:50 ` Jacob Keller
  2015-08-14 17:50 ` [PATCH v5 2/4] notes: add tests for --commit/--abort/--strategy exclusivity Jacob Keller
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Jacob Keller @ 2015-08-14 17:50 UTC (permalink / raw)
  To: git
  Cc: Johan Herland, Michael Haggerty, Eric Sunshine, Junio C Hamano,
	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 | 3 ++-
 2 files changed, 4 insertions(+), 3 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..674682b34b83 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.
-- 
2.5.0.280.g4aaba03

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v5 2/4] notes: add tests for --commit/--abort/--strategy exclusivity
  2015-08-14 17:50 [PATCH v5 0/4] notes mergestrategy configuration option Jacob Keller
  2015-08-14 17:50 ` [PATCH v5 1/4] notes: document cat_sort_uniq rewriteMode Jacob Keller
@ 2015-08-14 17:50 ` Jacob Keller
  2015-08-14 17:50 ` [PATCH v5 3/4] notes: add notes.mergestrategy option to select default strategy Jacob Keller
  2015-08-14 17:50 ` [PATCH v5 4/4] notes: teach git-notes about notes.<ref>.mergestrategy option Jacob Keller
  3 siblings, 0 replies; 6+ messages in thread
From: Jacob Keller @ 2015-08-14 17:50 UTC (permalink / raw)
  To: git
  Cc: Johan Herland, Michael Haggerty, Eric Sunshine, Junio C Hamano,
	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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v5 3/4] notes: add notes.mergestrategy option to select default strategy
  2015-08-14 17:50 [PATCH v5 0/4] notes mergestrategy configuration option Jacob Keller
  2015-08-14 17:50 ` [PATCH v5 1/4] notes: document cat_sort_uniq rewriteMode Jacob Keller
  2015-08-14 17:50 ` [PATCH v5 2/4] notes: add tests for --commit/--abort/--strategy exclusivity Jacob Keller
@ 2015-08-14 17:50 ` Jacob Keller
  2015-08-14 18:07   ` Eric Sunshine
  2015-08-14 17:50 ` [PATCH v5 4/4] notes: teach git-notes about notes.<ref>.mergestrategy option Jacob Keller
  3 siblings, 1 reply; 6+ messages in thread
From: Jacob Keller @ 2015-08-14 17:50 UTC (permalink / raw)
  To: git
  Cc: Johan Herland, Michael Haggerty, Eric Sunshine, Junio C Hamano,
	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            |  7 +++++++
 Documentation/git-notes.txt         | 14 +++++++++++++-
 builtin/notes.c                     | 37 +++++++++++++++++++++++++------------
 notes-merge.h                       | 16 +++++++++-------
 t/t3309-notes-merge-auto-resolve.sh | 29 +++++++++++++++++++++++++++++
 5 files changed, 83 insertions(+), 20 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index de67ad1fdedf..5e3e03459de7 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1919,6 +1919,13 @@ 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 674682b34b83..89c8829a0543 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 042348082709..97109f8d419c 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -738,6 +738,24 @@ static int merge_commit(struct notes_merge_options *o)
 	return ret;
 }
 
+static int parse_notes_strategy(const char *arg, enum notes_merge_strategy *strategy)
+{
+	if (!strcmp(arg, "manual"))
+		*strategy = NOTES_MERGE_RESOLVE_MANUAL;
+	else if (!strcmp(arg, "ours"))
+		*strategy = NOTES_MERGE_RESOLVE_OURS;
+	else if (!strcmp(arg, "theirs"))
+		*strategy = NOTES_MERGE_RESOLVE_THEIRS;
+	else if (!strcmp(arg, "union"))
+		*strategy = NOTES_MERGE_RESOLVE_UNION;
+	else if (!strcmp(arg, "cat_sort_uniq"))
+		*strategy = NOTES_MERGE_RESOLVE_CAT_SORT_UNIQ;
+	else
+		return -1;
+
+	return 0;
+}
+
 static int merge(int argc, const char **argv, const char *prefix)
 {
 	struct strbuf remote_ref = STRBUF_INIT, msg = STRBUF_INIT;
@@ -746,7 +764,7 @@ static int merge(int argc, const char **argv, const char *prefix)
 	struct notes_merge_options o;
 	int do_merge = 0, do_commit = 0, do_abort = 0;
 	int verbosity = 0, result;
-	const char *strategy = NULL;
+	const char *strategy = NULL, *configured_strategy = NULL;
 	struct option options[] = {
 		OPT_GROUP(N_("General options")),
 		OPT__VERBOSITY(&verbosity),
@@ -796,21 +814,16 @@ static int merge(int argc, const char **argv, const char *prefix)
 	expand_notes_ref(&remote_ref);
 	o.remote_ref = remote_ref.buf;
 
+	git_config_get_string_const("notes.mergestrategy", &configured_strategy);
+
 	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_strategy(strategy, &o.strategy)) {
 			error("Unknown -s/--strategy: %s", strategy);
 			usage_with_options(git_notes_merge_usage, options);
 		}
+	} else if (configured_strategy) {
+		if (parse_notes_strategy(configured_strategy, &o.strategy))
+			die("Unknown notes merge strategy: %s", configured_strategy);
 	}
 
 	t = init_notes_check("merge");
diff --git a/notes-merge.h b/notes-merge.h
index 1d01f6aacf54..bda8c0c8d348 100644
--- a/notes-merge.h
+++ b/notes-merge.h
@@ -8,18 +8,20 @@ enum notes_merge_verbosity {
 	NOTES_MERGE_VERBOSITY_MAX = 5
 };
 
+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_merge_options {
 	const char *local_ref;
 	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/t/t3309-notes-merge-auto-resolve.sh b/t/t3309-notes-merge-auto-resolve.sh
index 461fd84755d7..476b9f5306f1 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
-- 
2.5.0.280.g4aaba03

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v5 4/4] notes: teach git-notes about notes.<ref>.mergestrategy option
  2015-08-14 17:50 [PATCH v5 0/4] notes mergestrategy configuration option Jacob Keller
                   ` (2 preceding siblings ...)
  2015-08-14 17:50 ` [PATCH v5 3/4] notes: add notes.mergestrategy option to select default strategy Jacob Keller
@ 2015-08-14 17:50 ` Jacob Keller
  3 siblings, 0 replies; 6+ messages in thread
From: Jacob Keller @ 2015-08-14 17:50 UTC (permalink / raw)
  To: git
  Cc: Johan Herland, Michael Haggerty, Eric Sunshine, Junio C Hamano,
	Jacob Keller

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

Add new option "notes.<ref>.mergestrategy" option which specifies the merge
strategy for merging into a given notes ref. This option enables
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 <ref> 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. Also,
notes.<ref>.merge overrides the general behavior as it is more specific.

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

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 5e3e03459de7..47478311367e 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1926,6 +1926,13 @@ notes.mergestrategy::
 	STRATEGIES" section of linkgit:git-notes[1] for more information
 	on each strategy.
 
+notes.<localref>.mergestrategy::
+	Which merge strategy to choose if the local ref for a notes merge
+	matches <localref>, overriding "notes.mergestrategy". <localref> must
+	be the short name of a ref under refs/notes/. See "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 89c8829a0543..b99809fc81a6 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.<localref>.mergestrategy::
+	Which strategy to choose when merging into <localref>. The set of
+	allowed values is the same as "notes.mergestrategy". <localref> must be
+	the short name of a ref under refs/notes/. See "NOTES MERGE STRATEGIES"
+	section above for more information about each 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 97109f8d419c..00c9ae402bab 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -758,13 +758,13 @@ static int parse_notes_strategy(const char *arg, enum notes_merge_strategy *stra
 
 static int merge(int argc, const char **argv, const char *prefix)
 {
-	struct strbuf remote_ref = STRBUF_INIT, msg = STRBUF_INIT;
+	struct strbuf remote_ref = STRBUF_INIT, msg = STRBUF_INIT, merge_key = STRBUF_INIT;
 	unsigned char result_sha1[20];
 	struct notes_tree *t;
 	struct notes_merge_options o;
 	int do_merge = 0, do_commit = 0, do_abort = 0;
 	int verbosity = 0, result;
-	const char *strategy = NULL, *configured_strategy = NULL;
+	const char *strategy = NULL, *configured_strategy = NULL, *short_ref = NULL;
 	struct option options[] = {
 		OPT_GROUP(N_("General options")),
 		OPT__VERBOSITY(&verbosity),
@@ -814,7 +814,14 @@ static int merge(int argc, const char **argv, const char *prefix)
 	expand_notes_ref(&remote_ref);
 	o.remote_ref = remote_ref.buf;
 
-	git_config_get_string_const("notes.mergestrategy", &configured_strategy);
+	if (!skip_prefix(o.local_ref, "refs/notes/", &short_ref))
+		die("Refusing to merge notes into %s (outside of refs/notes/)",
+		    o.local_ref);
+
+	strbuf_addf(&merge_key, "notes.%s.mergestrategy", short_ref);
+
+	if (git_config_get_string_const(merge_key.buf, &configured_strategy))
+		git_config_get_string_const("notes.mergestrategy", &configured_strategy);
 
 	if (strategy) {
 		if (parse_notes_strategy(strategy, &o.strategy)) {
diff --git a/t/t3309-notes-merge-auto-resolve.sh b/t/t3309-notes-merge-auto-resolve.sh
index 476b9f5306f1..560e75259798 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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v5 3/4] notes: add notes.mergestrategy option to select default strategy
  2015-08-14 17:50 ` [PATCH v5 3/4] notes: add notes.mergestrategy option to select default strategy Jacob Keller
@ 2015-08-14 18:07   ` Eric Sunshine
  0 siblings, 0 replies; 6+ messages in thread
From: Eric Sunshine @ 2015-08-14 18:07 UTC (permalink / raw)
  To: Jacob Keller
  Cc: Git List, Johan Herland, Michael Haggerty, Junio C Hamano, Jacob Keller

On Fri, Aug 14, 2015 at 1:50 PM, Jacob Keller <jacob.e.keller@intel.com> wrote:
> 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>
> ---
> diff --git a/builtin/notes.c b/builtin/notes.c
> index 042348082709..97109f8d419c 100644
> --- a/builtin/notes.c
> +++ b/builtin/notes.c
> @@ -796,21 +814,16 @@ static int merge(int argc, const char **argv, const char *prefix)
>         expand_notes_ref(&remote_ref);
>         o.remote_ref = remote_ref.buf;
>
> +       git_config_get_string_const("notes.mergestrategy", &configured_strategy);
> +
>         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_strategy(strategy, &o.strategy)) {
>                         error("Unknown -s/--strategy: %s", strategy);

This error message points the user at the source of the problem
("--strategy"). Good.

>                         usage_with_options(git_notes_merge_usage, options);
>                 }
> +       } else if (configured_strategy) {
> +               if (parse_notes_strategy(configured_strategy, &o.strategy))
> +                       die("Unknown notes merge strategy: %s", configured_strategy);

This error message doesn't. Perhaps it should mention the config
variable "notes.mergestrategy" to help the user track down the problem
more quickly?

>         }
>
>         t = init_notes_check("merge");

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2015-08-14 18:07 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-08-14 17:50 [PATCH v5 0/4] notes mergestrategy configuration option Jacob Keller
2015-08-14 17:50 ` [PATCH v5 1/4] notes: document cat_sort_uniq rewriteMode Jacob Keller
2015-08-14 17:50 ` [PATCH v5 2/4] notes: add tests for --commit/--abort/--strategy exclusivity Jacob Keller
2015-08-14 17:50 ` [PATCH v5 3/4] notes: add notes.mergestrategy option to select default strategy Jacob Keller
2015-08-14 18:07   ` Eric Sunshine
2015-08-14 17:50 ` [PATCH v5 4/4] notes: teach git-notes about notes.<ref>.mergestrategy option Jacob Keller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).