{"thread":{"id":"39977","subject":"[PATCH v5 1/5] refs: introduce pseudoref and per-worktree ref concepts","startedAt":"2015-07-31T06:06:17Z","lastAt":"2015-08-11T22:53:43Z","messageCount":13,"participants":["David Turner","Stefan Beller","Duy Nguyen","Junio C Hamano"],"isPatch":true,"patchVersion":5,"patchTotal":5},"messages":[{"id":"267186","messageId":"1438322781-21181-1-git-send-email-dturner@twopensource.com","threadId":"39977","inReplyTo":null,"subject":"[PATCH v5 1/5] refs: introduce pseudoref and per-worktree ref concepts","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2015-07-31T06:06:17Z","receivedAt":"2015-07-31T06:06:17Z","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 \n---\n\nThis version squashes in Junio's glossary and capitalization\ncorrections, and corrects the spelling of pseudoref (thanks to Johan\nHerland).\n\n---\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..8c6478b 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 <>\n \tinterface than the <>.\n \n+[[def_per_worktree_ref]]per-worktree ref::\n+\tRefs that are per-<>, rather than\n+\tglobal. This is presently only <>, 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. Pseudorefs both have names that are all-caps,\n+\tand always start with a line consisting of a\n+\t<> 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<>, 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 <> means to <> it and\n \t<> it. See also linkgit:git-pull[1].\n-- \n2.0.4.315.gad8727a-twtrsrc\n"},{"id":"267187","messageId":"1438322781-21181-2-git-send-email-dturner@twopensource.com","threadId":"39977","inReplyTo":"1438322781-21181-1-git-send-email-dturner@twopensource.com","subject":"[PATCH v5 2/5] refs: add ref_type function","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2015-07-31T06:06:18Z","receivedAt":"2015-07-31T06:06:18Z","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 \n---\n refs.c | 26 ++++++++++++++++++++++++++\n refs.h | 8 ++++++++\n 2 files changed, 34 insertions(+)\n\ndiff --git a/refs.c b/refs.c\nindex 0b96ece..0f87884 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2848,6 +2848,32 @@ 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+\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":"267188","messageId":"1438322781-21181-3-git-send-email-dturner@twopensource.com","threadId":"39977","inReplyTo":"1438322781-21181-1-git-send-email-dturner@twopensource.com","subject":"[PATCH v5 3/5] pseudorefs: create and use pseudoref update and delete functions","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2015-07-31T06:06:19Z","receivedAt":"2015-07-31T06:06:19Z","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 \n---\n refs.c | 101 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------\n 1 file changed, 93 insertions(+), 8 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 0f87884..e6fc3fe 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2874,12 +2874,88 @@ 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\trollback_lock_file(&lock);\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@@ -3973,17 +4049,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@@ -3998,7 +4082,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":"267189","messageId":"1438322781-21181-4-git-send-email-dturner@twopensource.com","threadId":"39977","inReplyTo":"1438322781-21181-1-git-send-email-dturner@twopensource.com","subject":"[PATCH v5 4/5] bisect: use update_ref","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2015-07-31T06:06:20Z","receivedAt":"2015-07-31T06:06:20Z","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 \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":"267190","messageId":"1438322781-21181-5-git-send-email-dturner@twopensource.com","threadId":"39977","inReplyTo":"1438322781-21181-1-git-send-email-dturner@twopensource.com","subject":"[PATCH v5 5/5] sequencer: replace write_cherry_pick_head with update_ref","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2015-07-31T06:06:21Z","receivedAt":"2015-07-31T06:06:21Z","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 \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":"267243","messageId":"CAGZ79kZ-e-GU5FfKWDAUxQPUnyrmGjSvHGnjWn=pwpDAj37umQ@mail.gmail.com","threadId":"39977","inReplyTo":"1438322781-21181-3-git-send-email-dturner@twopensource.com","subject":"Re: [PATCH v5 3/5] pseudorefs: create and use pseudoref update and delete functions","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-07-31T23:40:16Z","receivedAt":"2015-07-31T23:40:16Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"I am sorry for being late to the review, I looked into coverity today as Duy\nbugged me to fix the memory allocation stuff[1]\n\n[1] $gmane/275046\n\nOn Thu, Jul 30, 2015 at 11:06 PM, David Turner wrote:\n\n> +\n> + if (old_sha1) {\n> + unsigned char actual_old_sha1[20];\n> + read_ref(pseudoref, actual_old_sha1);\n\nWhat about the return value of read_ref?\nIn most cases of the code base (19/21) we check the return of that.\nSo maybe\n\n if (read_ref(pseudoref, actual_old_sha1) <0)\n die(\"Could not read ref %s\", pseudoref);\n\n\n> + if (fd < 0)\n> + die_errno(_(\"Could not open '%s' for writing\"), filename);\n> + read_ref(pseudoref, actual_old_sha1);\n\nsame here.\n"},{"id":"267313","messageId":"CACsJy8DK9k67yM_mJqbqgDxNKdGKpoLPOQQ4beqGgzUobzyFUQ@mail.gmail.com","threadId":"39977","inReplyTo":"1438322781-21181-2-git-send-email-dturner@twopensource.com","subject":"Re: [PATCH v5 2/5] refs: add ref_type function","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2015-08-03T13:55:59Z","receivedAt":"2015-08-03T13:55:59Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Jul 31, 2015 at 1:06 PM, David Turner wrote:\n> Add a function ref_type, which categorizes refs as per-worktree,\n> pseudoref, or normal ref.\n\nFor per-worktree refs, you probably should follow common_list[] in\npath.c because that's how file-based ref namespace is splitted between\nper-repo and per-worktree, even though just as simple as \"everything\noutside refs/ is per-worktree\" (with an exception of NOTES_MERGE_REF,\nwhich should be on the list as well). At least the two should be\naligned so that the default file-based backend works the same way as\nnew backends.\n\nGoing further, I think you need to pass the \"worktree identifier\" to\nref backend, at least in ref_transaction_begin_fn. Each backend is\nfree to store per-worktree refs however it wants. Of course if I ask\nfor refs/foo of worktree A, you should not return me refs/foo of\nworktree B. ref_transaction_begin_fn can return a fault code if it\ndoes not support multiple worktrees, which is fine.\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\nI'm not so sure that this can't be hidden behind backends and they can\nhave total control on falling back to file-based, or store them in\nsome secondary storage. I haven't re-read your discussion with Junio\nyet (only skimmed through long ago) so I may be missing some important\npoints.\n\n>\n> Signed-off-by: David Turner \n> ---\n> refs.c | 26 ++++++++++++++++++++++++++\n> refs.h | 8 ++++++++\n> 2 files changed, 34 insertions(+)\n>\n> diff --git a/refs.c b/refs.c\n> index 0b96ece..0f87884 100644\n> --- a/refs.c\n> +++ b/refs.c\n> @@ -2848,6 +2848,32 @@ 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> + for (c = refname; *c; c++) {\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> +\n> int delete_ref(const char *refname, const unsigned char *old_sha1,\n> unsigned int flags)\n> {\n> diff --git a/refs.h b/refs.h\n> index 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> + REF_TYPE_PER_WORKTREE,\n> + REF_TYPE_PSEUDOREF,\n> + REF_TYPE_NORMAL,\n> +};\n> +\n> +enum ref_type ref_type(const char *refname);\n> +\n> enum expire_reflog_flags {\n> EXPIRE_REFLOGS_DRY_RUN = 1 << 0,\n> EXPIRE_REFLOGS_UPDATE_REF = 1 << 1,\n> --\n> 2.0.4.315.gad8727a-twtrsrc\n>\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n\n\n\n-- \nDuy\n"},{"id":"267342","messageId":"1438634689.7348.35.camel@twopensource.com","threadId":"39977","inReplyTo":"CACsJy8DK9k67yM_mJqbqgDxNKdGKpoLPOQQ4beqGgzUobzyFUQ@mail.gmail.com","subject":"Re: [PATCH v5 2/5] refs: add ref_type function","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2015-08-03T20:44:49Z","receivedAt":"2015-08-03T20:44:49Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Mon, 2015-08-03 at 20:55 +0700, Duy Nguyen wrote:\n> On Fri, Jul 31, 2015 at 1:06 PM, David Turner wrote:\n> > Add a function ref_type, which categorizes refs as per-worktree,\n> > pseudoref, or normal ref.\n> \n> For per-worktree refs, you probably should follow common_list[] in\n> path.c because that's how file-based ref namespace is splitted between\n> per-repo and per-worktree, even though just as simple as \"everything\n> outside refs/ is per-worktree\" (with an exception of NOTES_MERGE_REF,\n> which should be on the list as well). At least the two should be\n> aligned so that the default file-based backend works the same way as\n> new backends.\n\nThat would be cleaner, I think. I'm maybe a little worried about\nperformance if we do this, but I guess we could optimize later. \n\nBefore I re-roll, I'll wait until we come to a conclusion on the \nother per-worktree ref thread.\n\nI think we discussed NOTES_MERGE_REF[1], and decided that it should work\nlike HEAD. Does that seem right to you?\n\n> Going further, I think you need to pass the \"worktree identifier\" to\n> ref backend, at least in ref_transaction_begin_fn. Each backend is\n> free to store per-worktree refs however it wants. Of course if I ask\n> for refs/foo of worktree A, you should not return me refs/foo of\n> worktree B. ref_transaction_begin_fn can return a fault code if it\n> does not support multiple worktrees, which is fine.\n\nIf we did that, we would have to add it all over the place -- not just\nref_transaction_begin, but also update_ref. \n\nI think it's better to encapsulate this knowledge inside the refs code.\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> I'm not so sure that this can't be hidden behind backends and they can\n> have total control on falling back to file-based, or store them in\n> some secondary storage. I haven't re-read your discussion with Junio\n> yet (only skimmed through long ago) so I may be missing some important\n> points.\n\nThe worry is symbolic refs -- a symbolic ref might be a per-worktree ref\npointing to a shared ref pointing to a per-worktree ref. This is why\nit's simpler to let backends handle things. If we had some rules about\nthis, we could maybe hide this from the backend, but so far, this was\nthe simplest thing to do (it works great!).\n\n\n[1] http://www.spinics.net/lists/git/msg256793.html\n"},{"id":"267846","messageId":"1439318358.5283.20.camel@twopensource.com","threadId":"39977","inReplyTo":"CACsJy8DK9k67yM_mJqbqgDxNKdGKpoLPOQQ4beqGgzUobzyFUQ@mail.gmail.com","subject":"Re: [PATCH v5 2/5] refs: add ref_type function","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2015-08-11T18:39:18Z","receivedAt":"2015-08-11T18:39:18Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Mon, 2015-08-03 at 20:55 +0700, Duy Nguyen wrote:\n> On Fri, Jul 31, 2015 at 1:06 PM, David Turner wrote:\n> > Add a function ref_type, which categorizes refs as per-worktree,\n> > pseudoref, or normal ref.\n> \n> For per-worktree refs, you probably should follow common_list[] in\n> path.c because that's how file-based ref namespace is splitted between\n> per-repo and per-worktree, even though just as simple as \"everything\n> outside refs/ is per-worktree\" (with an exception of NOTES_MERGE_REF,\n> which should be on the list as well). At least the two should be\n> aligned so that the default file-based backend works the same way as\n> new backends.\n\nI've looked into this, and decided not to follow common_list. That's\nbecause I've hacked the path.c code to treat refs/worktree specially;\nit's under refs (common), but it's per-worktree, so it's special-cased.\nYou may have seen this in the per-worktree-refs-for-bisect thread:\nhttp://permalink.gmane.org/gmane.comp.version-control.git/275673\n\nThis will require some special-casing in the alternate backends, but\nthey can use common the is_per_worktree_ref function. \n"},{"id":"267847","messageId":"1439318789.5283.23.camel@twopensource.com","threadId":"39977","inReplyTo":"CAGZ79kZ-e-GU5FfKWDAUxQPUnyrmGjSvHGnjWn=pwpDAj37umQ@mail.gmail.com","subject":"Re: [PATCH v5 3/5] pseudorefs: create and use pseudoref update and delete functions","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2015-08-11T18:46:29Z","receivedAt":"2015-08-11T18:46:29Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"\nOn Fri, 2015-07-31 at 16:40 -0700, Stefan Beller wrote:\n> I am sorry for being late to the review, I looked into coverity today as Duy\n> bugged me to fix the memory allocation stuff[1]\n\nThanks. Junio, can you pleas substitute the attached patch instead? It\naddresses Stefan's issues. If you prefer, I can resend the whole\nseries, but I thought this might be easier.\n\n\nFrom 6c86c38f533c6b35db3a557270aab95b342875c9 Mon Sep 17 00:00:00 2001\nFrom: David Turner \nDate: Wed, 15 Jul 2015 18:05:28 -0400\nSubject: [PATCH 3/5] pseudorefs: create and use pseudoref update and delete\n functions\n\nPseudorefs 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 \n---\n refs.c | 103 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----\n 1 file changed, 95 insertions(+), 8 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 0f87884..e44b88c 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2874,12 +2874,90 @@ 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\tif (read_ref(pseudoref, actual_old_sha1))\n+\t\t\tdie(\"Could not read ref %s\", pseudoref);\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\tif (read_ref(pseudoref, actual_old_sha1))\n+\t\t\tdie(\"Could not read ref %s\", pseudoref);\n+\t\tif (hashcmp(actual_old_sha1, old_sha1)) {\n+\t\t\twarning(\"Unexpected sha1 when deleting %s\", pseudoref);\n+\t\t\trollback_lock_file(&lock);\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@@ -3973,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@@ -3998,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.4.2.617.g3d4cca8-twtrsrc\n\n"},{"id":"267876","messageId":"xmqqh9o5o3r2.fsf@gitster.dls.corp.google.com","threadId":"39977","inReplyTo":"1439318789.5283.23.camel@twopensource.com","subject":"Re: [PATCH v5 3/5] pseudorefs: create and use pseudoref update and delete functions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-08-11T22:32:33Z","receivedAt":"2015-08-11T22:32:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Turner writes:\n\n> On Fri, 2015-07-31 at 16:40 -0700, Stefan Beller wrote:\n>> I am sorry for being late to the review, I looked into coverity today as Duy\n>> bugged me to fix the memory allocation stuff[1]\n>\n> Thanks. Junio, can you pleas substitute the attached patch instead?\n\nNo. The topic is already in 'next', no?\n"},{"id":"267880","messageId":"xmqqbnedo31z.fsf@gitster.dls.corp.google.com","threadId":"39977","inReplyTo":"xmqqh9o5o3r2.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v5 3/5] pseudorefs: create and use pseudoref update and delete functions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-08-11T22:47:36Z","receivedAt":"2015-08-11T22:47:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> David Turner writes:\n>\n>> On Fri, 2015-07-31 at 16:40 -0700, Stefan Beller wrote:\n>>> I am sorry for being late to the review, I looked into coverity today as Duy\n>>> bugged me to fix the memory allocation stuff[1]\n>>\n>> Thanks. Junio, can you pleas substitute the attached patch instead?\n>\n> No. The topic is already in 'next', no?\n\nYes, the topic is already in 'next'. A follow-up fix would be good.\n\nThe patch didn't apply cleanly on top of 74ec19d^ to replace 74ec19d\nanyway, so I was about to discard it, but after conflict resolution,\nthe interdiff turns out just these two hunks.\n\n-- >8 --\nSubject: pseudoref: check return values from read_ref()\nFrom: David Turner \nDate: Wed, 15 Jul 2015 18:05:28 -0400\n\nThese codepaths attempt to compare the \"expected\" current value with\nthe actual current value, but did not check if we successfully read\nthe current value before comparison.\n\nSigned-off-by: David Turner \nSigned-off-by: Junio C Hamano \n---\n\n * It is likely that we would end up comparing the expected value with\n garbage when the read fails, and the most likely outcome is that\n they do not match and we fail the transaction, which is all fine.\n\n So in that sense, this is not all that urgent, but it is nice to\n fix it when we know the code is not kosher.\n\n refs.c | 7 +++++--\n 1 file changed, 5 insertions(+), 2 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 522b19b..1db3654 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2868,7 +2868,9 @@ static int write_pseudoref(const char *pseudoref, const unsigned char *sha1,\n \n \tif (old_sha1) {\n \t\tunsigned char actual_old_sha1[20];\n-\t\tread_ref(pseudoref, actual_old_sha1);\n+\n+\t\tif (read_ref(pseudoref, actual_old_sha1))\n+\t\t\tdie(\"could not read ref '%s'\", pseudoref);\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@@ -2904,7 +2906,8 @@ static int delete_pseudoref(const char *pseudoref, const unsigned char *old_sha1\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 (read_ref(pseudoref, actual_old_sha1))\n+\t\t\tdie(\"could not read ref '%s'\", pseudoref);\n \t\tif (hashcmp(actual_old_sha1, old_sha1)) {\n \t\t\twarning(\"Unexpected sha1 when deleting %s\", pseudoref);\n \t\t\trollback_lock_file(&lock);\n"},{"id":"267879","messageId":"1439333623.5283.37.camel@twopensource.com","threadId":"39977","inReplyTo":"xmqqbnedo31z.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v5 3/5] pseudorefs: create and use pseudoref update and delete functions","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2015-08-11T22:53:43Z","receivedAt":"2015-08-11T22:53:43Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Tue, 2015-08-11 at 15:47 -0700, Junio C Hamano wrote:\n> Junio C Hamano writes:\n> \n> > David Turner writes:\n> >\n> >> On Fri, 2015-07-31 at 16:40 -0700, Stefan Beller wrote:\n> >>> I am sorry for being late to the review, I looked into coverity today as Duy\n> >>> bugged me to fix the memory allocation stuff[1]\n> >>\n> >> Thanks. Junio, can you pleas substitute the attached patch instead?\n> >\n> > No. The topic is already in 'next', no?\n> \n> Yes, the topic is already in 'next'. A follow-up fix would be good.\n> \n> The patch didn't apply cleanly on top of 74ec19d^ to replace 74ec19d\n> anyway, so I was about to discard it, but after conflict resolution,\n> the interdiff turns out just these two hunks.\n> \n> -- >8 --\n> Subject: pseudoref: check return values from read_ref()\n> From: David Turner \n> Date: Wed, 15 Jul 2015 18:05:28 -0400\n> \n> These codepaths attempt to compare the \"expected\" current value with\n> the actual current value, but did not check if we successfully read\n> the current value before comparison.\n> \n> Signed-off-by: David Turner \n> Signed-off-by: Junio C Hamano \n> ---\n> \n> * It is likely that we would end up comparing the expected value with\n> garbage when the read fails, and the most likely outcome is that\n> they do not match and we fail the transaction, which is all fine.\n> \n> So in that sense, this is not all that urgent, but it is nice to\n> fix it when we know the code is not kosher.\n> \n> refs.c | 7 +++++--\n> 1 file changed, 5 insertions(+), 2 deletions(-)\n> \n> diff --git a/refs.c b/refs.c\n> index 522b19b..1db3654 100644\n> --- a/refs.c\n> +++ b/refs.c\n> @@ -2868,7 +2868,9 @@ static int write_pseudoref(const char *pseudoref, const unsigned char *sha1,\n> \n> \tif (old_sha1) {\n> \t\tunsigned char actual_old_sha1[20];\n> -\t\tread_ref(pseudoref, actual_old_sha1);\n> +\n> +\t\tif (read_ref(pseudoref, actual_old_sha1))\n> +\t\t\tdie(\"could not read ref '%s'\", pseudoref);\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> @@ -2904,7 +2906,8 @@ static int delete_pseudoref(const char *pseudoref, const unsigned char *old_sha1\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 (read_ref(pseudoref, actual_old_sha1))\n> +\t\t\tdie(\"could not read ref '%s'\", pseudoref);\n> \t\tif (hashcmp(actual_old_sha1, old_sha1)) {\n> \t\t\twarning(\"Unexpected sha1 when deleting %s\", pseudoref);\n> \t\t\trollback_lock_file(&lock);\n> \n\nLGTM.\n"}]}