{"thread":{"id":"40112","subject":"[PATCH v8 5/8] notes: implement parse_combine_rewrite_fn using parse_notes_merge_strategy","startedAt":"2015-08-17T08:46:23Z","lastAt":"2015-08-17T19:35:28Z","messageCount":14,"participants":["Jacob Keller","Johan Herland","Junio C Hamano"],"isPatch":true,"patchVersion":8,"patchTotal":8},"messages":[{"id":"268169","messageId":"1439801191-3026-1-git-send-email-jacob.e.keller@intel.com","threadId":"40112","inReplyTo":null,"subject":"[PATCH v8 0/8] implement notes.mergeStrategy option","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2015-08-17T08:46:23Z","receivedAt":"2015-08-17T08:46:23Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"From: Jacob Keller \n\n- Changes since v7 -\n* add patches to make rewrite and merge take same options\n* camel case mergeStrategy\n* move init_notes_check above reading git-config in merge()\n This is necessary as it ensures refs are inside refs/notes/*\n\nIt should be noted that git-notes already blocks all operations outside\nof refs/notes, including merging from refs outside of refs/notes. This\nseries leaves the ability to merge from non-refs/notes refs as an\nenhancement for a future author. However, git-notes also (correctly)\nprevents merge (and any other option) into non refs/notes/* refs. You\nmay notice a \"BUG:\" print statement after skip_prefix, this is only as a\nfailsafe and is marked BUG since it should never be true.\n\nHopefully everything looks good now. I chose to add patches which\ncombine rewrite and notes-merge options to take the same parameters.\nThis does cause some weirdness since ours and theirs is swapped much\nlike in a rebase merge conflict.\n\nIf we dont' like this, I am ok with re-writing this to not document the\nsynonyms. However, I think that a good future direction would be to make\nrewrite use the exact same flow as merge, and support the `manual` mode\nas well using the full notes-merge harness.\n\nJacob Keller (8):\n notes: document cat_sort_uniq rewriteMode\n notes: extract enum notes_merge_strategy to notes-utils.h\n note: extract parse_notes_merge_strategy to notes-utils\n notes: allow use of the \"rewrite\" terminology for merge strategies\n notes: implement parse_combine_rewrite_fn using\n parse_notes_merge_strategy\n notes: add tests for --commit/--abort/--strategy exclusivity\n notes: add notes.mergeStrategy option to select default strategy\n notes: teach git-notes about notes..mergeStrategy option\n\n Documentation/config.txt | 22 +++++-\n Documentation/git-notes.txt | 43 +++++++++--\n builtin/notes.c | 43 +++++++----\n notes-merge.h | 10 +--\n notes-utils.c | 50 ++++++++++---\n notes-utils.h | 9 +++\n t/t3301-notes.sh | 55 ++++++++++++++\n t/t3309-notes-merge-auto-resolve.sh | 135 ++++++++++++++++++++++++++++++++++\n t/t3310-notes-merge-manual-resolve.sh | 12 +++\n 9 files changed, 341 insertions(+), 38 deletions(-)\n\n-- \n2.5.0.280.g4aaba03\n"},{"id":"268176","messageId":"1439801191-3026-2-git-send-email-jacob.e.keller@intel.com","threadId":"40112","inReplyTo":"1439801191-3026-1-git-send-email-jacob.e.keller@intel.com","subject":"[PATCH v8 1/8] notes: document cat_sort_uniq rewriteMode","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2015-08-17T08:46:24Z","receivedAt":"2015-08-17T08:46:24Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"From: Jacob Keller \n\nTeach documentation about the cat_sort_uniq rewriteMode that got added\nat the same time as the equivalent merge strategy.\n\nSigned-off-by: Jacob Keller \n---\n Documentation/config.txt | 4 ++--\n Documentation/git-notes.txt | 5 +++--\n 2 files changed, 5 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 75ec02e8e90a..de67ad1fdedf 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1947,8 +1947,8 @@ notes.rewriteMode::\n \tWhen copying notes during a rewrite (see the\n \t\"notes.rewrite.\" option), determines what to do if\n \tthe target commit already has a note. Must be one of\n-\t`overwrite`, `concatenate`, or `ignore`. Defaults to\n-\t`concatenate`.\n+\t`overwrite`, `concatenate`, `cat_sort_uniq`, or `ignore`.\n+\tDefaults to `concatenate`.\n +\n This setting can be overridden with the `GIT_NOTES_REWRITE_MODE`\n environment variable.\ndiff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt\nindex 851518d531b5..00c84be33ca9 100644\n--- a/Documentation/git-notes.txt\n+++ b/Documentation/git-notes.txt\n@@ -331,7 +331,8 @@ environment variable.\n notes.rewriteMode::\n \tWhen copying notes during a rewrite, what to do if the target\n \tcommit already has a note. Must be one of `overwrite`,\n-\t`concatenate`, and `ignore`. Defaults to `concatenate`.\n+\t`concatenate`, `cat_sort_uniq`, or `ignore`. Defaults to\n+\t`concatenate`.\n +\n This setting can be overridden with the `GIT_NOTES_REWRITE_MODE`\n environment variable.\n@@ -368,7 +369,7 @@ does not match any refs is silently ignored.\n 'GIT_NOTES_REWRITE_MODE'::\n \tWhen copying notes during a rewrite, what to do if the target\n \tcommit already has a note.\n-\tMust be one of `overwrite`, `concatenate`, and `ignore`.\n+\tMust be one of `overwrite`, `concatenate`, `cat_sort_uniq`, or `ignore`.\n \tThis overrides the `core.rewriteMode` setting.\n \n 'GIT_NOTES_REWRITE_REF'::\n-- \n2.5.0.280.g4aaba03\n"},{"id":"268173","messageId":"1439801191-3026-3-git-send-email-jacob.e.keller@intel.com","threadId":"40112","inReplyTo":"1439801191-3026-1-git-send-email-jacob.e.keller@intel.com","subject":"[PATCH v8 2/8] notes: extract enum notes_merge_strategy to notes-utils.h","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2015-08-17T08:46:25Z","receivedAt":"2015-08-17T08:46:25Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"From: Jacob Keller \n\nA future patch will extract parsing of the --strategy string into a\nhelper function in notes.c and will require the enumeration definition.\n\nSigned-off-by: Jacob Keller \n---\n notes-merge.h | 10 +++-------\n notes-utils.h | 8 ++++++++\n 2 files changed, 11 insertions(+), 7 deletions(-)\n\ndiff --git a/notes-merge.h b/notes-merge.h\nindex 1d01f6aacf54..0d890563b5f4 100644\n--- a/notes-merge.h\n+++ b/notes-merge.h\n@@ -1,6 +1,8 @@\n #ifndef NOTES_MERGE_H\n #define NOTES_MERGE_H\n \n+#include \"notes-utils.h\"\n+\n #define NOTES_MERGE_WORKTREE \"NOTES_MERGE_WORKTREE\"\n \n enum notes_merge_verbosity {\n@@ -13,13 +15,7 @@ struct notes_merge_options {\n \tconst char *remote_ref;\n \tstruct strbuf commit_msg;\n \tint verbosity;\n-\tenum {\n-\t\tNOTES_MERGE_RESOLVE_MANUAL = 0,\n-\t\tNOTES_MERGE_RESOLVE_OURS,\n-\t\tNOTES_MERGE_RESOLVE_THEIRS,\n-\t\tNOTES_MERGE_RESOLVE_UNION,\n-\t\tNOTES_MERGE_RESOLVE_CAT_SORT_UNIQ\n-\t} strategy;\n+\tenum notes_merge_strategy strategy;\n \tunsigned has_worktree:1;\n };\n \ndiff --git a/notes-utils.h b/notes-utils.h\nindex 890ddb33e13a..db5811e3f718 100644\n--- a/notes-utils.h\n+++ b/notes-utils.h\n@@ -19,6 +19,14 @@ void create_notes_commit(struct notes_tree *t, struct commit_list *parents,\n \n void commit_notes(struct notes_tree *t, const char *msg);\n \n+enum notes_merge_strategy {\n+\t\tNOTES_MERGE_RESOLVE_MANUAL = 0,\n+\t\tNOTES_MERGE_RESOLVE_OURS,\n+\t\tNOTES_MERGE_RESOLVE_THEIRS,\n+\t\tNOTES_MERGE_RESOLVE_UNION,\n+\t\tNOTES_MERGE_RESOLVE_CAT_SORT_UNIQ\n+};\n+\n struct notes_rewrite_cfg {\n \tstruct notes_tree **trees;\n \tconst char *cmd;\n-- \n2.5.0.280.g4aaba03\n"},{"id":"268170","messageId":"1439801191-3026-4-git-send-email-jacob.e.keller@intel.com","threadId":"40112","inReplyTo":"1439801191-3026-1-git-send-email-jacob.e.keller@intel.com","subject":"[PATCH v8 3/8] note: extract parse_notes_merge_strategy to notes-utils","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2015-08-17T08:46:26Z","receivedAt":"2015-08-17T08:46:26Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"From: Jacob Keller \n\nAllow future code to re-use the parsing functionality.\n\nSigned-off-by: Jacob Keller \n---\n builtin/notes.c | 12 +-----------\n notes-utils.c | 18 ++++++++++++++++++\n notes-utils.h | 1 +\n 3 files changed, 20 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin/notes.c b/builtin/notes.c\nindex 042348082709..0e7aba98f4d7 100644\n--- a/builtin/notes.c\n+++ b/builtin/notes.c\n@@ -797,17 +797,7 @@ static int merge(int argc, const char **argv, const char *prefix)\n \to.remote_ref = remote_ref.buf;\n \n \tif (strategy) {\n-\t\tif (!strcmp(strategy, \"manual\"))\n-\t\t\to.strategy = NOTES_MERGE_RESOLVE_MANUAL;\n-\t\telse if (!strcmp(strategy, \"ours\"))\n-\t\t\to.strategy = NOTES_MERGE_RESOLVE_OURS;\n-\t\telse if (!strcmp(strategy, \"theirs\"))\n-\t\t\to.strategy = NOTES_MERGE_RESOLVE_THEIRS;\n-\t\telse if (!strcmp(strategy, \"union\"))\n-\t\t\to.strategy = NOTES_MERGE_RESOLVE_UNION;\n-\t\telse if (!strcmp(strategy, \"cat_sort_uniq\"))\n-\t\t\to.strategy = NOTES_MERGE_RESOLVE_CAT_SORT_UNIQ;\n-\t\telse {\n+\t\tif (parse_notes_merge_strategy(strategy, &o.strategy)) {\n \t\t\terror(\"Unknown -s/--strategy: %s\", strategy);\n \t\t\tusage_with_options(git_notes_merge_usage, options);\n \t\t}\ndiff --git a/notes-utils.c b/notes-utils.c\nindex ccbf0737a34e..299e34bccc58 100644\n--- a/notes-utils.c\n+++ b/notes-utils.c\n@@ -54,6 +54,24 @@ void commit_notes(struct notes_tree *t, const char *msg)\n \tstrbuf_release(&buf);\n }\n \n+int parse_notes_merge_strategy(const char *v, enum notes_merge_strategy *s)\n+{\n+\tif (!strcmp(v, \"manual\"))\n+\t\t*s = NOTES_MERGE_RESOLVE_MANUAL;\n+\telse if (!strcmp(v, \"ours\"))\n+\t\t*s = NOTES_MERGE_RESOLVE_OURS;\n+\telse if (!strcmp(v, \"theirs\"))\n+\t\t*s = NOTES_MERGE_RESOLVE_THEIRS;\n+\telse if (!strcmp(v, \"union\"))\n+\t\t*s = NOTES_MERGE_RESOLVE_UNION;\n+\telse if (!strcmp(v, \"cat_sort_uniq\"))\n+\t\t*s = NOTES_MERGE_RESOLVE_CAT_SORT_UNIQ;\n+\telse\n+\t\treturn -1;\n+\n+\treturn 0;\n+}\n+\n static combine_notes_fn parse_combine_notes_fn(const char *v)\n {\n \tif (!strcasecmp(v, \"overwrite\"))\ndiff --git a/notes-utils.h b/notes-utils.h\nindex db5811e3f718..fa538e1d9502 100644\n--- a/notes-utils.h\n+++ b/notes-utils.h\n@@ -37,6 +37,7 @@ struct notes_rewrite_cfg {\n \tint mode_from_env;\n };\n \n+int parse_notes_merge_strategy(const char *v, enum notes_merge_strategy *s);\n struct notes_rewrite_cfg *init_copy_notes_for_rewrite(const char *cmd);\n int copy_note_for_rewrite(struct notes_rewrite_cfg *c,\n \t\t\t const unsigned char *from_obj, const unsigned char *to_obj);\n-- \n2.5.0.280.g4aaba03\n"},{"id":"268171","messageId":"1439801191-3026-5-git-send-email-jacob.e.keller@intel.com","threadId":"40112","inReplyTo":"1439801191-3026-1-git-send-email-jacob.e.keller@intel.com","subject":"[PATCH v8 4/8] notes: allow use of the \"rewrite\" terminology for merge strategies","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2015-08-17T08:46:27Z","receivedAt":"2015-08-17T08:46:27Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"From: Jacob Keller \n\nnotes-merge.c already re-uses the same functions for the automatic merge\nstrategies used by the rewrite functionality. Teach the -s/--strategy\noption how to interpret the equivalent rewrite terminology for\nconsistency.\n\nAdd tests for the new synonyms.\n\nTeaching rewrite how to understand merge terminology is left for a\nfollowing patch.\n\nSigned-off-by: Jacob Keller \n---\n Documentation/git-notes.txt | 6 +++++\n notes-utils.c | 6 +++++\n t/t3309-notes-merge-auto-resolve.sh | 45 +++++++++++++++++++++++++++++++++++++\n 3 files changed, 57 insertions(+)\n\ndiff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt\nindex 00c84be33ca9..5028e9355de5 100644\n--- a/Documentation/git-notes.txt\n+++ b/Documentation/git-notes.txt\n@@ -250,13 +250,19 @@ When done, the user can either finalize the merge with\n \"ours\" automatically resolves conflicting notes in favor of the local\n version (i.e. the current notes ref).\n \n+\"ignore\" is an alternative spelling for \"ours\".\n+\n \"theirs\" automatically resolves notes conflicts in favor of the remote\n version (i.e. the given notes ref being merged into the current notes\n ref).\n \n+\"overwrite\" is an alternative spelling for \"theirs\".\n+\n \"union\" automatically resolves notes conflicts by concatenating the\n local and remote versions.\n \n+\"concatenate\" is an alternative spelling for \"union\".\n+\n \"cat_sort_uniq\" is similar to \"union\", but in addition to concatenating\n the local and remote versions, this strategy also sorts the resulting\n lines, and removes duplicate lines from the result. This is equivalent\ndiff --git a/notes-utils.c b/notes-utils.c\nindex 299e34bccc58..656b0ea152e2 100644\n--- a/notes-utils.c\n+++ b/notes-utils.c\n@@ -60,10 +60,16 @@ int parse_notes_merge_strategy(const char *v, enum notes_merge_strategy *s)\n \t\t*s = NOTES_MERGE_RESOLVE_MANUAL;\n \telse if (!strcmp(v, \"ours\"))\n \t\t*s = NOTES_MERGE_RESOLVE_OURS;\n+\telse if (!strcmp(v, \"ignore\"))\n+\t\t*s = NOTES_MERGE_RESOLVE_OURS;\n \telse if (!strcmp(v, \"theirs\"))\n \t\t*s = NOTES_MERGE_RESOLVE_THEIRS;\n+\telse if (!strcmp(v, \"overwrite\"))\n+\t\t*s = NOTES_MERGE_RESOLVE_THEIRS;\n \telse if (!strcmp(v, \"union\"))\n \t\t*s = NOTES_MERGE_RESOLVE_UNION;\n+\telse if (!strcmp(v, \"concatenate\"))\n+\t\t*s = NOTES_MERGE_RESOLVE_UNION;\n \telse if (!strcmp(v, \"cat_sort_uniq\"))\n \t\t*s = NOTES_MERGE_RESOLVE_CAT_SORT_UNIQ;\n \telse\ndiff --git a/t/t3309-notes-merge-auto-resolve.sh b/t/t3309-notes-merge-auto-resolve.sh\nindex 461fd84755d7..909900672446 100755\n--- a/t/t3309-notes-merge-auto-resolve.sh\n+++ b/t/t3309-notes-merge-auto-resolve.sh\n@@ -365,6 +365,17 @@ test_expect_success 'reset to pre-merge state (y)' '\n \tverify_notes y y\n '\n \n+test_expect_success 'merge z into y with \"ignore\" strategy => Non-conflicting 3-way merge' '\n+\tgit notes merge --strategy=ignore z &&\n+\tverify_notes y ours\n+'\n+\n+test_expect_success 'reset to pre-merge state (y)' '\n+\tgit update-ref refs/notes/y refs/notes/y^1 &&\n+\t# Verify pre-merge state\n+\tverify_notes y y\n+'\n+\n cat <expect_notes_theirs\n 9b4b2c61f0615412da3c10f98ff85b57c04ec765 $commit_sha15\n 5de7ea7ad4f47e7ff91989fb82234634730f75df $commit_sha14\n@@ -432,6 +443,17 @@ test_expect_success 'reset to pre-merge state (y)' '\n \tverify_notes y y\n '\n \n+test_expect_success 'merge z into y with \"overwrite\" strategy => Non-conflicting 3-way merge' '\n+\tgit notes merge --strategy=overwrite z &&\n+\tverify_notes y theirs\n+'\n+\n+test_expect_success 'reset to pre-merge state (y)' '\n+\tgit update-ref refs/notes/y refs/notes/y^1 &&\n+\t# Verify pre-merge state\n+\tverify_notes y y\n+'\n+\n cat <expect_notes_union\n 7c4e546efd0fe939f876beb262ece02797880b54 $commit_sha15\n 5de7ea7ad4f47e7ff91989fb82234634730f75df $commit_sha14\n@@ -505,6 +527,17 @@ test_expect_success 'reset to pre-merge state (y)' '\n \tverify_notes y y\n '\n \n+test_expect_success 'merge z into y with \"concatenate\" strategy => Non-conflicting 3-way merge' '\n+\tgit notes merge --strategy=concatenate z &&\n+\tverify_notes y union\n+'\n+\n+test_expect_success 'reset to pre-merge state (y)' '\n+\tgit update-ref refs/notes/y refs/notes/y^1 &&\n+\t# Verify pre-merge state\n+\tverify_notes y y\n+'\n+\n cat <expect_notes_union2\n d682107b8bf7a7aea1e537a8d5cb6a12b60135f1 $commit_sha15\n 5de7ea7ad4f47e7ff91989fb82234634730f75df $commit_sha14\n@@ -579,6 +612,18 @@ test_expect_success 'reset to pre-merge state (z)' '\n \tverify_notes z z\n '\n \n+test_expect_success 'merge y into z with \"concatenate\" strategy => Non-conflicting 3-way merge' '\n+\tgit config core.notesRef refs/notes/z &&\n+\tgit notes merge --strategy=concatenate y &&\n+\tverify_notes z union2\n+'\n+\n+test_expect_success 'reset to pre-merge state (z)' '\n+\tgit update-ref refs/notes/z refs/notes/z^1 &&\n+\t# Verify pre-merge state\n+\tverify_notes z z\n+'\n+\n cat <expect_notes_cat_sort_uniq\n 6be90240b5f54594203e25d9f2f64b7567175aee $commit_sha15\n 5de7ea7ad4f47e7ff91989fb82234634730f75df $commit_sha14\n-- \n2.5.0.280.g4aaba03\n"},{"id":"268168","messageId":"1439801191-3026-6-git-send-email-jacob.e.keller@intel.com","threadId":"40112","inReplyTo":"1439801191-3026-1-git-send-email-jacob.e.keller@intel.com","subject":"[PATCH v8 5/8] notes: implement parse_combine_rewrite_fn using parse_notes_merge_strategy","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2015-08-17T08:46:28Z","receivedAt":"2015-08-17T08:46:28Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"From: Jacob Keller \n\nTeach the rewrite combine notes to use the same names as git-notes\nmerge. This will support all current names plus a few new synonyms.\n\nUpdate documentation to point to NOTES MERGE STRATEGIES to explain the\nvarious rewrite options available.\n\nImplementing rewrite functionality completely in terms of merging is\nleft as an exercise for a future contributor.\n\nSigned-off-by: Jacob Keller \n---\n Documentation/config.txt | 8 ++++---\n Documentation/git-notes.txt | 18 ++++++++++-----\n notes-utils.c | 26 +++++++++++++--------\n t/t3301-notes.sh | 55 +++++++++++++++++++++++++++++++++++++++++++++\n 4 files changed, 89 insertions(+), 18 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex de67ad1fdedf..4daa804b1eab 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1946,9 +1946,11 @@ notes.rewrite.::\n notes.rewriteMode::\n \tWhen copying notes during a rewrite (see the\n \t\"notes.rewrite.\" option), determines what to do if\n-\tthe target commit already has a note. Must be one of\n-\t`overwrite`, `concatenate`, `cat_sort_uniq`, or `ignore`.\n-\tDefaults to `concatenate`.\n+\tthe target commit already has a note. With the exception of `manual`,\n+\tany automatic merge strategy may be chosen. Beware that in the\n+\tre-write context the typical notion of `ours` and `theirs` is reversed.\n+\tSee the \"NOTES MERGE STRATEGIES\" section above for information on each\n+\tof the available strategies.\n +\n This setting can be overridden with the `GIT_NOTES_REWRITE_MODE`\n environment variable.\ndiff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt\nindex 5028e9355de5..678dadfdf3c3 100644\n--- a/Documentation/git-notes.txt\n+++ b/Documentation/git-notes.txt\n@@ -336,9 +336,11 @@ environment variable.\n \n notes.rewriteMode::\n \tWhen copying notes during a rewrite, what to do if the target\n-\tcommit already has a note. Must be one of `overwrite`,\n-\t`concatenate`, `cat_sort_uniq`, or `ignore`. Defaults to\n-\t`concatenate`.\n+\tcommit already has a note. With the exception of `manual`, any\n+\tautomatic merge strategy may be chosen. Beware that in the re-write\n+\tcontext the typical notion of `ours` and `theirs` is reversed . See\n+\tthe \"NOTES MERGE STRATEGIES\" section above for information on each of\n+\tthe available strategies.\n +\n This setting can be overridden with the `GIT_NOTES_REWRITE_MODE`\n environment variable.\n@@ -374,9 +376,13 @@ does not match any refs is silently ignored.\n \n 'GIT_NOTES_REWRITE_MODE'::\n \tWhen copying notes during a rewrite, what to do if the target\n-\tcommit already has a note.\n-\tMust be one of `overwrite`, `concatenate`, `cat_sort_uniq`, or `ignore`.\n-\tThis overrides the `core.rewriteMode` setting.\n+\tcommit already has a note. With the exception of `manual`, any\n+\tautomatic merge strategy may be chosen. Beware that in the re-write\n+\tcontext the typical notion of `ours` and `theirs` is reversed . See\n+\tthe \"NOTES MERGE STRATEGIES\" section above for information on each of\n+\tthe available strategies.\n++\n+Overrides the notes.rewriteMode configuration option.\n \n 'GIT_NOTES_REWRITE_REF'::\n \tWhen rewriting commits, which notes to copy from the original\ndiff --git a/notes-utils.c b/notes-utils.c\nindex 656b0ea152e2..8d4cc8909bcf 100644\n--- a/notes-utils.c\n+++ b/notes-utils.c\n@@ -80,16 +80,24 @@ int parse_notes_merge_strategy(const char *v, enum notes_merge_strategy *s)\n \n static combine_notes_fn parse_combine_notes_fn(const char *v)\n {\n-\tif (!strcasecmp(v, \"overwrite\"))\n-\t\treturn combine_notes_overwrite;\n-\telse if (!strcasecmp(v, \"ignore\"))\n-\t\treturn combine_notes_ignore;\n-\telse if (!strcasecmp(v, \"concatenate\"))\n-\t\treturn combine_notes_concatenate;\n-\telse if (!strcasecmp(v, \"cat_sort_uniq\"))\n-\t\treturn combine_notes_cat_sort_uniq;\n-\telse\n+\tenum notes_merge_strategy s;\n+\n+\tif (parse_notes_merge_strategy(v, &s))\n \t\treturn NULL;\n+\n+\tswitch (s) {\n+\tcase NOTES_MERGE_RESOLVE_OURS:\n+\t\treturn combine_notes_ignore;\n+\tcase NOTES_MERGE_RESOLVE_THEIRS:\n+\t\treturn combine_notes_overwrite;\n+\tcase NOTES_MERGE_RESOLVE_UNION:\n+\t\treturn combine_notes_concatenate;\n+\tcase NOTES_MERGE_RESOLVE_CAT_SORT_UNIQ:\n+\t\treturn combine_notes_cat_sort_uniq;\n+\tcase NOTES_MERGE_RESOLVE_MANUAL:\n+\tdefault:\n+\t\treturn NULL;\n+\t}\n }\n \n static int notes_rewrite_config(const char *k, const char *v, void *cb)\ndiff --git a/t/t3301-notes.sh b/t/t3301-notes.sh\nindex 8cffd35fb03d..eb1b5824a422 100755\n--- a/t/t3301-notes.sh\n+++ b/t/t3301-notes.sh\n@@ -997,6 +997,26 @@ test_expect_success 'git notes copy --for-rewrite (overwrite)' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'git notes copy --for-rewrite (theirs)' '\n+\tcat >expect <<-EOF &&\n+\t\tcommit 4acf42e847e7fffbbf89ee365c20ac7caf40de89\n+\t\tAuthor: A U Thor \n+\t\tDate: Thu Apr 7 15:27:13 2005 -0700\n+\n+\t\t${indent}15th\n+\n+\t\tNotes:\n+\t\t${indent}a fresh note\n+\tEOF\n+\tgit notes add -f -m\"a fresh note\" HEAD^ &&\n+\ttest_config notes.rewriteMode theirs &&\n+\ttest_config notes.rewriteRef \"refs/notes/*\" &&\n+\techo $(git rev-parse HEAD^) $(git rev-parse HEAD) |\n+\tgit notes copy --for-rewrite=foo &&\n+\tgit log -1 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'git notes copy --for-rewrite (ignore)' '\n \ttest_config notes.rewriteMode ignore &&\n \ttest_config notes.rewriteRef \"refs/notes/*\" &&\n@@ -1006,6 +1026,15 @@ test_expect_success 'git notes copy --for-rewrite (ignore)' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'git notes copy --for-rewrite (ours)' '\n+\ttest_config notes.rewriteMode ours &&\n+\ttest_config notes.rewriteRef \"refs/notes/*\" &&\n+\techo $(git rev-parse HEAD^) $(git rev-parse HEAD) |\n+\tgit notes copy --for-rewrite=foo &&\n+\tgit log -1 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'git notes copy --for-rewrite (append)' '\n \tcat >expect <<-EOF &&\n \t\tcommit 4acf42e847e7fffbbf89ee365c20ac7caf40de89\n@@ -1028,6 +1057,30 @@ test_expect_success 'git notes copy --for-rewrite (append)' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'git notes copy --for-rewrite (union append)' '\n+\tcat >expect <<-EOF &&\n+\t\tcommit 4acf42e847e7fffbbf89ee365c20ac7caf40de89\n+\t\tAuthor: A U Thor \n+\t\tDate: Thu Apr 7 15:27:13 2005 -0700\n+\n+\t\t${indent}15th\n+\n+\t\tNotes:\n+\t\t${indent}a fresh note\n+\t\t${indent}\n+\t\t${indent}another fresh note\n+\t\t${indent}\n+\t\t${indent}yet again a fresh note\n+\tEOF\n+\tgit notes add -f -m\"yet again a fresh note\" HEAD^ &&\n+\ttest_config notes.rewriteMode union &&\n+\ttest_config notes.rewriteRef \"refs/notes/*\" &&\n+\techo $(git rev-parse HEAD^) $(git rev-parse HEAD) |\n+\tgit notes copy --for-rewrite=foo &&\n+\tgit log -1 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'git notes copy --for-rewrite (append two to one)' '\n \tcat >expect <<-EOF &&\n \t\tcommit 4acf42e847e7fffbbf89ee365c20ac7caf40de89\n@@ -1041,6 +1094,8 @@ test_expect_success 'git notes copy --for-rewrite (append two to one)' '\n \t\t${indent}\n \t\t${indent}another fresh note\n \t\t${indent}\n+\t\t${indent}yet again a fresh note\n+\t\t${indent}\n \t\t${indent}append 1\n \t\t${indent}\n \t\t${indent}append 2\n-- \n2.5.0.280.g4aaba03\n"},{"id":"268175","messageId":"1439801191-3026-7-git-send-email-jacob.e.keller@intel.com","threadId":"40112","inReplyTo":"1439801191-3026-1-git-send-email-jacob.e.keller@intel.com","subject":"[PATCH v8 6/8] notes: add tests for --commit/--abort/--strategy exclusivity","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2015-08-17T08:46:29Z","receivedAt":"2015-08-17T08:46:29Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"From: Jacob Keller \n\nAdd new tests to ensure that --commit, --abort, and --strategy are\nmutually exclusive.\n\nSigned-off-by: Jacob Keller \n---\n t/t3310-notes-merge-manual-resolve.sh | 12 ++++++++++++\n 1 file changed, 12 insertions(+)\n\ndiff --git a/t/t3310-notes-merge-manual-resolve.sh b/t/t3310-notes-merge-manual-resolve.sh\nindex 195bb97f859d..d5572121da69 100755\n--- a/t/t3310-notes-merge-manual-resolve.sh\n+++ b/t/t3310-notes-merge-manual-resolve.sh\n@@ -314,6 +314,18 @@ y and z notes on 1st commit\n \n EOF\n \n+test_expect_success 'do not allow mixing --commit and --abort' '\n+\ttest_must_fail git notes merge --commit --abort\n+'\n+\n+test_expect_success 'do not allow mixing --commit and --strategy' '\n+\ttest_must_fail git notes merge --commit --strategy theirs\n+'\n+\n+test_expect_success 'do not allow mixing --abort and --strategy' '\n+\ttest_must_fail git notes merge --abort --strategy theirs\n+'\n+\n test_expect_success 'finalize conflicting merge (z => m)' '\n \t# Resolve conflicts and finalize merge\n \tcat >.git/NOTES_MERGE_WORKTREE/$commit_sha1 <\n\nTeach git-notes about \"notes.mergeStrategy\" to select a general strategy\nfor all notes merges. This enables a user to always get expected merge\nstrategy such as \"cat_sort_uniq\" without having to pass the \"-s\" option\nmanually.\n\nSigned-off-by: Jacob Keller \n---\n Documentation/config.txt | 7 +++++\n Documentation/git-notes.txt | 14 +++++++++-\n builtin/notes.c | 19 ++++++++++++--\n t/t3309-notes-merge-auto-resolve.sh | 51 +++++++++++++++++++++++++++++++++++++\n 4 files changed, 88 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 4daa804b1eab..56e20446f587 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1919,6 +1919,13 @@ mergetool.writeToTemp::\n mergetool.prompt::\n \tPrompt before each invocation of the merge resolution program.\n \n+notes.mergeStrategy::\n+\tWhich merge strategy to choose by default when resolving notes\n+\tconflicts. Must be one of `manual`, `ours`, `theirs`, `union`,\n+\tor `cat_sort_uniq`. Defaults to `manual`. See \"NOTES MERGE\n+\tSTRATEGIES\" section of linkgit:git-notes[1] for more information\n+\ton each strategy.\n+\n notes.displayRef::\n \tThe (fully qualified) refname from which to show notes when\n \tshowing commit messages. The value of this variable can be set\ndiff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt\nindex 678dadfdf3c3..66b5017939c6 100644\n--- a/Documentation/git-notes.txt\n+++ b/Documentation/git-notes.txt\n@@ -101,7 +101,7 @@ merge::\n \tany) into the current notes ref (called \"local\").\n +\n If conflicts arise and a strategy for automatically resolving\n-conflicting notes (see the -s/--strategy option) is not given,\n+conflicting notes (see the \"NOTES MERGE STRATEGIES\" section) is not given,\n the \"manual\" resolver is used. This resolver checks out the\n conflicting notes in a special worktree (`.git/NOTES_MERGE_WORKTREE`),\n and instructs the user to manually resolve the conflicts there.\n@@ -183,6 +183,7 @@ OPTIONS\n \tWhen merging notes, resolve notes conflicts using the given\n \tstrategy. The following strategies are recognized: \"manual\"\n \t(default), \"ours\", \"theirs\", \"union\" and \"cat_sort_uniq\".\n+\tThis option overrides the \"notes.mergeStrategy\" configuration setting.\n \tSee the \"NOTES MERGE STRATEGIES\" section below for more\n \tinformation on each notes merge strategy.\n \n@@ -247,6 +248,9 @@ When done, the user can either finalize the merge with\n 'git notes merge --commit', or abort the merge with\n 'git notes merge --abort'.\n \n+Users may select an automated merge strategy from among the following using\n+either -s/--strategy option or configuring notes.mergeStrategy accordingly:\n+\n \"ours\" automatically resolves conflicting notes in favor of the local\n version (i.e. the current notes ref).\n \n@@ -316,6 +320,14 @@ core.notesRef::\n \tThis setting can be overridden through the environment and\n \tcommand line.\n \n+notes.mergeStrategy::\n+\tWhich merge strategy to choose by default when resolving notes\n+\tconflicts. Must be one of `manual`, `ours`, `theirs`, `union`,\n+\tor `cat_sort_uniq`. Defaults to `manual`. See \"NOTES MERGE\n+\tSTRATEGIES\" section above for more information on each strategy.\n++\n+This setting can be overridden by passing the `--strategy` option.\n+\n notes.displayRef::\n \tWhich ref (or refs, if a glob or specified more than once), in\n \taddition to the default set by `core.notesRef` or\ndiff --git a/builtin/notes.c b/builtin/notes.c\nindex 0e7aba98f4d7..91f7a6547b0f 100644\n--- a/builtin/notes.c\n+++ b/builtin/notes.c\n@@ -738,6 +738,19 @@ static int merge_commit(struct notes_merge_options *o)\n \treturn ret;\n }\n \n+static int git_config_get_notes_strategy(const char *key,\n+\t\t\t\t\t enum notes_merge_strategy *strategy)\n+{\n+\tconst char *value;\n+\n+\tif (git_config_get_string_const(key, &value))\n+\t\treturn 1;\n+\tif (parse_notes_merge_strategy(value, strategy))\n+\t\tgit_die_config(key, \"unknown notes merge strategy %s\", value);\n+\n+\treturn 0;\n+}\n+\n static int merge(int argc, const char **argv, const char *prefix)\n {\n \tstruct strbuf remote_ref = STRBUF_INIT, msg = STRBUF_INIT;\n@@ -796,15 +809,17 @@ static int merge(int argc, const char **argv, const char *prefix)\n \texpand_notes_ref(&remote_ref);\n \to.remote_ref = remote_ref.buf;\n \n+\tt = init_notes_check(\"merge\");\n+\n \tif (strategy) {\n \t\tif (parse_notes_merge_strategy(strategy, &o.strategy)) {\n \t\t\terror(\"Unknown -s/--strategy: %s\", strategy);\n \t\t\tusage_with_options(git_notes_merge_usage, options);\n \t\t}\n+\t} else {\n+\t\tgit_config_get_notes_strategy(\"notes.mergeStrategy\", &o.strategy);\n \t}\n \n-\tt = init_notes_check(\"merge\");\n-\n \tstrbuf_addf(&msg, \"notes: Merged notes from %s into %s\",\n \t\t remote_ref.buf, default_notes_ref());\n \tstrbuf_add(&(o.commit_msg), msg.buf + 7, msg.len - 7); /* skip \"notes: \" */\ndiff --git a/t/t3309-notes-merge-auto-resolve.sh b/t/t3309-notes-merge-auto-resolve.sh\nindex 909900672446..1bd71d947e60 100755\n--- a/t/t3309-notes-merge-auto-resolve.sh\n+++ b/t/t3309-notes-merge-auto-resolve.sh\n@@ -298,6 +298,13 @@ test_expect_success 'merge z into y with invalid strategy => Fail/No changes' '\n \tverify_notes y y\n '\n \n+test_expect_success 'merge z into y with invalid configuration option => Fail/No changes' '\n+\tgit config core.notesRef refs/notes/y &&\n+\ttest_must_fail git -c notes.mergeStrategy=\"foo\" notes merge z &&\n+\t# Verify no changes (y)\n+\tverify_notes y y\n+'\n+\n cat <expect_notes_ours\n 68b8630d25516028bed862719855b3d6768d7833 $commit_sha15\n 5de7ea7ad4f47e7ff91989fb82234634730f75df $commit_sha14\n@@ -376,6 +383,28 @@ test_expect_success 'reset to pre-merge state (y)' '\n \tverify_notes y y\n '\n \n+test_expect_success 'merge z into y with \"ours\" configuration option => Non-conflicting 3-way merge' '\n+\tgit -c notes.mergeStrategy=\"ours\" notes merge z &&\n+\tverify_notes y ours\n+'\n+\n+test_expect_success 'reset to pre-merge state (y)' '\n+\tgit update-ref refs/notes/y refs/notes/y^1 &&\n+\t# Verify pre-merge state\n+\tverify_notes y y\n+'\n+\n+test_expect_success 'merge z into y with \"ignore\" configuration option => Non-conflicting 3-way merge' '\n+\tgit -c notes.mergeStrategy=\"ignore\" notes merge z &&\n+\tverify_notes y ours\n+'\n+\n+test_expect_success 'reset to pre-merge state (y)' '\n+\tgit update-ref refs/notes/y refs/notes/y^1 &&\n+\t# Verify pre-merge state\n+\tverify_notes y y\n+'\n+\n cat <expect_notes_theirs\n 9b4b2c61f0615412da3c10f98ff85b57c04ec765 $commit_sha15\n 5de7ea7ad4f47e7ff91989fb82234634730f75df $commit_sha14\n@@ -454,6 +483,28 @@ test_expect_success 'reset to pre-merge state (y)' '\n \tverify_notes y y\n '\n \n+test_expect_success 'merge z into y with \"theirs\" strategy overriding configuration option \"ours\" => Non-conflicting 3-way merge' '\n+\tgit -c notes.mergeStrategy=\"ours\" notes merge --strategy=theirs z &&\n+\tverify_notes y theirs\n+'\n+\n+test_expect_success 'reset to pre-merge state (y)' '\n+\tgit update-ref refs/notes/y refs/notes/y^1 &&\n+\t# Verify pre-merge state\n+\tverify_notes y y\n+'\n+\n+test_expect_success 'merge z into y with \"overwrite\" strategy overriding configuration option \"ours\" => Non-conflicting 3-way merge' '\n+\tgit -c notes.mergeStrategy=\"ours\" notes merge --strategy=overwrite z &&\n+\tverify_notes y theirs\n+'\n+\n+test_expect_success 'reset to pre-merge state (y)' '\n+\tgit update-ref refs/notes/y refs/notes/y^1 &&\n+\t# Verify pre-merge state\n+\tverify_notes y y\n+'\n+\n cat <expect_notes_union\n 7c4e546efd0fe939f876beb262ece02797880b54 $commit_sha15\n 5de7ea7ad4f47e7ff91989fb82234634730f75df $commit_sha14\n-- \n2.5.0.280.g4aaba03\n"},{"id":"268174","messageId":"1439801191-3026-9-git-send-email-jacob.e.keller@intel.com","threadId":"40112","inReplyTo":"1439801191-3026-1-git-send-email-jacob.e.keller@intel.com","subject":"[PATCH v8 8/8] notes: teach git-notes about notes..mergeStrategy option","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2015-08-17T08:46:31Z","receivedAt":"2015-08-17T08:46:31Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"From: Jacob Keller \n\nAdd new option \"notes..mergeStrategy\" option which specifies the merge\nstrategy for merging into a given notes ref. This option enables\nselection of merge strategy for particular notes refs, rather than all\nnotes ref merges, as user may not want cat_sort_uniq for all refs, but\nonly some. Note that the is the local reference we are merging\ninto, not the remote ref we merged from. The assumption is that users\nwill mostly want to configure separate local ref merge strategies rather\nthan strategies depending on which remote ref they merge from. Also,\nnotes..merge overrides the general behavior as it is more specific.\n\nSigned-off-by: Jacob Keller \n---\n Documentation/config.txt | 7 +++++++\n Documentation/git-notes.txt | 6 ++++++\n builtin/notes.c | 14 ++++++++++++-\n t/t3309-notes-merge-auto-resolve.sh | 39 +++++++++++++++++++++++++++++++++++++\n 4 files changed, 65 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 56e20446f587..a48c111d3ce0 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1926,6 +1926,13 @@ notes.mergeStrategy::\n \tSTRATEGIES\" section of linkgit:git-notes[1] for more information\n \ton each strategy.\n \n+notes..mergeStrategy::\n+\tWhich merge strategy to choose if the local ref for a notes merge\n+\tmatches , overriding \"notes.mergeStrategy\". must\n+\tbe the short name of a ref under refs/notes/. See the \"NOTES MERGE\n+\tSTRATEGIES\" section in linkgit:git-notes[1] for more information on the\n+\tavailable strategies.\n+\n notes.displayRef::\n \tThe (fully qualified) refname from which to show notes when\n \tshowing commit messages. The value of this variable can be set\ndiff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt\nindex 66b5017939c6..0167bc6e347b 100644\n--- a/Documentation/git-notes.txt\n+++ b/Documentation/git-notes.txt\n@@ -328,6 +328,12 @@ notes.mergeStrategy::\n +\n This setting can be overridden by passing the `--strategy` option.\n \n+notes..mergeStrategy::\n+\tWhich strategy to choose when merging into . The set of\n+\tallowed values is the same as \"notes.mergeStrategy\". must be\n+\tthe short name of a ref under refs/notes/. See the \"NOTES MERGE STRATEGIES\"\n+\tsection above for more information about each strategy.\n+\n notes.displayRef::\n \tWhich ref (or refs, if a glob or specified more than once), in\n \taddition to the default set by `core.notesRef` or\ndiff --git a/builtin/notes.c b/builtin/notes.c\nindex 91f7a6547b0f..3608c6478528 100644\n--- a/builtin/notes.c\n+++ b/builtin/notes.c\n@@ -817,7 +817,19 @@ static int merge(int argc, const char **argv, const char *prefix)\n \t\t\tusage_with_options(git_notes_merge_usage, options);\n \t\t}\n \t} else {\n-\t\tgit_config_get_notes_strategy(\"notes.mergeStrategy\", &o.strategy);\n+\t\tstruct strbuf merge_key = STRBUF_INIT;\n+\t\tconst char *short_ref = NULL;\n+\n+\t\tif (!skip_prefix(o.local_ref, \"refs/notes/\", &short_ref))\n+\t\t\tdie(\"BUG: local ref %s is outside of refs/notes/\",\n+\t\t\t o.local_ref);\n+\n+\t\tstrbuf_addf(&merge_key, \"notes.%s.mergeStrategy\", short_ref);\n+\n+\t\tif (git_config_get_notes_strategy(merge_key.buf, &o.strategy))\n+\t\t\tgit_config_get_notes_strategy(\"notes.mergeStrategy\", &o.strategy);\n+\n+\t\tstrbuf_release(&merge_key);\n \t}\n \n \tstrbuf_addf(&msg, \"notes: Merged notes from %s into %s\",\ndiff --git a/t/t3309-notes-merge-auto-resolve.sh b/t/t3309-notes-merge-auto-resolve.sh\nindex 1bd71d947e60..44898ffba9fb 100755\n--- a/t/t3309-notes-merge-auto-resolve.sh\n+++ b/t/t3309-notes-merge-auto-resolve.sh\n@@ -405,6 +405,17 @@ test_expect_success 'reset to pre-merge state (y)' '\n \tverify_notes y y\n '\n \n+test_expect_success 'merge z into y with \"ours\" per-ref configuration option => Non-conflicting 3-way merge' '\n+\tgit -c notes.y.mergeStrategy=\"ours\" notes merge z &&\n+\tverify_notes y ours\n+'\n+\n+test_expect_success 'reset to pre-merge state (y)' '\n+\tgit update-ref refs/notes/y refs/notes/y^1 &&\n+\t# Verify pre-merge state\n+\tverify_notes y y\n+'\n+\n cat <expect_notes_theirs\n 9b4b2c61f0615412da3c10f98ff85b57c04ec765 $commit_sha15\n 5de7ea7ad4f47e7ff91989fb82234634730f75df $commit_sha14\n@@ -589,6 +600,34 @@ test_expect_success 'reset to pre-merge state (y)' '\n \tverify_notes y y\n '\n \n+test_expect_success 'merge z into y with \"union\" strategy overriding per-ref configuration => Non-conflicting 3-way merge' '\n+\tgit -c notes.y.mergeStrategy=\"theirs\" notes merge --strategy=union z &&\n+\tverify_notes y union\n+'\n+\n+test_expect_success 'reset to pre-merge state (y)' '\n+\tgit update-ref refs/notes/y refs/notes/y^1 &&\n+\t# Verify pre-merge state\n+\tverify_notes y y\n+'\n+\n+test_expect_success 'merge z into y with \"union\" per-ref overriding general configuration => Non-conflicting 3-way merge' '\n+\tgit -c notes.y.mergeStrategy=\"union\" -c notes.mergeStrategy=\"theirs\" notes merge z &&\n+\tverify_notes y union\n+'\n+\n+test_expect_success 'reset to pre-merge state (y)' '\n+\tgit update-ref refs/notes/y refs/notes/y^1 &&\n+\t# Verify pre-merge state\n+\tverify_notes y y\n+'\n+\n+test_expect_success 'merge z into y with \"manual\" per-ref only checks specific ref configuration => Conflicting 3-way merge' '\n+\ttest_must_fail git -c notes.z.mergeStrategy=\"union\" notes merge z &&\n+\tgit notes merge --abort &&\n+\tverify_notes y y\n+'\n+\n cat <expect_notes_union2\n d682107b8bf7a7aea1e537a8d5cb6a12b60135f1 $commit_sha15\n 5de7ea7ad4f47e7ff91989fb82234634730f75df $commit_sha14\n-- \n2.5.0.280.g4aaba03\n"},{"id":"268181","messageId":"CALKQrgfLzWdRxC5saBXJ_-iKmVDfs+mBfDKKrSU2-tP7eO5+Zg@mail.gmail.com","threadId":"40112","inReplyTo":"1439801191-3026-5-git-send-email-jacob.e.keller@intel.com","subject":"Re: [PATCH v8 4/8] notes: allow use of the \"rewrite\" terminology for merge strategies","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2015-08-17T12:54:18Z","receivedAt":"2015-08-17T12:54:18Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Mon, Aug 17, 2015 at 10:46 AM, Jacob Keller wrote:\n> From: Jacob Keller \n>\n> notes-merge.c already re-uses the same functions for the automatic merge\n> strategies used by the rewrite functionality. Teach the -s/--strategy\n> option how to interpret the equivalent rewrite terminology for\n> consistency.\n\nI'm somewhat negative to this patch. IMHO, adding the rewrite modes as\nmerge strategy synonyms adds no benefit - only potential confusion -\nto the existing merge strategies. Words that have a sensible meaning\nin the context of rewrite, do not necessarily have the same sensible\nmeaning in the context of merge (and vice versa). I'd rather have the\nrewrite code map ignore/overwrite/concatenate to ours/theirs/union,\nwithout teaching the notes-merge code about these words. Or maybe even\ndrop this patch (and the next?) entirely, and let the future author\n(who implements notes rewrite in terms of notes merge) decide how to\ndeal with this? By committing to these synonyms now, you might\nactually be making things harder for the future author: once the\nsynonyms are part of the user-visible and documented interface, they\ncannot easily be removed/changed again.\n\n...Johan\n\n> Add tests for the new synonyms.\n>\n> Teaching rewrite how to understand merge terminology is left for a\n> following patch.\n>\n> Signed-off-by: Jacob Keller \n\n\n\n-- \nJohan Herland, \nwww.herland.net\n"},{"id":"268183","messageId":"CALKQrgeBVfkWOPZYJLnLqJiBPG5XR++G2Mmma2tyc-kwZ5aW_A@mail.gmail.com","threadId":"40112","inReplyTo":"1439801191-3026-9-git-send-email-jacob.e.keller@intel.com","subject":"Re: [PATCH v8 8/8] notes: teach git-notes about notes..mergeStrategy option","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2015-08-17T13:21:09Z","receivedAt":"2015-08-17T13:21:09Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"Allow me to suggest a different wording, somewhat inspired by the\nbranch..* documentation...\n\nOn Mon, Aug 17, 2015 at 10:46 AM, Jacob Keller wrote:\n> From: Jacob Keller \n>\n> Add new option \"notes..mergeStrategy\" option which specifies the merge\n> strategy for merging into a given notes ref.\n\nAdd new \"notes..mergeStrategy\" config, which specifies the merge\nstrategy for notes merges into refs/notes/.\n\n> This option enables\n> selection of merge strategy for particular notes refs, rather than all\n> notes ref merges, as user may not want cat_sort_uniq for all refs, but\n> only some. Note that the is the local reference we are merging\n\ns///\n\n> into, not the remote ref we merged from. The assumption is that users\n> will mostly want to configure separate local ref merge strategies rather\n> than strategies depending on which remote ref they merge from. Also,\n> notes..merge overrides the general behavior as it is more specific.\n\nsame here\n\n>\n> Signed-off-by: Jacob Keller \n> ---\n> Documentation/config.txt | 7 +++++++\n> Documentation/git-notes.txt | 6 ++++++\n> builtin/notes.c | 14 ++++++++++++-\n> t/t3309-notes-merge-auto-resolve.sh | 39 +++++++++++++++++++++++++++++++++++++\n> 4 files changed, 65 insertions(+), 1 deletion(-)\n>\n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> index 56e20446f587..a48c111d3ce0 100644\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -1926,6 +1926,13 @@ notes.mergeStrategy::\n> STRATEGIES\" section of linkgit:git-notes[1] for more information\n> on each strategy.\n>\n> +notes..mergeStrategy::\n> + Which merge strategy to choose if the local ref for a notes merge\n> + matches , overriding \"notes.mergeStrategy\". must\n> + be the short name of a ref under refs/notes/.\n\nnotes..mergeStrategy::\n Which merge strategy to use when doing a notes merge into\n refs/notes/. This overrides the more general\n\"notes.mergeStrategy\".\n\nOtherwise, the series (except possibly #4/#5, see separate discussion)\nlooks good to me.\n\n\n...Johan\n\n\n\n-- \nJohan Herland, \nwww.herland.net\n"},{"id":"268199","messageId":"CA+P7+xok1z+d2RAduPQhs6=a+SrX+jKGjKHsamUEA-ih7qBemQ@mail.gmail.com","threadId":"40112","inReplyTo":"CALKQrgeBVfkWOPZYJLnLqJiBPG5XR++G2Mmma2tyc-kwZ5aW_A@mail.gmail.com","subject":"Re: [PATCH v8 8/8] notes: teach git-notes about notes..mergeStrategy option","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2015-08-17T18:25:47Z","receivedAt":"2015-08-17T18:25:47Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"Hi,\n\nOn Mon, Aug 17, 2015 at 6:21 AM, Johan Herland wrote:\n> Allow me to suggest a different wording, somewhat inspired by the\n> branch..* documentation...\n>\n> On Mon, Aug 17, 2015 at 10:46 AM, Jacob Keller wrote:\n>> From: Jacob Keller \n>>\n>> Add new option \"notes..mergeStrategy\" option which specifies the merge\n>> strategy for merging into a given notes ref.\n>\n> Add new \"notes..mergeStrategy\" config, which specifies the merge\n> strategy for notes merges into refs/notes/.\n>\n>> This option enables\n>> selection of merge strategy for particular notes refs, rather than all\n>> notes ref merges, as user may not want cat_sort_uniq for all refs, but\n>> only some. Note that the is the local reference we are merging\n>\n> s///\n>\n>> into, not the remote ref we merged from. The assumption is that users\n>> will mostly want to configure separate local ref merge strategies rather\n>> than strategies depending on which remote ref they merge from. Also,\n>> notes..merge overrides the general behavior as it is more specific.\n>\n> same here\n\nAgreed, will fix in the next respin.\n\nRegards,\nJake\n"},{"id":"268200","messageId":"CA+P7+xq5qTPVsSq0JOSUm5o9w=szD1c2oFm0e3s_f97E0=Zh+A@mail.gmail.com","threadId":"40112","inReplyTo":"CALKQrgfLzWdRxC5saBXJ_-iKmVDfs+mBfDKKrSU2-tP7eO5+Zg@mail.gmail.com","subject":"Re: [PATCH v8 4/8] notes: allow use of the \"rewrite\" terminology for merge strategies","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2015-08-17T18:28:41Z","receivedAt":"2015-08-17T18:28:41Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"Hi,\n\nOn Mon, Aug 17, 2015 at 5:54 AM, Johan Herland wrote:\n> On Mon, Aug 17, 2015 at 10:46 AM, Jacob Keller wrote:\n>> From: Jacob Keller \n>>\n>> notes-merge.c already re-uses the same functions for the automatic merge\n>> strategies used by the rewrite functionality. Teach the -s/--strategy\n>> option how to interpret the equivalent rewrite terminology for\n>> consistency.\n>\n> I'm somewhat negative to this patch. IMHO, adding the rewrite modes as\n> merge strategy synonyms adds no benefit - only potential confusion -\n> to the existing merge strategies. Words that have a sensible meaning\n> in the context of rewrite, do not necessarily have the same sensible\n> meaning in the context of merge (and vice versa). I'd rather have the\n> rewrite code map ignore/overwrite/concatenate to ours/theirs/union,\n> without teaching the notes-merge code about these words. Or maybe even\n> drop this patch (and the next?) entirely, and let the future author\n> (who implements notes rewrite in terms of notes merge) decide how to\n> deal with this? By committing to these synonyms now, you might\n> actually be making things harder for the future author: once the\n> synonyms are part of the user-visible and documented interface, they\n> cannot easily be removed/changed again.\n>\n> ...Johan\n>\n\nI am ok dropping these, I really only added them after Junio brought\nit up. I think that documenting union/concatenate is fine, but I think\nthat ours/theirs is very confusing. However, I think you're right that\na future author can deal with this when working on rewrite -> merge. I\ndon't think we need to do this now.\n\nI'll drop these patches, but I will leave the\nparse_notes_merge_strategy in it's location here.\n\nWe could document concatenate as a synonym of union but I don't think\nthat is a big deal.\n\nRegards,\nJake\n"},{"id":"268213","messageId":"xmqqoai5d7y7.fsf@gitster.dls.corp.google.com","threadId":"40112","inReplyTo":"CALKQrgfLzWdRxC5saBXJ_-iKmVDfs+mBfDKKrSU2-tP7eO5+Zg@mail.gmail.com","subject":"Re: [PATCH v8 4/8] notes: allow use of the \"rewrite\" terminology for merge strategies","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-08-17T19:35:28Z","receivedAt":"2015-08-17T19:35:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johan Herland writes:\n\n> On Mon, Aug 17, 2015 at 10:46 AM, Jacob Keller wrote:\n>> From: Jacob Keller \n>>\n>> notes-merge.c already re-uses the same functions for the automatic merge\n>> strategies used by the rewrite functionality. Teach the -s/--strategy\n>> option how to interpret the equivalent rewrite terminology for\n>> consistency.\n>\n> I'm somewhat negative to this patch. IMHO, adding the rewrite modes as\n> merge strategy synonyms adds no benefit - only potential confusion -\n> to the existing merge strategies. ...\n> ... By committing to these synonyms now, you might\n> actually be making things harder for the future author: once the\n> synonyms are part of the user-visible and documented interface, they\n> cannot easily be removed/changed again.\n\nOK. Thanks.\n"}]}