{"thread":{"id":"40049","subject":"[PATCH v6 1/2] worktrees: add find_shared_symref","startedAt":"2015-08-10T17:52:44Z","lastAt":"2015-08-11T13:56:45Z","messageCount":6,"participants":["David Turner","Eric Sunshine","Johan Herland"],"isPatch":true,"patchVersion":6,"patchTotal":2},"messages":[{"id":"267783","messageId":"1439229165-25773-1-git-send-email-dturner@twopensource.com","threadId":"40049","inReplyTo":null,"subject":"[PATCH v6 1/2] worktrees: add find_shared_symref","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2015-08-10T17:52:44Z","receivedAt":"2015-08-10T17:52:44Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"Add a new function, find_shared_symref, which contains the heart of\ndie_if_checked_out, but works for any symref, not just HEAD. Refactor\ndie_if_checked_out to use the same infrastructure as\nfind_shared_symref.\n\nSoon, we will use find_shared_symref to protect notes merges in\nworktrees.\n\nSigned-off-by: David Turner \n---\n\nPlease disregard v6.\n\nThis version addresses Eric Sunshine's comments on v5. It fixes an error\nmessage and cleans up the code.\n\n---\n branch.c | 46 ++++++++++++++++++++++++++++++++++------------\n branch.h | 8 ++++++++\n 2 files changed, 42 insertions(+), 12 deletions(-)\n\ndiff --git a/branch.c b/branch.c\nindex c85be07..07c8b1e 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -311,21 +311,23 @@ void remove_branch_state(void)\n \tunlink(git_path(\"SQUASH_MSG\"));\n }\n \n-static void check_linked_checkout(const char *branch, const char *id)\n+static char *find_linked_symref(const char *symref, const char *branch,\n+\t\t\t\tconst char *id)\n {\n \tstruct strbuf sb = STRBUF_INIT;\n \tstruct strbuf path = STRBUF_INIT;\n \tstruct strbuf gitdir = STRBUF_INIT;\n+\tchar *existing = NULL;\n \n \t/*\n-\t * $GIT_COMMON_DIR/HEAD is practically outside\n-\t * $GIT_DIR so resolve_ref_unsafe() won't work (it\n-\t * uses git_path). Parse the ref ourselves.\n+\t * $GIT_COMMON_DIR/$symref (e.g. HEAD) is practically outside\n+\t * $GIT_DIR so resolve_ref_unsafe() won't work (it uses\n+\t * git_path). Parse the ref ourselves.\n \t */\n \tif (id)\n-\t\tstrbuf_addf(&path, \"%s/worktrees/%s/HEAD\", get_git_common_dir(), id);\n+\t\tstrbuf_addf(&path, \"%s/worktrees/%s/%s\", get_git_common_dir(), id, symref);\n \telse\n-\t\tstrbuf_addf(&path, \"%s/HEAD\", get_git_common_dir());\n+\t\tstrbuf_addf(&path, \"%s/%s\", get_git_common_dir(), symref);\n \n \tif (!strbuf_readlink(&sb, path.buf, 0)) {\n \t\tif (!starts_with(sb.buf, \"refs/\") ||\n@@ -347,33 +349,53 @@ static void check_linked_checkout(const char *branch, const char *id)\n \t\tstrbuf_rtrim(&gitdir);\n \t} else\n \t\tstrbuf_addstr(&gitdir, get_git_common_dir());\n-\tskip_prefix(branch, \"refs/heads/\", &branch);\n \tstrbuf_strip_suffix(&gitdir, \".git\");\n-\tdie(_(\"'%s' is already checked out at '%s'\"), branch, gitdir.buf);\n+\n+\texisting = strbuf_detach(&gitdir, NULL);\n done:\n \tstrbuf_release(&path);\n \tstrbuf_release(&sb);\n \tstrbuf_release(&gitdir);\n+\n+\treturn existing;\n }\n \n-void die_if_checked_out(const char *branch)\n+char *find_shared_symref(const char *symref, const char *target)\n {\n \tstruct strbuf path = STRBUF_INIT;\n \tDIR *dir;\n \tstruct dirent *d;\n+\tchar *existing;\n \n-\tcheck_linked_checkout(branch, NULL);\n+\tif ((existing = find_linked_symref(symref, target, NULL)))\n+\t\treturn existing;\n \n \tstrbuf_addf(&path, \"%s/worktrees\", get_git_common_dir());\n \tdir = opendir(path.buf);\n \tstrbuf_release(&path);\n \tif (!dir)\n-\t\treturn;\n+\t\treturn NULL;\n \n \twhile ((d = readdir(dir)) != NULL) {\n \t\tif (!strcmp(d->d_name, \".\") || !strcmp(d->d_name, \"..\"))\n \t\t\tcontinue;\n-\t\tcheck_linked_checkout(branch, d->d_name);\n+\t\texisting = find_linked_symref(symref, target, d->d_name);\n+\t\tif (existing)\n+\t\t\tgoto done;\n \t}\n+done:\n \tclosedir(dir);\n+\n+\treturn existing;\n+}\n+\n+void die_if_checked_out(const char *branch)\n+{\n+\tchar *existing;\n+\n+\texisting = find_shared_symref(\"HEAD\", branch);\n+\tif (existing) {\n+\t\tskip_prefix(branch, \"refs/heads/\", &branch);\n+\t\tdie(_(\"'%s' is already checked out at '%s'\"), branch, existing);\n+\t}\n }\ndiff --git a/branch.h b/branch.h\nindex 58aa45f..d3446ed 100644\n--- a/branch.h\n+++ b/branch.h\n@@ -59,4 +59,12 @@ extern int read_branch_desc(struct strbuf *, const char *branch_name);\n */\n extern void die_if_checked_out(const char *branch);\n \n+/*\n+ * Check if a per-worktree symref points to a ref in the main worktree\n+ * or any linked worktree, and return the path to the exising worktree\n+ * if it is. Returns NULL if there is no existing ref. The caller is\n+ * responsible for freeing the returned path.\n+ */\n+extern char *find_shared_symref(const char *symref, const char *target);\n+\n #endif\n-- \n2.0.4.315.gad8727a-twtrsrc\n"},{"id":"267784","messageId":"1439229165-25773-2-git-send-email-dturner@twopensource.com","threadId":"40049","inReplyTo":"1439229165-25773-1-git-send-email-dturner@twopensource.com","subject":"[PATCH v6 2/2] notes: handle multiple worktrees","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2015-08-10T17:52:45Z","receivedAt":"2015-08-10T17:52:45Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"Before creating NOTES_MERGE_REF, check NOTES_MERGE_REF using\nfind_shared_symref and die if we find one. This prevents simultaneous\nmerges to the same notes branch from different worktrees.\n\nSigned-off-by: David Turner \n---\n builtin/notes.c | 6 ++++\n t/t3320-notes-merge-worktrees.sh | 72 ++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 78 insertions(+)\n create mode 100755 t/t3320-notes-merge-worktrees.sh\n\ndiff --git a/builtin/notes.c b/builtin/notes.c\nindex 63f95fc..0423480 100644\n--- a/builtin/notes.c\n+++ b/builtin/notes.c\n@@ -19,6 +19,7 @@\n #include \"string-list.h\"\n #include \"notes-merge.h\"\n #include \"notes-utils.h\"\n+#include \"branch.h\"\n \n static const char * const git_notes_usage[] = {\n \tN_(\"git notes [--ref ] [list []]\"),\n@@ -825,10 +826,15 @@ static int merge(int argc, const char **argv, const char *prefix)\n \t\tupdate_ref(msg.buf, default_notes_ref(), result_sha1, NULL,\n \t\t\t 0, UPDATE_REFS_DIE_ON_ERR);\n \telse { /* Merge has unresolved conflicts */\n+\t\tchar *existing;\n \t\t/* Update .git/NOTES_MERGE_PARTIAL with partial merge result */\n \t\tupdate_ref(msg.buf, \"NOTES_MERGE_PARTIAL\", result_sha1, NULL,\n \t\t\t 0, UPDATE_REFS_DIE_ON_ERR);\n \t\t/* Store ref-to-be-updated into .git/NOTES_MERGE_REF */\n+\t\texisting = find_shared_symref(\"NOTES_MERGE_REF\", default_notes_ref());\n+\t\tif (existing)\n+\t\t\tdie(_(\"A notes merge into %s is already in-progress at %s\"),\n+\t\t\t default_notes_ref(), existing);\n \t\tif (create_symref(\"NOTES_MERGE_REF\", default_notes_ref(), NULL))\n \t\t\tdie(\"Failed to store link to current notes ref (%s)\",\n \t\t\t default_notes_ref());\ndiff --git a/t/t3320-notes-merge-worktrees.sh b/t/t3320-notes-merge-worktrees.sh\nnew file mode 100755\nindex 0000000..a7beef2\n--- /dev/null\n+++ b/t/t3320-notes-merge-worktrees.sh\n@@ -0,0 +1,72 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2015 Twitter, Inc\n+#\n+\n+test_description='Test merging of notes trees in multiple worktrees'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'setup commit' '\n+\ttest_commit tantrum\n+'\n+\n+commit_tantrum=$(git rev-parse tantrum^{commit})\n+\n+test_expect_success 'setup notes ref (x)' '\n+\tgit config core.notesRef refs/notes/x &&\n+\tgit notes add -m \"x notes on tantrum\" tantrum\n+'\n+\n+test_expect_success 'setup local branch (y)' '\n+\tgit update-ref refs/notes/y refs/notes/x &&\n+\tgit config core.notesRef refs/notes/y &&\n+\tgit notes remove tantrum\n+'\n+\n+test_expect_success 'setup remote branch (z)' '\n+\tgit update-ref refs/notes/z refs/notes/x &&\n+\tgit config core.notesRef refs/notes/z &&\n+\tgit notes add -f -m \"conflicting notes on tantrum\" tantrum\n+'\n+\n+test_expect_success 'modify notes ref ourselves (x)' '\n+\tgit config core.notesRef refs/notes/x &&\n+\tgit notes add -f -m \"more conflicting notes on tantrum\" tantrum\n+'\n+\n+test_expect_success 'create some new worktrees' '\n+\tgit worktree add -b newbranch worktree master &&\n+\tgit worktree add -b newbranch2 worktree2 master\n+'\n+\n+test_expect_success 'merge z into y fails and sets NOTES_MERGE_REF' '\n+\tgit config core.notesRef refs/notes/y &&\n+\ttest_must_fail git notes merge z &&\n+\techo \"ref: refs/notes/y\" > expect &&\n+\ttest_cmp .git/NOTES_MERGE_REF expect\n+'\n+\n+test_expect_success 'merge z into y while mid-merge in another workdir fails' '\n+\t(\n+\t\tcd worktree &&\n+\t\tgit config core.notesRef refs/notes/y &&\n+\t\ttest_must_fail git notes merge z 2>err &&\n+\t\tgrep \"A notes merge into refs/notes/y is already in-progress at\" err\n+\t) &&\n+\ttest_path_is_missing .git/worktrees/worktree/NOTES_MERGE_REF\n+'\n+\n+test_expect_success 'merge z into x while mid-merge on y succeeds' '\n+\t(\n+\t\tcd worktree2 &&\n+\t\tgit config core.notesRef refs/notes/x &&\n+\t\ttest_must_fail git notes merge z 2>&1 >out &&\n+\t\tgrep \"Automatic notes merge failed\" out &&\n+\t\tgrep -v \"A notes merge into refs/notes/x is already in-progress in\" out\n+\t) &&\n+\techo \"ref: refs/notes/x\" > expect &&\n+\ttest_cmp .git/worktrees/worktree2/NOTES_MERGE_REF expect\n+'\n+\n+test_done\n-- \n2.0.4.315.gad8727a-twtrsrc\n"},{"id":"267797","messageId":"CAPig+cTBLx2Q0n2ZcA-QgwNQMHZm9G45ThHbGc+Orkz-6c5cUg@mail.gmail.com","threadId":"40049","inReplyTo":"1439229165-25773-1-git-send-email-dturner@twopensource.com","subject":"Re: [PATCH v6 1/2] worktrees: add find_shared_symref","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-08-10T22:30:31Z","receivedAt":"2015-08-10T22:30:31Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Aug 10, 2015 at 1:52 PM, David Turner wrote:\n> worktrees: add find_shared_symref\n\ns/worktrees/branch/ perhaps?\n\n> Add a new function, find_shared_symref, which contains the heart of\n> die_if_checked_out, but works for any symref, not just HEAD. Refactor\n> die_if_checked_out to use the same infrastructure as\n> find_shared_symref.\n>\n> Soon, we will use find_shared_symref to protect notes merges in\n> worktrees.\n>\n> Signed-off-by: David Turner \n> ---\n> This version addresses Eric Sunshine's comments on v5. It fixes an error\n> message and cleans up the code.\n\nAll issues identified in previous versions seem to have been\naddressed, and nothing else pops out at me. Thanks.\n"},{"id":"267799","messageId":"1439246526.5283.3.camel@twopensource.com","threadId":"40049","inReplyTo":"CAPig+cTBLx2Q0n2ZcA-QgwNQMHZm9G45ThHbGc+Orkz-6c5cUg@mail.gmail.com","subject":"Re: [PATCH v6 1/2] worktrees: add find_shared_symref","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2015-08-10T22:42:06Z","receivedAt":"2015-08-10T22:42:06Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Mon, 2015-08-10 at 18:30 -0400, Eric Sunshine wrote:\n> On Mon, Aug 10, 2015 at 1:52 PM, David Turner wrote:\n> > worktrees: add find_shared_symref\n> \n> s/worktrees/branch/ perhaps?\n\nDo you mean \"this is in branch.c, so should be labeled with branch\"? \n\nBecause this change is mostly about non-branch refs, so I think saying\n\"branch\" is confusing. That's why I labelled it \"worktrees\"; that's the\nbroad topic that's being addressed.\n\n> > Add a new function, find_shared_symref, which contains the heart of\n> > die_if_checked_out, but works for any symref, not just HEAD. Refactor\n> > die_if_checked_out to use the same infrastructure as\n> > find_shared_symref.\n> >\n> > Soon, we will use find_shared_symref to protect notes merges in\n> > worktrees.\n> >\n> > Signed-off-by: David Turner \n> > ---\n> > This version addresses Eric Sunshine's comments on v5. It fixes an error\n> > message and cleans up the code.\n> \n> All issues identified in previous versions seem to have been\n> addressed, and nothing else pops out at me. Thanks.\n"},{"id":"267801","messageId":"CAPig+cQ-Y6=A-bUEtBzHCV_roDShFcm9qnv8PC6gge2Y0anU_w@mail.gmail.com","threadId":"40049","inReplyTo":"1439246526.5283.3.camel@twopensource.com","subject":"Re: [PATCH v6 1/2] worktrees: add find_shared_symref","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-08-10T22:49:49Z","receivedAt":"2015-08-10T22:49:49Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Aug 10, 2015 at 6:42 PM, David Turner wrote:\n> On Mon, 2015-08-10 at 18:30 -0400, Eric Sunshine wrote:\n>> On Mon, Aug 10, 2015 at 1:52 PM, David Turner wrote:\n>> > worktrees: add find_shared_symref\n>>\n>> s/worktrees/branch/ perhaps?\n>\n> Do you mean \"this is in branch.c, so should be labeled with branch\"?\n>\n> Because this change is mostly about non-branch refs, so I think saying\n> \"branch\" is confusing. That's why I labelled it \"worktrees\"; that's the\n> broad topic that's being addressed.\n\nOkay.\n"},{"id":"267831","messageId":"CALKQrgco6wJ0CpNX5=DF+Z7G009e0G5wz=UUTOqAzv81hKh7bg@mail.gmail.com","threadId":"40049","inReplyTo":"1439229165-25773-2-git-send-email-dturner@twopensource.com","subject":"Re: [PATCH v6 2/2] notes: handle multiple worktrees","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2015-08-11T13:56:45Z","receivedAt":"2015-08-11T13:56:45Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Mon, Aug 10, 2015 at 7:52 PM, David Turner wrote:\n> Before creating NOTES_MERGE_REF, check NOTES_MERGE_REF using\n> find_shared_symref and die if we find one. This prevents simultaneous\n> merges to the same notes branch from different worktrees.\n>\n> Signed-off-by: David Turner \n\nStill looks good to me, AFAICS. Feel free to add my Reviewed-by.\n\n...Johan\n\n\n\n-- \nJohan Herland, \nwww.herland.net\n"}]}