{"thread":{"id":"40756","subject":"[PATCH v7 00/11] refs backend pre-vtable","startedAt":"2015-11-09T17:03:37Z","lastAt":"2015-11-10T07:40:02Z","messageCount":18,"participants":["Michael Haggerty","Ramsay Jones","David Turner"],"isPatch":true,"patchVersion":7,"patchTotal":11},"messages":[{"id":"273085","messageId":"cover.1447085798.git.mhagger@alum.mit.edu","threadId":"40756","inReplyTo":null,"subject":"[PATCH v7 00/11] refs backend pre-vtable","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2015-11-09T17:03:37Z","receivedAt":"2015-11-09T17:03:37Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"This is another reroll of the pre-vtable part of the refs-backend\npatch series dt/refs-backend-pre-vtable. v6 [1] proved cumbersome\nbecause it conflicted messily with lf/ref-is-hidden-namespace [2]. The\nconflicts were partly due to the motion of code across files but, even\nworse, due to the change of order of function definitions between old\nand new files.\n\nSo I have heavily \"optimized\" this reroll for reviewability and to\nminimize conflicts with other work in the area. The only such work\nthat I know of is lf/ref-is-hidden-namespace, which can now be merged\nwith this series *without conflicts*.\n\nChanges since v6:\n\n* It doesn't move refs.c to the refs/ subdirectory, as v6 did.\n Instead, leave the shared code in the existing file, refs.c.\n\n* Don't rename refs.c to refs/files-backend.c and then selectively\n move content back to refs.c. Instead, move content only once and\n only in one direction, namely from refs.c -> refs/files-backend.c\n (in patch 08/11). This is a giant commit but *it makes no other\n changes* so it should be easy to review. (The \"--patience\" option of\n \"git diff\" is quite helpful here. It was turned on when this patch\n series was generated.)\n\n* Preserve the order of code during the move. Aside from a few lines\n of boilerplate, each of the following commands, when applied to\n commit 08, shows only lines being deleted:\n\n git diff --patience $commit^:refs.c $commit:refs.c\n git diff --patience $commit^:refs.c $commit:refs/files-backend.c\n\n* To make all of the above possible, patches 01 through 06 do a little\n bit of preparatory code untangling. These commits have themselves\n been split up a bit to make them as \"obviously correct\" as possible.\n Patch 07 creates the new header file, refs/refs-internal.h, thereby\n increasing the visibility of some declarations.\n\n* The final patches 09, 10, and 11 are not quite as \"obviously\n correct\" as the first eight, but I left them in to keep the logical\n contents of this patch series the same as v6. But these last three\n commits could just as well be postponed until the next tranche of\n patches if that helps speed the way of the first eight patches into\n master.\n\n* I also fixed a commit message and fixed the implementation of the\n new verify_refname_available() function to return a negative number\n on error as documented (previously it returned 1 on error).\n\nI've tried to attribute authorship of these changes as fairly as\npossible based on who initiated the corresponding changes. If anybody\nfeels that I have appropriated his work or, conversely, put words into\nhis mouth, just let me know and I would be happy to adjust the\nauthorship.\n\nIt would be great to get this patch series (at least the first eight\npatches) reviewed and merged as soon as possible. Even though it no\nlonger conflicts with lf/ref-is-hidden-namespace, it is still very\nprone to conflicting with any other work in the references code.\n\nThis patch series is also available on my GitHub fork [3] as branch\n\"refs-backend-pre-vtable\".\n\nMichael\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/280325/focus=280754\n[2] http://article.gmane.org/gmane.comp.version-control.git/281004\n[3] https://github.com/mhagger/git\n\nDavid Turner (5):\n refs: make is_branch public\n copy_msg(): rename to copy_reflog_msg()\n initdb: make safe_create_dir public\n files_log_ref_write: new function\n refs: break out ref conflict checks\n\nMichael Haggerty (4):\n pack_if_possible_fn(): use ref_type() instead of is_per_worktree_ref()\n refname_is_safe(): improve docstring\n refs/refs-internal.h: new header file\n refs: split filesystem-based refs code into a new file\n\nRonnie Sahlberg (2):\n verify_refname_available(): rename function\n verify_refname_available(): new function\n\n Makefile | 3 +-\n builtin/init-db.c | 12 -\n cache.h | 8 +\n path.c | 12 +\n refs.c | 3709 +---------------------------------------\n refs.h | 2 +\n refs.c => refs/files-backend.c | 1287 +-------------\n refs/refs-internal.h | 202 +++\n 8 files changed, 315 insertions(+), 4920 deletions(-)\n copy refs.c => refs/files-backend.c (75%)\n create mode 100644 refs/refs-internal.h\n\n-- \n2.6.2\n"},{"id":"273086","messageId":"39e0a2a708fb1a42b69e45bc5c0b17d565d3662e.1447085798.git.mhagger@alum.mit.edu","threadId":"40756","inReplyTo":"cover.1447085798.git.mhagger@alum.mit.edu","subject":"[PATCH v7 01/11] refs: make is_branch public","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2015-11-09T17:03:38Z","receivedAt":"2015-11-09T17:03:38Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"From: David Turner \n\nis_branch was already non-static, but this patch declares it in the\nheader.\n\nSigned-off-by: Ronnie Sahlberg \nSigned-off-by: David Turner \nSigned-off-by: Junio C Hamano \nSigned-off-by: Michael Haggerty \n---\n refs.h | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/refs.h b/refs.h\nindex 6d30c98..39b8edc 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -217,6 +217,8 @@ extern void warn_dangling_symrefs(FILE *fp, const char *msg_fmt, const struct st\n */\n int pack_refs(unsigned int flags);\n \n+int is_branch(const char *refname);\n+\n /*\n * Flags controlling ref_transaction_update(), ref_transaction_create(), etc.\n * REF_NODEREF: act on the ref directly, instead of dereferencing\n-- \n2.6.2\n"},{"id":"273092","messageId":"178f98e23cf3d515d34e0e739a1d610ad83c1f9a.1447085798.git.mhagger@alum.mit.edu","threadId":"40756","inReplyTo":"cover.1447085798.git.mhagger@alum.mit.edu","subject":"[PATCH v7 02/11] verify_refname_available(): rename function","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2015-11-09T17:03:39Z","receivedAt":"2015-11-09T17:03:39Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"From: Ronnie Sahlberg \n\nRename verify_refname_available() to verify_refname_available_dir() to\nmake the old name available for a more general purpose.\n\nSigned-off-by: Ronnie Sahlberg \nSigned-off-by: David Turner \nSigned-off-by: Junio C Hamano \nSigned-off-by: Michael Haggerty \n---\n refs.c | 44 ++++++++++++++++++++++----------------------\n 1 file changed, 22 insertions(+), 22 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 132eff5..0617e0c 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -279,7 +279,7 @@ struct ref_dir {\n * presence of an empty subdirectory does not block the creation of a\n * similarly-named reference. (The fact that reference names with the\n * same leading components can conflict *with each other* is a\n- * separate issue that is regulated by verify_refname_available().)\n+ * separate issue that is regulated by verify_refname_available_dir().)\n *\n * Please note that the name field contains the fully-qualified\n * reference (or subdirectory) name. Space could be saved by only\n@@ -911,11 +911,11 @@ static int nonmatching_ref_fn(struct ref_entry *entry, void *vdata)\n *\n * extras and skip must be sorted.\n */\n-static int verify_refname_available(const char *refname,\n-\t\t\t\t const struct string_list *extras,\n-\t\t\t\t const struct string_list *skip,\n-\t\t\t\t struct ref_dir *dir,\n-\t\t\t\t struct strbuf *err)\n+static int verify_refname_available_dir(const char *refname,\n+\t\t\t\t\tconst struct string_list *extras,\n+\t\t\t\t\tconst struct string_list *skip,\n+\t\t\t\t\tstruct ref_dir *dir,\n+\t\t\t\t\tstruct strbuf *err)\n {\n \tconst char *slash;\n \tint pos;\n@@ -2465,8 +2465,8 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,\n \t\tstrbuf_git_path(&orig_ref_file, \"%s\", orig_refname);\n \t\tif (remove_empty_directories(&orig_ref_file)) {\n \t\t\tlast_errno = errno;\n-\t\t\tif (!verify_refname_available(orig_refname, extras, skip,\n-\t\t\t\t\t\t get_loose_refs(&ref_cache), err))\n+\t\t\tif (!verify_refname_available_dir(orig_refname, extras, skip,\n+\t\t\t\t\t\t\t get_loose_refs(&ref_cache), err))\n \t\t\t\tstrbuf_addf(err, \"there are still refs under '%s'\",\n \t\t\t\t\t orig_refname);\n \t\t\tgoto error_return;\n@@ -2479,8 +2479,8 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,\n \tif (!refname) {\n \t\tlast_errno = errno;\n \t\tif (last_errno != ENOTDIR ||\n-\t\t !verify_refname_available(orig_refname, extras, skip,\n-\t\t\t\t\t get_loose_refs(&ref_cache), err))\n+\t\t !verify_refname_available_dir(orig_refname, extras, skip,\n+\t\t\t\t\t\t get_loose_refs(&ref_cache), err))\n \t\t\tstrbuf_addf(err, \"unable to resolve reference %s: %s\",\n \t\t\t\t orig_refname, strerror(last_errno));\n \n@@ -2493,8 +2493,8 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,\n \t * our refname.\n \t */\n \tif (is_null_oid(&lock->old_oid) &&\n-\t verify_refname_available(refname, extras, skip,\n-\t\t\t\t get_packed_refs(&ref_cache), err)) {\n+\t verify_refname_available_dir(refname, extras, skip,\n+\t\t\t\t\t get_packed_refs(&ref_cache), err)) {\n \t\tlast_errno = ENOTDIR;\n \t\tgoto error_return;\n \t}\n@@ -3127,10 +3127,10 @@ static int rename_ref_available(const char *oldname, const char *newname)\n \tint ret;\n \n \tstring_list_insert(&skip, oldname);\n-\tret = !verify_refname_available(newname, NULL, &skip,\n-\t\t\t\t\tget_packed_refs(&ref_cache), &err)\n-\t\t&& !verify_refname_available(newname, NULL, &skip,\n-\t\t\t\t\t get_loose_refs(&ref_cache), &err);\n+\tret = !verify_refname_available_dir(newname, NULL, &skip,\n+\t\t\t\t\t get_packed_refs(&ref_cache), &err)\n+\t\t&& !verify_refname_available_dir(newname, NULL, &skip,\n+\t\t\t\t\t\t get_loose_refs(&ref_cache), &err);\n \tif (!ret)\n \t\terror(\"%s\", err.buf);\n \n@@ -4376,12 +4376,12 @@ int initial_ref_transaction_commit(struct ref_transaction *transaction,\n \t\tif ((update->flags & REF_HAVE_OLD) &&\n \t\t !is_null_sha1(update->old_sha1))\n \t\t\tdie(\"BUG: initial ref transaction with old_sha1 set\");\n-\t\tif (verify_refname_available(update->refname,\n-\t\t\t\t\t &affected_refnames, NULL,\n-\t\t\t\t\t loose_refs, err) ||\n-\t\t verify_refname_available(update->refname,\n-\t\t\t\t\t &affected_refnames, NULL,\n-\t\t\t\t\t packed_refs, err)) {\n+\t\tif (verify_refname_available_dir(update->refname,\n+\t\t\t\t\t\t &affected_refnames, NULL,\n+\t\t\t\t\t\t loose_refs, err) ||\n+\t\t verify_refname_available_dir(update->refname,\n+\t\t\t\t\t\t &affected_refnames, NULL,\n+\t\t\t\t\t\t packed_refs, err)) {\n \t\t\tret = TRANSACTION_NAME_CONFLICT;\n \t\t\tgoto cleanup;\n \t\t}\n-- \n2.6.2\n"},{"id":"273087","messageId":"23d05cbc247e5ef9036cb6f20184810745ac540f.1447085798.git.mhagger@alum.mit.edu","threadId":"40756","inReplyTo":"cover.1447085798.git.mhagger@alum.mit.edu","subject":"[PATCH v7 03/11] verify_refname_available(): new function","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2015-11-09T17:03:40Z","receivedAt":"2015-11-09T17:03:40Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"From: Ronnie Sahlberg \n\nAdd a new verify_refname_available() function, which checks whether the\nrefname is available for use, taking all references (both packed and\nloose) into account. This function, unlike the old\nverify_refname_available(), has semantics independent of the choice of\nreference storage, and can therefore be implemented by alternative\nreference backends.\n\nUse the new function in a couple of places.\n\nSigned-off-by: Ronnie Sahlberg \nSigned-off-by: David Turner \nSigned-off-by: Junio C Hamano \nSigned-off-by: Michael Haggerty \n---\n refs.c | 66 ++++++++++++++++++++++++++++++++++++++++--------------------------\n 1 file changed, 40 insertions(+), 26 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 0617e0c..ddcdf81 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -279,7 +279,7 @@ struct ref_dir {\n * presence of an empty subdirectory does not block the creation of a\n * similarly-named reference. (The fact that reference names with the\n * same leading components can conflict *with each other* is a\n- * separate issue that is regulated by verify_refname_available_dir().)\n+ * separate issue that is regulated by verify_refname_available().)\n *\n * Please note that the name field contains the fully-qualified\n * reference (or subdirectory) name. Space could be saved by only\n@@ -897,19 +897,7 @@ static int nonmatching_ref_fn(struct ref_entry *entry, void *vdata)\n /*\n * Return 0 if a reference named refname could be created without\n * conflicting with the name of an existing reference in dir.\n- * Otherwise, return a negative value and write an explanation to err.\n- * If extras is non-NULL, it is a list of additional refnames with\n- * which refname is not allowed to conflict. If skip is non-NULL,\n- * ignore potential conflicts with refs in skip (e.g., because they\n- * are scheduled for deletion in the same operation). Behavior is\n- * undefined if the same name is listed in both extras and skip.\n- *\n- * Two reference names conflict if one of them exactly matches the\n- * leading components of the other; e.g., \"refs/foo/bar\" conflicts\n- * with both \"refs/foo\" and with \"refs/foo/bar/baz\" but not with\n- * \"refs/foo/bar\" or \"refs/foo/barbados\".\n- *\n- * extras and skip must be sorted.\n+ * See verify_refname_available for more information.\n */\n static int verify_refname_available_dir(const char *refname,\n \t\t\t\t\tconst struct string_list *extras,\n@@ -3120,6 +3108,40 @@ out:\n \treturn ret;\n }\n \n+/*\n+ * Return 0 if a reference named refname could be created without\n+ * conflicting with the name of an existing reference. Otherwise,\n+ * return a negative value and write an explanation to err. If extras\n+ * is non-NULL, it is a list of additional refnames with which refname\n+ * is not allowed to conflict. If skip is non-NULL, ignore potential\n+ * conflicts with refs in skip (e.g., because they are scheduled for\n+ * deletion in the same operation). Behavior is undefined if the same\n+ * name is listed in both extras and skip.\n+ *\n+ * Two reference names conflict if one of them exactly matches the\n+ * leading components of the other; e.g., \"foo/bar\" conflicts with\n+ * both \"foo\" and with \"foo/bar/baz\" but not with \"foo/bar\" or\n+ * \"foo/barbados\".\n+ *\n+ * extras and skip must be sorted.\n+ */\n+static int verify_refname_available(const char *newname,\n+\t\t\t\t struct string_list *extras,\n+\t\t\t\t struct string_list *skip,\n+\t\t\t\t struct strbuf *err)\n+{\n+\tstruct ref_dir *packed_refs = get_packed_refs(&ref_cache);\n+\tstruct ref_dir *loose_refs = get_loose_refs(&ref_cache);\n+\n+\tif (verify_refname_available_dir(newname, extras, skip,\n+\t\t\t\t\t packed_refs, err) ||\n+\t verify_refname_available_dir(newname, extras, skip,\n+\t\t\t\t\t loose_refs, err))\n+\t\treturn -1;\n+\n+\treturn 0;\n+}\n+\n static int rename_ref_available(const char *oldname, const char *newname)\n {\n \tstruct string_list skip = STRING_LIST_INIT_NODUP;\n@@ -3127,10 +3149,7 @@ static int rename_ref_available(const char *oldname, const char *newname)\n \tint ret;\n \n \tstring_list_insert(&skip, oldname);\n-\tret = !verify_refname_available_dir(newname, NULL, &skip,\n-\t\t\t\t\t get_packed_refs(&ref_cache), &err)\n-\t\t&& !verify_refname_available_dir(newname, NULL, &skip,\n-\t\t\t\t\t\t get_loose_refs(&ref_cache), &err);\n+\tret = !verify_refname_available(newname, NULL, &skip, &err);\n \tif (!ret)\n \t\terror(\"%s\", err.buf);\n \n@@ -4334,8 +4353,6 @@ static int ref_present(const char *refname,\n int initial_ref_transaction_commit(struct ref_transaction *transaction,\n \t\t\t\t struct strbuf *err)\n {\n-\tstruct ref_dir *loose_refs = get_loose_refs(&ref_cache);\n-\tstruct ref_dir *packed_refs = get_packed_refs(&ref_cache);\n \tint ret = 0, i;\n \tint n = transaction->nr;\n \tstruct ref_update **updates = transaction->updates;\n@@ -4376,12 +4393,9 @@ int initial_ref_transaction_commit(struct ref_transaction *transaction,\n \t\tif ((update->flags & REF_HAVE_OLD) &&\n \t\t !is_null_sha1(update->old_sha1))\n \t\t\tdie(\"BUG: initial ref transaction with old_sha1 set\");\n-\t\tif (verify_refname_available_dir(update->refname,\n-\t\t\t\t\t\t &affected_refnames, NULL,\n-\t\t\t\t\t\t loose_refs, err) ||\n-\t\t verify_refname_available_dir(update->refname,\n-\t\t\t\t\t\t &affected_refnames, NULL,\n-\t\t\t\t\t\t packed_refs, err)) {\n+\t\tif (verify_refname_available(update->refname,\n+\t\t\t\t\t &affected_refnames, NULL,\n+\t\t\t\t\t err)) {\n \t\t\tret = TRANSACTION_NAME_CONFLICT;\n \t\t\tgoto cleanup;\n \t\t}\n-- \n2.6.2\n"},{"id":"273093","messageId":"482af811a0f3ed88740a0d1a3c62e925991decf3.1447085798.git.mhagger@alum.mit.edu","threadId":"40756","inReplyTo":"cover.1447085798.git.mhagger@alum.mit.edu","subject":"[PATCH v7 04/11] copy_msg(): rename to copy_reflog_msg()","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2015-11-09T17:03:41Z","receivedAt":"2015-11-09T17:03:41Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"From: David Turner \n\nWe will soon increase the visibility of this function, so make its name\nmore distinctive.\n\nSigned-off-by: David Turner \nSigned-off-by: Junio C Hamano \nSigned-off-by: Michael Haggerty \n---\n refs.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex ddcdf81..480de9a 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3287,7 +3287,7 @@ static int commit_ref(struct ref_lock *lock)\n * large, while cleaning up the whitespaces. Especially, convert LF to space,\n * because reflog file is one line per entry.\n */\n-static int copy_msg(char *buf, const char *msg)\n+static int copy_reflog_msg(char *buf, const char *msg)\n {\n \tchar *cp = buf;\n \tchar c;\n@@ -3391,7 +3391,7 @@ static int log_ref_write_fd(int fd, const unsigned char *old_sha1,\n \t\t\tsha1_to_hex(new_sha1),\n \t\t\tcommitter);\n \tif (msglen)\n-\t\tlen += copy_msg(logrec + len - 1, msg) - 1;\n+\t\tlen += copy_reflog_msg(logrec + len - 1, msg) - 1;\n \n \twritten = len <= maxlen ? write_in_full(fd, logrec, len) : -1;\n \tfree(logrec);\n-- \n2.6.2\n"},{"id":"273089","messageId":"d229d316ab0cf086f5b93e5d720f011701129791.1447085798.git.mhagger@alum.mit.edu","threadId":"40756","inReplyTo":"cover.1447085798.git.mhagger@alum.mit.edu","subject":"[PATCH v7 05/11] pack_if_possible_fn(): use ref_type() instead of is_per_worktree_ref()","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2015-11-09T17:03:42Z","receivedAt":"2015-11-09T17:03:42Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"is_per_worktree_ref() will soon be made private, so use the public\ninterface, ref_type(), in its place. And now that we're using\nref_type(), we can make it clear that we won't pack pseudorefs. This was\nthe case before, but due to the not-so-obvious reason that this function\nis applied to references via the loose reference cache, which only\nincludes references that live inside \"refs/\".\n\nSigned-off-by: Michael Haggerty \n---\n refs.c | 4 +---\n 1 file changed, 1 insertion(+), 3 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 480de9a..82129f0 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2671,8 +2671,6 @@ struct pack_refs_cb_data {\n \tstruct ref_to_prune *ref_to_prune;\n };\n \n-static int is_per_worktree_ref(const char *refname);\n-\n /*\n * An each_ref_entry_fn that is run over loose references only. If\n * the loose reference can be packed, add an entry in the packed ref\n@@ -2687,7 +2685,7 @@ static int pack_if_possible_fn(struct ref_entry *entry, void *cb_data)\n \tint is_tag_ref = starts_with(entry->name, \"refs/tags/\");\n \n \t/* Do not pack per-worktree refs: */\n-\tif (is_per_worktree_ref(entry->name))\n+\tif (ref_type(entry->name) != REF_TYPE_NORMAL)\n \t\treturn 0;\n \n \t/* ALWAYS pack tags */\n-- \n2.6.2\n"},{"id":"273091","messageId":"cffa0c3c8d4f466eedaf6102b783dcef31db4b4d.1447085798.git.mhagger@alum.mit.edu","threadId":"40756","inReplyTo":"cover.1447085798.git.mhagger@alum.mit.edu","subject":"[PATCH v7 06/11] refname_is_safe(): improve docstring","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2015-11-09T17:03:43Z","receivedAt":"2015-11-09T17:03:43Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Signed-off-by: Michael Haggerty \n---\n refs.c | 16 ++++++++++------\n 1 file changed, 10 insertions(+), 6 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 82129f0..f48c58a 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -341,13 +341,17 @@ static struct ref_dir *get_ref_dir(struct ref_entry *entry)\n }\n \n /*\n- * Check if a refname is safe.\n- * For refs that start with \"refs/\" we consider it safe as long they do\n- * not try to resolve to outside of refs/.\n+ * Return true iff refname is minimally safe. \"Safe\" here means that\n+ * deleting a loose reference by this name will not do any damage, for\n+ * example by causing a file that is not a reference to be deleted.\n+ * This function does not check that the reference name is legal; for\n+ * that, use check_refname_format().\n *\n- * For all other refs we only consider them safe iff they only contain\n- * upper case characters and '_' (like \"HEAD\" AND \"MERGE_HEAD\", and not like\n- * \"config\").\n+ * We consider a refname that starts with \"refs/\" to be safe as long\n+ * as any \"..\" components that it might contain do not escape \"refs/\".\n+ * Names that do not start with \"refs/\" are considered safe iff they\n+ * consist entirely of upper case characters and '_' (like \"HEAD\" and\n+ * \"MERGE_HEAD\" but not \"config\" or \"FOO/BAR\").\n */\n static int refname_is_safe(const char *refname)\n {\n-- \n2.6.2\n"},{"id":"273088","messageId":"5f62351689318a4ef9feeb3639e6a7a00b354a02.1447085798.git.mhagger@alum.mit.edu","threadId":"40756","inReplyTo":"cover.1447085798.git.mhagger@alum.mit.edu","subject":"[PATCH v7 07/11] refs/refs-internal.h: new header file","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2015-11-09T17:03:44Z","receivedAt":"2015-11-09T17:03:44Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"There are a number of constants, structs, and static functions defined\nin refs.c and treated as private to the references module. But we want\nto support multiple reference backends within the reference module,\nand those backends will need access to some heretofore private\ndeclarations.\n\nWe don't want those declarations to be visible to non-refs code, so we\ndon't want to move them to refs.h. Instead, add a new header file,\nrefs/refs-internal.h, that is intended to be included only from within\nthe refs module. Make some functions non-static and move some\ndeclarations (and their corresponding docstrings) from refs.c to this\nfile.\n\nIn a moment we will add more content to the \"refs\" subdirectory.\n\nSigned-off-by: Michael Haggerty \n---\n refs.c | 175 +++----------------------------------------------\n refs/refs-internal.h | 182 +++++++++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 191 insertions(+), 166 deletions(-)\n create mode 100644 refs/refs-internal.h\n\ndiff --git a/refs.c b/refs.c\nindex f48c58a..9aff0c8 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1,6 +1,6 @@\n #include \"cache.h\"\n+#include \"refs/refs-internal.h\"\n #include \"lockfile.h\"\n-#include \"refs.h\"\n #include \"object.h\"\n #include \"tag.h\"\n #include \"dir.h\"\n@@ -35,41 +35,6 @@ static unsigned char refname_disposition[256] = {\n };\n \n /*\n- * Flag passed to lock_ref_sha1_basic() telling it to tolerate broken\n- * refs (i.e., because the reference is about to be deleted anyway).\n- */\n-#define REF_DELETING\t0x02\n-\n-/*\n- * Used as a flag in ref_update::flags when a loose ref is being\n- * pruned.\n- */\n-#define REF_ISPRUNING\t0x04\n-\n-/*\n- * Used as a flag in ref_update::flags when the reference should be\n- * updated to new_sha1.\n- */\n-#define REF_HAVE_NEW\t0x08\n-\n-/*\n- * Used as a flag in ref_update::flags when old_sha1 should be\n- * checked.\n- */\n-#define REF_HAVE_OLD\t0x10\n-\n-/*\n- * Used as a flag in ref_update::flags when the lockfile needs to be\n- * committed.\n- */\n-#define REF_NEEDS_COMMIT 0x20\n-\n-/*\n- * 0x40 is REF_FORCE_CREATE_REFLOG, so skip it if you're adding a\n- * value to ref_update::flags\n- */\n-\n-/*\n * Try to read one refname component from the front of refname.\n * Return the length of the component found, or -1 if the component is\n * not legal. It is legal if it is something reasonable to have under\n@@ -340,20 +305,7 @@ static struct ref_dir *get_ref_dir(struct ref_entry *entry)\n \treturn dir;\n }\n \n-/*\n- * Return true iff refname is minimally safe. \"Safe\" here means that\n- * deleting a loose reference by this name will not do any damage, for\n- * example by causing a file that is not a reference to be deleted.\n- * This function does not check that the reference name is legal; for\n- * that, use check_refname_format().\n- *\n- * We consider a refname that starts with \"refs/\" to be safe as long\n- * as any \"..\" components that it might contain do not escape \"refs/\".\n- * Names that do not start with \"refs/\" are considered safe iff they\n- * consist entirely of upper case characters and '_' (like \"HEAD\" and\n- * \"MERGE_HEAD\" but not \"config\" or \"FOO/BAR\").\n- */\n-static int refname_is_safe(const char *refname)\n+int refname_is_safe(const char *refname)\n {\n \tif (starts_with(refname, \"refs/\")) {\n \t\tchar *buf;\n@@ -1823,39 +1775,7 @@ static int filter_refs(const char *refname, const struct object_id *oid,\n \treturn filter->fn(refname, oid, flags, filter->cb_data);\n }\n \n-enum peel_status {\n-\t/* object was peeled successfully: */\n-\tPEEL_PEELED = 0,\n-\n-\t/*\n-\t * object cannot be peeled because the named object (or an\n-\t * object referred to by a tag in the peel chain), does not\n-\t * exist.\n-\t */\n-\tPEEL_INVALID = -1,\n-\n-\t/* object cannot be peeled because it is not a tag: */\n-\tPEEL_NON_TAG = -2,\n-\n-\t/* ref_entry contains no peeled value because it is a symref: */\n-\tPEEL_IS_SYMREF = -3,\n-\n-\t/*\n-\t * ref_entry cannot be peeled because it is broken (i.e., the\n-\t * symbolic reference cannot even be resolved to an object\n-\t * name):\n-\t */\n-\tPEEL_BROKEN = -4\n-};\n-\n-/*\n- * Peel the named object; i.e., if the object is a tag, resolve the\n- * tag recursively until a non-tag is found. If successful, store the\n- * result to sha1 and return PEEL_PEELED. If the object is not a tag\n- * or is not valid, return PEEL_NON_TAG or PEEL_INVALID, respectively,\n- * and leave sha1 unchanged.\n- */\n-static enum peel_status peel_object(const unsigned char *name, unsigned char *sha1)\n+enum peel_status peel_object(const unsigned char *name, unsigned char *sha1)\n {\n \tstruct object *o = lookup_unknown_object(name);\n \n@@ -3110,27 +3030,10 @@ out:\n \treturn ret;\n }\n \n-/*\n- * Return 0 if a reference named refname could be created without\n- * conflicting with the name of an existing reference. Otherwise,\n- * return a negative value and write an explanation to err. If extras\n- * is non-NULL, it is a list of additional refnames with which refname\n- * is not allowed to conflict. If skip is non-NULL, ignore potential\n- * conflicts with refs in skip (e.g., because they are scheduled for\n- * deletion in the same operation). Behavior is undefined if the same\n- * name is listed in both extras and skip.\n- *\n- * Two reference names conflict if one of them exactly matches the\n- * leading components of the other; e.g., \"foo/bar\" conflicts with\n- * both \"foo\" and with \"foo/bar/baz\" but not with \"foo/bar\" or\n- * \"foo/barbados\".\n- *\n- * extras and skip must be sorted.\n- */\n-static int verify_refname_available(const char *newname,\n-\t\t\t\t struct string_list *extras,\n-\t\t\t\t struct string_list *skip,\n-\t\t\t\t struct strbuf *err)\n+int verify_refname_available(const char *newname,\n+\t\t\t struct string_list *extras,\n+\t\t\t struct string_list *skip,\n+\t\t\t struct strbuf *err)\n {\n \tstruct ref_dir *packed_refs = get_packed_refs(&ref_cache);\n \tstruct ref_dir *loose_refs = get_loose_refs(&ref_cache);\n@@ -3284,12 +3187,7 @@ static int commit_ref(struct ref_lock *lock)\n \treturn 0;\n }\n \n-/*\n- * copy the reflog message msg to buf, which has been allocated sufficiently\n- * large, while cleaning up the whitespaces. Especially, convert LF to space,\n- * because reflog file is one line per entry.\n- */\n-static int copy_reflog_msg(char *buf, const char *msg)\n+int copy_reflog_msg(char *buf, const char *msg)\n {\n \tchar *cp = buf;\n \tchar c;\n@@ -3310,7 +3208,7 @@ static int copy_reflog_msg(char *buf, const char *msg)\n \treturn cp - buf;\n }\n \n-static int should_autocreate_reflog(const char *refname)\n+int should_autocreate_reflog(const char *refname)\n {\n \tif (!log_all_ref_updates)\n \t\treturn 0;\n@@ -3963,61 +3861,6 @@ int for_each_reflog(each_ref_fn fn, void *cb_data)\n \treturn retval;\n }\n \n-/**\n- * Information needed for a single ref update. Set new_sha1 to the new\n- * value or to null_sha1 to delete the ref. To check the old value\n- * while the ref is locked, set (flags & REF_HAVE_OLD) and set\n- * old_sha1 to the old value, or to null_sha1 to ensure the ref does\n- * not exist before update.\n- */\n-struct ref_update {\n-\t/*\n-\t * If (flags & REF_HAVE_NEW), set the reference to this value:\n-\t */\n-\tunsigned char new_sha1[20];\n-\t/*\n-\t * If (flags & REF_HAVE_OLD), check that the reference\n-\t * previously had this value:\n-\t */\n-\tunsigned char old_sha1[20];\n-\t/*\n-\t * One or more of REF_HAVE_NEW, REF_HAVE_OLD, REF_NODEREF,\n-\t * REF_DELETING, and REF_ISPRUNING:\n-\t */\n-\tunsigned int flags;\n-\tstruct ref_lock *lock;\n-\tint type;\n-\tchar *msg;\n-\tconst char refname[FLEX_ARRAY];\n-};\n-\n-/*\n- * Transaction states.\n- * OPEN: The transaction is in a valid state and can accept new updates.\n- * An OPEN transaction can be committed.\n- * CLOSED: A closed transaction is no longer active and no other operations\n- * than free can be used on it in this state.\n- * A transaction can either become closed by successfully committing\n- * an active transaction or if there is a failure while building\n- * the transaction thus rendering it failed/inactive.\n- */\n-enum ref_transaction_state {\n-\tREF_TRANSACTION_OPEN = 0,\n-\tREF_TRANSACTION_CLOSED = 1\n-};\n-\n-/*\n- * Data structure for holding a reference transaction, which can\n- * consist of checks and updates to multiple references, carried out\n- * as atomically as possible. This structure is opaque to callers.\n- */\n-struct ref_transaction {\n-\tstruct ref_update **updates;\n-\tsize_t alloc;\n-\tsize_t nr;\n-\tenum ref_transaction_state state;\n-};\n-\n struct ref_transaction *ref_transaction_begin(struct strbuf *err)\n {\n \tassert(err);\ndiff --git a/refs/refs-internal.h b/refs/refs-internal.h\nnew file mode 100644\nindex 0000000..8f5cfb4\n--- /dev/null\n+++ b/refs/refs-internal.h\n@@ -0,0 +1,182 @@\n+#ifndef REFS_REFS_INTERNAL_H\n+#define REFS_REFS_INTERNAL_H\n+\n+/*\n+ * Data structures and functions for the internal use of the refs\n+ * module. Code outside of the refs module should use only the public\n+ * functions defined in \"refs.h\", and should *not* include this file.\n+ */\n+\n+#include \"../refs.h\"\n+\n+/*\n+ * Flag passed to lock_ref_sha1_basic() telling it to tolerate broken\n+ * refs (i.e., because the reference is about to be deleted anyway).\n+ */\n+#define REF_DELETING\t0x02\n+\n+/*\n+ * Used as a flag in ref_update::flags when a loose ref is being\n+ * pruned.\n+ */\n+#define REF_ISPRUNING\t0x04\n+\n+/*\n+ * Used as a flag in ref_update::flags when the reference should be\n+ * updated to new_sha1.\n+ */\n+#define REF_HAVE_NEW\t0x08\n+\n+/*\n+ * Used as a flag in ref_update::flags when old_sha1 should be\n+ * checked.\n+ */\n+#define REF_HAVE_OLD\t0x10\n+\n+/*\n+ * Used as a flag in ref_update::flags when the lockfile needs to be\n+ * committed.\n+ */\n+#define REF_NEEDS_COMMIT 0x20\n+\n+/*\n+ * 0x40 is REF_FORCE_CREATE_REFLOG, so skip it if you're adding a\n+ * value to ref_update::flags\n+ */\n+\n+/*\n+ * Return true iff refname is minimally safe. \"Safe\" here means that\n+ * deleting a loose reference by this name will not do any damage, for\n+ * example by causing a file that is not a reference to be deleted.\n+ * This function does not check that the reference name is legal; for\n+ * that, use check_refname_format().\n+ *\n+ * We consider a refname that starts with \"refs/\" to be safe as long\n+ * as any \"..\" components that it might contain do not escape \"refs/\".\n+ * Names that do not start with \"refs/\" are considered safe iff they\n+ * consist entirely of upper case characters and '_' (like \"HEAD\" and\n+ * \"MERGE_HEAD\" but not \"config\" or \"FOO/BAR\").\n+ */\n+int refname_is_safe(const char *refname);\n+\n+enum peel_status {\n+\t/* object was peeled successfully: */\n+\tPEEL_PEELED = 0,\n+\n+\t/*\n+\t * object cannot be peeled because the named object (or an\n+\t * object referred to by a tag in the peel chain), does not\n+\t * exist.\n+\t */\n+\tPEEL_INVALID = -1,\n+\n+\t/* object cannot be peeled because it is not a tag: */\n+\tPEEL_NON_TAG = -2,\n+\n+\t/* ref_entry contains no peeled value because it is a symref: */\n+\tPEEL_IS_SYMREF = -3,\n+\n+\t/*\n+\t * ref_entry cannot be peeled because it is broken (i.e., the\n+\t * symbolic reference cannot even be resolved to an object\n+\t * name):\n+\t */\n+\tPEEL_BROKEN = -4\n+};\n+\n+/*\n+ * Peel the named object; i.e., if the object is a tag, resolve the\n+ * tag recursively until a non-tag is found. If successful, store the\n+ * result to sha1 and return PEEL_PEELED. If the object is not a tag\n+ * or is not valid, return PEEL_NON_TAG or PEEL_INVALID, respectively,\n+ * and leave sha1 unchanged.\n+ */\n+enum peel_status peel_object(const unsigned char *name, unsigned char *sha1);\n+\n+/*\n+ * Return 0 if a reference named refname could be created without\n+ * conflicting with the name of an existing reference. Otherwise,\n+ * return a negative value and write an explanation to err. If extras\n+ * is non-NULL, it is a list of additional refnames with which refname\n+ * is not allowed to conflict. If skip is non-NULL, ignore potential\n+ * conflicts with refs in skip (e.g., because they are scheduled for\n+ * deletion in the same operation). Behavior is undefined if the same\n+ * name is listed in both extras and skip.\n+ *\n+ * Two reference names conflict if one of them exactly matches the\n+ * leading components of the other; e.g., \"foo/bar\" conflicts with\n+ * both \"foo\" and with \"foo/bar/baz\" but not with \"foo/bar\" or\n+ * \"foo/barbados\".\n+ *\n+ * extras and skip must be sorted.\n+ */\n+int verify_refname_available(const char *newname,\n+\t\t\t struct string_list *extras,\n+\t\t\t struct string_list *skip,\n+\t\t\t struct strbuf *err);\n+\n+/*\n+ * Copy the reflog message msg to buf, which has been allocated sufficiently\n+ * large, while cleaning up the whitespaces. Especially, convert LF to space,\n+ * because reflog file is one line per entry.\n+ */\n+int copy_reflog_msg(char *buf, const char *msg);\n+\n+int should_autocreate_reflog(const char *refname);\n+\n+/**\n+ * Information needed for a single ref update. Set new_sha1 to the new\n+ * value or to null_sha1 to delete the ref. To check the old value\n+ * while the ref is locked, set (flags & REF_HAVE_OLD) and set\n+ * old_sha1 to the old value, or to null_sha1 to ensure the ref does\n+ * not exist before update.\n+ */\n+struct ref_update {\n+\t/*\n+\t * If (flags & REF_HAVE_NEW), set the reference to this value:\n+\t */\n+\tunsigned char new_sha1[20];\n+\t/*\n+\t * If (flags & REF_HAVE_OLD), check that the reference\n+\t * previously had this value:\n+\t */\n+\tunsigned char old_sha1[20];\n+\t/*\n+\t * One or more of REF_HAVE_NEW, REF_HAVE_OLD, REF_NODEREF,\n+\t * REF_DELETING, and REF_ISPRUNING:\n+\t */\n+\tunsigned int flags;\n+\tstruct ref_lock *lock;\n+\tint type;\n+\tchar *msg;\n+\tconst char refname[FLEX_ARRAY];\n+};\n+\n+/*\n+ * Transaction states.\n+ * OPEN: The transaction is in a valid state and can accept new updates.\n+ * An OPEN transaction can be committed.\n+ * CLOSED: A closed transaction is no longer active and no other operations\n+ * than free can be used on it in this state.\n+ * A transaction can either become closed by successfully committing\n+ * an active transaction or if there is a failure while building\n+ * the transaction thus rendering it failed/inactive.\n+ */\n+enum ref_transaction_state {\n+\tREF_TRANSACTION_OPEN = 0,\n+\tREF_TRANSACTION_CLOSED = 1\n+};\n+\n+/*\n+ * Data structure for holding a reference transaction, which can\n+ * consist of checks and updates to multiple references, carried out\n+ * as atomically as possible. This structure is opaque to callers.\n+ */\n+struct ref_transaction {\n+\tstruct ref_update **updates;\n+\tsize_t alloc;\n+\tsize_t nr;\n+\tenum ref_transaction_state state;\n+};\n+\n+#endif /* REFS_REFS_INTERNAL_H */\n-- \n2.6.2\n"},{"id":"273094","messageId":"3ca59b9ffc153e11361176249380ae605d73aafd.1447085798.git.mhagger@alum.mit.edu","threadId":"40756","inReplyTo":"cover.1447085798.git.mhagger@alum.mit.edu","subject":"[PATCH v7 09/11] initdb: make safe_create_dir public","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2015-11-09T17:03:46Z","receivedAt":"2015-11-09T17:03:46Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"From: David Turner \n\nSoon we will want to create initdb functions for ref backends, and\ncode from initdb that calls this function needs to move into the files\nbackend. So this function needs to be public.\n\nSigned-off-by: David Turner \nSigned-off-by: Junio C Hamano \nSigned-off-by: Michael Haggerty \n---\n builtin/init-db.c | 12 ------------\n cache.h | 8 ++++++++\n path.c | 12 ++++++++++++\n 3 files changed, 20 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/init-db.c b/builtin/init-db.c\nindex f59f407..07229d6 100644\n--- a/builtin/init-db.c\n+++ b/builtin/init-db.c\n@@ -24,18 +24,6 @@ static int init_shared_repository = -1;\n static const char *init_db_template_dir;\n static const char *git_link;\n \n-static void safe_create_dir(const char *dir, int share)\n-{\n-\tif (mkdir(dir, 0777) < 0) {\n-\t\tif (errno != EEXIST) {\n-\t\t\tperror(dir);\n-\t\t\texit(1);\n-\t\t}\n-\t}\n-\telse if (share && adjust_shared_perm(dir))\n-\t\tdie(_(\"Could not make %s writable by group\"), dir);\n-}\n-\n static void copy_templates_1(struct strbuf *path, struct strbuf *template,\n \t\t\t DIR *dir)\n {\ndiff --git a/cache.h b/cache.h\nindex 9a905a8..0f9808c 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1737,4 +1737,12 @@ void stat_validity_update(struct stat_validity *sv, int fd);\n int versioncmp(const char *s1, const char *s2);\n void sleep_millisec(int millisec);\n \n+/*\n+ * Create a directory and (if share is nonzero) adjust its permissions\n+ * according to the shared_repository setting. Only use this for\n+ * directories under $GIT_DIR. Don't use it for working tree\n+ * directories.\n+ */\n+void safe_create_dir(const char *dir, int share);\n+\n #endif /* CACHE_H */\ndiff --git a/path.c b/path.c\nindex 212695a..9e0283c 100644\n--- a/path.c\n+++ b/path.c\n@@ -723,6 +723,18 @@ int adjust_shared_perm(const char *path)\n \treturn 0;\n }\n \n+void safe_create_dir(const char *dir, int share)\n+{\n+\tif (mkdir(dir, 0777) < 0) {\n+\t\tif (errno != EEXIST) {\n+\t\t\tperror(dir);\n+\t\t\texit(1);\n+\t\t}\n+\t}\n+\telse if (share && adjust_shared_perm(dir))\n+\t\tdie(_(\"Could not make %s writable by group\"), dir);\n+}\n+\n static int have_same_root(const char *path1, const char *path2)\n {\n \tint is_abs1, is_abs2;\n-- \n2.6.2\n"},{"id":"273095","messageId":"b017ec518536e9f21c2900b3821b40a8a3a8f375.1447085798.git.mhagger@alum.mit.edu","threadId":"40756","inReplyTo":"cover.1447085798.git.mhagger@alum.mit.edu","subject":"[PATCH v7 10/11] files_log_ref_write: new function","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2015-11-09T17:03:47Z","receivedAt":"2015-11-09T17:03:47Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"From: David Turner \n\nBecause HEAD and stash are per-worktree, every refs backend needs to\ngo through the files backend to write these refs.\n\nSo create a new function, files_log_ref_write, and add it to\nrefs/refs-internal.h. Later, we will use this to handle reflog updates\nfor per-worktree symbolic refs (HEAD).\n\nSigned-off-by: David Turner \nSigned-off-by: Junio C Hamano \nSigned-off-by: Michael Haggerty \n---\n refs/files-backend.c | 8 ++++++++\n refs/refs-internal.h | 4 ++++\n 2 files changed, 12 insertions(+)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex a2e5a56..1094348 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -2735,6 +2735,14 @@ static int log_ref_write(const char *refname, const unsigned char *old_sha1,\n \t\t\t const unsigned char *new_sha1, const char *msg,\n \t\t\t int flags, struct strbuf *err)\n {\n+\treturn files_log_ref_write(refname, old_sha1, new_sha1, msg, flags,\n+\t\t\t\t err);\n+}\n+\n+int files_log_ref_write(const char *refname, const unsigned char *old_sha1,\n+\t\t\tconst unsigned char *new_sha1, const char *msg,\n+\t\t\tint flags, struct strbuf *err)\n+{\n \tstruct strbuf sb = STRBUF_INIT;\n \tint ret = log_ref_write_1(refname, old_sha1, new_sha1, msg, &sb, flags,\n \t\t\t\t err);\ndiff --git a/refs/refs-internal.h b/refs/refs-internal.h\nindex 8f5cfb4..88a5be0 100644\n--- a/refs/refs-internal.h\n+++ b/refs/refs-internal.h\n@@ -179,4 +179,8 @@ struct ref_transaction {\n \tenum ref_transaction_state state;\n };\n \n+int files_log_ref_write(const char *refname, const unsigned char *old_sha1,\n+\t\t\tconst unsigned char *new_sha1, const char *msg,\n+\t\t\tint flags, struct strbuf *err);\n+\n #endif /* REFS_REFS_INTERNAL_H */\n-- \n2.6.2\n"},{"id":"273090","messageId":"4dfc6ad3effed7afb511e3356eefa0ef48dd8b8e.1447085798.git.mhagger@alum.mit.edu","threadId":"40756","inReplyTo":"cover.1447085798.git.mhagger@alum.mit.edu","subject":"[PATCH v7 11/11] refs: break out ref conflict checks","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2015-11-09T17:03:48Z","receivedAt":"2015-11-09T17:03:48Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"From: David Turner \n\nCreate new function find_descendant_ref, to hold one of the ref\nconflict checks used in verify_refname_available. Multiple backends\nwill need this function, so move it to the common code.\n\nAlso move rename_ref_available to the common code, because alternate\nbackends might need it and it has no files-backend-specific code.\n\nSigned-off-by: David Turner \nSigned-off-by: Junio C Hamano \nSigned-off-by: Michael Haggerty \n---\n refs.c | 44 ++++++++++++++++++++++++++++++++++++++++++++\n refs/files-backend.c | 49 +++++++------------------------------------------\n refs/refs-internal.h | 16 ++++++++++++++++\n 3 files changed, 67 insertions(+), 42 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 175b3e9..0e2eaf8 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1027,3 +1027,47 @@ int ref_is_hidden(const char *refname)\n \t}\n \treturn 0;\n }\n+\n+const char *find_descendant_ref(const char *dirname,\n+\t\t\t\tconst struct string_list *extras,\n+\t\t\t\tconst struct string_list *skip)\n+{\n+\tint pos;\n+\n+\tif (!extras)\n+\t\treturn NULL;\n+\n+\t/*\n+\t * Look at the place where dirname would be inserted into\n+\t * extras. If there is an entry at that position that starts\n+\t * with dirname (remember, dirname includes the trailing\n+\t * slash) and is not in skip, then we have a conflict.\n+\t */\n+\tfor (pos = string_list_find_insert_index(extras, dirname, 0);\n+\t pos < extras->nr; pos++) {\n+\t\tconst char *extra_refname = extras->items[pos].string;\n+\n+\t\tif (!starts_with(extra_refname, dirname))\n+\t\t\tbreak;\n+\n+\t\tif (!skip || !string_list_has_string(skip, extra_refname))\n+\t\t\treturn extra_refname;\n+\t}\n+\treturn NULL;\n+}\n+\n+int rename_ref_available(const char *oldname, const char *newname)\n+{\n+\tstruct string_list skip = STRING_LIST_INIT_NODUP;\n+\tstruct strbuf err = STRBUF_INIT;\n+\tint ret;\n+\n+\tstring_list_insert(&skip, oldname);\n+\tret = !verify_refname_available(newname, NULL, &skip, &err);\n+\tif (!ret)\n+\t\terror(\"%s\", err.buf);\n+\n+\tstring_list_clear(&skip, 0);\n+\tstrbuf_release(&err);\n+\treturn ret;\n+}\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 1094348..8bb3728 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -729,6 +729,7 @@ static int verify_refname_available_dir(const char *refname,\n \t\t\t\t\tstruct strbuf *err)\n {\n \tconst char *slash;\n+\tconst char *extra_refname;\n \tint pos;\n \tstruct strbuf dirname = STRBUF_INIT;\n \tint ret = -1;\n@@ -834,32 +835,12 @@ static int verify_refname_available_dir(const char *refname,\n \t\t}\n \t}\n \n-\tif (extras) {\n-\t\t/*\n-\t\t * Check for entries in extras that start with\n-\t\t * \"$refname/\". We do that by looking for the place\n-\t\t * where \"$refname/\" would be inserted in extras. If\n-\t\t * there is an entry at that position that starts with\n-\t\t * \"$refname/\" and is not in skip, then we have a\n-\t\t * conflict.\n-\t\t */\n-\t\tfor (pos = string_list_find_insert_index(extras, dirname.buf, 0);\n-\t\t pos < extras->nr; pos++) {\n-\t\t\tconst char *extra_refname = extras->items[pos].string;\n-\n-\t\t\tif (!starts_with(extra_refname, dirname.buf))\n-\t\t\t\tbreak;\n-\n-\t\t\tif (!skip || !string_list_has_string(skip, extra_refname)) {\n-\t\t\t\tstrbuf_addf(err, \"cannot process '%s' and '%s' at the same time\",\n-\t\t\t\t\t refname, extra_refname);\n-\t\t\t\tgoto cleanup;\n-\t\t\t}\n-\t\t}\n-\t}\n-\n-\t/* No conflicts were found */\n-\tret = 0;\n+\textra_refname = find_descendant_ref(dirname.buf, extras, skip);\n+\tif (extra_refname)\n+\t\tstrbuf_addf(err, \"cannot process '%s' and '%s' at the same time\",\n+\t\t\t refname, extra_refname);\n+\telse\n+\t\tret = 0;\n \n cleanup:\n \tstrbuf_release(&dirname);\n@@ -2474,22 +2455,6 @@ int verify_refname_available(const char *newname,\n \treturn 0;\n }\n \n-static int rename_ref_available(const char *oldname, const char *newname)\n-{\n-\tstruct string_list skip = STRING_LIST_INIT_NODUP;\n-\tstruct strbuf err = STRBUF_INIT;\n-\tint ret;\n-\n-\tstring_list_insert(&skip, oldname);\n-\tret = !verify_refname_available(newname, NULL, &skip, &err);\n-\tif (!ret)\n-\t\terror(\"%s\", err.buf);\n-\n-\tstring_list_clear(&skip, 0);\n-\tstrbuf_release(&err);\n-\treturn ret;\n-}\n-\n static int write_ref_to_lockfile(struct ref_lock *lock,\n \t\t\t\t const unsigned char *sha1, struct strbuf *err);\n static int commit_ref_update(struct ref_lock *lock,\ndiff --git a/refs/refs-internal.h b/refs/refs-internal.h\nindex 88a5be0..6c29f45 100644\n--- a/refs/refs-internal.h\n+++ b/refs/refs-internal.h\n@@ -183,4 +183,20 @@ int files_log_ref_write(const char *refname, const unsigned char *old_sha1,\n \t\t\tconst unsigned char *new_sha1, const char *msg,\n \t\t\tint flags, struct strbuf *err);\n \n+/*\n+ * Check for entries in extras that are within the specified\n+ * directory, where dirname is a reference directory name including\n+ * the trailing slash (e.g., \"refs/heads/foo/\"). Ignore any\n+ * conflicting references that are found in skip. If there is a\n+ * conflicting reference, return its name.\n+ *\n+ * extras and skip must be sorted lists of reference names. Either one\n+ * can be NULL, signifying the empty list.\n+ */\n+const char *find_descendant_ref(const char *dirname,\n+\t\t\t\tconst struct string_list *extras,\n+\t\t\t\tconst struct string_list *skip);\n+\n+int rename_ref_available(const char *oldname, const char *newname);\n+\n #endif /* REFS_REFS_INTERNAL_H */\n-- \n2.6.2\n"},{"id":"273104","messageId":"5640EBD3.8040505@ramsayjones.plus.com","threadId":"40756","inReplyTo":"39e0a2a708fb1a42b69e45bc5c0b17d565d3662e.1447085798.git.mhagger@alum.mit.edu","subject":"Re: [PATCH v7 01/11] refs: make is_branch public","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2015-11-09T18:54:11Z","receivedAt":"2015-11-09T18:54:11Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 09/11/15 17:03, Michael Haggerty wrote:\n> From: David Turner \n> \n> is_branch was already non-static, but this patch declares it in the\n> header.\n> \n> Signed-off-by: Ronnie Sahlberg \n> Signed-off-by: David Turner \n> Signed-off-by: Junio C Hamano \n> Signed-off-by: Michael Haggerty \n> ---\n> refs.h | 2 ++\n> 1 file changed, 2 insertions(+)\n> \n> diff --git a/refs.h b/refs.h\n> index 6d30c98..39b8edc 100644\n> --- a/refs.h\n> +++ b/refs.h\n> @@ -217,6 +217,8 @@ extern void warn_dangling_symrefs(FILE *fp, const char *msg_fmt, const struct st\n> */\n> int pack_refs(unsigned int flags);\n> \n> +int is_branch(const char *refname);\n> +\n> /*\n> * Flags controlling ref_transaction_update(), ref_transaction_create(), etc.\n> * REF_NODEREF: act on the ref directly, instead of dereferencing\n> \n\nI don't understand, is_branch() is already declared in refs.h, see line 67.\n\nThis is true in master, next and pu now appears to have two declarations.\n\nATB,\nRamsay Jones\n"},{"id":"273107","messageId":"5640F823.5010902@ramsayjones.plus.com","threadId":"40756","inReplyTo":"5f62351689318a4ef9feeb3639e6a7a00b354a02.1447085798.git.mhagger@alum.mit.edu","subject":"Re: [PATCH v7 07/11] refs/refs-internal.h: new header file","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2015-11-09T19:46:43Z","receivedAt":"2015-11-09T19:46:43Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 09/11/15 17:03, Michael Haggerty wrote:\n> There are a number of constants, structs, and static functions defined\n> in refs.c and treated as private to the references module. But we want\n> to support multiple reference backends within the reference module,\n> and those backends will need access to some heretofore private\n> declarations.\n> \n> We don't want those declarations to be visible to non-refs code, so we\n> don't want to move them to refs.h. Instead, add a new header file,\n> refs/refs-internal.h, that is intended to be included only from within\n> the refs module. Make some functions non-static and move some\n> declarations (and their corresponding docstrings) from refs.c to this\n> file.\n> \n> In a moment we will add more content to the \"refs\" subdirectory.\n> \n> Signed-off-by: Michael Haggerty \n> ---\n> refs.c | 175 +++----------------------------------------------\n> refs/refs-internal.h | 182 +++++++++++++++++++++++++++++++++++++++++++++++++++\n> 2 files changed, 191 insertions(+), 166 deletions(-)\n> create mode 100644 refs/refs-internal.h\n> \n> diff --git a/refs.c b/refs.c\n> index f48c58a..9aff0c8 100644\n> --- a/refs.c\n> +++ b/refs.c\n> @@ -1,6 +1,6 @@\n> #include \"cache.h\"\n> +#include \"refs/refs-internal.h\"\n> #include \"lockfile.h\"\n> -#include \"refs.h\"\n\nThis looked wrong to me, until I had read the remainder of the\npatch and noticed that the 'internal' header #included the\n'public' interface header.\n\nUnfortunately, this still feels wrong to me! I would rather that\nthe internal header _not_ include the public header (so, include\nthem _both_ when necessary). Just my opinion, which you can simply\nignore. :-D\n\nATB,\nRamsay Jones\n"},{"id":"273110","messageId":"1447104175.20147.0.camel@twopensource.com","threadId":"40756","inReplyTo":"5640EBD3.8040505@ramsayjones.plus.com","subject":"Re: [PATCH v7 01/11] refs: make is_branch public","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2015-11-09T21:22:55Z","receivedAt":"2015-11-09T21:22:55Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Mon, 2015-11-09 at 18:54 +0000, Ramsay Jones wrote:\n> \n> On 09/11/15 17:03, Michael Haggerty wrote:\n> > From: David Turner \n> > \n> > is_branch was already non-static, but this patch declares it in the\n> > header.\n> > \n> > Signed-off-by: Ronnie Sahlberg \n> > Signed-off-by: David Turner \n> > Signed-off-by: Junio C Hamano \n> > Signed-off-by: Michael Haggerty \n> > ---\n> > refs.h | 2 ++\n> > 1 file changed, 2 insertions(+)\n> > \n> > diff --git a/refs.h b/refs.h\n> > index 6d30c98..39b8edc 100644\n> > --- a/refs.h\n> > +++ b/refs.h\n> > @@ -217,6 +217,8 @@ extern void warn_dangling_symrefs(FILE *fp, const char *msg_fmt, const struct st\n> > */\n> > int pack_refs(unsigned int flags);\n> > \n> > +int is_branch(const char *refname);\n> > +\n> > /*\n> > * Flags controlling ref_transaction_update(), ref_transaction_create(), etc.\n> > * REF_NODEREF: act on the ref directly, instead of dereferencing\n> > \n> \n> I don't understand, is_branch() is already declared in refs.h, see line 67.\n> \n> This is true in master, next and pu now appears to have two declarations.\n\nAgreed. This was necessary at the time I started on this work, but this\npatch apparently stuck around past the time it was needed. Please\ndrop.\n"},{"id":"273111","messageId":"1447104235.20147.1.camel@twopensource.com","threadId":"40756","inReplyTo":"5640F823.5010902@ramsayjones.plus.com","subject":"Re: [PATCH v7 07/11] refs/refs-internal.h: new header file","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2015-11-09T21:23:55Z","receivedAt":"2015-11-09T21:23:55Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Mon, 2015-11-09 at 19:46 +0000, Ramsay Jones wrote:\n> \n> On 09/11/15 17:03, Michael Haggerty wrote:\n> > There are a number of constants, structs, and static functions defined\n> > in refs.c and treated as private to the references module. But we want\n> > to support multiple reference backends within the reference module,\n> > and those backends will need access to some heretofore private\n> > declarations.\n> > \n> > We don't want those declarations to be visible to non-refs code, so we\n> > don't want to move them to refs.h. Instead, add a new header file,\n> > refs/refs-internal.h, that is intended to be included only from within\n> > the refs module. Make some functions non-static and move some\n> > declarations (and their corresponding docstrings) from refs.c to this\n> > file.\n> > \n> > In a moment we will add more content to the \"refs\" subdirectory.\n> > \n> > Signed-off-by: Michael Haggerty \n> > ---\n> > refs.c | 175 +++----------------------------------------------\n> > refs/refs-internal.h | 182 +++++++++++++++++++++++++++++++++++++++++++++++++++\n> > 2 files changed, 191 insertions(+), 166 deletions(-)\n> > create mode 100644 refs/refs-internal.h\n> > \n> > diff --git a/refs.c b/refs.c\n> > index f48c58a..9aff0c8 100644\n> > --- a/refs.c\n> > +++ b/refs.c\n> > @@ -1,6 +1,6 @@\n> > #include \"cache.h\"\n> > +#include \"refs/refs-internal.h\"\n> > #include \"lockfile.h\"\n> > -#include \"refs.h\"\n> \n> This looked wrong to me, until I had read the remainder of the\n> patch and noticed that the 'internal' header #included the\n> 'public' interface header.\n> \n> Unfortunately, this still feels wrong to me! I would rather that\n> the internal header _not_ include the public header (so, include\n> them _both_ when necessary). Just my opinion, which you can simply\n> ignore. :-D\n\n+1 on this.\n"},{"id":"273112","messageId":"1447104501.20147.3.camel@twopensource.com","threadId":"40756","inReplyTo":"cover.1447085798.git.mhagger@alum.mit.edu","subject":"Re: [PATCH v7 00/11] refs backend pre-vtable","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2015-11-09T21:28:21Z","receivedAt":"2015-11-09T21:28:21Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Mon, 2015-11-09 at 18:03 +0100, Michael Haggerty wrote:\n> This is another reroll of the pre-vtable part of the refs-backend\n> patch series dt/refs-backend-pre-vtable. v6 [1] proved cumbersome\n> because it conflicted messily with lf/ref-is-hidden-namespace [2]. The\n> conflicts were partly due to the motion of code across files but, even\n> worse, due to the change of order of function definitions between old\n> and new files.\n>\n> So I have heavily \"optimized\" this reroll for reviewability and to\n> minimize conflicts with other work in the area. The only such work\n> that I know of is lf/ref-is-hidden-namespace, which can now be merged\n> with this series *without conflicts*.\n\nModulo Ramsey's comments, this works for me. AIUI, Ronnie's original\npatchset was written the way it was to avoid merge conflicts. Because\nof the changes in the interim, it turns out not to quite work that way.\nSo it's totally OK with me to write it this new way.\n"},{"id":"273130","messageId":"56419EBF.5020404@alum.mit.edu","threadId":"40756","inReplyTo":"5640EBD3.8040505@ramsayjones.plus.com","subject":"Re: [PATCH v7 01/11] refs: make is_branch public","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2015-11-10T07:37:35Z","receivedAt":"2015-11-10T07:37:35Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 11/09/2015 07:54 PM, Ramsay Jones wrote:\n> On 09/11/15 17:03, Michael Haggerty wrote:\n>> [...]\n>> diff --git a/refs.h b/refs.h\n>> index 6d30c98..39b8edc 100644\n>> --- a/refs.h\n>> +++ b/refs.h\n>> @@ -217,6 +217,8 @@ extern void warn_dangling_symrefs(FILE *fp, const char *msg_fmt, const struct st\n>> */\n>> int pack_refs(unsigned int flags);\n>> \n>> +int is_branch(const char *refname);\n>> +\n>> /*\n>> * Flags controlling ref_transaction_update(), ref_transaction_create(), etc.\n>> * REF_NODEREF: act on the ref directly, instead of dereferencing\n>>\n> \n> I don't understand, is_branch() is already declared in refs.h, see line 67.\n> \n> This is true in master, next and pu now appears to have two declarations.\n\nThanks for noticing! I'll drop that patch from the next round.\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\n"},{"id":"273131","messageId":"56419F52.9050806@alum.mit.edu","threadId":"40756","inReplyTo":"5640F823.5010902@ramsayjones.plus.com","subject":"Re: [PATCH v7 07/11] refs/refs-internal.h: new header file","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2015-11-10T07:40:02Z","receivedAt":"2015-11-10T07:40:02Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 11/09/2015 08:46 PM, Ramsay Jones wrote:\n> On 09/11/15 17:03, Michael Haggerty wrote:\n>> [...]\n>> diff --git a/refs.c b/refs.c\n>> index f48c58a..9aff0c8 100644\n>> --- a/refs.c\n>> +++ b/refs.c\n>> @@ -1,6 +1,6 @@\n>> #include \"cache.h\"\n>> +#include \"refs/refs-internal.h\"\n>> #include \"lockfile.h\"\n>> -#include \"refs.h\"\n> \n> This looked wrong to me, until I had read the remainder of the\n> patch and noticed that the 'internal' header #included the\n> 'public' interface header.\n> \n> Unfortunately, this still feels wrong to me! I would rather that\n> the internal header _not_ include the public header (so, include\n> them _both_ when necessary). Just my opinion, which you can simply\n> ignore. :-D\n\nYeah, I was of two minds about this. I will change this in the next\nround. Thanks for your review!\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\n"}]}