{"thread":{"id":"40844","subject":"[PATCH v2] notes: allow merging from arbitrary references","startedAt":"2015-11-20T18:51:09Z","lastAt":"2015-11-20T18:51:09Z","messageCount":1,"participants":["Jacob Keller"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"273551","messageId":"1448045469-12971-1-git-send-email-jacob.e.keller@intel.com","threadId":"40844","inReplyTo":null,"subject":"[PATCH v2] notes: allow merging from arbitrary references","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2015-11-20T18:51:09Z","receivedAt":"2015-11-20T18:51:09Z","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\nCreate a new expansion function, expand_loose_notes_ref which will first\ncheck whether the ref can be found using get_sha1. If it can't be found\nthen it will fallback to using expand_notes_ref. The content of the\nstrbuf will not be changed if the notes ref can be located using\nget_sha1. Otherwise, it may be updated as done by expand_notes_ref.\n\nSince we now support merging from non-notes refs, remove the test case\nassociated with that behavior. Add a test case for merging from a\nnon-notes ref.\n\nSigned-off-by: Jacob Keller <jacob.keller@gmail.com>\n---\n\nSince we already use get_sha1 inside the notes machinery, there is\nlittle reason to expand get_sha1's object back into a strbuf so we can\njust drop that code which will make expand_loose_notes_ref a little more\nsane. This should be useful if we want to use expand_loose_notes_ref in\nthe future for non-write operations.\n\nNotes:\n    - v2\n    * don't expand notes-ref to the sha1, in order to support get-ref better\n\n builtin/notes.c        |  4 ++--\n notes.c                | 10 ++++++++++\n notes.h                |  7 +++++++\n t/t3308-notes-merge.sh | 22 +++++++++++-----------\n 4 files changed, 30 insertions(+), 13 deletions(-)\n\ndiff --git a/builtin/notes.c b/builtin/notes.c\nindex e0f5d308d206..4a86cc90ee92 100644\n--- a/builtin/notes.c\n+++ b/builtin/notes.c\n@@ -809,7 +809,7 @@ static int merge(int argc, const char **argv, const char *prefix)\n \n \to.local_ref = default_notes_ref();\n \tstrbuf_addstr(&remote_ref, argv[0]);\n-\texpand_notes_ref(&remote_ref);\n+\texpand_loose_notes_ref(&remote_ref);\n \to.remote_ref = remote_ref.buf;\n \n \tt = init_notes_check(\"merge\", NOTES_INIT_WRITABLE);\n@@ -836,7 +836,7 @@ static int merge(int argc, const char **argv, const char *prefix)\n \t}\n \n \tstrbuf_addf(&msg, \"notes: Merged notes from %s into %s\",\n-\t\t    remote_ref.buf, default_notes_ref());\n+\t\t    argv[0], default_notes_ref());\n \tstrbuf_add(&(o.commit_msg), msg.buf + 7, msg.len - 7); /* skip \"notes: \" */\n \n \tresult = notes_merge(&o, t, result_sha1);\ndiff --git a/notes.c b/notes.c\nindex 358e2fdb74eb..c1e503559076 100644\n--- a/notes.c\n+++ b/notes.c\n@@ -1306,3 +1306,13 @@ void expand_notes_ref(struct strbuf *sb)\n \telse\n \t\tstrbuf_insert(sb, 0, \"refs/notes/\", 11);\n }\n+\n+void expand_loose_notes_ref(struct strbuf *sb)\n+{\n+\tunsigned char object[20];\n+\n+\tif (get_sha1(sb->buf, object)) {\n+\t\t/* fallback to expand_notes_ref */\n+\t\texpand_notes_ref(sb);\n+\t}\n+}\ndiff --git a/notes.h b/notes.h\nindex e5d67fd3754a..5345642cfd1c 100644\n--- a/notes.h\n+++ b/notes.h\n@@ -302,4 +302,11 @@ void string_list_add_refs_from_colon_sep(struct string_list *list,\n /* Expand inplace a note ref like \"foo\" or \"notes/foo\" into \"refs/notes/foo\" */\n void expand_notes_ref(struct strbuf *sb);\n \n+/*\n+ * Similar to expand_notes_ref, but will check whether the ref can be located\n+ * via get_sha1 first, and only falls back to expand_notes_ref in the case\n+ * where get_sha1 fails.\n+ */\n+void expand_loose_notes_ref(struct strbuf *sb);\n+\n #endif\ndiff --git a/t/t3308-notes-merge.sh b/t/t3308-notes-merge.sh\nindex 24d82b49bbea..19aed7ec953b 100755\n--- a/t/t3308-notes-merge.sh\n+++ b/t/t3308-notes-merge.sh\n@@ -18,7 +18,9 @@ test_expect_success setup '\n \tgit notes add -m \"Notes on 1st commit\" 1st &&\n \tgit notes add -m \"Notes on 2nd commit\" 2nd &&\n \tgit notes add -m \"Notes on 3rd commit\" 3rd &&\n-\tgit notes add -m \"Notes on 4th commit\" 4th\n+\tgit notes add -m \"Notes on 4th commit\" 4th &&\n+\t# Copy notes to remote-notes\n+\tgit fetch . refs/notes/*:refs/remote-notes/origin/*\n '\n \n commit_sha1=$(git rev-parse 1st^{commit})\n@@ -66,7 +68,9 @@ test_expect_success 'verify initial notes (x)' '\n '\n \n cp expect_notes_x expect_notes_y\n+cp expect_notes_x expect_notes_v\n cp expect_log_x expect_log_y\n+cp expect_log_x expect_log_v\n \n test_expect_success 'fail to merge empty notes ref into empty notes ref (z => y)' '\n \ttest_must_fail git -c \"core.notesRef=refs/notes/y\" notes merge z\n@@ -84,16 +88,12 @@ test_expect_success 'fail to merge into various non-notes refs' '\n \ttest_must_fail git -c \"core.notesRef=refs/notes/foo^{bar\" notes merge x\n '\n \n-test_expect_success 'fail to merge various non-note-trees' '\n-\tgit config core.notesRef refs/notes/y &&\n-\ttest_must_fail git notes merge refs/notes &&\n-\ttest_must_fail git notes merge refs/notes/ &&\n-\ttest_must_fail git notes merge refs/notes/dir &&\n-\ttest_must_fail git notes merge refs/notes/dir/ &&\n-\ttest_must_fail git notes merge refs/heads/master &&\n-\ttest_must_fail git notes merge x: &&\n-\ttest_must_fail git notes merge x:foo &&\n-\ttest_must_fail git notes merge foo^{bar\n+test_expect_success 'merge non-notes ref into empty notes ref (remote-notes/origin/x => v)' '\n+\tgit config core.notesRef refs/notes/v &&\n+\tgit notes merge refs/remote-notes/origin/x &&\n+\tverify_notes v &&\n+\t# refs/remote-notes/origin/x and v should point to the same notes commit\n+\ttest \"$(git rev-parse refs/remote-notes/origin/x)\" = \"$(git rev-parse refs/notes/v)\"\n '\n \n test_expect_success 'merge notes into empty notes ref (x => y)' '\n-- \n2.6.3.505.g5cc1fd1\n"}]}