{"thread":{"id":"36850","subject":"[PATCH 1/2] refs.c: write updates to packed refs when a transaction has more than one ref","startedAt":"2014-06-05T23:26:26Z","lastAt":"2014-06-09T16:20:26Z","messageCount":5,"participants":["Ronnie Sahlberg","Eric Sunshine"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"243437","messageId":"1402010788-8236-1-git-send-email-sahlberg@google.com","threadId":"36850","inReplyTo":null,"subject":"[PATCH 0/2] Write multi-ref updates directly to packed refs","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-06-05T23:26:26Z","receivedAt":"2014-06-05T23:26:26Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"This patch adds support to the transaction system to update all non-refs as\npacked refs instead of loose refs.\n\nThis series can also be found at\nhttps://github.com/rsahlberg/git/tree/ref-transactions-req-packed-refs\n\nThis series adds support to have transaction write updates to packed refs\ninstead of as loose refs.\nIf the transaction only contains a single update to a single ref, such as the\ncommit case, we continue to update this as locking and writing to the loose\nref file.\nBut If on the other hand the transaction will update more than one ref, then\nwe perform all these updates inside the packed refs file and remove all the\naffected loose refs files. This allows us to both perform deletes (added\npreviously) as well as updates-of-more-than-one-ref as one single atomic rename\noperation.\n\nSince the transactions will now by default write directly to the packed refs\nfile for updates to more than one ref, we no longer need to provide a\npacekd refs api to builtin/clone.c any more. So lets change clone.c to\nuse a transaction. This removes 4 fucntions from the refs.c API we can now\nunexport and make static.\n\nThis change means that clone will now apear as one single atomic operation\nto any external observer where all refs appear at once instead of one\nref becoming visible at a time.\n\n\nRonnie Sahlberg (2):\n  refs.c: write updates to packed refs when a transaction has more than\n    one ref\n  refs.c: make the *_packed_refs functions static\n\n builtin/clone.c | 16 ++++++++---\n refs.c          | 82 ++++++++++++++++++++++++++++++++++++++++-----------------\n refs.h          | 28 --------------------\n 3 files changed, 70 insertions(+), 56 deletions(-)\n\n-- \n2.0.0.583.g402232d\n"},{"id":"243434","messageId":"1402010788-8236-2-git-send-email-sahlberg@google.com","threadId":"36850","inReplyTo":"1402010788-8236-1-git-send-email-sahlberg@google.com","subject":"[PATCH 1/2] refs.c: write updates to packed refs when a transaction has more than one ref","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-06-05T23:26:27Z","receivedAt":"2014-06-05T23:26:27Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"When we are updating more than one single ref, i.e. not a commit, then\nwrite the updated refs directly to the packed refs file instead of writing\nthem as loose refs.\n\nChange clone to use a transaction instead of using the pacekd refs api.\n\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\n---\n builtin/clone.c | 16 +++++++++----\n refs.c          | 74 +++++++++++++++++++++++++++++++++++++++++----------------\n 2 files changed, 66 insertions(+), 24 deletions(-)\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex b12989d..000a236 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -497,17 +497,25 @@ static struct ref *wanted_peer_refs(const struct ref *refs,\n static void write_remote_refs(const struct ref *local_refs)\n {\n \tconst struct ref *r;\n+\tstruct ref_transaction *transaction;\n+\tstruct strbuf err = STRBUF_INIT;\n \n-\tlock_packed_refs(LOCK_DIE_ON_ERROR);\n+\ttransaction = transaction_begin(&err);\n+\tif (!transaction)\n+\t\tdie(\"%s\", err.buf);\n \n \tfor (r = local_refs; r; r = r->next) {\n \t\tif (!r->peer_ref)\n \t\t\tcontinue;\n-\t\tadd_packed_ref(r->peer_ref->name, r->old_sha1);\n+\t\tif (transaction_update_sha1(transaction, r->peer_ref->name,\n+\t\t\t\t\t    r->old_sha1, NULL, 0, 0, NULL,\n+\t\t\t\t\t    &err))\n+\t\t\tdie(\"%s\", err.buf);\n \t}\n \n-\tif (commit_packed_refs())\n-\t\tdie_errno(\"unable to overwrite old ref-pack file\");\n+\tif (transaction_commit(transaction, &err))\n+\t\tdie(\"%s\", err.buf);\n+\ttransaction_free(transaction);\n }\n \n static void write_followtags(const struct ref *refs, const char *msg)\ndiff --git a/refs.c b/refs.c\nindex 4281011..6aa64dd 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2481,31 +2481,18 @@ static int repack_without_refs(const char **refnames, int n, struct strbuf *err)\n \tstruct ref_dir *packed;\n \tstruct string_list refs_to_delete = STRING_LIST_INIT_DUP;\n \tstruct string_list_item *ref_to_delete;\n-\tint i, ret, removed = 0;\n+\tint i, ret;\n \n \t/* Look for a packed ref */\n \tfor (i = 0; i < n; i++)\n \t\tif (get_packed_ref(refnames[i]))\n \t\t\tbreak;\n \n-\t/* Avoid processing if we have nothing to do */\n-\tif (i == n)\n-\t\treturn 0; /* no refname exists in packed refs */\n-\n \tpacked = get_packed_refs(&ref_cache);\n \n \t/* Remove refnames from the cache */\n \tfor (i = 0; i < n; i++)\n-\t\tif (remove_entry(packed, refnames[i]) != -1)\n-\t\t\tremoved = 1;\n-\tif (!removed) {\n-\t\t/*\n-\t\t * All packed entries disappeared while we were\n-\t\t * acquiring the lock.\n-\t\t */\n-\t\trollback_packed_refs();\n-\t\treturn 0;\n-\t}\n+\t\tremove_entry(packed, refnames[i]);\n \n \t/* Remove any other accumulated cruft */\n \tdo_for_each_entry_in_dir(packed, 0, curate_packed_ref_fn, &refs_to_delete);\n@@ -3614,6 +3601,7 @@ int transaction_commit(struct ref_transaction *transaction,\n \t\t\t   struct strbuf *err)\n {\n \tint ret = 0, delnum = 0, i, df_conflict = 0, need_repack = 0;\n+\tint num_updates = 0;\n \tconst char **delnames;\n \tint n = transaction->nr;\n \tstruct packed_ref_cache *packed_ref_cache;\n@@ -3647,18 +3635,38 @@ int transaction_commit(struct ref_transaction *transaction,\n \t\tgoto cleanup;\n \t}\n \n-\t/* any loose refs are to be deleted are first copied to packed refs */\n+\t/* count how many refs we are updating (not deleting) */\n \tfor (i = 0; i < n; i++) {\n \t\tstruct ref_update *update = updates[i];\n \t\tunsigned char sha1[20];\n \n \t\tif (update->update_type != UPDATE_SHA1)\n \t\t\tcontinue;\n-\t\tif (!is_null_sha1(update->new_sha1))\n+\t\tif (is_null_sha1(update->new_sha1))\n+\t\t\tcontinue;\n+\n+\t\tnum_updates++;\n+\t}\n+\n+\t/*\n+\t * Always copy loose refs that are to be deleted to the packed refs.\n+\t * If we are updating multiple refs then copy all non symref refs\n+\t * to the packed refs too.\n+\t */\n+\tfor (i = 0; i < n; i++) {\n+\t\tstruct ref_update *update = updates[i];\n+\t\tunsigned char sha1[20];\n+\t\tint flag;\n+\n+\t\tif (update->update_type != UPDATE_SHA1)\n+\t\t\tcontinue;\n+\t\tif (num_updates < 2 && !is_null_sha1(update->new_sha1))\n \t\t\tcontinue;\n \t\tif (get_packed_ref(update->refname))\n \t\t\tcontinue;\n-\t\tif (!resolve_ref_unsafe(update->refname, sha1, 1, NULL))\n+\t\tif (!resolve_ref_unsafe(update->refname, sha1, 1, &flag))\n+\t\t\tcontinue;\n+\t\tif (flag & REF_ISSYMREF)\n \t\t\tcontinue;\n \n \t\tadd_packed_ref(update->refname, sha1);\n@@ -3682,11 +3690,14 @@ int transaction_commit(struct ref_transaction *transaction,\n \t\t\tgoto cleanup;\n \t\t}\n \t}\n+\tneed_repack = 0;\n \n \t/*\n \t * At this stage any refs that are to be deleted have been moved to the\n \t * packed refs file anf the packed refs file is deleted. We can now\n \t * safely delete these loose refs.\n+\t * If we are updating multiple normal refs, then those will also be in\n+\t * the packed refs file so we can delete them too.\n \t */\n \n \t/* Unlink any loose refs scheduled for deletion */\n@@ -3724,7 +3735,10 @@ int transaction_commit(struct ref_transaction *transaction,\n \t\tupdate->lock = NULL;\n \t}\n \n-\t/* Acquire all ref locks for updates while verifying old values */\n+\t/*\n+\t * Acquire all ref locks for updates while verifying old values.\n+\t * If we are multi-updating then update them in packed refs.\n+\t */\n \tfor (i = 0; i < n; i++) {\n \t\tstruct ref_update *update = updates[i];\n \n@@ -3748,6 +3762,22 @@ int transaction_commit(struct ref_transaction *transaction,\n \t\t\tret = -1;\n \t\t\tgoto cleanup;\n \t\t}\n+\t\tif (num_updates < 2 || update->type & REF_ISSYMREF)\n+\t\t\tcontinue;\n+\n+\t\tif (delete_ref_loose(update->lock, update->type, err)) {\n+\t\t\tret = -1;\n+\t\t\tgoto cleanup;\n+\t\t}\n+\t\tunlock_ref(update->lock);\n+\t\tupdate->lock = NULL;\n+\n+\t\tpacked = get_packed_refs(&ref_cache);;\n+\t\tremove_entry(packed, update->refname);\n+\t\tadd_packed_ref(update->refname, update->new_sha1);\n+\t\tneed_repack = 1;\n+\n+\t\ttry_remove_empty_parents((char *)update->refname);\n \t}\n \n \t/* delete reflog for all deleted refs */\n@@ -3795,7 +3825,7 @@ int transaction_commit(struct ref_transaction *transaction,\n \n \t\tif (update->update_type != UPDATE_SHA1)\n \t\t\tcontinue;\n-\t\tif (!is_null_sha1(update->new_sha1)) {\n+\t\tif (update->lock && !is_null_sha1(update->new_sha1)) {\n \t\t\tret = write_ref_sha1(update->lock, update->new_sha1,\n \t\t\t\t\t     update->msg);\n \t\t\tupdate->lock = NULL; /* freed by write_ref_sha1 */\n@@ -3860,6 +3890,10 @@ int transaction_commit(struct ref_transaction *transaction,\n \t\t}\n \t}\n \n+\tif (need_repack) {\n+\t\tpacked = get_packed_refs(&ref_cache);\n+\t\tsort_ref_dir(packed);\n+\t}\n \tif (repack_without_refs(delnames, delnum, err))\n \t\tret = -1;\n \tclear_loose_ref_cache(&ref_cache);\n-- \n2.0.0.583.g402232d\n"},{"id":"243435","messageId":"1402010788-8236-3-git-send-email-sahlberg@google.com","threadId":"36850","inReplyTo":"1402010788-8236-1-git-send-email-sahlberg@google.com","subject":"[PATCH 2/2] refs.c: make the *_packed_refs functions static","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-06-05T23:26:28Z","receivedAt":"2014-06-05T23:26:28Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"We no longer need to expose the lock/add/commit/rollback functions\nfor packed refs anymore so make them static and remove them from the api.\n\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\n---\n refs.c |  8 ++++----\n refs.h | 28 ----------------------------\n 2 files changed, 4 insertions(+), 32 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 6aa64dd..530947e 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1123,7 +1123,7 @@ static struct ref_dir *get_packed_refs(struct ref_cache *refs)\n \treturn get_packed_ref_dir(get_packed_ref_cache(refs));\n }\n \n-void add_packed_ref(const char *refname, const unsigned char *sha1)\n+static void add_packed_ref(const char *refname, const unsigned char *sha1)\n {\n \tstruct packed_ref_cache *packed_ref_cache =\n \t\tget_packed_ref_cache(&ref_cache);\n@@ -2211,7 +2211,7 @@ static int write_packed_entry_fn(struct ref_entry *entry, void *cb_data)\n \treturn 0;\n }\n \n-int lock_packed_refs(int flags)\n+static int lock_packed_refs(int flags)\n {\n \tstruct packed_ref_cache *packed_ref_cache;\n \n@@ -2234,7 +2234,7 @@ int lock_packed_refs(int flags)\n  * Commit the packed refs changes.\n  * On error we must make sure that errno contains a meaningful value.\n  */\n-int commit_packed_refs(void)\n+static int commit_packed_refs(void)\n {\n \tstruct packed_ref_cache *packed_ref_cache =\n \t\tget_packed_ref_cache(&ref_cache);\n@@ -2259,7 +2259,7 @@ int commit_packed_refs(void)\n \treturn error;\n }\n \n-void rollback_packed_refs(void)\n+static void rollback_packed_refs(void)\n {\n \tstruct packed_ref_cache *packed_ref_cache =\n \t\tget_packed_ref_cache(&ref_cache);\ndiff --git a/refs.h b/refs.h\nindex 3c99619..e92cb92 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -118,34 +118,6 @@ extern int for_each_rawref(each_ref_fn, void *);\n extern void warn_dangling_symref(FILE *fp, const char *msg_fmt, const char *refname);\n \n /*\n- * Lock the packed-refs file for writing.  Flags is passed to\n- * hold_lock_file_for_update().  Return 0 on success.\n- */\n-extern int lock_packed_refs(int flags);\n-\n-/*\n- * Add a reference to the in-memory packed reference cache.  This may\n- * only be called while the packed-refs file is locked (see\n- * lock_packed_refs()).  To actually write the packed-refs file, call\n- * commit_packed_refs().\n- */\n-extern void add_packed_ref(const char *refname, const unsigned char *sha1);\n-\n-/*\n- * Write the current version of the packed refs cache from memory to\n- * disk.  The packed-refs file must already be locked for writing (see\n- * lock_packed_refs()).  Return zero on success.\n- */\n-extern int commit_packed_refs(void);\n-\n-/*\n- * Rollback the lockfile for the packed-refs file, and discard the\n- * in-memory packed reference cache.  (The packed-refs file will be\n- * read anew if it is needed again after this function is called.)\n- */\n-extern void rollback_packed_refs(void);\n-\n-/*\n  * Flags for controlling behaviour of pack_refs()\n  * PACK_REFS_PRUNE: Prune loose refs after packing\n  * PACK_REFS_ALL:   Pack _all_ refs, not just tags and already packed refs\n-- \n2.0.0.583.g402232d\n"},{"id":"243600","messageId":"CAPig+cTQocaZBHciFkP591A11w9qcKAvG5a2H+R=7QHVvxU4EQ@mail.gmail.com","threadId":"36850","inReplyTo":"1402010788-8236-2-git-send-email-sahlberg@google.com","subject":"Re: [PATCH 1/2] refs.c: write updates to packed refs when a transaction has more than one ref","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-06-08T09:03:12Z","receivedAt":"2014-06-08T09:03:12Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Jun 5, 2014 at 7:26 PM, Ronnie Sahlberg <sahlberg@google.com> wrote:\n> When we are updating more than one single ref, i.e. not a commit, then\n> write the updated refs directly to the packed refs file instead of writing\n> them as loose refs.\n>\n> Change clone to use a transaction instead of using the pacekd refs api.\n\ns/pacekd/packed/\n\n> Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>\n"},{"id":"243627","messageId":"CAL=YDWk+KsctVnLMFv6tWC-=Z6uAaTbYLnJJUyBaY+yxwmM7Mw@mail.gmail.com","threadId":"36850","inReplyTo":"CAPig+cTQocaZBHciFkP591A11w9qcKAvG5a2H+R=7QHVvxU4EQ@mail.gmail.com","subject":"Re: [PATCH 1/2] refs.c: write updates to packed refs when a transaction has more than one ref","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-06-09T16:20:26Z","receivedAt":"2014-06-09T16:20:26Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"thanks\n\nOn Sun, Jun 8, 2014 at 2:03 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:\n> On Thu, Jun 5, 2014 at 7:26 PM, Ronnie Sahlberg <sahlberg@google.com> wrote:\n>> When we are updating more than one single ref, i.e. not a commit, then\n>> write the updated refs directly to the packed refs file instead of writing\n>> them as loose refs.\n>>\n>> Change clone to use a transaction instead of using the pacekd refs api.\n>\n> s/pacekd/packed/\n>\n>> Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>\n"}]}