{"thread":{"id":"39957","subject":"[PATCH v3 1/6] refs: Introduce pseudoref and per-worktree ref concepts","startedAt":"2015-07-28T18:12:18Z","lastAt":"2015-08-03T13:27:45Z","messageCount":41,"participants":["David Turner","Junio C Hamano","Eric Sunshine","Johan Herland","Michael Haggerty","Jacob Keller","Duy Nguyen"],"isPatch":true,"patchVersion":3,"patchTotal":6},"messages":[{"id":"266955","messageId":"1438107144-24293-1-git-send-email-dturner@twopensource.com","threadId":"39957","inReplyTo":null,"subject":"[PATCH v3 0/6] pseudorefs","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2015-07-28T18:12:18Z","receivedAt":"2015-07-28T18:12:18Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"This version fixes documentation issues found by Eric Sunshine.\n\nIt also adds a new patch so as not to create static functions that\naren't immediately used; Eric also noticed that issue.\n\nI refactored the functions to classify a ref into a single public\nref_type function.  This makes it easy for backends that want to treat\nall non-normal refs the same; in previous patch versions, backends\nwould have had to say is_pseudoref || is_per_worktree_ref, and that\nwould have caused the strcmp in is_per_worktree_ref to be called\ntwice.  Now they can just say ref_type(ref) != REF_TYPE_NORMAL.\n\nI also fixed another issues that I noticed myself: I removed a stray\ndebugging \"&& 0\" condition.\n"},{"id":"266954","messageId":"1438107144-24293-2-git-send-email-dturner@twopensource.com","threadId":"39957","inReplyTo":"1438107144-24293-1-git-send-email-dturner@twopensource.com","subject":"[PATCH v3 1/6] refs: Introduce pseudoref and per-worktree ref concepts","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2015-07-28T18:12:19Z","receivedAt":"2015-07-28T18:12:19Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"Add glossary entries for both concepts.\n\nPseudorefs and per-worktree refs do not yet have special handling,\nbecause the files refs backend already handles them correctly.  Later,\nwe will make the LMDB backend call out to the files backend to handle\nper-worktree refs.\n\nSigned-off-by: David Turner <dturner@twopensource.com>\n---\n Documentation/glossary-content.txt | 21 +++++++++++++++++++++\n 1 file changed, 21 insertions(+)\n\ndiff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt\nindex ab18f4b..ff14079 100644\n--- a/Documentation/glossary-content.txt\n+++ b/Documentation/glossary-content.txt\n@@ -411,6 +411,27 @@ exclude;;\n \tcore Git. Porcelains expose more of a <<def_SCM,SCM>>\n \tinterface than the <<def_plumbing,plumbing>>.\n \n+[[def_per_worktree_ref]]per-worktree ref::\n+\tRefs that are per-<<def_worktree,worktree>>, rather than\n+\tglobal.  This is presently only <<def_HEAD,HEAD>>, but might\n+\tlater include other unusual refs.\n+\n+[[def_pseudoref]]pseudoref::\n+\tPseudorefs are a class of files under `$GIT_DIR` which behave\n+\tlike refs for the purposes of rev-parse, but which are treated\n+\tspecially by git.  Psuedorefs both have names that are all-caps,\n+\tand always start with a line consisting of a\n+\t<<def_sha1,SHA-1>> followed by whitespace.  So, HEAD is not a\n+\tpseudoref, because it is sometimes a symbolic ref.  They might\n+\toptionally contain some additional data.  `MERGE_HEAD` and\n+\t`CHERRY_PICK_HEAD` are examples.  Unlike\n+\t<<def_per_worktree_ref,per-worktree refs>>, these files cannot\n+\tbe symbolic refs, and never have reflogs.  They also cannot be\n+\tupdated through the normal ref update machinery.  Instead,\n+\tthey are updated by directly writing to the files.  However,\n+\tthey can be read as if they were refs, so `git rev-parse\n+\tMERGE_HEAD` will work.\n+\n [[def_pull]]pull::\n \tPulling a <<def_branch,branch>> means to <<def_fetch,fetch>> it and\n \t<<def_merge,merge>> it.  See also linkgit:git-pull[1].\n-- \n2.0.4.315.gad8727a-twtrsrc\n"},{"id":"266958","messageId":"1438107144-24293-3-git-send-email-dturner@twopensource.com","threadId":"39957","inReplyTo":"1438107144-24293-1-git-send-email-dturner@twopensource.com","subject":"[PATCH v3 2/6] notes: replace pseudorefs with real refs","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2015-07-28T18:12:20Z","receivedAt":"2015-07-28T18:12:20Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"All-caps files like NOTES_MERGE_REF are pseudorefs, and thus are\nper-worktree.  We don't want multiple notes merges happening at once\n(in different worktrees), so we want to make these refs true refs.\n\nSo, we lowercase NOTES_MERGE_REF and friends.  That way, backends\nthat distinguish between pseudorefs and real refs can correctly\nhandle notes merges.\n\nThis will also enable us to prevent pseudorefs from being updated by\nthe ref update machinery e.g. git update-ref.\n\nSigned-off-by: David Turner <dturner@twopensource.com>\n---\n Documentation/git-notes.txt           | 12 ++---\n builtin/notes.c                       | 38 ++++++++--------\n notes-merge.c                         | 10 ++--\n notes-merge.h                         |  8 ++--\n t/t3310-notes-merge-manual-resolve.sh | 86 +++++++++++++++++------------------\n t/t3311-notes-merge-fanout.sh         |  6 +--\n 6 files changed, 80 insertions(+), 80 deletions(-)\n\ndiff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt\nindex 851518d..82fa3fd 100644\n--- a/Documentation/git-notes.txt\n+++ b/Documentation/git-notes.txt\n@@ -103,7 +103,7 @@ merge::\n If conflicts arise and a strategy for automatically resolving\n conflicting notes (see the -s/--strategy option) 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+conflicting notes in a special worktree (`.git/notes_merge_worktree`),\n and instructs the user to manually resolve the conflicts there.\n When done, the user can either finalize the merge with\n 'git notes merge --commit', or abort the merge with\n@@ -189,11 +189,11 @@ OPTIONS\n --commit::\n \tFinalize an in-progress 'git notes merge'. Use this option\n \twhen you have resolved the conflicts that 'git notes merge'\n-\tstored in .git/NOTES_MERGE_WORKTREE. This amends the partial\n+\tstored in .git/notes_merge_worktree. This amends the partial\n \tmerge commit created by 'git notes merge' (stored in\n-\t.git/NOTES_MERGE_PARTIAL) by adding the notes in\n-\t.git/NOTES_MERGE_WORKTREE. The notes ref stored in the\n-\t.git/NOTES_MERGE_REF symref is updated to the resulting commit.\n+\t.git/notes_merge_partial) by adding the notes in\n+\t.git/notes_merge_worktree. The notes ref stored in the\n+\t.git/notes_merge_ref symref is updated to the resulting commit.\n \n --abort::\n \tAbort/reset a in-progress 'git notes merge', i.e. a notes merge\n@@ -241,7 +241,7 @@ NOTES MERGE STRATEGIES\n \n The default notes merge strategy is \"manual\", which checks out\n conflicting notes in a special work tree for resolving notes conflicts\n-(`.git/NOTES_MERGE_WORKTREE`), and instructs the user to resolve the\n+(`.git/notes_merge_worktree`), and instructs the user to resolve the\n conflicts in that work tree.\n When done, the user can either finalize the merge with\n 'git notes merge --commit', or abort the merge with\ndiff --git a/builtin/notes.c b/builtin/notes.c\nindex 63f95fc..e0b8a02 100644\n--- a/builtin/notes.c\n+++ b/builtin/notes.c\n@@ -670,14 +670,14 @@ static int merge_abort(struct notes_merge_options *o)\n \tint ret = 0;\n \n \t/*\n-\t * Remove .git/NOTES_MERGE_PARTIAL and .git/NOTES_MERGE_REF, and call\n-\t * notes_merge_abort() to remove .git/NOTES_MERGE_WORKTREE.\n+\t * Remove .git/notes_merge_partial and .git/notes_merge_ref, and call\n+\t * notes_merge_abort() to remove .git/notes_merge_worktree.\n \t */\n \n-\tif (delete_ref(\"NOTES_MERGE_PARTIAL\", NULL, 0))\n-\t\tret += error(\"Failed to delete ref NOTES_MERGE_PARTIAL\");\n-\tif (delete_ref(\"NOTES_MERGE_REF\", NULL, REF_NODEREF))\n-\t\tret += error(\"Failed to delete ref NOTES_MERGE_REF\");\n+\tif (delete_ref(\"notes_merge_partial\", NULL, 0))\n+\t\tret += error(\"Failed to delete ref notes_merge_partial\");\n+\tif (delete_ref(\"notes_merge_ref\", NULL, REF_NODEREF))\n+\t\tret += error(\"Failed to delete ref notes_merge_ref\");\n \tif (notes_merge_abort(o))\n \t\tret += error(\"Failed to remove 'git notes merge' worktree\");\n \treturn ret;\n@@ -694,16 +694,16 @@ static int merge_commit(struct notes_merge_options *o)\n \tint ret;\n \n \t/*\n-\t * Read partial merge result from .git/NOTES_MERGE_PARTIAL,\n-\t * and target notes ref from .git/NOTES_MERGE_REF.\n+\t * Read partial merge result from .git/notes_merge_partial,\n+\t * and target notes ref from .git/notes_merge_ref.\n \t */\n \n-\tif (get_sha1(\"NOTES_MERGE_PARTIAL\", sha1))\n-\t\tdie(\"Failed to read ref NOTES_MERGE_PARTIAL\");\n+\tif (get_sha1(\"notes_merge_partial\", sha1))\n+\t\tdie(\"Failed to read ref notes_merge_partial\");\n \telse if (!(partial = lookup_commit_reference(sha1)))\n-\t\tdie(\"Could not find commit from NOTES_MERGE_PARTIAL.\");\n+\t\tdie(\"Could not find commit from notes_merge_partial.\");\n \telse if (parse_commit(partial))\n-\t\tdie(\"Could not parse commit from NOTES_MERGE_PARTIAL.\");\n+\t\tdie(\"Could not parse commit from notes_merge_partial.\");\n \n \tif (partial->parents)\n \t\thashcpy(parent_sha1, partial->parents->item->object.sha1);\n@@ -711,12 +711,12 @@ static int merge_commit(struct notes_merge_options *o)\n \t\thashclr(parent_sha1);\n \n \tt = xcalloc(1, sizeof(struct notes_tree));\n-\tinit_notes(t, \"NOTES_MERGE_PARTIAL\", combine_notes_overwrite, 0);\n+\tinit_notes(t, \"notes_merge_partial\", combine_notes_overwrite, 0);\n \n \to->local_ref = local_ref_to_free =\n-\t\tresolve_refdup(\"NOTES_MERGE_REF\", 0, sha1, NULL);\n+\t\tresolve_refdup(\"notes_merge_ref\", 0, sha1, NULL);\n \tif (!o->local_ref)\n-\t\tdie(\"Failed to resolve NOTES_MERGE_REF\");\n+\t\tdie(\"Failed to resolve notes_merge_ref\");\n \n \tif (notes_merge_commit(o, t, partial, sha1))\n \t\tdie(\"Failed to finalize notes merge\");\n@@ -825,11 +825,11 @@ 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\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/* 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\tif (create_symref(\"NOTES_MERGE_REF\", default_notes_ref(), NULL))\n+\t\t/* Store ref-to-be-updated into .git/notes_merge_ref */\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());\n \t\tprintf(\"Automatic notes merge failed. Fix conflicts in %s and \"\ndiff --git a/notes-merge.c b/notes-merge.c\nindex 0b2b82c..58a8637 100644\n--- a/notes-merge.c\n+++ b/notes-merge.c\n@@ -274,10 +274,10 @@ static void check_notes_merge_worktree(struct notes_merge_options *o)\n \t\t\t\t    \"'git notes merge --commit' or 'git notes \"\n \t\t\t\t    \"merge --abort' to commit/abort the \"\n \t\t\t\t    \"previous merge before you start a new \"\n-\t\t\t\t    \"notes merge.\", git_path(\"NOTES_MERGE_*\"));\n+\t\t\t\t    \"notes merge.\", git_path(\"notes_merge_*\"));\n \t\t\telse\n \t\t\t\tdie(\"You have not concluded your notes merge \"\n-\t\t\t\t    \"(%s exists).\", git_path(\"NOTES_MERGE_*\"));\n+\t\t\t\t    \"(%s exists).\", git_path(\"notes_merge_*\"));\n \t\t}\n \n \t\tif (safe_create_leading_directories_const(git_path(\n@@ -663,7 +663,7 @@ int notes_merge_commit(struct notes_merge_options *o,\n \t\t       unsigned char *result_sha1)\n {\n \t/*\n-\t * Iterate through files in .git/NOTES_MERGE_WORKTREE and add all\n+\t * Iterate through files in .git/notes_merge_workree and add all\n \t * found notes to 'partial_tree'. Write the updated notes tree to\n \t * the DB, and commit the resulting tree object while reusing the\n \t * commit message and parents from 'partial_commit'.\n@@ -734,8 +734,8 @@ int notes_merge_commit(struct notes_merge_options *o,\n int notes_merge_abort(struct notes_merge_options *o)\n {\n \t/*\n-\t * Remove all files within .git/NOTES_MERGE_WORKTREE. We do not remove\n-\t * the .git/NOTES_MERGE_WORKTREE directory itself, since it might be\n+\t * Remove all files within .git/notes_merge_workree. We do not remove\n+\t * the .git/notes_merge_workree directory itself, since it might be\n \t * the current working directory of the user.\n \t */\n \tstruct strbuf buf = STRBUF_INIT;\ndiff --git a/notes-merge.h b/notes-merge.h\nindex 1d01f6a..18705d6 100644\n--- a/notes-merge.h\n+++ b/notes-merge.h\n@@ -1,7 +1,7 @@\n #ifndef NOTES_MERGE_H\n #define NOTES_MERGE_H\n \n-#define NOTES_MERGE_WORKTREE \"NOTES_MERGE_WORKTREE\"\n+#define NOTES_MERGE_WORKTREE \"notes_merge_worktree\"\n \n enum notes_merge_verbosity {\n \tNOTES_MERGE_VERBOSITY_DEFAULT = 2,\n@@ -46,7 +46,7 @@ void init_notes_merge_options(struct notes_merge_options *o);\n  *    are stored in 'local_tree', and the SHA1 or the resulting commit\n  *    (to be amended when the conflicts have been resolved) is written into\n  *    'result_sha1'. The unmerged entries are written into the\n- *    .git/NOTES_MERGE_WORKTREE directory with conflict markers.\n+ *    .git/notes_merge_worktree directory with conflict markers.\n  *    -1 is returned.\n  *\n  * Both o->local_ref and o->remote_ref must be given (non-NULL), but either ref\n@@ -65,7 +65,7 @@ int notes_merge(struct notes_merge_options *o,\n  * the given 'partial_commit', the partial result commit created by a previous\n  * call to notes_merge().\n  *\n- * This function will add the (now resolved) notes in .git/NOTES_MERGE_WORKTREE\n+ * This function will add the (now resolved) notes in .git/notes_merge_worktree\n  * to 'partial_tree', and create a final notes merge commit, the SHA1 of which\n  * will be stored in 'result_sha1'.\n  */\n@@ -77,7 +77,7 @@ int notes_merge_commit(struct notes_merge_options *o,\n /*\n  * Abort conflict resolution from an earlier notes_merge()\n  *\n- * Removes the notes merge worktree in .git/NOTES_MERGE_WORKTREE.\n+ * Removes the notes merge worktree in .git/notes_merge_worktree.\n  */\n int notes_merge_abort(struct notes_merge_options *o);\n \ndiff --git a/t/t3310-notes-merge-manual-resolve.sh b/t/t3310-notes-merge-manual-resolve.sh\nindex 195bb97..dc9ecd5 100755\n--- a/t/t3310-notes-merge-manual-resolve.sh\n+++ b/t/t3310-notes-merge-manual-resolve.sh\n@@ -178,12 +178,12 @@ test_expect_success 'merge z into m (== y) with default (\"manual\") resolver => C\n \tgit config core.notesRef refs/notes/m &&\n \ttest_must_fail git notes merge z >output &&\n \t# Output should point to where to resolve conflicts\n-\tgrep -q \"\\\\.git/NOTES_MERGE_WORKTREE\" output &&\n+\tgrep -q \"\\\\.git/notes_merge_worktree\" output &&\n \t# Inspect merge conflicts\n-\tls .git/NOTES_MERGE_WORKTREE >output_conflicts &&\n+\tls .git/notes_merge_worktree >output_conflicts &&\n \ttest_cmp expect_conflicts output_conflicts &&\n \t( for f in $(cat expect_conflicts); do\n-\t\ttest_cmp \"expect_conflict_$f\" \".git/NOTES_MERGE_WORKTREE/$f\" ||\n+\t\ttest_cmp \"expect_conflict_$f\" \".git/notes_merge_worktree/$f\" ||\n \t\texit 1\n \tdone ) &&\n \t# Verify that current notes tree (pre-merge) has not changed (m == y)\n@@ -222,10 +222,10 @@ test_expect_success 'change notes in z' '\n '\n \n test_expect_success 'cannot do merge w/conflicts when previous merge is unfinished' '\n-\ttest -d .git/NOTES_MERGE_WORKTREE &&\n+\ttest -d .git/notes_merge_worktree &&\n \ttest_must_fail git notes merge z >output 2>&1 &&\n \t# Output should indicate what is wrong\n-\tgrep -q \"\\\\.git/NOTES_MERGE_\\\\* exists\" output\n+\tgrep -q \"\\\\.git/notes_merge_\\\\* exists\" output\n '\n \n # Setup non-conflicting merge between x and new notes ref w\n@@ -282,7 +282,7 @@ w notes on 1st commit\n EOF\n \n test_expect_success 'can do merge without conflicts even if previous merge is unfinished (x => w)' '\n-\ttest -d .git/NOTES_MERGE_WORKTREE &&\n+\ttest -d .git/notes_merge_worktree &&\n \tgit notes merge x &&\n \tverify_notes w &&\n \t# Verify that other notes refs has not changed (x and y)\n@@ -316,15 +316,15 @@ EOF\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+\tcat >.git/notes_merge_worktree/$commit_sha1 <<EOF &&\n y and z notes on 1st commit\n EOF\n-\tcat >.git/NOTES_MERGE_WORKTREE/$commit_sha4 <<EOF &&\n+\tcat >.git/notes_merge_worktree/$commit_sha4 <<EOF &&\n y and z notes on 4th commit\n EOF\n \tgit notes merge --commit &&\n-\t# No .git/NOTES_MERGE_* files left\n-\ttest_might_fail ls .git/NOTES_MERGE_* >output 2>/dev/null &&\n+\t# No .git/notes_merge_* files left\n+\ttest_might_fail ls .git/notes_merge_* >output 2>/dev/null &&\n \ttest_cmp /dev/null output &&\n \t# Merge commit has pre-merge y and pre-merge z as parents\n \ttest \"$(git rev-parse refs/notes/m^1)\" = \"$(cat pre_merge_y)\" &&\n@@ -369,12 +369,12 @@ test_expect_success 'redo merge of z into m (== y) with default (\"manual\") resol\n \tgit config core.notesRef refs/notes/m &&\n \ttest_must_fail git notes merge z >output &&\n \t# Output should point to where to resolve conflicts\n-\tgrep -q \"\\\\.git/NOTES_MERGE_WORKTREE\" output &&\n+\tgrep -q \"\\\\.git/notes_merge_worktree\" output &&\n \t# Inspect merge conflicts\n-\tls .git/NOTES_MERGE_WORKTREE >output_conflicts &&\n+\tls .git/notes_merge_worktree >output_conflicts &&\n \ttest_cmp expect_conflicts output_conflicts &&\n \t( for f in $(cat expect_conflicts); do\n-\t\ttest_cmp \"expect_conflict_$f\" \".git/NOTES_MERGE_WORKTREE/$f\" ||\n+\t\ttest_cmp \"expect_conflict_$f\" \".git/notes_merge_worktree/$f\" ||\n \t\texit 1\n \tdone ) &&\n \t# Verify that current notes tree (pre-merge) has not changed (m == y)\n@@ -385,8 +385,8 @@ test_expect_success 'redo merge of z into m (== y) with default (\"manual\") resol\n \n test_expect_success 'abort notes merge' '\n \tgit notes merge --abort &&\n-\t# No .git/NOTES_MERGE_* files left\n-\ttest_might_fail ls .git/NOTES_MERGE_* >output 2>/dev/null &&\n+\t# No .git/notes_merge_* files left\n+\ttest_might_fail ls .git/notes_merge_* >output 2>/dev/null &&\n \ttest_cmp /dev/null output &&\n \t# m has not moved (still == y)\n \ttest \"$(git rev-parse refs/notes/m)\" = \"$(cat pre_merge_y)\" &&\n@@ -403,12 +403,12 @@ git rev-parse refs/notes/z > pre_merge_z\n test_expect_success 'redo merge of z into m (== y) with default (\"manual\") resolver => Conflicting 3-way merge' '\n \ttest_must_fail git notes merge z >output &&\n \t# Output should point to where to resolve conflicts\n-\tgrep -q \"\\\\.git/NOTES_MERGE_WORKTREE\" output &&\n+\tgrep -q \"\\\\.git/notes_merge_worktree\" output &&\n \t# Inspect merge conflicts\n-\tls .git/NOTES_MERGE_WORKTREE >output_conflicts &&\n+\tls .git/notes_merge_worktree >output_conflicts &&\n \ttest_cmp expect_conflicts output_conflicts &&\n \t( for f in $(cat expect_conflicts); do\n-\t\ttest_cmp \"expect_conflict_$f\" \".git/NOTES_MERGE_WORKTREE/$f\" ||\n+\t\ttest_cmp \"expect_conflict_$f\" \".git/notes_merge_worktree/$f\" ||\n \t\texit 1\n \tdone ) &&\n \t# Verify that current notes tree (pre-merge) has not changed (m == y)\n@@ -441,19 +441,19 @@ EOF\n \n test_expect_success 'add + remove notes in finalized merge (z => m)' '\n \t# Resolve one conflict\n-\tcat >.git/NOTES_MERGE_WORKTREE/$commit_sha1 <<EOF &&\n+\tcat >.git/notes_merge_worktree/$commit_sha1 <<EOF &&\n y and z notes on 1st commit\n EOF\n \t# Remove another conflict\n-\trm .git/NOTES_MERGE_WORKTREE/$commit_sha4 &&\n+\trm .git/notes_merge_worktree/$commit_sha4 &&\n \t# Remove a D/F conflict\n-\trm .git/NOTES_MERGE_WORKTREE/$commit_sha3 &&\n+\trm .git/notes_merge_worktree/$commit_sha3 &&\n \t# Add a new note\n-\techo \"new note on 5th commit\" > .git/NOTES_MERGE_WORKTREE/$commit_sha5 &&\n+\techo \"new note on 5th commit\" > .git/notes_merge_worktree/$commit_sha5 &&\n \t# Finalize merge\n \tgit notes merge --commit &&\n-\t# No .git/NOTES_MERGE_* files left\n-\ttest_might_fail ls .git/NOTES_MERGE_* >output 2>/dev/null &&\n+\t# No .git/notes_merge_* files left\n+\ttest_might_fail ls .git/notes_merge_* >output 2>/dev/null &&\n \ttest_cmp /dev/null output &&\n \t# Merge commit has pre-merge y and pre-merge z as parents\n \ttest \"$(git rev-parse refs/notes/m^1)\" = \"$(cat pre_merge_y)\" &&\n@@ -484,12 +484,12 @@ test_expect_success 'redo merge of z into m (== y) with default (\"manual\") resol\n \tgit update-ref refs/notes/m refs/notes/y &&\n \ttest_must_fail git notes merge z >output &&\n \t# Output should point to where to resolve conflicts\n-\tgrep -q \"\\\\.git/NOTES_MERGE_WORKTREE\" output &&\n+\tgrep -q \"\\\\.git/notes_merge_worktree\" output &&\n \t# Inspect merge conflicts\n-\tls .git/NOTES_MERGE_WORKTREE >output_conflicts &&\n+\tls .git/notes_merge_worktree >output_conflicts &&\n \ttest_cmp expect_conflicts output_conflicts &&\n \t( for f in $(cat expect_conflicts); do\n-\t\ttest_cmp \"expect_conflict_$f\" \".git/NOTES_MERGE_WORKTREE/$f\" ||\n+\t\ttest_cmp \"expect_conflict_$f\" \".git/notes_merge_worktree/$f\" ||\n \t\texit 1\n \tdone ) &&\n \t# Verify that current notes tree (pre-merge) has not changed (m == y)\n@@ -507,31 +507,31 @@ test_expect_success 'reset notes ref m to somewhere else (w)' '\n \ttest \"$(git rev-parse refs/notes/m)\" = \"$(git rev-parse refs/notes/w)\"\n '\n \n-test_expect_success 'fail to finalize conflicting merge if underlying ref has moved in the meantime (m != NOTES_MERGE_PARTIAL^1)' '\n+test_expect_success 'fail to finalize conflicting merge if underlying ref has moved in the meantime (m != notes_merge_partial^1)' '\n \t# Resolve conflicts\n-\tcat >.git/NOTES_MERGE_WORKTREE/$commit_sha1 <<EOF &&\n+\tcat >.git/notes_merge_worktree/$commit_sha1 <<EOF &&\n y and z notes on 1st commit\n EOF\n-\tcat >.git/NOTES_MERGE_WORKTREE/$commit_sha4 <<EOF &&\n+\tcat >.git/notes_merge_worktree/$commit_sha4 <<EOF &&\n y and z notes on 4th commit\n EOF\n \t# Fail to finalize merge\n \ttest_must_fail git notes merge --commit >output 2>&1 &&\n-\t# .git/NOTES_MERGE_* must remain\n-\ttest -f .git/NOTES_MERGE_PARTIAL &&\n-\ttest -f .git/NOTES_MERGE_REF &&\n-\ttest -f .git/NOTES_MERGE_WORKTREE/$commit_sha1 &&\n-\ttest -f .git/NOTES_MERGE_WORKTREE/$commit_sha2 &&\n-\ttest -f .git/NOTES_MERGE_WORKTREE/$commit_sha3 &&\n-\ttest -f .git/NOTES_MERGE_WORKTREE/$commit_sha4 &&\n+\t# .git/notes_merge_* must remain\n+\tgit rev-parse --verify notes_merge_partial &&\n+\tgit rev-parse --verify notes_merge_ref &&\n+\ttest -f .git/notes_merge_worktree/$commit_sha1 &&\n+\ttest -f .git/notes_merge_worktree/$commit_sha2 &&\n+\ttest -f .git/notes_merge_worktree/$commit_sha3 &&\n+\ttest -f .git/notes_merge_worktree/$commit_sha4 &&\n \t# Refs are unchanged\n \ttest \"$(git rev-parse refs/notes/m)\" = \"$(git rev-parse refs/notes/w)\" &&\n-\ttest \"$(git rev-parse refs/notes/y)\" = \"$(git rev-parse NOTES_MERGE_PARTIAL^1)\" &&\n-\ttest \"$(git rev-parse refs/notes/m)\" != \"$(git rev-parse NOTES_MERGE_PARTIAL^1)\" &&\n+\ttest \"$(git rev-parse refs/notes/y)\" = \"$(git rev-parse notes_merge_partial^1)\" &&\n+\ttest \"$(git rev-parse refs/notes/m)\" != \"$(git rev-parse notes_merge_partial^1)\" &&\n \t# Mention refs/notes/m, and its current and expected value in output\n \tgrep -q \"refs/notes/m\" output &&\n \tgrep -q \"$(git rev-parse refs/notes/m)\" output &&\n-\tgrep -q \"$(git rev-parse NOTES_MERGE_PARTIAL^1)\" output &&\n+\tgrep -q \"$(git rev-parse notes_merge_partial^1)\" output &&\n \t# Verify that other notes refs has not changed (w, x, y and z)\n \tverify_notes w &&\n \tverify_notes x &&\n@@ -541,8 +541,8 @@ EOF\n \n test_expect_success 'resolve situation by aborting the notes merge' '\n \tgit notes merge --abort &&\n-\t# No .git/NOTES_MERGE_* files left\n-\ttest_might_fail ls .git/NOTES_MERGE_* >output 2>/dev/null &&\n+\t# No .git/notes_merge_* files left\n+\ttest_might_fail ls .git/notes_merge_* >output 2>/dev/null &&\n \ttest_cmp /dev/null output &&\n \t# m has not moved (still == w)\n \ttest \"$(git rev-parse refs/notes/m)\" = \"$(git rev-parse refs/notes/w)\" &&\n@@ -563,7 +563,7 @@ test_expect_success 'switch cwd before committing notes merge' '\n \tgit notes --ref=other add -m bar HEAD &&\n \ttest_must_fail git notes merge refs/notes/other &&\n \t(\n-\t\tcd .git/NOTES_MERGE_WORKTREE &&\n+\t\tcd .git/notes_merge_worktree &&\n \t\techo \"foo\" > $(git rev-parse HEAD) &&\n \t\techo \"bar\" >> $(git rev-parse HEAD) &&\n \t\tgit notes merge --commit\ndiff --git a/t/t3311-notes-merge-fanout.sh b/t/t3311-notes-merge-fanout.sh\nindex 93516ef..eeaea62 100755\n--- a/t/t3311-notes-merge-fanout.sh\n+++ b/t/t3311-notes-merge-fanout.sh\n@@ -387,10 +387,10 @@ other notes for commit4\n EOF\n \n test_expect_success 'verify conflict entries (with no fanout)' '\n-\tls .git/NOTES_MERGE_WORKTREE >output_conflicts &&\n+\tls .git/notes_merge_worktree >output_conflicts &&\n \ttest_cmp expect_conflicts output_conflicts &&\n \t( for f in $(cat expect_conflicts); do\n-\t\ttest_cmp \"expect_conflict_$f\" \".git/NOTES_MERGE_WORKTREE/$f\" ||\n+\t\ttest_cmp \"expect_conflict_$f\" \".git/notes_merge_worktree/$f\" ||\n \t\texit 1\n \tdone ) &&\n \t# Verify that current notes tree (pre-merge) has not changed (m == w)\n@@ -417,7 +417,7 @@ other notes for commit1\n EOF\n \n test_expect_success 'resolve and finalize merge (z => w)' '\n-\tcat >.git/NOTES_MERGE_WORKTREE/$commit_sha3 <<EOF &&\n+\tcat >.git/notes_merge_worktree/$commit_sha3 <<EOF &&\n other notes for commit3\n \n appended notes for commit3\n-- \n2.0.4.315.gad8727a-twtrsrc\n"},{"id":"266960","messageId":"1438107144-24293-4-git-send-email-dturner@twopensource.com","threadId":"39957","inReplyTo":"1438107144-24293-1-git-send-email-dturner@twopensource.com","subject":"[PATCH v3 3/6] refs: add ref_type function","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2015-07-28T18:12:21Z","receivedAt":"2015-07-28T18:12:21Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"Add a function ref_type, which categorizes refs as per-worktree,\npseudoref, or normal ref.\n\nLater, we will use this in refs.c to treat pseudorefs specially.\nAlternate ref backends may use it to treat both pseudorefs and\nper-worktree refs differently.\n\nSigned-off-by: David Turner <dturner@twopensource.com>\n---\n refs.c | 29 +++++++++++++++++++++++++++++\n refs.h |  8 ++++++++\n 2 files changed, 37 insertions(+)\n\ndiff --git a/refs.c b/refs.c\nindex 0b96ece..553ae8b 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2848,6 +2848,35 @@ static int delete_ref_loose(struct ref_lock *lock, int flag, struct strbuf *err)\n \treturn 0;\n }\n \n+static int is_per_worktree_ref(const char *refname)\n+{\n+\treturn !strcmp(refname, \"HEAD\");\n+}\n+\n+static int is_pseudoref_syntax(const char *refname)\n+{\n+\tconst char *c;\n+\n+\tif (strchr(refname, '/'))\n+\t\treturn 0;\n+\n+\tfor (c = refname; *c; ++c) {\n+\t\tif (!isupper(*c) && *c != '-' && *c != '_')\n+\t\t\treturn 0;\n+\t}\n+\n+\treturn 1;\n+}\n+\n+enum ref_type ref_type(const char *refname)\n+{\n+\tif (is_per_worktree_ref(refname))\n+\t\treturn REF_TYPE_PER_WORKTREE;\n+\tif (is_pseudoref_syntax(refname))\n+\t\treturn REF_TYPE_PSEUDOREF;\n+       return REF_TYPE_NORMAL;\n+}\n+\n int delete_ref(const char *refname, const unsigned char *old_sha1,\n \t       unsigned int flags)\n {\ndiff --git a/refs.h b/refs.h\nindex e4e46c3..dca4fb5 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -445,6 +445,14 @@ extern int parse_hide_refs_config(const char *var, const char *value, const char\n \n extern int ref_is_hidden(const char *);\n \n+enum ref_type {\n+\tREF_TYPE_PER_WORKTREE,\n+\tREF_TYPE_PSEUDOREF,\n+\tREF_TYPE_NORMAL,\n+};\n+\n+enum ref_type ref_type(const char *refname);\n+\n enum expire_reflog_flags {\n \tEXPIRE_REFLOGS_DRY_RUN = 1 << 0,\n \tEXPIRE_REFLOGS_UPDATE_REF = 1 << 1,\n-- \n2.0.4.315.gad8727a-twtrsrc\n"},{"id":"266957","messageId":"1438107144-24293-5-git-send-email-dturner@twopensource.com","threadId":"39957","inReplyTo":"1438107144-24293-1-git-send-email-dturner@twopensource.com","subject":"[PATCH v3 4/6] pseudorefs: create and use pseudoref update and delete functions","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2015-07-28T18:12:22Z","receivedAt":"2015-07-28T18:12:22Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"Pseudorefs should not be updated through the ref transaction\nAPI, because alternate ref backends still need to store pseudorefs\nin GIT_DIR (instead of wherever they store refs).  Instead,\nchange update_ref and delete_ref to call pseudoref-specific\nfunctions.\n\nSigned-off-by: David Turner <dturner@twopensource.com>\n---\n refs.c | 100 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------\n 1 file changed, 92 insertions(+), 8 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 553ae8b..2bd6aa6 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2877,12 +2877,87 @@ enum ref_type ref_type(const char *refname)\n        return REF_TYPE_NORMAL;\n }\n \n+static int write_pseudoref(const char *pseudoref, const unsigned char *sha1,\n+\t\t\t   const unsigned char *old_sha1, struct strbuf *err)\n+{\n+\tconst char *filename;\n+\tint fd;\n+\tstatic struct lock_file lock;\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tint ret = -1;\n+\n+\tstrbuf_addf(&buf, \"%s\\n\", sha1_to_hex(sha1));\n+\n+\tfilename = git_path(\"%s\", pseudoref);\n+\tfd = hold_lock_file_for_update(&lock, filename, LOCK_DIE_ON_ERROR);\n+\tif (fd < 0) {\n+\t\tstrbuf_addf(err, \"Could not open '%s' for writing: %s\",\n+\t\t\t    filename, strerror(errno));\n+\t\treturn -1;\n+\t}\n+\n+\tif (old_sha1) {\n+\t\tunsigned char actual_old_sha1[20];\n+\t\tread_ref(pseudoref, actual_old_sha1);\n+\t\tif (hashcmp(actual_old_sha1, old_sha1)) {\n+\t\t\tstrbuf_addf(err, \"Unexpected sha1 when writing %s\", pseudoref);\n+\t\t\trollback_lock_file(&lock);\n+\t\t\tgoto done;\n+\t\t}\n+\t}\n+\n+\tif (write_in_full(fd, buf.buf, buf.len) != buf.len) {\n+\t\tstrbuf_addf(err, \"Could not write to '%s'\", filename);\n+\t\trollback_lock_file(&lock);\n+\t\tgoto done;\n+\t}\n+\n+\tcommit_lock_file(&lock);\n+\tret = 0;\n+done:\n+\tstrbuf_release(&buf);\n+\treturn ret;\n+}\n+\n+static int delete_pseudoref(const char *pseudoref, const unsigned char *old_sha1)\n+{\n+\tstatic struct lock_file lock;\n+\tconst char *filename;\n+\n+\tfilename = git_path(\"%s\", pseudoref);\n+\n+\tif (old_sha1 && !is_null_sha1(old_sha1)) {\n+\t\tint fd;\n+\t\tunsigned char actual_old_sha1[20];\n+\n+\t\tfd = hold_lock_file_for_update(&lock, filename,\n+\t\t\t\t\t       LOCK_DIE_ON_ERROR);\n+\t\tif (fd < 0)\n+\t\t\tdie_errno(_(\"Could not open '%s' for writing\"), filename);\n+\t\tread_ref(pseudoref, actual_old_sha1);\n+\t\tif (hashcmp(actual_old_sha1, old_sha1)) {\n+\t\t\twarning(\"Unexpected sha1 when deleting %s\", pseudoref);\n+\t\t\treturn -1;\n+\t\t}\n+\n+\t\tunlink(filename);\n+\t\trollback_lock_file(&lock);\n+\t} else {\n+\t\tunlink(filename);\n+\t}\n+\n+\treturn 0;\n+}\n+\n int delete_ref(const char *refname, const unsigned char *old_sha1,\n \t       unsigned int flags)\n {\n \tstruct ref_transaction *transaction;\n \tstruct strbuf err = STRBUF_INIT;\n \n+\tif (ref_type(refname) == REF_TYPE_PSEUDOREF)\n+\t\treturn delete_pseudoref(refname, old_sha1);\n+\n \ttransaction = ref_transaction_begin(&err);\n \tif (!transaction ||\n \t    ref_transaction_delete(transaction, refname, old_sha1,\n@@ -3976,17 +4051,25 @@ int update_ref(const char *msg, const char *refname,\n \t       const unsigned char *new_sha1, const unsigned char *old_sha1,\n \t       unsigned int flags, enum action_on_err onerr)\n {\n-\tstruct ref_transaction *t;\n+\tstruct ref_transaction *t = NULL;\n \tstruct strbuf err = STRBUF_INIT;\n+\tint ret = 0;\n \n-\tt = ref_transaction_begin(&err);\n-\tif (!t ||\n-\t    ref_transaction_update(t, refname, new_sha1, old_sha1,\n-\t\t\t\t   flags, msg, &err) ||\n-\t    ref_transaction_commit(t, &err)) {\n+\tif (ref_type(refname) == REF_TYPE_PSEUDOREF) {\n+\t\tret = write_pseudoref(refname, new_sha1, old_sha1, &err);\n+\t} else {\n+\t\tt = ref_transaction_begin(&err);\n+\t\tif (!t ||\n+\t\t    ref_transaction_update(t, refname, new_sha1, old_sha1,\n+\t\t\t\t\t   flags, msg, &err) ||\n+\t\t    ref_transaction_commit(t, &err)) {\n+\t\t\tret = 1;\n+\t\t\tref_transaction_free(t);\n+\t\t}\n+\t}\n+\tif (ret) {\n \t\tconst char *str = \"update_ref failed for ref '%s': %s\";\n \n-\t\tref_transaction_free(t);\n \t\tswitch (onerr) {\n \t\tcase UPDATE_REFS_MSG_ON_ERR:\n \t\t\terror(str, refname, err.buf);\n@@ -4001,7 +4084,8 @@ int update_ref(const char *msg, const char *refname,\n \t\treturn 1;\n \t}\n \tstrbuf_release(&err);\n-\tref_transaction_free(t);\n+\tif (t)\n+\t\tref_transaction_free(t);\n \treturn 0;\n }\n \n-- \n2.0.4.315.gad8727a-twtrsrc\n"},{"id":"266956","messageId":"1438107144-24293-6-git-send-email-dturner@twopensource.com","threadId":"39957","inReplyTo":"1438107144-24293-1-git-send-email-dturner@twopensource.com","subject":"[PATCH v3 5/6] rebase: use update_ref","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2015-07-28T18:12:23Z","receivedAt":"2015-07-28T18:12:23Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"Instead of manually writing a pseudoref (in one case) and shelling out\nto git update-ref (in another), use the update_ref function.  This\nis much simpler.\n\nSigned-off-by: David Turner <dturner@twopensource.com>\n---\n bisect.c | 37 ++++++++-----------------------------\n 1 file changed, 8 insertions(+), 29 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex 857cf59..33ac88d 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -19,7 +19,6 @@ static struct object_id *current_bad_oid;\n \n static const char *argv_checkout[] = {\"checkout\", \"-q\", NULL, \"--\", NULL};\n static const char *argv_show_branch[] = {\"show-branch\", NULL, NULL};\n-static const char *argv_update_ref[] = {\"update-ref\", \"--no-deref\", \"BISECT_HEAD\", NULL, NULL};\n \n static const char *term_bad;\n static const char *term_good;\n@@ -675,34 +674,16 @@ static int is_expected_rev(const struct object_id *oid)\n \treturn res;\n }\n \n-static void mark_expected_rev(char *bisect_rev_hex)\n-{\n-\tint len = strlen(bisect_rev_hex);\n-\tconst char *filename = git_path(\"BISECT_EXPECTED_REV\");\n-\tint fd = open(filename, O_CREAT | O_TRUNC | O_WRONLY, 0600);\n-\n-\tif (fd < 0)\n-\t\tdie_errno(\"could not create file '%s'\", filename);\n-\n-\tbisect_rev_hex[len] = '\\n';\n-\twrite_or_die(fd, bisect_rev_hex, len + 1);\n-\tbisect_rev_hex[len] = '\\0';\n-\n-\tif (close(fd) < 0)\n-\t\tdie(\"closing file %s: %s\", filename, strerror(errno));\n-}\n-\n-static int bisect_checkout(char *bisect_rev_hex, int no_checkout)\n+static int bisect_checkout(const unsigned char *bisect_rev, int no_checkout)\n {\n+\tchar bisect_rev_hex[GIT_SHA1_HEXSZ + 1];\n \n-\tmark_expected_rev(bisect_rev_hex);\n+\tmemcpy(bisect_rev_hex, sha1_to_hex(bisect_rev), GIT_SHA1_HEXSZ + 1);\n+\tupdate_ref(NULL, \"BISECT_EXPECTED_REV\", bisect_rev, NULL, 0, UPDATE_REFS_DIE_ON_ERR);\n \n \targv_checkout[2] = bisect_rev_hex;\n \tif (no_checkout) {\n-\t\targv_update_ref[3] = bisect_rev_hex;\n-\t\tif (run_command_v_opt(argv_update_ref, RUN_GIT_CMD))\n-\t\t\tdie(\"update-ref --no-deref HEAD failed on %s\",\n-\t\t\t    bisect_rev_hex);\n+\t\tupdate_ref(NULL, \"BISECT_HEAD\", bisect_rev, NULL, 0, UPDATE_REFS_DIE_ON_ERR);\n \t} else {\n \t\tint res;\n \t\tres = run_command_v_opt(argv_checkout, RUN_GIT_CMD);\n@@ -804,7 +785,7 @@ static void check_merge_bases(int no_checkout)\n \t\t\thandle_skipped_merge_base(mb);\n \t\t} else {\n \t\t\tprintf(\"Bisecting: a merge base must be tested\\n\");\n-\t\t\texit(bisect_checkout(sha1_to_hex(mb), no_checkout));\n+\t\t\texit(bisect_checkout(mb, no_checkout));\n \t\t}\n \t}\n \n@@ -948,7 +929,6 @@ int bisect_next_all(const char *prefix, int no_checkout)\n \tstruct commit_list *tried;\n \tint reaches = 0, all = 0, nr, steps;\n \tconst unsigned char *bisect_rev;\n-\tchar bisect_rev_hex[GIT_SHA1_HEXSZ + 1];\n \n \tread_bisect_terms(&term_bad, &term_good);\n \tif (read_bisect_refs())\n@@ -986,11 +966,10 @@ int bisect_next_all(const char *prefix, int no_checkout)\n \t}\n \n \tbisect_rev = revs.commits->item->object.sha1;\n-\tmemcpy(bisect_rev_hex, sha1_to_hex(bisect_rev), GIT_SHA1_HEXSZ + 1);\n \n \tif (!hashcmp(bisect_rev, current_bad_oid->hash)) {\n \t\texit_if_skipped_commits(tried, current_bad_oid);\n-\t\tprintf(\"%s is the first %s commit\\n\", bisect_rev_hex,\n+\t\tprintf(\"%s is the first %s commit\\n\", sha1_to_hex(bisect_rev),\n \t\t\tterm_bad);\n \t\tshow_diff_tree(prefix, revs.commits->item);\n \t\t/* This means the bisection process succeeded. */\n@@ -1003,7 +982,7 @@ int bisect_next_all(const char *prefix, int no_checkout)\n \t       \"(roughly %d step%s)\\n\", nr, (nr == 1 ? \"\" : \"s\"),\n \t       steps, (steps == 1 ? \"\" : \"s\"));\n \n-\treturn bisect_checkout(bisect_rev_hex, no_checkout);\n+\treturn bisect_checkout(bisect_rev, no_checkout);\n }\n \n static inline int log2i(int n)\n-- \n2.0.4.315.gad8727a-twtrsrc\n"},{"id":"266959","messageId":"1438107144-24293-7-git-send-email-dturner@twopensource.com","threadId":"39957","inReplyTo":"1438107144-24293-1-git-send-email-dturner@twopensource.com","subject":"[PATCH v3 6/6] sequencer: replace write_cherry_pick_head with update_ref","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2015-07-28T18:12:24Z","receivedAt":"2015-07-28T18:12:24Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"Now update_ref (via write_pseudoref) does almost exactly what\nwrite_cherry_pick_head did, so we can remove write_cherry_pick_head\nand just use update_ref.\n\nSigned-off-by: David Turner <dturner@twopensource.com>\n---\n sequencer.c | 23 ++++-------------------\n 1 file changed, 4 insertions(+), 19 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex c4f4b7d..554a704 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -158,23 +158,6 @@ static void free_message(struct commit *commit, struct commit_message *msg)\n \tunuse_commit_buffer(commit, msg->message);\n }\n \n-static void write_cherry_pick_head(struct commit *commit, const char *pseudoref)\n-{\n-\tconst char *filename;\n-\tint fd;\n-\tstruct strbuf buf = STRBUF_INIT;\n-\n-\tstrbuf_addf(&buf, \"%s\\n\", sha1_to_hex(commit->object.sha1));\n-\n-\tfilename = git_path(\"%s\", pseudoref);\n-\tfd = open(filename, O_WRONLY | O_CREAT, 0666);\n-\tif (fd < 0)\n-\t\tdie_errno(_(\"Could not open '%s' for writing\"), filename);\n-\tif (write_in_full(fd, buf.buf, buf.len) != buf.len || close(fd))\n-\t\tdie_errno(_(\"Could not write to '%s'\"), filename);\n-\tstrbuf_release(&buf);\n-}\n-\n static void print_advice(int show_hint, struct replay_opts *opts)\n {\n \tchar *msg = getenv(\"GIT_CHERRY_PICK_HELP\");\n@@ -607,9 +590,11 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)\n \t * write it at all.\n \t */\n \tif (opts->action == REPLAY_PICK && !opts->no_commit && (res == 0 || res == 1))\n-\t\twrite_cherry_pick_head(commit, \"CHERRY_PICK_HEAD\");\n+\t\tupdate_ref(NULL, \"CHERRY_PICK_HEAD\", commit->object.sha1, NULL,\n+\t\t\t   REF_NODEREF, UPDATE_REFS_DIE_ON_ERR);\n \tif (opts->action == REPLAY_REVERT && ((opts->no_commit && res == 0) || res == 1))\n-\t\twrite_cherry_pick_head(commit, \"REVERT_HEAD\");\n+\t\tupdate_ref(NULL, \"REVERT_HEAD\", commit->object.sha1, NULL,\n+\t\t\t   REF_NODEREF, UPDATE_REFS_DIE_ON_ERR);\n \n \tif (res) {\n \t\terror(opts->action == REPLAY_REVERT\n-- \n2.0.4.315.gad8727a-twtrsrc\n"},{"id":"266966","messageId":"xmqqpp3cds44.fsf@gitster.dls.corp.google.com","threadId":"39957","inReplyTo":"1438107144-24293-3-git-send-email-dturner@twopensource.com","subject":"Re: [PATCH v3 2/6] notes: replace pseudorefs with real refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-28T19:00:59Z","receivedAt":"2015-07-28T19:00:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Turner <dturner@twopensource.com> writes:\n\n> All-caps files like NOTES_MERGE_REF are pseudorefs, and thus are\n> per-worktree.  We don't want multiple notes merges happening at once\n> (in different worktrees), so we want to make these refs true refs.\n>\n> So, we lowercase NOTES_MERGE_REF and friends.  That way, backends\n> that distinguish between pseudorefs and real refs can correctly\n> handle notes merges.\n>\n> This will also enable us to prevent pseudorefs from being updated by\n> the ref update machinery e.g. git update-ref.\n>\n> Signed-off-by: David Turner <dturner@twopensource.com>\n> ---\n\nThis seems to do a bit more than what it claims to do.  As this kind\nof changes are very error-prone to review, I did a bulk replace of\nthe three all-caps NOTES_*THING* and compared the result with what\nthis patch gives to spot this:\n\n>  \t# Fail to finalize merge\n>  \ttest_must_fail git notes merge --commit >output 2>&1 &&\n> -\t# .git/NOTES_MERGE_* must remain\n> -\ttest -f .git/NOTES_MERGE_PARTIAL &&\n> -\ttest -f .git/NOTES_MERGE_REF &&\n> -\ttest -f .git/NOTES_MERGE_WORKTREE/$commit_sha1 &&\n> -\ttest -f .git/NOTES_MERGE_WORKTREE/$commit_sha2 &&\n> -\ttest -f .git/NOTES_MERGE_WORKTREE/$commit_sha3 &&\n> -\ttest -f .git/NOTES_MERGE_WORKTREE/$commit_sha4 &&\n> +\t# .git/notes_merge_* must remain\n> +\tgit rev-parse --verify notes_merge_partial &&\n> +\tgit rev-parse --verify notes_merge_ref &&\n> +\ttest -f .git/notes_merge_worktree/$commit_sha1 &&\n> +\ttest -f .git/notes_merge_worktree/$commit_sha2 &&\n> +\ttest -f .git/notes_merge_worktree/$commit_sha3 &&\n> +\ttest -f .git/notes_merge_worktree/$commit_sha4 &&\n\nThe two \"rev-parse --verify\" looks semi-sensible [*1*];\nnotes_merge_partial is all lowercase and it refers to\n$GIT_DIR/notes_merge_partial, because they are shared across working\ntree. \n\nBut then why are $GIT_DIR/notes_merge_worktree/* still checked with\n\"test -f\"?  If they are not refs or ref-like things, why should they\nbe downcased?  If they are, why not \"rev-parse --verify\"?\n\n[Footnote]\n\n*1* I say \"semi-\" sensible, because it looks ugly.  All ref-like\n    things immediately below $GIT_DIR/ are all-caps by convention.\n    Perhaps it is a better idea to move it under refs/; \"everything\n    under refs/ is shared across working trees\" is probably a much\n    better rule than \"all caps but HEAD is special\".\n"},{"id":"266967","messageId":"xmqqlhe0ds2v.fsf@gitster.dls.corp.google.com","threadId":"39957","inReplyTo":"1438107144-24293-1-git-send-email-dturner@twopensource.com","subject":"Re: [PATCH v3 0/6] pseudorefs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-28T19:01:44Z","receivedAt":"2015-07-28T19:01:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"On top of what work is this series expected to be applied?\n"},{"id":"266968","messageId":"1438110421.18134.15.camel@twopensource.com","threadId":"39957","inReplyTo":"xmqqlhe0ds2v.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v3 0/6] pseudorefs","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2015-07-28T19:07:01Z","receivedAt":"2015-07-28T19:07:01Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Tue, 2015-07-28 at 12:01 -0700, Junio C Hamano wrote:\n> On top of what work is this series expected to be applied?\n\nI think I started from 'next' as of a few days ago:\n\ncommit df7aaa5e3454bbcbb1f142dd6b95b214d0b8efad\nAuthor: Zoë Blade <zoe@bytenoise.co.uk>\nDate:   Tue Jul 21 14:22:46 2015 +0100\n\n    userdiff: add support for Fountain documents\n\nLet me know if you need me to rebase on something else.\n"},{"id":"266970","messageId":"1438111470.18134.24.camel@twopensource.com","threadId":"39957","inReplyTo":"xmqqpp3cds44.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v3 2/6] notes: replace pseudorefs with real refs","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2015-07-28T19:24:30Z","receivedAt":"2015-07-28T19:24:30Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Tue, 2015-07-28 at 12:00 -0700, Junio C Hamano wrote:\n> David Turner <dturner@twopensource.com> writes:\n> \n> > All-caps files like NOTES_MERGE_REF are pseudorefs, and thus are\n> > per-worktree.  We don't want multiple notes merges happening at once\n> > (in different worktrees), so we want to make these refs true refs.\n> >\n> > So, we lowercase NOTES_MERGE_REF and friends.  That way, backends\n> > that distinguish between pseudorefs and real refs can correctly\n> > handle notes merges.\n> >\n> > This will also enable us to prevent pseudorefs from being updated by\n> > the ref update machinery e.g. git update-ref.\n> >\n> > Signed-off-by: David Turner <dturner@twopensource.com>\n> > ---\n> \n> This seems to do a bit more than what it claims to do.  As this kind\n> of changes are very error-prone to review, I did a bulk replace of\n> the three all-caps NOTES_*THING* and compared the result with what\n> this patch gives to spot this:\n> \n> >  \t# Fail to finalize merge\n> >  \ttest_must_fail git notes merge --commit >output 2>&1 &&\n> > -\t# .git/NOTES_MERGE_* must remain\n> > -\ttest -f .git/NOTES_MERGE_PARTIAL &&\n> > -\ttest -f .git/NOTES_MERGE_REF &&\n> > -\ttest -f .git/NOTES_MERGE_WORKTREE/$commit_sha1 &&\n> > -\ttest -f .git/NOTES_MERGE_WORKTREE/$commit_sha2 &&\n> > -\ttest -f .git/NOTES_MERGE_WORKTREE/$commit_sha3 &&\n> > -\ttest -f .git/NOTES_MERGE_WORKTREE/$commit_sha4 &&\n> > +\t# .git/notes_merge_* must remain\n> > +\tgit rev-parse --verify notes_merge_partial &&\n> > +\tgit rev-parse --verify notes_merge_ref &&\n> > +\ttest -f .git/notes_merge_worktree/$commit_sha1 &&\n> > +\ttest -f .git/notes_merge_worktree/$commit_sha2 &&\n> > +\ttest -f .git/notes_merge_worktree/$commit_sha3 &&\n> > +\ttest -f .git/notes_merge_worktree/$commit_sha4 &&\n> \n> The two \"rev-parse --verify\" looks semi-sensible [*1*];\n> notes_merge_partial is all lowercase and it refers to\n> $GIT_DIR/notes_merge_partial, because they are shared across working\n> tree. \n> \n> But then why are $GIT_DIR/notes_merge_worktree/* still checked with\n> \"test -f\"?  If they are not refs or ref-like things, why should they\n> be downcased?  If they are, why not \"rev-parse --verify\"?\n\nThey are downcased for consistency with the other notes_merge_* stuff.\n\n> [Footnote]\n> \n> *1* I say \"semi-\" sensible, because it looks ugly.  All ref-like\n>     things immediately below $GIT_DIR/ are all-caps by convention.\n>     Perhaps it is a better idea to move it under refs/; \"everything\n>     under refs/ is shared across working trees\" is probably a much\n>     better rule than \"all caps but HEAD is special\".\n\nDo you mean move all current pseudorefs?  Or just the notes stuff? \n\nMoving MERGE_HEAD means that if someone upgrades in the middle of a\nmerge, they'll end up confused.  The same is true of the notes stuff,\nbut I imagine that many fewer people use notes than use git merge, so I\nwasn't going to worry about that.  What do you think?\n"},{"id":"266972","messageId":"xmqqegjsdq3n.fsf@gitster.dls.corp.google.com","threadId":"39957","inReplyTo":"xmqqpp3cds44.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v3 2/6] notes: replace pseudorefs with real refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-28T19:44:28Z","receivedAt":"2015-07-28T19:44:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> David Turner <dturner@twopensource.com> writes:\n>\n>> All-caps files like NOTES_MERGE_REF are pseudorefs, and thus are\n>> per-worktree.  We don't want multiple notes merges happening at once\n>> (in different worktrees), so we want to make these refs true refs.\n>> ...\n> The two \"rev-parse --verify\" looks semi-sensible [*1*];\n> notes_merge_partial is all lowercase and it refers to\n> $GIT_DIR/notes_merge_partial, because they are shared across working\n> tree. \n>\n> But then why are $GIT_DIR/notes_merge_worktree/* still checked with\n> \"test -f\"?  If they are not refs or ref-like things, why should they\n> be downcased?  If they are, why not \"rev-parse --verify\"?\n>\n> [Footnote]\n>\n> *1* I say \"semi-\" sensible, because it looks ugly.  All ref-like\n>     things immediately below $GIT_DIR/ are all-caps by convention.\n>     Perhaps it is a better idea to move it under refs/; \"everything\n>     under refs/ is shared across working trees\" is probably a much\n>     better rule than \"all caps but HEAD is special\".\n\nI am not sure if \"we don't want multiple notes merges at once\" is a\nvalid cause for this inconsistency in the first place.  When you try\nto start merging a notes tree in one place (leaving NOTES_MERGE_REF\nand friends in the ref storage shared across worktrees), should you\nbe prevented from merging a different notes tree in another?  Isn't\nthe whole point of having refs/notes/ hierarchy to allow different\nnotes to hang underneath there, and isn't NOTES_MERGE_REF symref\nabout keeping track of which one of them is being worked on in this\nworking tree?\n\nWe don't want multiple usual merges at once in different worktrees,\neither; rather, we don't want conflicting updates to the same branch,\nbe it done by a merge or a single-parent commit, from different\nworktrees.  And the way we protect ourselves is by forbidding two\ncheckout of the same branch by controlling what HEAD can point at.\n\nMaking NOTES_MERGE_REF shared across working trees feels like\nsolving that \"no multiple checkouts\" problem by making HEAD shared\nacross working trees, ensuring that only one of them can have usable\nHEAD.  Instead, shouldn't we be solving the issue by allowing\nNOTES_MERGE_REF in multiple working trees point at different refs\nbut ensuring that there is at most one working tree that updates one\nparticular ref in refs/notes/ hierarchy?  Can't we learn from (and\neven better, reuse) the logic that controls HEAD for this purpose?\n\nI think it is OK for us to admit that the \"notes\" subsystem is not\nquite ready to work well with multiple working tree world yet [*1*],\nand move this series forward without worrying about them.\n\n\n[Footnote]\n\n*1* I am not saying that the notes subsystem can forever stay to be\n    second class citizen that does not know about multiple worktrees\n    or pluggable ref backends.  But my brief scan of builtin/notes.c\n    seems to indicate that there are quite a lot of work to be done\n    to prepare it for these two big changes.  My gut feeling is\n    that:\n\n    - NOTES_MERGE_REF symref is like HEAD, that is per working tree\n      but is controlled across working trees---no two working trees\n      can \"checkout\" the same refs/notes/* tree for conflict\n      resolution at the same time.  It should be all-caps and live\n      directly under $GIT_DIR/.\n\n    - NOTES_MERGE_PARTIAL is like MERGE_HEAD or the index in that it\n      is merely a state of in-progress merge that is private to a\n      single working tree.  $GIT_DIR/NOTES_MERGE_PARTIAL is just\n      fine, and we do not have to change it.\n\n    - NOTES_MERGE_WORKTREE is a temporary area in the filesystem\n      that houses bunch of blob files (i.e. notes contents).  It\n      should be per-working tree but it is not even refs.\n      $GIT_DIR/NOTES_MERGE_WORKTREE is just fine, and we do not have\n      to change it.\n\n    And as to adjusting to the \"ref backend with pseudo ref\" world,\n    I think the code is more-or-less ready, especially because your\n    recent round of this series teaches the \"pseudorefs private to\n    working tree\" to update_ref() and delete_ref(), relieving the\n    ref API users like notes subsystem from having to worry about\n    them.\n\n    So the only potential issue in the notes subsystem is to ensure\n    NOTES_MERGE_REF from multiple working trees do not point at the\n    same thing at the same time, with a similar protection we have\n    for HEAD, I think.\n"},{"id":"267004","messageId":"1438118624-26107-1-git-send-email-dturner@twopensource.com","threadId":"39957","inReplyTo":"xmqqegjsdq3n.fsf@gitster.dls.corp.google.com","subject":"[PATCH] notes: handle multiple worktrees","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2015-07-28T21:23:44Z","receivedAt":"2015-07-28T21:23:44Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"Prevent merges to the same notes branch from different worktrees.\nBefore creating NOTES_MERGE_REF, check NOTES_MERGE_REF using the same\ncode we use to check that two HEADs in different worktrees don't point\nto the same branch.  Modify that code, die_if_checked_out, to take a\n\"head\" ref to examine; previously, it just looked at HEAD.\n\nReported-by: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: David Turner <dturner@twopensource.com>\n---\n branch.c                         | 15 +++++----\n branch.h                         |  2 +-\n builtin/checkout.c               |  2 +-\n builtin/notes.c                  |  2 ++\n builtin/worktree.c               |  2 +-\n t/t3320-notes-merge-worktrees.sh | 71 ++++++++++++++++++++++++++++++++++++++++\n 6 files changed, 84 insertions(+), 10 deletions(-)\n create mode 100755 t/t3320-notes-merge-worktrees.sh\n\ndiff --git a/branch.c b/branch.c\nindex c85be07..60eadc6 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -311,21 +311,22 @@ 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 void check_linked_checkout(const char *head, const char *branch,\n+\t\t\t\t  const char *id)\n {\n \tstruct strbuf sb = STRBUF_INIT;\n \tstruct strbuf path = STRBUF_INIT;\n \tstruct strbuf gitdir = STRBUF_INIT;\n \n \t/*\n-\t * $GIT_COMMON_DIR/HEAD is practically outside\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 */\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, head);\n \telse\n-\t\tstrbuf_addf(&path, \"%s/HEAD\", get_git_common_dir());\n+\t\tstrbuf_addf(&path, \"%s/%s\", get_git_common_dir(), head);\n \n \tif (!strbuf_readlink(&sb, path.buf, 0)) {\n \t\tif (!starts_with(sb.buf, \"refs/\") ||\n@@ -356,13 +357,13 @@ done:\n \tstrbuf_release(&gitdir);\n }\n \n-void die_if_checked_out(const char *branch)\n+void die_if_checked_out(const char *head, const char *branch)\n {\n \tstruct strbuf path = STRBUF_INIT;\n \tDIR *dir;\n \tstruct dirent *d;\n \n-\tcheck_linked_checkout(branch, NULL);\n+\tcheck_linked_checkout(head, branch, NULL);\n \n \tstrbuf_addf(&path, \"%s/worktrees\", get_git_common_dir());\n \tdir = opendir(path.buf);\n@@ -373,7 +374,7 @@ void die_if_checked_out(const char *branch)\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\tcheck_linked_checkout(head, branch, d->d_name);\n \t}\n \tclosedir(dir);\n }\ndiff --git a/branch.h b/branch.h\nindex 58aa45f..3577fe5 100644\n--- a/branch.h\n+++ b/branch.h\n@@ -57,6 +57,6 @@ extern int read_branch_desc(struct strbuf *, const char *branch_name);\n  * worktree and die (with a message describing its checkout location) if\n  * it is.\n  */\n-extern void die_if_checked_out(const char *branch);\n+extern void die_if_checked_out(const char *head, const char *branch);\n \n #endif\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex e1403be..a5e9f2d 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -1107,7 +1107,7 @@ static int checkout_branch(struct checkout_opts *opts,\n \t\tchar *head_ref = resolve_refdup(\"HEAD\", 0, sha1, &flag);\n \t\tif (head_ref &&\n \t\t    (!(flag & REF_ISSYMREF) || strcmp(head_ref, new->path)))\n-\t\t\tdie_if_checked_out(new->path);\n+\t\t\tdie_if_checked_out(\"HEAD\", new->path);\n \t\tfree(head_ref);\n \t}\n \ndiff --git a/builtin/notes.c b/builtin/notes.c\nindex 63f95fc..8794ba1 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 <notes-ref>] [list [<object>]]\"),\n@@ -829,6 +830,7 @@ static int merge(int argc, const char **argv, const char *prefix)\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\tdie_if_checked_out(\"NOTES_MERGE_REF\", default_notes_ref());\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/builtin/worktree.c b/builtin/worktree.c\nindex 430b51e..bfdb90f 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -203,7 +203,7 @@ static int add_worktree(const char *path, const char *refname,\n \telse if (!opts->detach && !strbuf_check_branch_ref(&symref, refname) &&\n \t\t ref_exists(symref.buf)) { /* it's a branch */\n \t\tif (!opts->force)\n-\t\t\tdie_if_checked_out(symref.buf);\n+\t\t\tdie_if_checked_out(\"HEAD\", symref.buf);\n \t} else { /* must be a commit */\n \t\tcommit = lookup_commit_reference_by_name(refname);\n \t\tif (!commit)\ndiff --git a/t/t3320-notes-merge-worktrees.sh b/t/t3320-notes-merge-worktrees.sh\nnew file mode 100755\nindex 0000000..b102c23\n--- /dev/null\n+++ b/t/t3320-notes-merge-worktrees.sh\n@@ -0,0 +1,71 @@\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 \"is already checked out\" 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 -v \"is already checked out\" 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":"267007","messageId":"1438119752.18134.43.camel@twopensource.com","threadId":"39957","inReplyTo":"1438118624-26107-1-git-send-email-dturner@twopensource.com","subject":"Re: [PATCH] notes: handle multiple worktrees","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2015-07-28T21:42:32Z","receivedAt":"2015-07-28T21:42:32Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"Sorry, this one is on top of next.\n\nOn Tue, 2015-07-28 at 17:23 -0400, David Turner wrote:\n> Prevent merges to the same notes branch from different worktrees.\n> Before creating NOTES_MERGE_REF, check NOTES_MERGE_REF using the same\n> code we use to check that two HEADs in different worktrees don't point\n> to the same branch.  Modify that code, die_if_checked_out, to take a\n> \"head\" ref to examine; previously, it just looked at HEAD.\n> \n> Reported-by: Junio C Hamano <gitster@pobox.com>\n> Signed-off-by: David Turner <dturner@twopensource.com>\n> ---\n>  branch.c                         | 15 +++++----\n>  branch.h                         |  2 +-\n>  builtin/checkout.c               |  2 +-\n>  builtin/notes.c                  |  2 ++\n>  builtin/worktree.c               |  2 +-\n>  t/t3320-notes-merge-worktrees.sh | 71 ++++++++++++++++++++++++++++++++++++++++\n>  6 files changed, 84 insertions(+), 10 deletions(-)\n>  create mode 100755 t/t3320-notes-merge-worktrees.sh\n> \n> diff --git a/branch.c b/branch.c\n> index c85be07..60eadc6 100644\n> --- a/branch.c\n> +++ b/branch.c\n> @@ -311,21 +311,22 @@ 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 void check_linked_checkout(const char *head, const char *branch,\n> +\t\t\t\t  const char *id)\n>  {\n>  \tstruct strbuf sb = STRBUF_INIT;\n>  \tstruct strbuf path = STRBUF_INIT;\n>  \tstruct strbuf gitdir = STRBUF_INIT;\n>  \n>  \t/*\n> -\t * $GIT_COMMON_DIR/HEAD is practically outside\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 */\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, head);\n>  \telse\n> -\t\tstrbuf_addf(&path, \"%s/HEAD\", get_git_common_dir());\n> +\t\tstrbuf_addf(&path, \"%s/%s\", get_git_common_dir(), head);\n>  \n>  \tif (!strbuf_readlink(&sb, path.buf, 0)) {\n>  \t\tif (!starts_with(sb.buf, \"refs/\") ||\n> @@ -356,13 +357,13 @@ done:\n>  \tstrbuf_release(&gitdir);\n>  }\n>  \n> -void die_if_checked_out(const char *branch)\n> +void die_if_checked_out(const char *head, const char *branch)\n>  {\n>  \tstruct strbuf path = STRBUF_INIT;\n>  \tDIR *dir;\n>  \tstruct dirent *d;\n>  \n> -\tcheck_linked_checkout(branch, NULL);\n> +\tcheck_linked_checkout(head, branch, NULL);\n>  \n>  \tstrbuf_addf(&path, \"%s/worktrees\", get_git_common_dir());\n>  \tdir = opendir(path.buf);\n> @@ -373,7 +374,7 @@ void die_if_checked_out(const char *branch)\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\tcheck_linked_checkout(head, branch, d->d_name);\n>  \t}\n>  \tclosedir(dir);\n>  }\n> diff --git a/branch.h b/branch.h\n> index 58aa45f..3577fe5 100644\n> --- a/branch.h\n> +++ b/branch.h\n> @@ -57,6 +57,6 @@ extern int read_branch_desc(struct strbuf *, const char *branch_name);\n>   * worktree and die (with a message describing its checkout location) if\n>   * it is.\n>   */\n> -extern void die_if_checked_out(const char *branch);\n> +extern void die_if_checked_out(const char *head, const char *branch);\n>  \n>  #endif\n> diff --git a/builtin/checkout.c b/builtin/checkout.c\n> index e1403be..a5e9f2d 100644\n> --- a/builtin/checkout.c\n> +++ b/builtin/checkout.c\n> @@ -1107,7 +1107,7 @@ static int checkout_branch(struct checkout_opts *opts,\n>  \t\tchar *head_ref = resolve_refdup(\"HEAD\", 0, sha1, &flag);\n>  \t\tif (head_ref &&\n>  \t\t    (!(flag & REF_ISSYMREF) || strcmp(head_ref, new->path)))\n> -\t\t\tdie_if_checked_out(new->path);\n> +\t\t\tdie_if_checked_out(\"HEAD\", new->path);\n>  \t\tfree(head_ref);\n>  \t}\n>  \n> diff --git a/builtin/notes.c b/builtin/notes.c\n> index 63f95fc..8794ba1 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 <notes-ref>] [list [<object>]]\"),\n> @@ -829,6 +830,7 @@ static int merge(int argc, const char **argv, const char *prefix)\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\tdie_if_checked_out(\"NOTES_MERGE_REF\", default_notes_ref());\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());\n> diff --git a/builtin/worktree.c b/builtin/worktree.c\n> index 430b51e..bfdb90f 100644\n> --- a/builtin/worktree.c\n> +++ b/builtin/worktree.c\n> @@ -203,7 +203,7 @@ static int add_worktree(const char *path, const char *refname,\n>  \telse if (!opts->detach && !strbuf_check_branch_ref(&symref, refname) &&\n>  \t\t ref_exists(symref.buf)) { /* it's a branch */\n>  \t\tif (!opts->force)\n> -\t\t\tdie_if_checked_out(symref.buf);\n> +\t\t\tdie_if_checked_out(\"HEAD\", symref.buf);\n>  \t} else { /* must be a commit */\n>  \t\tcommit = lookup_commit_reference_by_name(refname);\n>  \t\tif (!commit)\n> diff --git a/t/t3320-notes-merge-worktrees.sh b/t/t3320-notes-merge-worktrees.sh\n> new file mode 100755\n> index 0000000..b102c23\n> --- /dev/null\n> +++ b/t/t3320-notes-merge-worktrees.sh\n> @@ -0,0 +1,71 @@\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 \"is already checked out\" 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 -v \"is already checked out\" 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"},{"id":"267009","messageId":"xmqqy4i0aqoa.fsf@gitster.dls.corp.google.com","threadId":"39957","inReplyTo":"1438119752.18134.43.camel@twopensource.com","subject":"Re: [PATCH] notes: handle multiple worktrees","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-28T22:00:21Z","receivedAt":"2015-07-28T22:00:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Turner <dturner@twopensource.com> writes:\n\n> Sorry, this one is on top of next.\n\nThanks, but I'd appreciate if you can be more specific next time.\n\nA topic that truly depends on everything in 'next' cannot graduate\nbefore all the others do, but in this particular case, I think this\nchange only needs to depend on Eric's es/worktree-add-cleanup and no\nother topic, so it is more manageable ;-)\n"},{"id":"267010","messageId":"xmqqtwsoaq45.fsf@gitster.dls.corp.google.com","threadId":"39957","inReplyTo":"1438118624-26107-1-git-send-email-dturner@twopensource.com","subject":"Re: [PATCH] notes: handle multiple worktrees","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-28T22:12:26Z","receivedAt":"2015-07-28T22:12:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Turner <dturner@twopensource.com> writes:\n\n> Prevent merges to the same notes branch from different worktrees.\n> Before creating NOTES_MERGE_REF, check NOTES_MERGE_REF using the same\n> code we use to check that two HEADs in different worktrees don't point\n> to the same branch.  Modify that code, die_if_checked_out, to take a\n> \"head\" ref to examine; previously, it just looked at HEAD.\n>\n> Reported-by: Junio C Hamano <gitster@pobox.com>\n> Signed-off-by: David Turner <dturner@twopensource.com>\n> ---\n\nThanks for following through.  As I didn't report anything, I do not\ndeserve that label, but it's OK ;-)\n\nI know that it is a requirement to protect NOTES_MERGE_REF from\nbeing used by multiple places for \"notes merge\" to play well with\nthe multi-worktree world order, but because I do not know if that is\nsufficient, I'm asking a few people for further review.\n\nThis is a totally unrelated tangent, but I wonder if \"branch -M\" to\nrename the current branch to an existing branch misbehaves in the\nmulti-worktree world.  I do not see any die-if-checked-out call in\nrename_branch() where create_symref() is used to adjust HEAD for the\ncurrent branch.  Duy?\n\n>  branch.c                         | 15 +++++----\n>  branch.h                         |  2 +-\n>  builtin/checkout.c               |  2 +-\n>  builtin/notes.c                  |  2 ++\n>  builtin/worktree.c               |  2 +-\n>  t/t3320-notes-merge-worktrees.sh | 71 ++++++++++++++++++++++++++++++++++++++++\n>  6 files changed, 84 insertions(+), 10 deletions(-)\n>  create mode 100755 t/t3320-notes-merge-worktrees.sh\n\n> diff --git a/branch.c b/branch.c\n> index c85be07..60eadc6 100644\n> --- a/branch.c\n> +++ b/branch.c\n> @@ -311,21 +311,22 @@ 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 void check_linked_checkout(const char *head, const char *branch,\n> +\t\t\t\t  const char *id)\n>  {\n>  \tstruct strbuf sb = STRBUF_INIT;\n>  \tstruct strbuf path = STRBUF_INIT;\n>  \tstruct strbuf gitdir = STRBUF_INIT;\n>  \n>  \t/*\n> -\t * $GIT_COMMON_DIR/HEAD is practically outside\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 */\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, head);\n>  \telse\n> -\t\tstrbuf_addf(&path, \"%s/HEAD\", get_git_common_dir());\n> +\t\tstrbuf_addf(&path, \"%s/%s\", get_git_common_dir(), head);\n>  \n>  \tif (!strbuf_readlink(&sb, path.buf, 0)) {\n>  \t\tif (!starts_with(sb.buf, \"refs/\") ||\n> @@ -356,13 +357,13 @@ done:\n>  \tstrbuf_release(&gitdir);\n>  }\n>  \n> -void die_if_checked_out(const char *branch)\n> +void die_if_checked_out(const char *head, const char *branch)\n>  {\n>  \tstruct strbuf path = STRBUF_INIT;\n>  \tDIR *dir;\n>  \tstruct dirent *d;\n>  \n> -\tcheck_linked_checkout(branch, NULL);\n> +\tcheck_linked_checkout(head, branch, NULL);\n>  \n>  \tstrbuf_addf(&path, \"%s/worktrees\", get_git_common_dir());\n>  \tdir = opendir(path.buf);\n> @@ -373,7 +374,7 @@ void die_if_checked_out(const char *branch)\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\tcheck_linked_checkout(head, branch, d->d_name);\n>  \t}\n>  \tclosedir(dir);\n>  }\n> diff --git a/branch.h b/branch.h\n> index 58aa45f..3577fe5 100644\n> --- a/branch.h\n> +++ b/branch.h\n> @@ -57,6 +57,6 @@ extern int read_branch_desc(struct strbuf *, const char *branch_name);\n>   * worktree and die (with a message describing its checkout location) if\n>   * it is.\n>   */\n> -extern void die_if_checked_out(const char *branch);\n> +extern void die_if_checked_out(const char *head, const char *branch);\n>  \n>  #endif\n> diff --git a/builtin/checkout.c b/builtin/checkout.c\n> index e1403be..a5e9f2d 100644\n> --- a/builtin/checkout.c\n> +++ b/builtin/checkout.c\n> @@ -1107,7 +1107,7 @@ static int checkout_branch(struct checkout_opts *opts,\n>  \t\tchar *head_ref = resolve_refdup(\"HEAD\", 0, sha1, &flag);\n>  \t\tif (head_ref &&\n>  \t\t    (!(flag & REF_ISSYMREF) || strcmp(head_ref, new->path)))\n> -\t\t\tdie_if_checked_out(new->path);\n> +\t\t\tdie_if_checked_out(\"HEAD\", new->path);\n>  \t\tfree(head_ref);\n>  \t}\n>  \n> diff --git a/builtin/notes.c b/builtin/notes.c\n> index 63f95fc..8794ba1 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 <notes-ref>] [list [<object>]]\"),\n> @@ -829,6 +830,7 @@ static int merge(int argc, const char **argv, const char *prefix)\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\tdie_if_checked_out(\"NOTES_MERGE_REF\", default_notes_ref());\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());\n> diff --git a/builtin/worktree.c b/builtin/worktree.c\n> index 430b51e..bfdb90f 100644\n> --- a/builtin/worktree.c\n> +++ b/builtin/worktree.c\n> @@ -203,7 +203,7 @@ static int add_worktree(const char *path, const char *refname,\n>  \telse if (!opts->detach && !strbuf_check_branch_ref(&symref, refname) &&\n>  \t\t ref_exists(symref.buf)) { /* it's a branch */\n>  \t\tif (!opts->force)\n> -\t\t\tdie_if_checked_out(symref.buf);\n> +\t\t\tdie_if_checked_out(\"HEAD\", symref.buf);\n>  \t} else { /* must be a commit */\n>  \t\tcommit = lookup_commit_reference_by_name(refname);\n>  \t\tif (!commit)\n> diff --git a/t/t3320-notes-merge-worktrees.sh b/t/t3320-notes-merge-worktrees.sh\n> new file mode 100755\n> index 0000000..b102c23\n> --- /dev/null\n> +++ b/t/t3320-notes-merge-worktrees.sh\n> @@ -0,0 +1,71 @@\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 \"is already checked out\" 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 -v \"is already checked out\" 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"},{"id":"267011","messageId":"CAPig+cRjXjWPrvFdY081HRLPEjdX4kePBgZn+akWGNkycM2Z7w@mail.gmail.com","threadId":"39957","inReplyTo":"1438118624-26107-1-git-send-email-dturner@twopensource.com","subject":"Re: [PATCH] notes: handle multiple worktrees","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-07-28T22:17:18Z","receivedAt":"2015-07-28T22:17:18Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Jul 28, 2015 at 5:23 PM, David Turner <dturner@twopensource.com> wrote:\n> Prevent merges to the same notes branch from different worktrees.\n> Before creating NOTES_MERGE_REF, check NOTES_MERGE_REF using the same\n> code we use to check that two HEADs in different worktrees don't point\n> to the same branch.  Modify that code, die_if_checked_out, to take a\n> \"head\" ref to examine; previously, it just looked at HEAD.\n\ndie_if_checked_out() seems to be grossly misnamed following this\nchange since a branch (or detached head) can be \"checked out\", but\nasking it if a HEAD or NOTES_MERGE_REF is \"checked out\" sounds weird.\nPerhaps rename it to die_if_symref_shared() or something?\n\nThe new argument order, die_if_checked_out(\"HEAD\", \"branchname\"), also\nseems backward to me, at least with the current function name\n(die_if_checked_out). With a better function name, maybe it wouldn't\nbe so bad.\n\nI could also see die_if_checked_out() being refactored to move the\nunderlying logic to a new (better named) function for checking the\nvalue of a particular file (HEAD, NOTES_MERGE_REF) in each linked\nworktree. And then die_if_checked_out() would be a thin wrapper over\nthat new function. A reason for preferring this approach and keeping\ndie_if_checked_out() as a convenience wrapper is that it's likely that\nit's going to be needed in more places in the future.\n\n> Reported-by: Junio C Hamano <gitster@pobox.com>\n> Signed-off-by: David Turner <dturner@twopensource.com>\n> ---\n> diff --git a/branch.c b/branch.c\n> index c85be07..60eadc6 100644\n> --- a/branch.c\n> +++ b/branch.c\n> @@ -311,21 +311,22 @@ void remove_branch_state(void)\n>         unlink(git_path(\"SQUASH_MSG\"));\n>  }\n>\n> -static void check_linked_checkout(const char *branch, const char *id)\n> +static void check_linked_checkout(const char *head, const char *branch,\n> +                                 const char *id)\n>  {\n>         struct strbuf sb = STRBUF_INIT;\n>         struct strbuf path = STRBUF_INIT;\n>         struct strbuf gitdir = STRBUF_INIT;\n>\n>         /*\n> -        * $GIT_COMMON_DIR/HEAD is practically outside\n> +        * $GIT_COMMON_DIR/$head is practically outside\n>          * $GIT_DIR so resolve_ref_unsafe() won't work (it\n>          * uses git_path). Parse the ref ourselves.\n>          */\n>         if (id)\n> -               strbuf_addf(&path, \"%s/worktrees/%s/HEAD\", get_git_common_dir(), id);\n> +               strbuf_addf(&path, \"%s/worktrees/%s/%s\", get_git_common_dir(), id, head);\n>         else\n> -               strbuf_addf(&path, \"%s/HEAD\", get_git_common_dir());\n> +               strbuf_addf(&path, \"%s/%s\", get_git_common_dir(), head);\n>\n>         if (!strbuf_readlink(&sb, path.buf, 0)) {\n>                 if (!starts_with(sb.buf, \"refs/\") ||\n> @@ -356,13 +357,13 @@ done:\n>         strbuf_release(&gitdir);\n>  }\n>\n> -void die_if_checked_out(const char *branch)\n> +void die_if_checked_out(const char *head, const char *branch)\n>  {\n>         struct strbuf path = STRBUF_INIT;\n>         DIR *dir;\n>         struct dirent *d;\n>\n> -       check_linked_checkout(branch, NULL);\n> +       check_linked_checkout(head, branch, NULL);\n>\n>         strbuf_addf(&path, \"%s/worktrees\", get_git_common_dir());\n>         dir = opendir(path.buf);\n> @@ -373,7 +374,7 @@ void die_if_checked_out(const char *branch)\n>         while ((d = readdir(dir)) != NULL) {\n>                 if (!strcmp(d->d_name, \".\") || !strcmp(d->d_name, \"..\"))\n>                         continue;\n> -               check_linked_checkout(branch, d->d_name);\n> +               check_linked_checkout(head, branch, d->d_name);\n>         }\n>         closedir(dir);\n>  }\n> diff --git a/branch.h b/branch.h\n> index 58aa45f..3577fe5 100644\n> --- a/branch.h\n> +++ b/branch.h\n> @@ -57,6 +57,6 @@ extern int read_branch_desc(struct strbuf *, const char *branch_name);\n>   * worktree and die (with a message describing its checkout location) if\n>   * it is.\n>   */\n> -extern void die_if_checked_out(const char *branch);\n> +extern void die_if_checked_out(const char *head, const char *branch);\n>\n>  #endif\n"},{"id":"267013","messageId":"CALKQrgdLC_sLkM3jyH6pnDoY+RTHr9wJR_VH6iirBWmvyHYcVA@mail.gmail.com","threadId":"39957","inReplyTo":"xmqqegjsdq3n.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v3 2/6] notes: replace pseudorefs with real refs","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2015-07-28T22:38:03Z","receivedAt":"2015-07-28T22:38:03Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Tue, Jul 28, 2015 at 9:44 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Junio C Hamano <gitster@pobox.com> writes:\n>> David Turner <dturner@twopensource.com> writes:\n>>> All-caps files like NOTES_MERGE_REF are pseudorefs, and thus are\n>>> per-worktree.  We don't want multiple notes merges happening at once\n>>> (in different worktrees), so we want to make these refs true refs.\n>>> ...\n>> The two \"rev-parse --verify\" looks semi-sensible [*1*];\n>> notes_merge_partial is all lowercase and it refers to\n>> $GIT_DIR/notes_merge_partial, because they are shared across working\n>> tree.\n>>\n>> But then why are $GIT_DIR/notes_merge_worktree/* still checked with\n>> \"test -f\"?  If they are not refs or ref-like things, why should they\n>> be downcased?  If they are, why not \"rev-parse --verify\"?\n>>\n>> [Footnote]\n>>\n>> *1* I say \"semi-\" sensible, because it looks ugly.  All ref-like\n>>     things immediately below $GIT_DIR/ are all-caps by convention.\n>>     Perhaps it is a better idea to move it under refs/; \"everything\n>>     under refs/ is shared across working trees\" is probably a much\n>>     better rule than \"all caps but HEAD is special\".\n>\n> I am not sure if \"we don't want multiple notes merges at once\" is a\n> valid cause for this inconsistency in the first place.  When you try\n> to start merging a notes tree in one place (leaving NOTES_MERGE_REF\n> and friends in the ref storage shared across worktrees), should you\n> be prevented from merging a different notes tree in another?  Isn't\n> the whole point of having refs/notes/ hierarchy to allow different\n> notes to hang underneath there, and isn't NOTES_MERGE_REF symref\n> about keeping track of which one of them is being worked on in this\n> working tree?\n\nI believe the current M.O. for notes merge is to allow only one\nsimultaneous notes merge per _repo_. AFAIR, the notes merge code is\ncompletely unrelated to the work tree (you could probably even do a\nnotes merge in a _bare_ repo), so IINM, the NOTES_MERGE_* refs should\nbe bound to the _repo_, NOT to the worktree.\n\nSo if I understand the pseudoref concept correctly, that should make\nNOTES_MERGE_* \"regular\" refs, and not pseudorefs.\n\n> We don't want multiple usual merges at once in different worktrees,\n> either; rather, we don't want conflicting updates to the same branch,\n> be it done by a merge or a single-parent commit, from different\n> worktrees.  And the way we protect ourselves is by forbidding two\n> checkout of the same branch by controlling what HEAD can point at.\n>\n> Making NOTES_MERGE_REF shared across working trees feels like\n> solving that \"no multiple checkouts\" problem by making HEAD shared\n> across working trees, ensuring that only one of them can have usable\n> HEAD.  Instead, shouldn't we be solving the issue by allowing\n> NOTES_MERGE_REF in multiple working trees point at different refs\n> but ensuring that there is at most one working tree that updates one\n> particular ref in refs/notes/ hierarchy?  Can't we learn from (and\n> even better, reuse) the logic that controls HEAD for this purpose?\n>\n> I think it is OK for us to admit that the \"notes\" subsystem is not\n> quite ready to work well with multiple working tree world yet [*1*],\n> and move this series forward without worrying about them.\n>\n>\n> [Footnote]\n>\n> *1* I am not saying that the notes subsystem can forever stay to be\n>     second class citizen that does not know about multiple worktrees\n>     or pluggable ref backends.  But my brief scan of builtin/notes.c\n>     seems to indicate that there are quite a lot of work to be done\n>     to prepare it for these two big changes.  My gut feeling is\n>     that:\n>\n>     - NOTES_MERGE_REF symref is like HEAD, that is per working tree\n>       but is controlled across working trees---no two working trees\n>       can \"checkout\" the same refs/notes/* tree for conflict\n>       resolution at the same time.\n\nThis sentence does not make sense to me. IMHO, a notes merge has\nnothing to do with the worktree at all. Instead, the notes merge\ncreates its _own_ worktree (under .git/NOTES_MERGE_WORKTREE) when\nneeded (i.e. when there are notes conflicts to be resolved).\n\n>       It should be all-caps and live\n>       directly under $GIT_DIR/.\n\nIt should definitely live under $GIT_DIR (and not inside each\nworktree's .git/). Whether or not it's all-caps is not that important\nto me.\n\n>\n>     - NOTES_MERGE_PARTIAL is like MERGE_HEAD or the index in that it\n>       is merely a state of in-progress merge that is private to a\n>       single working tree.  $GIT_DIR/NOTES_MERGE_PARTIAL is just\n>       fine, and we do not have to change it.\n\nAgreed.\n\n>\n>     - NOTES_MERGE_WORKTREE is a temporary area in the filesystem\n>       that houses bunch of blob files (i.e. notes contents).  It\n>       should be per-working tree but it is not even refs.\n>       $GIT_DIR/NOTES_MERGE_WORKTREE is just fine, and we do not have\n>       to change it.\n\n>From the notes merge perspective, NOTES_MERGE_WORKTREE _is_ the\nworktree. There is no other worktree that is relevant to the notes\nmerge code. So I'm not quite sure how to handle notes merges in the\nmultiple worktree scenario. Do we expect users running notes merges\nsimultaneously from mulitple worktrees? If that is something we need\nto support, why not take it one step further and support simultaneous\nnotes merges from a single worktree?\n\n>\n>     And as to adjusting to the \"ref backend with pseudo ref\" world,\n>     I think the code is more-or-less ready, especially because your\n>     recent round of this series teaches the \"pseudorefs private to\n>     working tree\" to update_ref() and delete_ref(), relieving the\n>     ref API users like notes subsystem from having to worry about\n>     them.\n>\n>     So the only potential issue in the notes subsystem is to ensure\n>     NOTES_MERGE_REF from multiple working trees do not point at the\n>     same thing at the same time, with a similar protection we have\n>     for HEAD, I think.\n\nAgreed. Namespacing the NOTES_MERGE_* files based on the contents of\nNOTES_MERGE_REF is probably the way to go here. I.e. instead of\n\n  .git/NOTES_MERGE_REF\n  .git/NOTES_MERGE_PARTIAL\n  .git/NOTES_MERGE_WORKTREE\n\nyou would have (assuming NOTES_MERGE_REF == refs/notes/foo):\n\n  .git/NOTES_MERGE/refs/notes/foo/REF\n  .git/NOTES_MERGE/refs/notes/foo/PARTIAL\n  .git/NOTES_MERGE/refs/notes/foo/WORKTREE\n\nand then a simultaneous notes-merge on a different notes ref\n(refs/notes/bar) would live in\n\n  .git/NOTES_MERGE/refs/notes/bar/REF\n  .git/NOTES_MERGE/refs/notes/bar/PARTIAL\n  .git/NOTES_MERGE/refs/notes/bar/WORKTREE\n\nAlso, we would need support on the command side, i.e. \"git notes merge\ncommit/abort\" needs to be told _which_ notes merge to commit/abort...\n\nHowever, in any case, notes merges are always per _repo_ and never per\n_worktree_, so this is all unrelated to the current patch/discussion\nAFAICS.\n\n\n...Johan\n\n-- \nJohan Herland, <johan@herland.net>\nwww.herland.net\n"},{"id":"267015","messageId":"CALKQrgcX2aPc4cac2p-w8FE9K7X+A5tFWtNcS-N6EOC9gTvfZw@mail.gmail.com","threadId":"39957","inReplyTo":"xmqqtwsoaq45.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] notes: handle multiple worktrees","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2015-07-28T22:50:56Z","receivedAt":"2015-07-28T22:50:56Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Wed, Jul 29, 2015 at 12:12 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> David Turner <dturner@twopensource.com> writes:\n>> Prevent merges to the same notes branch from different worktrees.\n>> Before creating NOTES_MERGE_REF, check NOTES_MERGE_REF using the same\n>> code we use to check that two HEADs in different worktrees don't point\n>> to the same branch.  Modify that code, die_if_checked_out, to take a\n>> \"head\" ref to examine; previously, it just looked at HEAD.\n>>\n>> Reported-by: Junio C Hamano <gitster@pobox.com>\n>> Signed-off-by: David Turner <dturner@twopensource.com>\n>> ---\n>\n> Thanks for following through.  As I didn't report anything, I do not\n> deserve that label, but it's OK ;-)\n>\n> I know that it is a requirement to protect NOTES_MERGE_REF from\n> being used by multiple places for \"notes merge\" to play well with\n> the multi-worktree world order, but because I do not know if that is\n> sufficient, I'm asking a few people for further review.\n\nAs just stated in a related thread, I don't think it makes sense to\nhave NOTES_MERGE_REF per worktree, as the notes merge is always\ncompletely unrelated to the current worktree (or the current branch\nfor that matter). AFAICS this patch is all about handling per-worktree\nNOTES_MERGE_REFs, and as such I'm NAK on this patch. Instead, there\nshould only be one NOTES_MERGE_REF per _repo_, and although we might\nwant to expand to allow multiple concurrent notes merges in the\nfuture, that is still a per-repo, and not a per-worktree thing, hence\ncompletely unrelated to David's current effort.\n\n...Johan\n"},{"id":"267016","messageId":"xmqqpp3bc2u6.fsf@gitster.dls.corp.google.com","threadId":"39957","inReplyTo":"CALKQrgdLC_sLkM3jyH6pnDoY+RTHr9wJR_VH6iirBWmvyHYcVA@mail.gmail.com","subject":"Re: [PATCH v3 2/6] notes: replace pseudorefs with real refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-28T22:52:17Z","receivedAt":"2015-07-28T22:52:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johan Herland <johan@herland.net> writes:\n\nJohan Herland <johan@herland.net> writes:\n\n> However, in any case, notes merges are always per _repo_ and never per\n> _worktree_, so this is all unrelated to the current patch/discussion\n> AFAICS.\n\nThanks for chiming in, but I actually think you are confused.\n\n\"git merge\" is always per _repo_ in the sense that the result of a\nmerge of a topic to the 'master' is recorded in the 'master' which\nis per-repo.  In the multi-worktree world order, that does not\nchange.  What changes is that you could have different worktrees\nthat check out different branches.  Worktree A may have 'master'\nchecked out and do the merge there to update the tip of 'master'.\nBut while worktree A is doing that, worktree B may have 'next'\nchecked out and do an unrelated merge there.  Once worktree A leaves\n'master' by checking out another branch, worktree B is free to check\nout 'master' and do further merges there.  Merging into 'master' is\nper _repo_, but the act of merging is per worktree.\n\nI think merges of refs/notes/commits and refs/notes/someotherthing\nworks exactly the same way.  In worktree A, you may decide to merge\na notes tree refs/notes/commits with somebody else's.  It may\nconflict and you may need to \"lock\" refs/notes/commits from being\ntouched by other worktrees while resolving that, but that does not\nmean other worktrees cannot do a merge of refs/notes/someotherthing\nat all.  The temporary area you use for merging notes, i.e. the\nworking tree as far as notes merge is concerned, is private to\nworktree A and does not need to be seen by other worktrees.\n\nSo while you are working on merging and resolving, that intermediate\nstate is *NOT* per _repo_ at all.  It is at most per worktree (Yes\nyou could extend and have one notes_merge_ref per each refs/notes/*\nref to make it even finer grained to allow more than one notes merge\ngoing on inside a single worktree, but I do not think it is worth\nit).\n"},{"id":"267017","messageId":"CALKQrgfxc83-yjrCWZqC+pyPhbQDgYbrpCDSaBk78YypO=BXOg@mail.gmail.com","threadId":"39957","inReplyTo":"xmqqpp3bc2u6.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v3 2/6] notes: replace pseudorefs with real refs","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2015-07-28T23:43:28Z","receivedAt":"2015-07-28T23:43:28Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Wed, Jul 29, 2015 at 12:52 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Johan Herland <johan@herland.net> writes:\n>> However, in any case, notes merges are always per _repo_ and never per\n>> _worktree_, so this is all unrelated to the current patch/discussion\n>> AFAICS.\n>\n> Thanks for chiming in, but I actually think you are confused.\n>\n> \"git merge\" is always per _repo_ in the sense that the result of a\n> merge of a topic to the 'master' is recorded in the 'master' which\n> is per-repo.  In the multi-worktree world order, that does not\n> change.  What changes is that you could have different worktrees\n> that check out different branches.  Worktree A may have 'master'\n> checked out and do the merge there to update the tip of 'master'.\n> But while worktree A is doing that, worktree B may have 'next'\n> checked out and do an unrelated merge there.  Once worktree A leaves\n> 'master' by checking out another branch, worktree B is free to check\n> out 'master' and do further merges there.  Merging into 'master' is\n> per _repo_, but the act of merging is per worktree.\n\nAgreed thus far.\n\n> I think merges of refs/notes/commits and refs/notes/someotherthing\n> works exactly the same way.  In worktree A, you may decide to merge\n> a notes tree refs/notes/commits with somebody else's.\n\nHere is where we start to differ. I would say that starting a notes\nmerge is completely unrelated to your worktree. Consider this:\n\nWhen you start a \"regular\" (non-notes) merge in worktree A, that merge\nwill \"occupy\" worktree A for the purpose of completing the merge, e.g.\nconflicting files will be checked out inside worktree A, and it is\nobvious that you cannot do other/unrelated things in worktree A until\nyou have resolved the conflicts and completed the merge. As such, a\nregular merge is inextricably bound to a specific worktree.\n\nThis is not the case for notes merges. If I start a notes merge from\nworktree A, there is no \"occupation\" of that worktree. Before the\nnotes merge is resolved, I can do whatever I want in worktree A\n(including checking out a different branch, performing a rebase,\nwhatever...). Instead, the notes merge creates its own worktree (that\nis \"occupied\" until the notes merge is completed), which is completely\nunrelated to worktree A.\n\n>  It may\n> conflict and you may need to \"lock\" refs/notes/commits from being\n> touched by other worktrees while resolving that, but that does not\n> mean other worktrees cannot do a merge of refs/notes/someotherthing\n> at all.\n\nIn principle, I agree that an ongoing notes-merge into\nrefs/notes/someotherthing should be able to coexist with an ongoing\nnotes-merge into refs/notes/commits. However, it does not make sense\nto bind those notes-merges to a specific worktree.\n\nLet's say I have two worktrees, A and B, and from worktree A, I have\nstarted a notes-merge into refs/notes/commits. Now:\n\n - From worktree B I should NOT be able to start another notes-merge\ninto refs/notes/commits.\n\n - From worktree B I SHOULD be able to start another notes-merge into\nrefs/notes/someotherthing\n\nBut this doesn't really have anything to do with worktree B. I can\njust as easily say:\n\n - From worktree A I should NOT be able to start another notes-merge\ninto refs/notes/commits.\n\n - From worktree A I SHOULD be able to start another notes-merge into\nrefs/notes/someotherthing\n\nMy conclusion is therefore that binding a notes merge to a specific\nworktree does not make any sense. Preventing simultaneous notes merges\ninto the same notes ref is something that must be solved per _repo_\n(and not per worktree), and since the worktree plays no part in the\nresolution/completion of the notes merge, it makes more sense to place\nall the notes-merge-related refs/files inside the _repo_, and not\ninside the worktree.\n\nNow, we do not yet support simultaneous notes merges at all, but my\nfollow-on point is that the addition of such support is wholly\nindependent of the multi-worktree support. For now, it would make more\nsense to only allow a single notes-merge across all worktrees. I.e.\nwhen starting a notes-merge from ANY worktree, it should simply fail\nif there is an existing unresolved notes merge (no matter which\nworktree started that unresolved notes merge).\n\n>  The temporary area you use for merging notes, i.e. the\n> working tree as far as notes merge is concerned, is private to\n> worktree A and does not need to be seen by other worktrees.\n\nDisagree. The private area used to resolve notes merges should be per\n_repo_, not per worktree. That is IMHO the only sane way to (in the\nfuture) prevent simultaneous notes merges going into the same notes\nref.\n\n> So while you are working on merging and resolving, that intermediate\n> state is *NOT* per _repo_ at all.  It is at most per worktree (Yes\n> you could extend and have one notes_merge_ref per each refs/notes/*\n> ref to make it even finer grained to allow more than one notes merge\n> going on inside a single worktree, but I do not think it is worth\n> it).\n\nAs stated above, my position is that while you are resolving a notes\nmerge, the worktree from which you started that notes merge is\ncompletely irrelevant. In fact, you can easily do a notes merge in a\n_bare_ repo...\n\n...Johan\n\n-- \nJohan Herland, <johan@herland.net>\nwww.herland.net\n"},{"id":"267019","messageId":"xmqqlhdzby5o.fsf@gitster.dls.corp.google.com","threadId":"39957","inReplyTo":"CALKQrgfxc83-yjrCWZqC+pyPhbQDgYbrpCDSaBk78YypO=BXOg@mail.gmail.com","subject":"Re: [PATCH v3 2/6] notes: replace pseudorefs with real refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-29T00:33:23Z","receivedAt":"2015-07-29T00:33:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johan Herland <johan@herland.net> writes:\n\n> Here is where we start to differ. I would say that starting a notes\n> merge is completely unrelated to your worktree. Consider this:\n> ...\n> This is not the case for notes merges. If I start a notes merge from\n> worktree A, there is no \"occupation\" of that worktree. Before the\n> notes merge is resolved, I can do whatever I want in worktree A\n> (including checking out a different branch, performing a rebase,\n> whatever...). Instead, the notes merge creates its own worktree (that\n> is \"occupied\" until the notes merge is completed), which is completely\n> unrelated to worktree A.\n\nThat does not mean the notes merge that you started when you were\nsitting in worktree A has to be shared with worktree B, by say doing\n\"vi .git/NOTES_MERGE_WORKTREE/$commit && git notes merge --commit\".\n\nAlso the above does not explain why it is sensible for you to forbid\nworktree B from doing an unrelated notes merge of a different ref\nunder refs/notes/* while your worktree A is doing a notes merge.\n\n> In principle, I agree that an ongoing notes-merge into\n> refs/notes/someotherthing should be able to coexist with an ongoing\n> notes-merge into refs/notes/commits. However, it does not make sense\n> to bind those notes-merges to a specific worktree.\n\nThe thing is, the choice is between per worktree or per repository.\nTaking the latter would mean you can only be doing one notes merge\nat a time, even though you prepared multiple worktrees so that you\ncan work on different things at a time.  It is true that there is\nnothing inherent that ties a note merge to a worktree (a worktree is\ntied to a branch that is checed out, and there is no tie between a\nbranch and a notes tree), but \"not inherantly tied to\" does not\nautomatically mean \"has to be one per repository\".  You'd ideally\nwant to allow N workspaces for N refs/notes/* refs.\n\nBut people work in worktrees, and that is their unit of working\nspace.  From that point of view, unless you are proposing a\ncompletely different design where the primary worktree can be used\nonly for manipulating notes (hence, you can have worktrees for\nresolving refs/notes/A and refs/notes/B, in addition to the other\nworktrees you use to advance branches), treating NOTES_MERGE_REF as\na per-worktree entity just like HEAD and the index is, would be the\nmost sensible comporise.\n\n> Let's say I have two worktrees, A and B, and from worktree A, I have\n> started a notes-merge into refs/notes/commits. Now:\n>\n>  - From worktree B I should NOT be able to start another notes-merge\n> into refs/notes/commits.\n\nCorrect.\n\n>  - From worktree B I SHOULD be able to start another notes-merge into\n> refs/notes/someotherthing\n\nCorrect.\n\n> But this doesn't really have anything to do with worktree B. \n\nCorrect. There is nothing inherent that ties someotherthing to B and\ncommits to A.  The only reason why I think \"per worktree\" is vastly\nsuperiour than \"only one per repository\" is because the end user did\nset up two worktrees so that he can do two things at once\nindependently.\n\n> I can\n> just as easily say:\n>\n>  - From worktree A I should NOT be able to start another notes-merge\n> into refs/notes/commits.\n\nCorrect.\n\n>  - From worktree A I SHOULD be able to start another notes-merge into\n> refs/notes/someotherthing\n\nIrrelevant and moving the goalpost.  We are not talking about\nextending capability of the system that does not use multi-worktree.\nWe do not do two regular merges in a single worktree at a time, and\nI do not think it is sensible to claim \"I SHOULD be able to\".\n\n> Now, we do not yet support simultaneous notes merges at all, but my\n> follow-on point is that the addition of such support is wholly\n> independent of the multi-worktree support.\n\nNow we are getting somewhere.  So is there more that is needed than\nseparating NOTES_MERGE_REF per worktree to make this work (remember,\nmultiple notes-merge in a single worktree is a non-goal, just like\nmultiple merge in a single worktree is not supported today and will\nnot be)?  Is there some other state that is not captured by\nNOTES_MERGE_REF and friends that you would end up recording a wrong\nmerge result, if two worktrees that have NOTES_MERGE_REF pointing at\na different ref in refs/notes/* try to do the notes-merge at the\nsame time?\n\n> For now, it would make more\n> sense to only allow a single notes-merge across all worktrees. I.e.\n> when starting a notes-merge from ANY worktree, it should simply fail\n> if there is an existing unresolved notes merge (no matter which\n> worktree started that unresolved notes merge).\n\nI do not understand why we need to have such a restriction.  It is\nlike saying you may have two worktrees but you cannot merge into\nmaster in worktree A if you are doing a merge into next in worktree\nB.  I'd need a clear answer to the question in the previous\nparagraph to say something like \"ah, ok, that limitation (either\nimposed by the current code design, or perhaps reliance of the\nfilesystem, or whatever) I didn't think about, and now what you say\nmakes sense to me\".\n\n> Disagree. The private area used to resolve notes merges should be per\n> _repo_, not per worktree.\n\nI can buy \"the private area for resolving refs/notes/common must be\na single place per repo that is differnet from the private area used\nfor refs/notes/somethingelse, which would also have its own single\nplace per repo\".\n\nBut then what you are talkinng about is per _ref_, not per _repo_.\n\nAnd I'd very much love to see this per _ref_; that is fantastic.\n\nBecause the suggestion is to allow a single notes ref refs/notes/*\nto be \"checked out\" only in a single worktree at a time, that per\n_ref_ thing falls out naturally from per _worktree_ arrangement.\n\nAnd that is how people work, by creating a separate worktree, they\ngain one additional workspace.\n"},{"id":"267021","messageId":"55B824BD.8000003@alum.mit.edu","threadId":"39957","inReplyTo":"xmqqlhdzby5o.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v3 2/6] notes: replace pseudorefs with real refs","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2015-07-29T00:56:29Z","receivedAt":"2015-07-29T00:56:29Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Johan Herland <johan@herland.net> writes:\n> Here is where we start to differ. I would say that starting a notes\n> merge is completely unrelated to your worktree. Consider this:\n\nIt sounds like what a notes merge really wants is a new linked worktree\nthat has branch refs/notes/foo checked out:\n\n* This would allow multiple notes merges to take place at the same time\nprovided they target different merge references.\n\n* This would prevent multiple notes merges to the same notes reference\nat the same time by the same mechanism that prevents the same branch\nfrom being checked out in two linked worktrees at the same time.\n\nIt's just a thought; I have no idea whether it is practical...\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\n"},{"id":"267022","messageId":"CA+P7+xrfr7S-Xc3eQeg2bMY3mkPA5sjGWsozv3MwQhAA=ENkhg@mail.gmail.com","threadId":"39957","inReplyTo":"55B824BD.8000003@alum.mit.edu","subject":"Re: [PATCH v3 2/6] notes: replace pseudorefs with real refs","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2015-07-29T01:23:07Z","receivedAt":"2015-07-29T01:23:07Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Tue, Jul 28, 2015 at 5:56 PM, Michael Haggerty <mhagger@alum.mit.edu> wrote:\n> Johan Herland <johan@herland.net> writes:\n>> Here is where we start to differ. I would say that starting a notes\n>> merge is completely unrelated to your worktree. Consider this:\n>\n> It sounds like what a notes merge really wants is a new linked worktree\n> that has branch refs/notes/foo checked out:\n>\n> * This would allow multiple notes merges to take place at the same time\n> provided they target different merge references.\n>\n> * This would prevent multiple notes merges to the same notes reference\n> at the same time by the same mechanism that prevents the same branch\n> from being checked out in two linked worktrees at the same time.\n>\n> It's just a thought; I have no idea whether it is practical...\n>\n> Michael\n>\n\nThat seems far more flexible and robust to me.\n\nRegards,\nJake\n"},{"id":"267023","messageId":"CALKQrgeevWUE6tdPLWRKzRQLj5v5_9NyFqJCBraRArrZB-QNyw@mail.gmail.com","threadId":"39957","inReplyTo":"55B824BD.8000003@alum.mit.edu","subject":"Re: [PATCH v3 2/6] notes: replace pseudorefs with real refs","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2015-07-29T01:24:59Z","receivedAt":"2015-07-29T01:24:59Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Wed, Jul 29, 2015 at 2:56 AM, Michael Haggerty <mhagger@alum.mit.edu> wrote:\n> Johan Herland <johan@herland.net> writes:\n>> Here is where we start to differ. I would say that starting a notes\n>> merge is completely unrelated to your worktree. Consider this:\n>\n> It sounds like what a notes merge really wants is a new linked worktree\n> that has branch refs/notes/foo checked out:\n\nYes, almost. There are some complications with the concept of\n\"checking out\" a notes tree:\n\n - The notes tree fanout must be flattened (so that when merging two\nnote trees with different fanout, conflicting notes (e.g. deadbeef...\nand de/adbeef....) are turned into a file-level conflict in the notes\nmerge worktree (i.e. contents with conflict markers in\n.git/NOTES_MERGE_WORKTREE/deadbeef...).\n\n - Notes trees may be very large (e.g. one note per commit for the\nentire project history), so we want to avoid checking out as many\nnotes as possible. Currently we only checkout the notes that actually\ndo conflict, and keep the rest referenced from NOTES_MERGE_PARTIAL.\n\n> * This would allow multiple notes merges to take place at the same time\n> provided they target different merge references.\n>\n> * This would prevent multiple notes merges to the same notes reference\n> at the same time by the same mechanism that prevents the same branch\n> from being checked out in two linked worktrees at the same time.\n>\n> It's just a thought; I have no idea whether it is practical...\n\nI'm not sure whether it's worth trying to reuse the same linked\nworktree mechanism for notes trees, when (a) the concept of \"checking\nout\" a notes tree is so different, as explained above, and (b)\ncurrently the only use case for a notes worktree is during a notes\nmerge.\n\n\n...Johan\n\n> Michael\n>\n> --\n> Michael Haggerty\n> mhagger@alum.mit.edu\n>\n\n-- \nJohan Herland, <johan@herland.net>\nwww.herland.net\n"},{"id":"267024","messageId":"xmqqh9onbu53.fsf@gitster.dls.corp.google.com","threadId":"39957","inReplyTo":"55B824BD.8000003@alum.mit.edu","subject":"Re: [PATCH v3 2/6] notes: replace pseudorefs with real refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-29T02:00:08Z","receivedAt":"2015-07-29T02:00:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Haggerty <mhagger@alum.mit.edu> writes:\n\n> It sounds like what a notes merge really wants is a new linked worktree\n> that has branch refs/notes/foo checked out:\n>\n> * This would allow multiple notes merges to take place at the same time\n> provided they target different merge references.\n>\n> * This would prevent multiple notes merges to the same notes reference\n> at the same time by the same mechanism that prevents the same branch\n> from being checked out in two linked worktrees at the same time.\n>\n> It's just a thought; I have no idea whether it is practical...\n\nThat was certainly one of the possibilities that crossed my mind.\n\nIn any case, the primary thing I am interested in at this point is\nto unblock David's \"prepare things so that we can put primary refs\nin a different ref backends more easily\" topic, and I've already\nmade my point a few messages ago upstream:\n\n    I think it is OK for us to admit that the \"notes\" subsystem is\n    not quite ready to work well with multiple working tree world\n    yet [*1*], and move this series forward without worrying about\n    them.\n\nSo doing the absolute minimum, leaving the \"now what can we do to\nimprove notes-merge process?\" outside the scope of the topic.\n\nImproving the notes-merge process is also an interesting topic, but\nit is clear that people started thinking about it today ;-), so it\ncan wait without blocking David's work.  The refs/notes/* hierarchy\nwill be handled exactly the same way as regular branches in the\npluggable ref-backends, and how the ephemeral REF_NOTES_REF and\nfriends are represented (some are refs, some may be pseudo-refs,\nsome may be just a filesystem entity) would need to be revamped if\nwe really do the \"improving notes-merge\" thing anyway, so David's\ntopic shouldn't be blocked by the \"possible future tweak\" of the\ncurrent design that hasn't happened yet.  Instead, improving\nnotes-merge can be done on top, potentially undoing and redoing\nDavi'd topic.\n"},{"id":"267025","messageId":"xmqqd1zbbtc4.fsf@gitster.dls.corp.google.com","threadId":"39957","inReplyTo":"CALKQrgfxc83-yjrCWZqC+pyPhbQDgYbrpCDSaBk78YypO=BXOg@mail.gmail.com","subject":"Re: [PATCH v3 2/6] notes: replace pseudorefs with real refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-29T02:17:31Z","receivedAt":"2015-07-29T02:17:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johan Herland <johan@herland.net> writes:\n\n> In fact, you can easily do a notes merge in a _bare_ repo...\n\nYou keep repeating that but I do not think it is relevant at all.\n\nIf you have a bare repository, you either\n\n (1) do not have any worktree associated with it; or\n\n (2) have worktrees associated with it elsewhere, but its parent\n     directory is *not* the root of any worktree.\n\nIf (1), what are found outside GIT_COMMON_DIR (e.g. HEAD) is found\ninside GIT_DIR, so if we leave NOTES_MERGE_REF and friends outside\nGIT_COMMON_DIR and create the NOTES_MERGE_WORKTREE in GIT_DIR, that\nwould work just as fine as it currently does.\n\nIf (2), what are found outside GIT_COMMON_DIR (e.g. HEAD) is still\nfound inside GIT_DIR (that is now per worktree, but for your bare\nrepository, that is the repository directory itself).  Again, if we\nleave NOTES_MERGE_REF and friends outside GIT_COMMON_DIR and create\nthe NOTES_MERGE_WORKTREE in GIT_DIR, that would work just as fine as\nit currently does.\n\nAnd as long as NOTES_MERGE_REF is made per $GIT_DIR (\"per worktree\"\nis the phrase I am refraining deliberately here, because all the\nworktrees have their own private area, and in addition, your bare\nrepository has one, too) that is protected against \"multiple\ncheckout\", all worktrees and your bare repository can perform\nindependent notes-merges.\n\nPerhaps you meant by \"per repo\" to mean \"per $GIT_DIR\" in this\nmessage, and if that is the case, then I think we are in agreement.\n"},{"id":"267026","messageId":"xmqq8u9zbsy5.fsf@gitster.dls.corp.google.com","threadId":"39957","inReplyTo":"CALKQrgeevWUE6tdPLWRKzRQLj5v5_9NyFqJCBraRArrZB-QNyw@mail.gmail.com","subject":"Re: [PATCH v3 2/6] notes: replace pseudorefs with real refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-29T02:25:54Z","receivedAt":"2015-07-29T02:25:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johan Herland <johan@herland.net> writes:\n\n> Yes, almost. There are some complications with the concept of\n> \"checking out\" a notes tree:\n>\n>  - The notes tree fanout must be flattened (so that when merging two\n> note trees with different fanout, conflicting notes (e.g. deadbeef...\n> and de/adbeef....) are turned into a file-level conflict in the notes\n> merge worktree (i.e. contents with conflict markers in\n> .git/NOTES_MERGE_WORKTREE/deadbeef...).\n\nTrue.  I however think Michael was envisioning further into the\nfuture, where the tree-level merges would take care of that detail\nand populate the index to express conflicts using \"canonicalised\"\npaths, removing these fan-out slashes, before externalising the\nconflicted paths that need user's attention on the filesystem.\n\n>  - Notes trees may be very large (e.g. one note per commit for the\n> entire project history), so we want to avoid checking out as many\n> notes as possible. Currently we only checkout the notes that actually\n> do conflict, and keep the rest referenced from NOTES_MERGE_PARTIAL.\n\nThis is a valid point, but nobody forces us to do a full checkout to\nperform a merge.  From day one, our merge machinery is prepared to\nmerge in an empty working tree, only checking out paths that need to\nbe modified by the merge.\n\n>> * This would allow multiple notes merges to take place at the same time\n>> provided they target different merge references.\n>>\n>> * This would prevent multiple notes merges to the same notes reference\n>> at the same time by the same mechanism that prevents the same branch\n>> from being checked out in two linked worktrees at the same time.\n>>\n>> It's just a thought; I have no idea whether it is practical...\n>\n> I'm not sure whether it's worth trying to reuse the same linked\n> worktree mechanism for notes trees, when (a) the concept of \"checking\n> out\" a notes tree is so different, as explained above, and (b)\n> currently the only use case for a notes worktree is during a notes\n> merge.\n\nI have a very similar feeling except that I'd replace \"linked\nworktree mechanism\" with \"checkout mechanism\".  If we were to\nintroduce such a new checkout mechanism that flattens a fan-out\npaths, with \"sparse checkout\"-like behaviour, the current \"checkout\nmechanism\" would not be reused, but the resulting system would\nbenefit from \"linked worktree mechanism\" just as much as the normal\nmultiple worktree layout benefits from it.  You'd want to make sure\nonly one such worktree has the checkout of one refs/notes/* ref,\neverything in refs/* is shared across the repository and its\nworktrees, and certain things like MERGE_HEAD and the index inside\n$GIT_DIR/ are not shared, which are what the linked worktree\nmechanism gives us.\n"},{"id":"267027","messageId":"CALKQrge31G7-Ng8kZmy8=MgB+WQ3-bKimxCxOHOOwUMDuvRBzA@mail.gmail.com","threadId":"39957","inReplyTo":"xmqqlhdzby5o.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v3 2/6] notes: replace pseudorefs with real refs","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2015-07-29T02:34:14Z","receivedAt":"2015-07-29T02:34:14Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Wed, Jul 29, 2015 at 2:33 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Johan Herland <johan@herland.net> writes:\n>\n>> Here is where we start to differ. I would say that starting a notes\n>> merge is completely unrelated to your worktree. Consider this:\n>> ...\n>> This is not the case for notes merges. If I start a notes merge from\n>> worktree A, there is no \"occupation\" of that worktree. Before the\n>> notes merge is resolved, I can do whatever I want in worktree A\n>> (including checking out a different branch, performing a rebase,\n>> whatever...). Instead, the notes merge creates its own worktree (that\n>> is \"occupied\" until the notes merge is completed), which is completely\n>> unrelated to worktree A.\n>\n> That does not mean the notes merge that you started when you were\n> sitting in worktree A has to be shared with worktree B, by say doing\n> \"vi .git/NOTES_MERGE_WORKTREE/$commit && git notes merge --commit\".\n>\n> Also the above does not explain why it is sensible for you to forbid\n> worktree B from doing an unrelated notes merge of a different ref\n> under refs/notes/* while your worktree A is doing a notes merge.\n\nI do not argue that it is sensible to forbid concurrent unrelated\nnotes merges. I argue that using linked worktrees is a poor solution\nfor concurrent unrelated notes merges.\n\nA better solution does not concern itself with worktrees at all, and\ndoes not need to add nonsensical conditions like:\n\n    die_if_checked_out(\"NOTES_MERGE_REF\", default_notes_ref());\n\nA better solution does not need to add complexity to the branch or\nlinked worktree code to deal with notes merges. Instead, it simply\norganizes the notes merge worktrees in such a manner that the correct\nsemantics naturally emerge.\n\nAgain, let's compare the two approaches (against the current situation):\n\nCurrent situation:\n - A single $GIT_COMMON_DIR/NOTES_MERGE_*\n - Concurrent (unrelated or not) notes merges are simply not supported\n\nProposal A (please correct me where I have misunderstood what's proposed):\n - Each worktree has its own $GIT_DIR/NOTES_MERGE_*\n - Concurrent unrelated notes merges are supported, provided that you\ncreate an additional linked worktree to \"host\" each concurrent notes\nmerge.\n - Logic must be added to ensure unrelated-ness, i.e. make sure that\nthe NOTES_MERGE_REF in worktree X is different from all other\nworktrees' NOTES_MERGE_REF.\n\nProposal B:\n - The repo has a $GIT_COMMON_DIR/notes-merge/$ref/* hierarchy for\norganizing concurrent notes merges\n - Concurrent unrelated notes merges are supported, independently of\nwhether you have zero, one, or several worktrees.\n - The notes merge code must be adjusted to work with the above\nhierarchy, and must naturally fail if the user attempts to start a new\nnotes merge that would clobber an in-progress notes merge (only notes\nmerges to the same notes ref will clobber).\n\nI obviously feel proposal B is superior to A, so I wonder what I have\nmissed about A that makes it preferable.\n\n>> In principle, I agree that an ongoing notes-merge into\n>> refs/notes/someotherthing should be able to coexist with an ongoing\n>> notes-merge into refs/notes/commits. However, it does not make sense\n>> to bind those notes-merges to a specific worktree.\n>\n> The thing is, the choice is between per worktree or per repository.\n> Taking the latter would mean you can only be doing one notes merge\n> at a time, even though you prepared multiple worktrees so that you\n> can work on different things at a time.  It is true that there is\n> nothing inherent that ties a note merge to a worktree (a worktree is\n> tied to a branch that is checed out, and there is no tie between a\n> branch and a notes tree), but \"not inherantly tied to\" does not\n> automatically mean \"has to be one per repository\".  You'd ideally\n> want to allow N workspaces for N refs/notes/* refs.\n>\n> But people work in worktrees, and that is their unit of working\n> space.  From that point of view, unless you are proposing a\n> completely different design where the primary worktree can be used\n> only for manipulating notes (hence, you can have worktrees for\n> resolving refs/notes/A and refs/notes/B, in addition to the other\n> worktrees you use to advance branches), treating NOTES_MERGE_REF as\n> a per-worktree entity just like HEAD and the index is, would be the\n> most sensible comporise.\n\nI believe it is a bad compromise. It complicates the code, and it\nprovides a concurrent notes merges that is unnecessarily tied to (and\ndependent on) worktrees. For example, if I wanted to perform N\nconcurrent notes merges in a project that happens to have a huge\nworktree, I would now have to create N copies of the huge worktree...\n\n\n...Johan\n\n-- \nJohan Herland, <johan@herland.net>\nwww.herland.net\n"},{"id":"267028","messageId":"CALKQrgdkGUqjFaX+Z_U5oNL1bGdCn+qSRfQktHXA8q82KwVAHw@mail.gmail.com","threadId":"39957","inReplyTo":"xmqqd1zbbtc4.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v3 2/6] notes: replace pseudorefs with real refs","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2015-07-29T02:37:07Z","receivedAt":"2015-07-29T02:37:07Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Wed, Jul 29, 2015 at 4:17 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Perhaps you meant by \"per repo\" to mean \"per $GIT_DIR\" in this\n> message, and if that is the case, then I think we are in agreement.\n\nNo, by per repo I mean per $GIT_COMMON_DIR (I haven't followed the\nlinked worktree effort, so this language is new to me. My apologies).\n\n...Johan\n\n-- \nJohan Herland, <johan@herland.net>\nwww.herland.net\n"},{"id":"267029","messageId":"CALKQrgff1AKBCsdFBTcthMb61gQwb4CjH=P7TPfpy1unMJ2_=g@mail.gmail.com","threadId":"39957","inReplyTo":"xmqqh9onbu53.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v3 2/6] notes: replace pseudorefs with real refs","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2015-07-29T02:53:39Z","receivedAt":"2015-07-29T02:53:39Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Wed, Jul 29, 2015 at 4:00 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Michael Haggerty <mhagger@alum.mit.edu> writes:\n>> It sounds like what a notes merge really wants is a new linked worktree\n>> that has branch refs/notes/foo checked out:\n>>\n>> * This would allow multiple notes merges to take place at the same time\n>> provided they target different merge references.\n>>\n>> * This would prevent multiple notes merges to the same notes reference\n>> at the same time by the same mechanism that prevents the same branch\n>> from being checked out in two linked worktrees at the same time.\n>>\n>> It's just a thought; I have no idea whether it is practical...\n>\n> That was certainly one of the possibilities that crossed my mind.\n>\n> In any case, the primary thing I am interested in at this point is\n> to unblock David's \"prepare things so that we can put primary refs\n> in a different ref backends more easily\" topic, and I've already\n> made my point a few messages ago upstream:\n>\n>     I think it is OK for us to admit that the \"notes\" subsystem is\n>     not quite ready to work well with multiple working tree world\n>     yet [*1*], and move this series forward without worrying about\n>     them.\n>\n> So doing the absolute minimum, leaving the \"now what can we do to\n> improve notes-merge process?\" outside the scope of the topic.\n\nThat's exactly what I was also trying to do: David's topic should not\nhave to deal with NOTES_MERGE_* at all. Simply leave it all as\nsomething that belongs in $GIT_COMMON_DIR, and let's solve concurrent\nunrelated notes merges as a wholly independent, separate topic.\n\n...Johan\n\n-- \nJohan Herland, <johan@herland.net>\nwww.herland.net\n"},{"id":"267032","messageId":"xmqq4mknblt0.fsf@gitster.dls.corp.google.com","threadId":"39957","inReplyTo":"CALKQrgff1AKBCsdFBTcthMb61gQwb4CjH=P7TPfpy1unMJ2_=g@mail.gmail.com","subject":"Re: [PATCH v3 2/6] notes: replace pseudorefs with real refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-29T05:00:11Z","receivedAt":"2015-07-29T05:00:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johan Herland <johan@herland.net> writes:\n\n> On Wed, Jul 29, 2015 at 4:00 AM, Junio C Hamano <gitster@pobox.com> wrote:\n>\n>> So doing the absolute minimum, leaving the \"now what can we do to\n>> improve notes-merge process?\" outside the scope of the topic.\n>\n> That's exactly what I was also trying to do: David's topic should not\n> have to deal with NOTES_MERGE_* at all. Simply leave it all as\n> something that belongs in $GIT_COMMON_DIR, and let's solve concurrent\n> unrelated notes merges as a wholly independent, separate topic.\n\nHere, it perhaps is showing that you are unfamiliar with the linked\nworktree topic.\n\nThings directly under .git/, like HEAD, MERGE_HEAD, etc., are per\nworktree (i.e. not shared across working trees).  You have to work\nto make them shared, i.e. per repo.  David's earlier one downcased\nthem while still keeing them directly under .git/ but I think it is\nugly and in general a wrong way to mark what is shared and what is\nper worktree.  E.g. index is not shared, even though the name is all\nlowercase.\n\nYou will be updating that mechanism when you truly make the\nnotes-merge a per refs/notes/* ref thing (not per repo, not per\nworktree).  Perhaps you will use refs/notes-merge/$foo that is\nshared across worktrees as the replacement for NOTES_MERGE_REF that\ncurrently is used to merge into refs/notes/$foo, or something like\nthat, to pursue your \"per ref\" goal.  At that point, NOTES_MERGE_REF\nbased design needs to be scrapped anyway; it should not matter if\nNOTES_MERGE_REF is per worktree (not shared) or per repo (shared) in\nthe meantime.\n\nDoing absolute minimum means that $GIT_DIR/NOTES_MERGE_REF should be\nleft per worktree, like MERGE_HEAD, etc., without doing anything\nlike downcasing or moving it under refs/notes-merge/ (which I think\nis a better way to make it shared, if we wanted to), which is\nadditional special casing that will be a wasted effort that would\nnot matter in the end result.\n"},{"id":"267033","messageId":"xmqqzj2fa764.fsf@gitster.dls.corp.google.com","threadId":"39957","inReplyTo":"CALKQrge31G7-Ng8kZmy8=MgB+WQ3-bKimxCxOHOOwUMDuvRBzA@mail.gmail.com","subject":"Re: [PATCH v3 2/6] notes: replace pseudorefs with real refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-29T05:01:39Z","receivedAt":"2015-07-29T05:01:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johan Herland <johan@herland.net> writes:\n\n> I believe it is a bad compromise. It complicates the code, and it\n> provides a concurrent notes merges that is unnecessarily tied to (and\n> dependent on) worktrees. For example, if I wanted to perform N\n> concurrent notes merges in a project that happens to have a huge\n> worktree, I would now have to create N copies of the huge worktree...\n\nWho said worktree has to be populated?  You should be able to have\nan absolutely empty checkout just like \"clone --no-checkout\".\n"},{"id":"267034","messageId":"CAPig+cSRsrmKPLshfn2vpwbk3PV9vJzmTndQT9AHfsVaMmMKug@mail.gmail.com","threadId":"39957","inReplyTo":"1438107144-24293-4-git-send-email-dturner@twopensource.com","subject":"Re: [PATCH v3 3/6] refs: add ref_type function","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-07-29T06:32:46Z","receivedAt":"2015-07-29T06:32:46Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Jul 28, 2015 at 2:12 PM, David Turner <dturner@twopensource.com> wrote:\n> Add a function ref_type, which categorizes refs as per-worktree,\n> pseudoref, or normal ref.\n>\n> Later, we will use this in refs.c to treat pseudorefs specially.\n> Alternate ref backends may use it to treat both pseudorefs and\n> per-worktree refs differently.\n>\n> Signed-off-by: David Turner <dturner@twopensource.com>\n> ---\n> diff --git a/refs.c b/refs.c\n> index 0b96ece..553ae8b 100644\n> --- a/refs.c\n> +++ b/refs.c\n> @@ -2848,6 +2848,35 @@ static int delete_ref_loose(struct ref_lock *lock, int flag, struct strbuf *err)\n>         return 0;\n>  }\n>\n> +static int is_per_worktree_ref(const char *refname)\n> +{\n> +       return !strcmp(refname, \"HEAD\");\n> +}\n> +\n> +static int is_pseudoref_syntax(const char *refname)\n> +{\n> +       const char *c;\n> +\n> +       if (strchr(refname, '/'))\n> +               return 0;\n\nIsn't this redundant? Won't the below for-loop already return 0 if it\nencounters a slash?\n\n> +       for (c = refname; *c; ++c) {\n\nStyle: c++\n\n> +               if (!isupper(*c) && *c != '-' && *c != '_')\n> +                       return 0;\n> +       }\n> +\n> +       return 1;\n> +}\n> +\n> +enum ref_type ref_type(const char *refname)\n> +{\n> +       if (is_per_worktree_ref(refname))\n> +               return REF_TYPE_PER_WORKTREE;\n> +       if (is_pseudoref_syntax(refname))\n> +               return REF_TYPE_PSEUDOREF;\n> +       return REF_TYPE_NORMAL;\n> +}\n"},{"id":"267035","messageId":"CAPig+cSe9jL465CefToBwSLMz9-HfJA6u+Vcc0rszJ=Co-gsBg@mail.gmail.com","threadId":"39957","inReplyTo":"1438107144-24293-5-git-send-email-dturner@twopensource.com","subject":"Re: [PATCH v3 4/6] pseudorefs: create and use pseudoref update and delete functions","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-07-29T06:38:26Z","receivedAt":"2015-07-29T06:38:26Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Jul 28, 2015 at 2:12 PM, David Turner <dturner@twopensource.com> wrote:\n> Pseudorefs should not be updated through the ref transaction\n> API, because alternate ref backends still need to store pseudorefs\n> in GIT_DIR (instead of wherever they store refs).  Instead,\n> change update_ref and delete_ref to call pseudoref-specific\n> functions.\n>\n> Signed-off-by: David Turner <dturner@twopensource.com>\n> ---\n> diff --git a/refs.c b/refs.c\n> index 553ae8b..2bd6aa6 100644\n> --- a/refs.c\n> +++ b/refs.c\n> @@ -2877,12 +2877,87 @@ enum ref_type ref_type(const char *refname)\n> +static int delete_pseudoref(const char *pseudoref, const unsigned char *old_sha1)\n> +{\n> +       static struct lock_file lock;\n> +       const char *filename;\n> +\n> +       filename = git_path(\"%s\", pseudoref);\n> +\n> +       if (old_sha1 && !is_null_sha1(old_sha1)) {\n> +               int fd;\n> +               unsigned char actual_old_sha1[20];\n> +\n> +               fd = hold_lock_file_for_update(&lock, filename,\n> +                                              LOCK_DIE_ON_ERROR);\n> +               if (fd < 0)\n> +                       die_errno(_(\"Could not open '%s' for writing\"), filename);\n> +               read_ref(pseudoref, actual_old_sha1);\n> +               if (hashcmp(actual_old_sha1, old_sha1)) {\n> +                       warning(\"Unexpected sha1 when deleting %s\", pseudoref);\n> +                       return -1;\n\nDoes this need to release the lock file before returning?\n\n> +               }\n> +\n> +               unlink(filename);\n> +               rollback_lock_file(&lock);\n> +       } else {\n> +               unlink(filename);\n> +       }\n> +\n> +       return 0;\n> +}\n"},{"id":"267046","messageId":"CALKQrgfeFrr5SygixW=pJoOCheGR9kUhqwjNyuXGvzJnWU_LyQ@mail.gmail.com","threadId":"39957","inReplyTo":"xmqqzj2fa764.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v3 2/6] notes: replace pseudorefs with real refs","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2015-07-29T13:19:28Z","receivedAt":"2015-07-29T13:19:28Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Wed, Jul 29, 2015 at 7:01 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Johan Herland <johan@herland.net> writes:\n>\n>> I believe it is a bad compromise. It complicates the code, and it\n>> provides a concurrent notes merges that is unnecessarily tied to (and\n>> dependent on) worktrees. For example, if I wanted to perform N\n>> concurrent notes merges in a project that happens to have a huge\n>> worktree, I would now have to create N copies of the huge worktree...\n>\n> Who said worktree has to be populated?  You should be able to have\n> an absolutely empty checkout just like \"clone --no-checkout\".\n\nIMHO that's an insane workaround that only serves to highlight the\nconceptual problems of binding notes merges (as they are implemented\ntoday) to worktrees.\n\nI'm much more excited about Michael's idea of formalising the concept\nof a notes tree checkout (although as already discussed, checking out\na notes tree is different from checking out a \"regular\" tree). Then, a\nnotes merge (as you already suggested) should be implemented by\ncreating a linked worktree whose HEAD is the notes ref being merged\ninto. I believe there are potential complications here as well (where\na notes-merge might behave differently from a regular merge), but\nthose should be solvable.\n\nBut, whatever. This is unrelated to David's current effort, and I\ndon't want to hold that up, so please move along, nothing to see here.\n\n\n...Johan\n\n-- \nJohan Herland, <johan@herland.net>\nwww.herland.net\n"},{"id":"267071","messageId":"xmqqpp3aapj9.fsf@gitster.dls.corp.google.com","threadId":"39957","inReplyTo":"CALKQrgfeFrr5SygixW=pJoOCheGR9kUhqwjNyuXGvzJnWU_LyQ@mail.gmail.com","subject":"Re: [PATCH v3 2/6] notes: replace pseudorefs with real refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-29T16:37:14Z","receivedAt":"2015-07-29T16:37:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johan Herland <johan@herland.net> writes:\n\n> On Wed, Jul 29, 2015 at 7:01 AM, Junio C Hamano <gitster@pobox.com> wrote:\n>> Johan Herland <johan@herland.net> writes:\n>>\n>>> I believe it is a bad compromise. It complicates the code, and it\n>>> provides a concurrent notes merges that is unnecessarily tied to (and\n>>> dependent on) worktrees. For example, if I wanted to perform N\n>>> concurrent notes merges in a project that happens to have a huge\n>>> worktree, I would now have to create N copies of the huge worktree...\n>>\n>> Who said worktree has to be populated?  You should be able to have\n>> an absolutely empty checkout just like \"clone --no-checkout\".\n>\n> IMHO that's an insane workaround that only serves to highlight the\n> conceptual problems of binding notes merges (as they are implemented\n> today) to worktrees.\n\nActually, the name \"linked worktree\" is probably a misnomer.\n\nThere is nothing fundamental in the mechanism or in the concept that\nsays that these multiple $GIT_DIR's must not be a bare one.  The\nmain thing the separation between $GIT_DIR and $GIT_COMMON_DIR\naffords you is that you can have some things shared across them\n(e.g. refs/*, objects) while making others per $GIT_DIR (e.g. HEAD,\nindex, etc.).\n\nWith that in mind, it is not an insane workaround but a very natural\nmechanism suited exactly for what you want to do: using a feature\n(e.g. \"notes merge\") that currently can have at most one instance\nrunning at a time because it stores its state inside $GIT_DIR, and\nyou want to have N concurrent one going.  You keep that \"state per\nrunning instance\" inside $GIT_DIR (i.e. not shared) and use the\n\"linked worktree\" mechanism to have multiple $GIT_DIR, connected\nto the same $GIT_COMMON_DIR.\n\n> But, whatever. This is unrelated to David's current effort, and I\n> don't want to hold that up, so please move along, nothing to see here.\n\nI need this part from an earlier message answered to unblock David's\ntopic:\n\n    Now we are getting somewhere.  So is there more that is needed\n    than separating NOTES_MERGE_REF per worktree to make this work\n    (remember, multiple notes-merge in a single worktree is a\n    non-goal, just like multiple merge in a single worktree is not\n    supported today and will not be)?  Is there some other state\n    that is not captured by NOTES_MERGE_REF and friends that you\n    would end up recording a wrong merge result, if two worktrees\n    that have NOTES_MERGE_REF pointing at a different ref in\n    refs/notes/* try to do the notes-merge at the same time?\n\nIf we do not change anything (not even applying the [v3 2/6] patch\nwe are discussing), all these things prefixed with NOTES_ will\nbecome per $GIT_DIR with linked worktrees.\n\n    NOTES_EDITMSG, NOTES_MERGE_REF, NOTES_MERGE_PARTIAL,\n    NOTES_MERGE_WORKTREE\n\nThe user could attempt to start different notes merges in her\nmultiple $GIT_DIRs.  The question is to what degree we want to\nsupport her.\n\nIs it sufficient to have these per $GIT_DIR, when the user has two\n$GIT_DIRs connected to the same repository and wants to do two\n\"notes merge\" acting on different ref in refs/notes/*?  Or are there\nsome other states in the shared part kept, which would be stomped on\nby simultaneously running \"notes merge\" instances in different\n$GIT_DIRs, that make this not to work?  Any other problems in the\nremainder of the current implementation of \"notes merge\"?\n\nIf there are reasons/limitations that make simultaneous \"notes\nmerge\" of different notes in different $GIT_DIRs impossible, then I\nagree we shouldn't bother with [v3 2/6] patch.  We should just\ndeclare \"do not do it, it does not (yet) work\".\n\nBut if there isn't, [v3 2/6] is the absolute minimum thing we could\ndo to make \"notes merge\" usable by making sure that the user does\nnot attempt merging the same refs/notes/commits in two different\nplaces.\n"},{"id":"267072","messageId":"xmqqio92aokc.fsf@gitster.dls.corp.google.com","threadId":"39957","inReplyTo":"xmqqpp3aapj9.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v3 2/6] notes: replace pseudorefs with real refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-29T16:58:11Z","receivedAt":"2015-07-29T16:58:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> If we do not change anything (not even applying the [v3 2/6] patch\n> we are discussing),...\n\nThis one and...\n\n> If there are reasons/limitations that make simultaneous \"notes\n> merge\" of different notes in different $GIT_DIRs impossible, then I\n> agree we shouldn't bother with [v3 2/6] patch.  We should just\n\n... this one and ...\n\n> declare \"do not do it, it does not (yet) work\".\n>\n> But if there isn't, [v3 2/6] is the absolute minimum thing we could\n\n... this one.  All of these reference to [v3 2/6] are wrong.\n\nThe patch I meant was \"[PATCH] notes: handle multiple worktrees\",\nhttp://thread.gmane.org/gmane.comp.version-control.git/274803/focus=274852\n\nSorry for the confusion.\n\n\n> do to make \"notes merge\" usable by making sure that the user does\n> not attempt merging the same refs/notes/commits in two different\n> places.\n"},{"id":"267121","messageId":"CALKQrgeLMWCKzEnPfv_WeL1-=eDQYtEk=tOejSG4JwPaoEGt=Q@mail.gmail.com","threadId":"39957","inReplyTo":"xmqqpp3aapj9.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v3 2/6] notes: replace pseudorefs with real refs","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2015-07-30T06:05:15Z","receivedAt":"2015-07-30T06:05:15Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Wed, Jul 29, 2015 at 6:37 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Johan Herland <johan@herland.net> writes:\n>> On Wed, Jul 29, 2015 at 7:01 AM, Junio C Hamano <gitster@pobox.com> wrote:\n>>> Johan Herland <johan@herland.net> writes:\n>>>\n>>>> I believe it is a bad compromise. It complicates the code, and it\n>>>> provides a concurrent notes merges that is unnecessarily tied to (and\n>>>> dependent on) worktrees. For example, if I wanted to perform N\n>>>> concurrent notes merges in a project that happens to have a huge\n>>>> worktree, I would now have to create N copies of the huge worktree...\n>>>\n>>> Who said worktree has to be populated?  You should be able to have\n>>> an absolutely empty checkout just like \"clone --no-checkout\".\n>>\n>> IMHO that's an insane workaround that only serves to highlight the\n>> conceptual problems of binding notes merges (as they are implemented\n>> today) to worktrees.\n>\n> Actually, the name \"linked worktree\" is probably a misnomer.\n>\n> There is nothing fundamental in the mechanism or in the concept that\n> says that these multiple $GIT_DIR's must not be a bare one.  The\n> main thing the separation between $GIT_DIR and $GIT_COMMON_DIR\n> affords you is that you can have some things shared across them\n> (e.g. refs/*, objects) while making others per $GIT_DIR (e.g. HEAD,\n> index, etc.).\n>\n> With that in mind, it is not an insane workaround but a very natural\n> mechanism suited exactly for what you want to do: using a feature\n> (e.g. \"notes merge\") that currently can have at most one instance\n> running at a time because it stores its state inside $GIT_DIR, and\n> you want to have N concurrent one going.  You keep that \"state per\n> running instance\" inside $GIT_DIR (i.e. not shared) and use the\n> \"linked worktree\" mechanism to have multiple $GIT_DIR, connected\n> to the same $GIT_COMMON_DIR.\n\nMakes sense, although currently, IINM, those multiple $GIT_DIRs must\nbe associated with strictly different branches, which is completely\nunrelated to the desired notes-merge restriction (which applies to\nnotes refs - not branches). But this has been discussed to death,\nalready.\n\n>> But, whatever. This is unrelated to David's current effort, and I\n>> don't want to hold that up, so please move along, nothing to see here.\n>\n> I need this part from an earlier message answered to unblock David's\n> topic:\n>\n>     Now we are getting somewhere.  So is there more that is needed\n>     than separating NOTES_MERGE_REF per worktree to make this work\n>     (remember, multiple notes-merge in a single worktree is a\n>     non-goal, just like multiple merge in a single worktree is not\n>     supported today and will not be)?  Is there some other state\n>     that is not captured by NOTES_MERGE_REF and friends that you\n>     would end up recording a wrong merge result, if two worktrees\n>     that have NOTES_MERGE_REF pointing at a different ref in\n>     refs/notes/* try to do the notes-merge at the same time?\n\nI believe the answer to both questions is \"No\".\n\n> If we do not change anything (not even applying the \"[PATCH] notes: handle multiple worktrees\" patch\n> we are discussing), all these things prefixed with NOTES_ will\n> become per $GIT_DIR with linked worktrees.\n>\n>     NOTES_EDITMSG, NOTES_MERGE_REF, NOTES_MERGE_PARTIAL,\n>     NOTES_MERGE_WORKTREE\n>\n> The user could attempt to start different notes merges in her\n> multiple $GIT_DIRs.  The question is to what degree we want to\n> support her.\n>\n> Is it sufficient to have these per $GIT_DIR, when the user has two\n> $GIT_DIRs connected to the same repository and wants to do two\n> \"notes merge\" acting on different ref in refs/notes/*?  Or are there\n> some other states in the shared part kept, which would be stomped on\n> by simultaneously running \"notes merge\" instances in different\n> $GIT_DIRs, that make this not to work?  Any other problems in the\n> remainder of the current implementation of \"notes merge\"?\n\nStill, I believe the answer is \"No\".\n\n> If there are reasons/limitations that make simultaneous \"notes\n> merge\" of different notes in different $GIT_DIRs impossible, then I\n> agree we shouldn't bother with \"[PATCH] notes: handle multiple worktrees\" patch.  We should just\n> declare \"do not do it, it does not (yet) work\".\n>\n> But if there isn't, \"[PATCH] notes: handle multiple worktrees\" is the absolute minimum thing we could\n> do to make \"notes merge\" usable by making sure that the user does\n> not attempt merging the same refs/notes/commits in two different\n> places.\n\nSure. There's no point in delaying a patch that works well in practice\njust because I have a delusion of a theoretically cleaner solution\nthat won't make any difference in practice.\n\n\n...Johan\n\n-- \nJohan Herland, <johan@herland.net>\nwww.herland.net\n"},{"id":"267168","messageId":"xmqqoait7gwt.fsf@gitster.dls.corp.google.com","threadId":"39957","inReplyTo":"CALKQrgeLMWCKzEnPfv_WeL1-=eDQYtEk=tOejSG4JwPaoEGt=Q@mail.gmail.com","subject":"Re: [PATCH v3 2/6] notes: replace pseudorefs with real refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-30T16:24:02Z","receivedAt":"2015-07-30T16:24:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johan Herland <johan@herland.net> writes:\n\n>> Actually, the name \"linked worktree\" is probably a misnomer.\n>> ...\n> Makes sense, although currently, IINM, those multiple $GIT_DIRs must\n> be associated with strictly different branches, which is completely\n> unrelated to the desired notes-merge restriction (which applies to\n> notes refs - not branches). But this has been discussed to death,\n> already.\n\nYeah, when we enhance \"linked worktree\" to make it easier to create\n\"bare worktree\", we'd need to make sure it is easy to make them in\nthe detached HEAD state, i.e. not tied to any particular branch.\n\n>> The user could attempt to start different notes merges in her\n>> multiple $GIT_DIRs.  The question is to what degree we want to\n>> support her.\n>>\n>> Is it sufficient to have these per $GIT_DIR, when the user has two\n>> $GIT_DIRs connected to the same repository and wants to do two\n>> \"notes merge\" acting on different ref in refs/notes/*?  Or are there\n>> some other states in the shared part kept, which would be stomped on\n>> by simultaneously running \"notes merge\" instances in different\n>> $GIT_DIRs, that make this not to work?  Any other problems in the\n>> remainder of the current implementation of \"notes merge\"?\n>\n> Still, I believe the answer is \"No\".\n\nGood.  So the following would be a viable way forward.\n\n>> But if there isn't, \"[PATCH] notes: handle multiple worktrees\" is\n>> the absolute minimum thing we could\n>> do to make \"notes merge\" usable by making sure that the user does\n>> not attempt merging the same refs/notes/commits in two different\n>> places.\n>\n> Sure. There's no point in delaying a patch that works well in practice\n> just because I have a delusion of a theoretically cleaner solution\n> that won't make any difference in practice.\n\nI think the part of that patch that refactors the mechanism to\nnotice and disallow symrefs pointing at the same thing (with various\ntweaks suggested during the review) is a good idea in the longer\nterm.  If we were to switch to a \"you can do multiple notes merges\ninside a single repository without any additional linked worktree\"\nparadigm in the future, we would not want (or need) to use that\n\"avoid pointing this symref to the same thing\" code for notes merge\ncodepath, but that is not something we need to decide now.\n"},{"id":"267310","messageId":"CACsJy8B-HV8NS_FCuoy3pvLRr4B7S4CGO4StNxXaB=WLt8=94w@mail.gmail.com","threadId":"39957","inReplyTo":"CALKQrgcX2aPc4cac2p-w8FE9K7X+A5tFWtNcS-N6EOC9gTvfZw@mail.gmail.com","subject":"Re: [PATCH] notes: handle multiple worktrees","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2015-08-03T13:27:45Z","receivedAt":"2015-08-03T13:27:45Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Jul 29, 2015 at 5:50 AM, Johan Herland <johan@herland.net> wrote:\n> On Wed, Jul 29, 2015 at 12:12 AM, Junio C Hamano <gitster@pobox.com> wrote:\n>> David Turner <dturner@twopensource.com> writes:\n>>> Prevent merges to the same notes branch from different worktrees.\n>>> Before creating NOTES_MERGE_REF, check NOTES_MERGE_REF using the same\n>>> code we use to check that two HEADs in different worktrees don't point\n>>> to the same branch.  Modify that code, die_if_checked_out, to take a\n>>> \"head\" ref to examine; previously, it just looked at HEAD.\n>>>\n>>> Reported-by: Junio C Hamano <gitster@pobox.com>\n>>> Signed-off-by: David Turner <dturner@twopensource.com>\n>>> ---\n>>\n>> Thanks for following through.  As I didn't report anything, I do not\n>> deserve that label, but it's OK ;-)\n>>\n>> I know that it is a requirement to protect NOTES_MERGE_REF from\n>> being used by multiple places for \"notes merge\" to play well with\n>> the multi-worktree world order, but because I do not know if that is\n>> sufficient, I'm asking a few people for further review.\n>\n> As just stated in a related thread, I don't think it makes sense to\n> have NOTES_MERGE_REF per worktree, as the notes merge is always\n> completely unrelated to the current worktree (or the current branch\n> for that matter). AFAICS this patch is all about handling per-worktree\n> NOTES_MERGE_REFs, and as such I'm NAK on this patch. Instead, there\n> should only be one NOTES_MERGE_REF per _repo_, and although we might\n> want to expand to allow multiple concurrent notes merges in the\n> future, that is still a per-repo, and not a per-worktree thing, hence\n> completely unrelated to David's current effort.\n\nI agree. Luckily sharing NOTES_MERGE_REF is as short as\n\ndiff --git a/path.c b/path.c\nindex 10f4cbf..52d8ee4 100644\n--- a/path.c\n+++ b/path.c\n@@ -94,7 +94,7 @@ static void replace_dir(struct strbuf *buf, int len,\nconst char *newdir)\n static const char *common_list[] = {\n  \"/branches\", \"/hooks\", \"/info\", \"!/logs\", \"/lost-found\",\n  \"/objects\", \"/refs\", \"/remotes\", \"/worktrees\", \"/rr-cache\", \"/svn\",\n- \"config\", \"!gc.pid\", \"packed-refs\", \"shallow\",\n+ \"config\", \"!gc.pid\", \"packed-refs\", \"shallow\", \"NOTES_MERGE_REF\",\n  NULL\n };\n --\nDuy\n"}]}