{"thread":{"id":"39993","subject":"[PATCH v3 0/4] notes: add support for notes.merge option","startedAt":"2015-08-02T10:10:22Z","lastAt":"2015-08-06T22:37:04Z","messageCount":8,"participants":["Jacob Keller","Junio C Hamano","Eric Sunshine"],"isPatch":true,"patchVersion":3,"patchTotal":4},"messages":[{"id":"267279","messageId":"1438510226-1163-1-git-send-email-jacob.e.keller@intel.com","threadId":"39993","inReplyTo":null,"subject":"[PATCH v3 0/4] notes: add support for notes.merge option","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2015-08-02T10:10:22Z","receivedAt":"2015-08-02T10:10:22Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"From: Jacob Keller <jacob.keller@gmail.com>\n\nThis series incorporates the feedback from both Johan and Eric. In\naddition, I included an RFC implementing suggestion from Johan regarding\nper-ref merge strategies.\n\nI split the tests for --merge/--commit/--strategy out into their own\npatch to help review and keep the \"one commit = one change\" logic.\n\nI am not certain the RFC is ready yet, but the first 3 patches (now\nbased on next) should be ready to go.\n\nJacob Keller (4):\n  notes: document cat_sort_uniq rewriteMode\n  notes: add tests for --commit/--abort/--strategy exclusivity\n  notes: add notes.merge option to select default strategy\n  notes: add per-ref configuration of merge strategy\n\n Documentation/config.txt              |  18 +++-\n Documentation/git-notes.txt           |  23 ++++-\n builtin/notes.c                       | 167 ++++++++++++++++++++++++++++++----\n notes-merge.h                         |  16 ++--\n t/t3309-notes-merge-auto-resolve.sh   |  79 ++++++++++++++++\n t/t3310-notes-merge-manual-resolve.sh |  12 +++\n 6 files changed, 288 insertions(+), 27 deletions(-)\n\n-- \n2.5.0.482.gfcd5645\n"},{"id":"267280","messageId":"1438510226-1163-2-git-send-email-jacob.e.keller@intel.com","threadId":"39993","inReplyTo":"1438510226-1163-1-git-send-email-jacob.e.keller@intel.com","subject":"[PATCH v3 1/4] notes: document cat_sort_uniq rewriteMode","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2015-08-02T10:10:23Z","receivedAt":"2015-08-02T10:10:23Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"From: Jacob Keller <jacob.keller@gmail.com>\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 <jacob.keller@gmail.com>\nCc: Johan Herland <johan@herland.net>\nCc: Michael Haggerty <mhagger@alum.mit.edu>\nCc: Eric Sunshine <sunshine@sunshineco.com>\n---\n Documentation/config.txt    | 4 ++--\n Documentation/git-notes.txt | 3 ++-\n 2 files changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 3f1c17814ceb..4ce5c553e50f 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1939,8 +1939,8 @@ notes.rewriteMode::\n \tWhen copying notes during a rewrite (see the\n \t\"notes.rewrite.<command>\" 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..674682b34b83 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-- \n2.5.0.482.gfcd5645\n"},{"id":"267281","messageId":"1438510226-1163-3-git-send-email-jacob.e.keller@intel.com","threadId":"39993","inReplyTo":"1438510226-1163-1-git-send-email-jacob.e.keller@intel.com","subject":"[PATCH v3 2/4] notes: add tests for --commit/--abort/--strategy exclusivity","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2015-08-02T10:10:24Z","receivedAt":"2015-08-02T10:10:24Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"From: Jacob Keller <jacob.keller@gmail.com>\n\nAdd new tests to ensure that --commit, --abort, and --strategy are\nmutually exclusive.\n\nSigned-off-by: Jacob Keller <jacob.keller@gmail.com>\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 <<EOF &&\n-- \n2.5.0.482.gfcd5645\n"},{"id":"267283","messageId":"1438510226-1163-4-git-send-email-jacob.e.keller@intel.com","threadId":"39993","inReplyTo":"1438510226-1163-1-git-send-email-jacob.e.keller@intel.com","subject":"[PATCH v3 3/4] notes: add notes.merge option to select default strategy","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2015-08-02T10:10:25Z","receivedAt":"2015-08-02T10:10:25Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"From: Jacob Keller <jacob.keller@gmail.com>\n\nTeach git-notes about a new configuration option \"notes.merge\" for\nselecting the default notes merge strategy. Document the option in\nconfig.txt and git-notes.txt\n\nAdd tests for use of the configuration option. Include a test to ensure\nthat --strategy correctly overrides the configured setting.\n\nSigned-off-by: Jacob Keller <jacob.keller@gmail.com>\nCc: Johan Herland <johan@herland.net>\nCc: Michael Haggerty <mhagger@alum.mit.edu>\nCc: Eric Sunshine <sunshine@sunshineco.com>\n---\n Documentation/config.txt            |  7 +++++\n Documentation/git-notes.txt         | 14 +++++++++-\n builtin/notes.c                     | 56 ++++++++++++++++++++++++++-----------\n notes-merge.h                       | 16 ++++++-----\n t/t3309-notes-merge-auto-resolve.sh | 29 +++++++++++++++++++\n 5 files changed, 98 insertions(+), 24 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 4ce5c553e50f..0fa88a29dd65 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1911,6 +1911,13 @@ mergetool.writeToTemp::\n mergetool.prompt::\n \tPrompt before each invocation of the merge resolution program.\n \n+notes.merge::\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 674682b34b83..9c4f8536182f 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.merge\" 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.merge accordingly:\n+\n \"ours\" automatically resolves conflicting notes in favor of the local\n version (i.e. the current notes ref).\n \n@@ -310,6 +314,14 @@ core.notesRef::\n \tThis setting can be overridden through the environment and\n \tcommand line.\n \n+notes.merge::\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 63f95fc55439..de0caa00df1b 100644\n--- a/builtin/notes.c\n+++ b/builtin/notes.c\n@@ -92,6 +92,8 @@ static const char * const git_notes_get_ref_usage[] = {\n static const char note_template[] =\n \t\"\\nWrite/edit the notes for the following object:\\n\";\n \n+static enum notes_merge_strategy merge_strategy;\n+\n struct note_data {\n \tint given;\n \tint use_editor;\n@@ -737,6 +739,24 @@ static int merge_commit(struct notes_merge_options *o)\n \treturn ret;\n }\n \n+static int parse_notes_strategy(const char *arg, enum notes_merge_strategy *strategy)\n+{\n+\tif (!strcmp(arg, \"manual\"))\n+\t\t*strategy = NOTES_MERGE_RESOLVE_MANUAL;\n+\telse if (!strcmp(arg, \"ours\"))\n+\t\t*strategy = NOTES_MERGE_RESOLVE_OURS;\n+\telse if (!strcmp(arg, \"theirs\"))\n+\t\t*strategy = NOTES_MERGE_RESOLVE_THEIRS;\n+\telse if (!strcmp(arg, \"union\"))\n+\t\t*strategy = NOTES_MERGE_RESOLVE_UNION;\n+\telse if (!strcmp(arg, \"cat_sort_uniq\"))\n+\t\t*strategy = NOTES_MERGE_RESOLVE_CAT_SORT_UNIQ;\n+\telse\n+\t\treturn -1;\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@@ -795,23 +815,13 @@ 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-\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\t\terror(\"Unknown -s/--strategy: %s\", strategy);\n-\t\t\tusage_with_options(git_notes_merge_usage, options);\n-\t\t}\n+\tif (strategy && parse_notes_strategy(strategy, &merge_strategy)) {\n+\t\terror(\"Unknown -s/--strategy: %s\", strategy);\n+\t\tusage_with_options(git_notes_merge_usage, options);\n \t}\n \n+\to.strategy = merge_strategy;\n+\n \tt = init_notes_check(\"merge\");\n \n \tstrbuf_addf(&msg, \"notes: Merged notes from %s into %s\",\n@@ -945,6 +955,20 @@ static int get_ref(int argc, const char **argv, const char *prefix)\n \treturn 0;\n }\n \n+static int git_notes_config(const char *var, const char *value, void *cb)\n+{\n+\tif (!strcmp(var, \"notes.merge\")) {\n+\t\tif (!value)\n+\t\t\treturn config_error_nonbool(var);\n+\t\tif (parse_notes_strategy(value, &merge_strategy))\n+\t\t\treturn error(\"Unknown notes merge strategy: %s\", value);\n+\t\telse\n+\t\t\treturn 0;\n+\t}\n+\n+\treturn git_default_config(var, value, cb);\n+}\n+\n int cmd_notes(int argc, const char **argv, const char *prefix)\n {\n \tint result;\n@@ -955,7 +979,7 @@ int cmd_notes(int argc, const char **argv, const char *prefix)\n \t\tOPT_END()\n \t};\n \n-\tgit_config(git_default_config, NULL);\n+\tgit_config(git_notes_config, NULL);\n \targc = parse_options(argc, argv, prefix, options, git_notes_usage,\n \t\t\t     PARSE_OPT_STOP_AT_NON_OPTION);\n \ndiff --git a/notes-merge.h b/notes-merge.h\nindex 1d01f6aacf54..bda8c0c8d348 100644\n--- a/notes-merge.h\n+++ b/notes-merge.h\n@@ -8,18 +8,20 @@ enum notes_merge_verbosity {\n \tNOTES_MERGE_VERBOSITY_MAX = 5\n };\n \n+enum notes_merge_strategy {\n+\tNOTES_MERGE_RESOLVE_MANUAL = 0,\n+\tNOTES_MERGE_RESOLVE_OURS,\n+\tNOTES_MERGE_RESOLVE_THEIRS,\n+\tNOTES_MERGE_RESOLVE_UNION,\n+\tNOTES_MERGE_RESOLVE_CAT_SORT_UNIQ\n+};\n+\n struct notes_merge_options {\n \tconst char *local_ref;\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/t/t3309-notes-merge-auto-resolve.sh b/t/t3309-notes-merge-auto-resolve.sh\nindex 461fd84755d7..a773b01b73db 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.merge=\"foo\" notes merge z &&\n+\t# Verify no changes (y)\n+\tverify_notes y y\n+'\n+\n cat <<EOF | sort >expect_notes_ours\n 68b8630d25516028bed862719855b3d6768d7833 $commit_sha15\n 5de7ea7ad4f47e7ff91989fb82234634730f75df $commit_sha14\n@@ -365,6 +372,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\" configuration option => Non-conflicting 3-way merge' '\n+\tgit -c notes.merge=\"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 <<EOF | sort >expect_notes_theirs\n 9b4b2c61f0615412da3c10f98ff85b57c04ec765 $commit_sha15\n 5de7ea7ad4f47e7ff91989fb82234634730f75df $commit_sha14\n@@ -432,6 +450,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 \"theirs\" strategy overriding configuration option \"ours\" => Non-conflicting 3-way merge' '\n+\tgit -c notes.merge=\"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 cat <<EOF | sort >expect_notes_union\n 7c4e546efd0fe939f876beb262ece02797880b54 $commit_sha15\n 5de7ea7ad4f47e7ff91989fb82234634730f75df $commit_sha14\n-- \n2.5.0.482.gfcd5645\n"},{"id":"267282","messageId":"1438510226-1163-5-git-send-email-jacob.e.keller@intel.com","threadId":"39993","inReplyTo":"1438510226-1163-1-git-send-email-jacob.e.keller@intel.com","subject":"[PATCH RFC 4/4] notes: add per-ref configuration of merge strategy","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2015-08-02T10:10:26Z","receivedAt":"2015-08-02T10:10:26Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"From: Jacob Keller <jacob.keller@gmail.com>\n\nTeach git-notes about a new configuration option\n\"notes.<localref>.merge\" which selects the merge strategy for a\nparticular ref. This allows selection of merge strategy different for\neach note reference, in addition to the default strategy for all\nreferences.\n\nSigned-off-by: Jacob Keller <jacob.keller@gmail.com>\nCc: Johan Herland <johan@herland.net>\nCc: Michael Haggerty <mhagger@alum.mit.edu>\nCc: Eric Sunshine <sunshine@sunshineco.com>\n---\n Documentation/config.txt            |   7 +++\n Documentation/git-notes.txt         |   6 ++\n builtin/notes.c                     | 115 +++++++++++++++++++++++++++++++++++-\n t/t3309-notes-merge-auto-resolve.sh |  50 ++++++++++++++++\n 4 files changed, 176 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 0fa88a29dd65..c53ec4538cd3 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1918,6 +1918,13 @@ notes.merge::\n \tSTRATEGIES\" section of linkgit:git-notes[1] for more information\n \ton each strategy.\n \n+notes.<localref>.merge::\n+\tWhich merge strategy to choose if the local ref for a notes merge\n+\tmatches <localref>. Is overridden by notes.merge and takes the same\n+\tvalues. <localref> may be fully qualified or just under refs/notes/.\n+\tSee \"NOTES MERGE STRATEGIES\" section in linkgit:git-notes[1] for more\n+\tinformation on 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 9c4f8536182f..1e001807f9d9 100644\n--- a/Documentation/git-notes.txt\n+++ b/Documentation/git-notes.txt\n@@ -322,6 +322,12 @@ notes.merge::\n +\n This setting can be overridden by passing the `--strategy` option.\n \n+notes.<localref>.merge::\n+\tWhich strategy to choose when merging into <localref>. Uses the same\n+\tvalues as notes.merge. <localref> may be either a fully qualified ref\n+\tor the shortname under \"refs/notes/\". See \"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 de0caa00df1b..b0174d1024dc 100644\n--- a/builtin/notes.c\n+++ b/builtin/notes.c\n@@ -92,6 +92,10 @@ static const char * const git_notes_get_ref_usage[] = {\n static const char note_template[] =\n \t\"\\nWrite/edit the notes for the following object:\\n\";\n \n+static struct note_ref **note_refs;\n+static int note_refs_alloc;\n+static int note_refs_nr;\n+static struct hashmap note_refs_hash;\n static enum notes_merge_strategy merge_strategy;\n \n struct note_data {\n@@ -757,12 +761,87 @@ static int parse_notes_strategy(const char *arg, enum notes_merge_strategy *stra\n \treturn 0;\n }\n \n+struct note_refs_hash_key {\n+\tconst char *str;\n+\tint len;\n+};\n+\n+struct note_ref {\n+\tstruct hashmap_entry ent; /* must be first */\n+\n+\tconst char *name;\n+\tenum notes_merge_strategy merge_strategy;\n+};\n+\n+static int note_refs_hash_cmp(const struct note_ref *a, const struct note_ref *b, const struct note_refs_hash_key *key)\n+{\n+\tif (key)\n+\t\treturn strncmp(a->name, key->str, key->len) || a->name[key->len];\n+\telse\n+\t\treturn strcmp(a->name, b->name);\n+}\n+\n+static inline void init_note_refs_hash(void)\n+{\n+\tif (!note_refs_hash.cmpfn)\n+\t\thashmap_init(&note_refs_hash, (hashmap_cmp_fn)note_refs_hash_cmp, 0);\n+}\n+\n+static struct note_ref *make_note_ref(const char *name, int len)\n+{\n+\tstruct note_ref *ret, *replaced;\n+\tstruct note_refs_hash_key lookup;\n+\tstruct hashmap_entry lookup_entry;\n+\n+\tif (!len)\n+\t\tlen = strlen(name);\n+\n+\tinit_note_refs_hash();\n+\tlookup.str = name;\n+\tlookup.len = len;\n+\thashmap_entry_init(&lookup_entry, memhash(name, len));\n+\n+\tif ((ret = hashmap_get(&note_refs_hash, &lookup_entry, &lookup)) != NULL)\n+\t\treturn ret;\n+\n+\tret = xcalloc(1, sizeof(struct note_ref));\n+\tALLOC_GROW(note_refs, note_refs_nr + 1, note_refs_alloc);\n+\tnote_refs[note_refs_nr++] = ret;\n+\tret->name = xstrndup(name, len);\n+\n+\thashmap_entry_init(ret, lookup_entry.hash);\n+\treplaced = hashmap_put(&note_refs_hash, ret);\n+\tassert(replaced == NULL);  /* no previous entry overwritten */\n+\treturn ret;\n+}\n+\n+static void set_strategy_for_ref(const char *ref)\n+{\n+\tconst char *name = ref;\n+\tstruct note_refs_hash_key lookup;\n+\tstruct hashmap_entry lookup_entry;\n+\tstruct note_ref *note;\n+\n+\tskip_prefix(ref, \"refs/notes/\", &name);\n+\n+\tinit_note_refs_hash();\n+\tlookup.str = name;\n+\tlookup.len = strlen(name);\n+\n+\thashmap_entry_init(&lookup_entry, memhash(name, lookup.len));\n+\n+\tnote = hashmap_get(&note_refs_hash, &lookup_entry, &lookup);\n+\tif (note != NULL)\n+\t\tmerge_strategy = note->merge_strategy;\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 \tunsigned char result_sha1[20];\n \tstruct notes_tree *t;\n \tstruct notes_merge_options o;\n+\tstruct note_ref;\n \tint do_merge = 0, do_commit = 0, do_abort = 0;\n \tint verbosity = 0, result;\n \tconst char *strategy = NULL;\n@@ -815,6 +894,8 @@ 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+\tset_strategy_for_ref(o.local_ref);\n+\n \tif (strategy && parse_notes_strategy(strategy, &merge_strategy)) {\n \t\terror(\"Unknown -s/--strategy: %s\", strategy);\n \t\tusage_with_options(git_notes_merge_usage, options);\n@@ -957,7 +1038,16 @@ static int get_ref(int argc, const char **argv, const char *prefix)\n \n static int git_notes_config(const char *var, const char *value, void *cb)\n {\n-\tif (!strcmp(var, \"notes.merge\")) {\n+\tconst char *ref;\n+\tconst char *subkey;\n+\tstruct note_ref *note;\n+\n+\tif (!starts_with(var, \"notes.\"))\n+\t\treturn git_default_config(var, value, cb);\n+\tref = var + 6;\n+\n+\t/* Handle notes.* variables */\n+\tif (!strcmp(ref, \"merge\")) {\n \t\tif (!value)\n \t\t\treturn config_error_nonbool(var);\n \t\tif (parse_notes_strategy(value, &merge_strategy))\n@@ -966,7 +1056,28 @@ static int git_notes_config(const char *var, const char *value, void *cb)\n \t\t\treturn 0;\n \t}\n \n-\treturn git_default_config(var, value, cb);\n+\tif (*ref == '/') {\n+\t\twarning(\"Config notes ref cannot begin with '/': %s\", ref);\n+\t\treturn 0;\n+\t}\n+\tsubkey = strrchr(ref, '.');\n+\tif (!subkey)\n+\t\treturn 0;\n+\n+\t/* skip refs/notes prefix if provided */\n+\tskip_prefix(ref, \"refs/notes/\", &ref);\n+\n+\tnote = make_note_ref(ref, subkey - ref);\n+\tif (!strcmp(subkey, \".merge\")) {\n+\t\tif (!value)\n+\t\t\treturn config_error_nonbool(var);\n+\t\tif (parse_notes_strategy(value, &note->merge_strategy))\n+\t\t\treturn error(\"Unknown notes merge strategy: %s\", value);\n+\t\telse\n+\t\t\treturn 0;\n+\t}\n+\n+\treturn 0;\n }\n \n int cmd_notes(int argc, const char **argv, const char *prefix)\ndiff --git a/t/t3309-notes-merge-auto-resolve.sh b/t/t3309-notes-merge-auto-resolve.sh\nindex a773b01b73db..15dd539cd141 100755\n--- a/t/t3309-notes-merge-auto-resolve.sh\n+++ b/t/t3309-notes-merge-auto-resolve.sh\n@@ -383,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\" per-ref configuration option => Non-conflicting 3-way merge' '\n+\tgit -c notes.y.merge=\"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 \"ours\" per-ref configuration option prefixed with \"refs/notes\" => Non-conflicting 3-way merge' '\n+\tgit -c notes.refs/notes/y.merge=\"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 <<EOF | sort >expect_notes_theirs\n 9b4b2c61f0615412da3c10f98ff85b57c04ec765 $commit_sha15\n 5de7ea7ad4f47e7ff91989fb82234634730f75df $commit_sha14\n@@ -534,6 +556,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.refs/notes/y.merge=\"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.refs/notes/y.merge=\"union\" -c notes.merge=\"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.refs/notes/z.merge=\"union\" notes merge z &&\n+\tgit notes merge --abort &&\n+\tverify_notes y y\n+'\n+\n cat <<EOF | sort >expect_notes_union2\n d682107b8bf7a7aea1e537a8d5cb6a12b60135f1 $commit_sha15\n 5de7ea7ad4f47e7ff91989fb82234634730f75df $commit_sha14\n-- \n2.5.0.482.gfcd5645\n"},{"id":"267539","messageId":"xmqqegjh31js.fsf@gitster.dls.corp.google.com","threadId":"39993","inReplyTo":"1438510226-1163-4-git-send-email-jacob.e.keller@intel.com","subject":"Re: [PATCH v3 3/4] notes: add notes.merge option to select default strategy","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-08-05T20:47:35Z","receivedAt":"2015-08-05T20:47:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jacob Keller <jacob.e.keller@intel.com> writes:\n\n> From: Jacob Keller <jacob.keller@gmail.com>\n>\n> Teach git-notes about a new configuration option \"notes.merge\" for\n> selecting the default notes merge strategy. Document the option in\n> config.txt and git-notes.txt\n>\n> Add tests for use of the configuration option. Include a test to ensure\n> that --strategy correctly overrides the configured setting.\n>\n> Signed-off-by: Jacob Keller <jacob.keller@gmail.com>\n> Cc: Johan Herland <johan@herland.net>\n> Cc: Michael Haggerty <mhagger@alum.mit.edu>\n> Cc: Eric Sunshine <sunshine@sunshineco.com>\n> ---\n\nPerhaps I am biased because we do not usually use Cc: around here,\nbut the above looks in a somewhat strange order.  Shouldn't your\nsign-off be at the end?\n\n> +static enum notes_merge_strategy merge_strategy;\n\nOK.\n\n> +static int parse_notes_strategy(const char *arg, enum notes_merge_strategy *strategy)\n> +{\n> +\tif (!strcmp(arg, \"manual\"))\n> +\t\t*strategy = NOTES_MERGE_RESOLVE_MANUAL;\n> +\telse if (!strcmp(arg, \"ours\"))\n> +\t\t*strategy = NOTES_MERGE_RESOLVE_OURS;\n> +\telse if (!strcmp(arg, \"theirs\"))\n> +\t\t*strategy = NOTES_MERGE_RESOLVE_THEIRS;\n> +\telse if (!strcmp(arg, \"union\"))\n> +\t\t*strategy = NOTES_MERGE_RESOLVE_UNION;\n> +\telse if (!strcmp(arg, \"cat_sort_uniq\"))\n> +\t\t*strategy = NOTES_MERGE_RESOLVE_CAT_SORT_UNIQ;\n> +\telse\n> +\t\treturn -1;\n> +\n> +\treturn 0;\n> +}\n\nOK.\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> @@ -795,23 +815,13 @@ 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> -\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\t\terror(\"Unknown -s/--strategy: %s\", strategy);\n> -\t\t\tusage_with_options(git_notes_merge_usage, options);\n> -\t\t}\n> +\tif (strategy && parse_notes_strategy(strategy, &merge_strategy)) {\n> +\t\terror(\"Unknown -s/--strategy: %s\", strategy);\n> +\t\tusage_with_options(git_notes_merge_usage, options);\n>  \t}\n>  \n> +\to.strategy = merge_strategy;\n> +\n\nThis may be a minor taste thing, but initializing \"o.strategy\" with\nmerge_strategy at the same place as \"o.remote_ref\" is initialized,\nand then passing &o.merge_strategy to parse_notes_strategy(), may be\neasier to follow.\n\nRenaming the global \"merge_strategy\" to \"configured_merge_strategy\"\nmight make it even easier to follow.  If anybody uses the variable\ninstead of o.strategy after this point, it would be immediately\nobvious that it is either a bug or it is deliberately using the\nvalue from the configuration file, not command line.\n\nThanks.\n"},{"id":"267542","messageId":"xmqqa8u530i4.fsf@gitster.dls.corp.google.com","threadId":"39993","inReplyTo":"1438510226-1163-5-git-send-email-jacob.e.keller@intel.com","subject":"Re: [PATCH RFC 4/4] notes: add per-ref configuration of merge strategy","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-08-05T21:10:11Z","receivedAt":"2015-08-05T21:10:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jacob Keller <jacob.e.keller@intel.com> writes:\n\n> +notes.<localref>.merge::\n> +\tWhich merge strategy to choose if the local ref for a notes merge\n> +\tmatches <localref>. Is overridden by notes.merge and takes the same\n> +\tvalues. <localref> may be fully qualified or just under refs/notes/.\n> +\tSee \"NOTES MERGE STRATEGIES\" section in linkgit:git-notes[1] for more\n> +\tinformation on each strategy.\n\nIf I have notes.refs/notes/commit.merge and notes.merge specified,\nI'd expect the former overrides the latter.  The second sentence may\nneed correcting.\n\nI think it is a mistake to allow both notes.refs/notes/commit.merge\nand notes.commit.merge.  You'd end up needing to implement quite a\ncomplicated \"the last one wins\" rule if you did so.\n\n> +notes.<localref>.merge::\n> +\tWhich strategy to choose when merging into <localref>. Uses the same\n> +\tvalues as notes.merge. <localref> may be either a fully qualified ref\n> +\tor the shortname under \"refs/notes/\". See \"NOTES MERGE STRATEGIES\"\n> +\tsection above for more information about each strategy.\n\nAs a reviewer, I can tell that \"Uses the same values\" wants to say\nthat the set of allowed values is the same, but a casual reader is\nbound to read it as \"notes.commit.merge must be set to the same\nvalue as the value set to notes.merge\".\n\n> diff --git a/builtin/notes.c b/builtin/notes.c\n> index de0caa00df1b..b0174d1024dc 100644\n> --- a/builtin/notes.c\n> +++ b/builtin/notes.c\n> @@ -92,6 +92,10 @@ static const char * const git_notes_get_ref_usage[] = {\n>  static const char note_template[] =\n>  \t\"\\nWrite/edit the notes for the following object:\\n\";\n>  \n> +static struct note_ref **note_refs;\n> +static int note_refs_alloc;\n> +static int note_refs_nr;\n> +static struct hashmap note_refs_hash;\n>  static enum notes_merge_strategy merge_strategy;\n>  \n>  struct note_data {\n> @@ -757,12 +761,87 @@ static int parse_notes_strategy(const char *arg, enum notes_merge_strategy *stra\n>  \treturn 0;\n>  }\n>  ...\n> +struct note_refs_hash_key {\n> +\tconst char *str;\n> +\tint len;\n> +};\n> + ...\n> +static void set_strategy_for_ref(const char *ref)\n> +{\n> + ...\n> +}\n\nHmmm, I do not see why you need all the complexity above.\n\nWhen you come to merge(), after calling default_notes_ref(), you\nknow exactly which notes ref you are merging into, no?  Shouldn't\nthen the change required for this feature just the matter of asking\nthe configuration system values for notes.$remote_ref.merge and\nnotes.merge?\n\nIOW,\n\n\tstruct strbuf key = STRBUF_INIT;\n\tchar *value = NULL;\n\n        strbuf_addf(&key, \"notes.%s.merge\", remote_ref.buf);\n\n\tgit_config_get_string(key.buf, &value) ||\n\tgit_config_get_string_const(\"notes.merge\", &value));\n\n\tif (value)\n        \tparse_notes_strategy(value, &configured_merge_strategy);\n\n\t...\n\n        parse_options();\n\tif (strategy)\n        \tparse_notes_strategy(value, &configured_merge_strategy);\n\nor something?\n"},{"id":"267594","messageId":"CAPig+cR==VEdBPoraqQAb_dp+3rd_NoZZidthDRn2Zd9BYJRCw@mail.gmail.com","threadId":"39993","inReplyTo":"1438510226-1163-4-git-send-email-jacob.e.keller@intel.com","subject":"Re: [PATCH v3 3/4] notes: add notes.merge option to select default strategy","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-08-06T22:37:04Z","receivedAt":"2015-08-06T22:37:04Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Aug 2, 2015 at 6:10 AM, Jacob Keller <jacob.e.keller@intel.com> wrote:\n> Teach git-notes about a new configuration option \"notes.merge\" for\n> selecting the default notes merge strategy. Document the option in\n> config.txt and git-notes.txt\n>\n> Add tests for use of the configuration option. Include a test to ensure\n> that --strategy correctly overrides the configured setting.\n>\n> Signed-off-by: Jacob Keller <jacob.keller@gmail.com>\n> ---\n> diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt\n> index 674682b34b83..9c4f8536182f 100644\n> --- a/Documentation/git-notes.txt\n> +++ b/Documentation/git-notes.txt\n> @@ -101,7 +101,7 @@ merge::\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>         When merging notes, resolve notes conflicts using the given\n>         strategy. The following strategies are recognized: \"manual\"\n>         (default), \"ours\", \"theirs\", \"union\" and \"cat_sort_uniq\".\n> +       This option overrides the \"notes.merge\" configuration setting.\n>         See the \"NOTES MERGE STRATEGIES\" section below for more\n>         information on each notes merge strategy.\n\nThese two documentation updates are much easier to digest than the\nnoisy-diff versions of the previous attempt; and the patch overall is\na more pleasant read than v1.\n\n> diff --git a/builtin/notes.c b/builtin/notes.c\n> index 63f95fc55439..de0caa00df1b 100644\n> --- a/builtin/notes.c\n> +++ b/builtin/notes.c\n> @@ -945,6 +955,20 @@ static int get_ref(int argc, const char **argv, const char *prefix)\n>         return 0;\n>  }\n>\n> +static int git_notes_config(const char *var, const char *value, void *cb)\n> +{\n> +       if (!strcmp(var, \"notes.merge\")) {\n> +               if (!value)\n> +                       return config_error_nonbool(var);\n> +               if (parse_notes_strategy(value, &merge_strategy))\n> +                       return error(\"Unknown notes merge strategy: %s\", value);\n> +               else\n> +                       return 0;\n\nA purely subjective stylistic suggestion, which you can freely ignore\nif your preference differs:\n\n    if (!value)\n        return ...;\n    if (parse_notes_strategy(...))\n        return ...;\n    return 0;\n\n> +       }\n> +\n> +       return git_default_config(var, value, cb);\n> +}\n> +\n"}]}