{"thread":{"id":"37136","subject":"[PATCH 02/20] refs.c: update ref_transaction_delete to check for error and return status","startedAt":"2014-07-15T23:33:58Z","lastAt":"2014-07-16T22:52:10Z","messageCount":24,"participants":["Ronnie Sahlberg","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":20},"messages":[{"id":"246120","messageId":"1405467258-24102-1-git-send-email-sahlberg@google.com","threadId":"37136","inReplyTo":null,"subject":"[PATCH 00/20] ref transactions part 2","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-07-15T23:33:58Z","receivedAt":"2014-07-15T23:33:58Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"This is the next 20 patches from my originally big patch series and follow\nthe previous 19 patches that is now in juns tree.\nThese patches were numbered 20-39 in the original 48-patch series.\n\nChanges since these patches were in the original series:\n\n- Addressing concerns from mhagger's review\n\n\nRonnie Sahlberg (20):\n  refs.c: change ref_transaction_create to do error checking and return\n    status\n  refs.c: update ref_transaction_delete to check for error and return\n    status\n  refs.c: make ref_transaction_begin take an err argument\n  refs.c: add transaction.status and track OPEN/CLOSED/ERROR\n  tag.c: use ref transactions when doing updates\n  replace.c: use the ref transaction functions for updates\n  commit.c: use ref transactions for updates\n  sequencer.c: use ref transactions for all ref updates\n  fast-import.c: change update_branch to use ref transactions\n  branch.c: use ref transaction for all ref updates\n  refs.c: change update_ref to use a transaction\n  receive-pack.c: use a reference transaction for updating the refs\n  fast-import.c: use a ref transaction when dumping tags\n  walker.c: use ref transaction for ref updates\n  refs.c: make lock_ref_sha1 static\n  refs.c: remove the update_ref_lock function\n  refs.c: remove the update_ref_write function\n  refs.c: remove lock_ref_sha1\n  refs.c: make prune_ref use a transaction to delete the ref\n  refs.c: make delete_ref use a transaction\n\n branch.c               |  30 +++---\n builtin/commit.c       |  24 +++--\n builtin/receive-pack.c |  96 +++++++++++++-------\n builtin/replace.c      |  15 +--\n builtin/tag.c          |  15 +--\n builtin/update-ref.c   |  11 ++-\n fast-import.c          |  53 +++++++----\n refs.c                 | 242 ++++++++++++++++++++++++++++---------------------\n refs.h                 |  78 ++++++++++++----\n sequencer.c            |  27 ++++--\n walker.c               |  59 +++++++-----\n 11 files changed, 403 insertions(+), 247 deletions(-)\n\n-- \n2.0.1.442.g7fe6834.dirty\n"},{"id":"246121","messageId":"1405467258-24102-2-git-send-email-sahlberg@google.com","threadId":"37136","inReplyTo":"1405467258-24102-1-git-send-email-sahlberg@google.com","subject":"[PATCH 01/20] refs.c: change ref_transaction_create to do error checking and return status","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-07-15T23:33:59Z","receivedAt":"2014-07-15T23:33:59Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Do basic error checking in ref_transaction_create() and make it return\nnon-zero on error. Update all callers to check the result of\nref_transaction_create(). There are currently no conditions in _create that\nwill return error but there will be in the future. Add an err argument that\nwill be updated on failure.\n\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\n---\n builtin/update-ref.c |  4 +++-\n refs.c               | 18 ++++++++++++------\n refs.h               | 48 +++++++++++++++++++++++++++++++++++++++++-------\n 3 files changed, 56 insertions(+), 14 deletions(-)\n\ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex 3067b11..41121fa 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -226,7 +226,9 @@ static const char *parse_cmd_create(struct strbuf *input, const char *next)\n \tif (*next != line_termination)\n \t\tdie(\"create %s: extra input: %s\", refname, next);\n \n-\tref_transaction_create(transaction, refname, new_sha1, update_flags);\n+\tif (ref_transaction_create(transaction, refname, new_sha1,\n+\t\t\t\t   update_flags, &err))\n+\t\tdie(\"%s\", err.buf);\n \n \tupdate_flags = 0;\n \tfree(refname);\ndiff --git a/refs.c b/refs.c\nindex 3f05e88..c49f1c6 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3449,18 +3449,24 @@ int ref_transaction_update(struct ref_transaction *transaction,\n \treturn 0;\n }\n \n-void ref_transaction_create(struct ref_transaction *transaction,\n-\t\t\t    const char *refname,\n-\t\t\t    const unsigned char *new_sha1,\n-\t\t\t    int flags)\n+int ref_transaction_create(struct ref_transaction *transaction,\n+\t\t\t   const char *refname,\n+\t\t\t   const unsigned char *new_sha1,\n+\t\t\t   int flags,\n+\t\t\t   struct strbuf *err)\n {\n-\tstruct ref_update *update = add_update(transaction, refname);\n+\tstruct ref_update *update;\n+\n+\tif (!new_sha1 || is_null_sha1(new_sha1))\n+\t\tdie(\"BUG: create ref with null new_sha1\");\n+\n+\tupdate = add_update(transaction, refname);\n \n-\tassert(!is_null_sha1(new_sha1));\n \thashcpy(update->new_sha1, new_sha1);\n \thashclr(update->old_sha1);\n \tupdate->flags = flags;\n \tupdate->have_old = 1;\n+\treturn 0;\n }\n \n void ref_transaction_delete(struct ref_transaction *transaction,\ndiff --git a/refs.h b/refs.h\nindex c5376ce..b648819 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -10,6 +10,38 @@ struct ref_lock {\n \tint force_write;\n };\n \n+/*\n+ * A ref_transaction represents a collection of ref updates\n+ * that should succeed or fail together.\n+ *\n+ * Calling sequence\n+ * ----------------\n+ * - Allocate and initialize a `struct ref_transaction` by calling\n+ *   `ref_transaction_begin()`.\n+ *\n+ * - List intended ref updates by calling functions like\n+ *   `ref_transaction_update()` and `ref_transaction_create()`.\n+ *\n+ * - Call `ref_transaction_commit()` to execute the transaction.\n+ *   If this succeeds, the ref updates will have taken place and\n+ *   the transaction cannot be rolled back.\n+ *\n+ * - At any time call `ref_transaction_free()` to discard the\n+ *   transaction and free associated resources.  In particular,\n+ *   this rolls back the transaction if it has not been\n+ *   successfully committed.\n+ *\n+ * Error handling\n+ * --------------\n+ *\n+ * On error, transaction functions append a message about what\n+ * went wrong to the 'err' argument.  The message mentions what\n+ * ref was being updated (if any) when the error occurred so it\n+ * can be passed to 'die' or 'error' as-is.\n+ *\n+ * The message is appended to err without first clearing err.\n+ * err will not be '\\n' terminated.\n+ */\n struct ref_transaction;\n \n /*\n@@ -248,7 +280,7 @@ struct ref_transaction *ref_transaction_begin(void);\n  * it must not have existed beforehand.\n  * Function returns 0 on success and non-zero on failure. A failure to update\n  * means that the transaction as a whole has failed and will need to be\n- * rolled back. On failure the err buffer will be updated.\n+ * rolled back.\n  */\n int ref_transaction_update(struct ref_transaction *transaction,\n \t\t\t   const char *refname,\n@@ -262,11 +294,15 @@ int ref_transaction_update(struct ref_transaction *transaction,\n  * that the reference should have after the update; it must not be the\n  * null SHA-1.  It is verified that the reference does not exist\n  * already.\n+ * Function returns 0 on success and non-zero on failure. A failure to create\n+ * means that the transaction as a whole has failed and will need to be\n+ * rolled back.\n  */\n-void ref_transaction_create(struct ref_transaction *transaction,\n-\t\t\t    const char *refname,\n-\t\t\t    const unsigned char *new_sha1,\n-\t\t\t    int flags);\n+int ref_transaction_create(struct ref_transaction *transaction,\n+\t\t\t   const char *refname,\n+\t\t\t   const unsigned char *new_sha1,\n+\t\t\t   int flags,\n+\t\t\t   struct strbuf *err);\n \n /*\n  * Add a reference deletion to transaction.  If have_old is true, then\n@@ -282,8 +318,6 @@ void ref_transaction_delete(struct ref_transaction *transaction,\n  * Commit all of the changes that have been queued in transaction, as\n  * atomically as possible.  Return a nonzero value if there is a\n  * problem.\n- * If err is non-NULL we will add an error string to it to explain why\n- * the transaction failed. The string does not end in newline.\n  */\n int ref_transaction_commit(struct ref_transaction *transaction,\n \t\t\t   const char *msg, struct strbuf *err);\n-- \n2.0.1.442.g7fe6834.dirty\n"},{"id":"246119","messageId":"1405467258-24102-3-git-send-email-sahlberg@google.com","threadId":"37136","inReplyTo":"1405467258-24102-1-git-send-email-sahlberg@google.com","subject":"[PATCH 02/20] refs.c: update ref_transaction_delete to check for error and return status","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-07-15T23:34:00Z","receivedAt":"2014-07-15T23:34:00Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Change ref_transaction_delete() to do basic error checking and return\nnon-zero of error. Update all callers to check the return for\nref_transaction_delete(). There are currently no conditions in _delete that\nwill return error but there will be in the future. Add an err argument that\nwill be updated on failure.\n\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\n---\n builtin/update-ref.c |  5 +++--\n refs.c               | 16 +++++++++++-----\n refs.h               | 12 ++++++++----\n 3 files changed, 22 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex 41121fa..7c9c248 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -258,8 +258,9 @@ static const char *parse_cmd_delete(struct strbuf *input, const char *next)\n \tif (*next != line_termination)\n \t\tdie(\"delete %s: extra input: %s\", refname, next);\n \n-\tref_transaction_delete(transaction, refname, old_sha1,\n-\t\t\t       update_flags, have_old);\n+\tif (ref_transaction_delete(transaction, refname, old_sha1,\n+\t\t\t\t   update_flags, have_old, &err))\n+\t\tdie(\"%s\", err.buf);\n \n \tupdate_flags = 0;\n \tfree(refname);\ndiff --git a/refs.c b/refs.c\nindex c49f1c6..40f04f4 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3469,19 +3469,25 @@ int ref_transaction_create(struct ref_transaction *transaction,\n \treturn 0;\n }\n \n-void ref_transaction_delete(struct ref_transaction *transaction,\n-\t\t\t    const char *refname,\n-\t\t\t    const unsigned char *old_sha1,\n-\t\t\t    int flags, int have_old)\n+int ref_transaction_delete(struct ref_transaction *transaction,\n+\t\t\t   const char *refname,\n+\t\t\t   const unsigned char *old_sha1,\n+\t\t\t   int flags, int have_old,\n+\t\t\t   struct strbuf *err)\n {\n-\tstruct ref_update *update = add_update(transaction, refname);\n+\tstruct ref_update *update;\n \n+\tif (have_old && !old_sha1)\n+\t\tdie(\"BUG: have_old is true but old_sha1 is NULL\");\n+\n+\tupdate = add_update(transaction, refname);\n \tupdate->flags = flags;\n \tupdate->have_old = have_old;\n \tif (have_old) {\n \t\tassert(!is_null_sha1(old_sha1));\n \t\thashcpy(update->old_sha1, old_sha1);\n \t}\n+\treturn 0;\n }\n \n int update_ref(const char *action, const char *refname,\ndiff --git a/refs.h b/refs.h\nindex b648819..71389a1 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -308,11 +308,15 @@ int ref_transaction_create(struct ref_transaction *transaction,\n  * Add a reference deletion to transaction.  If have_old is true, then\n  * old_sha1 holds the value that the reference should have had before\n  * the update (which must not be the null SHA-1).\n+ * Function returns 0 on success and non-zero on failure. A failure to delete\n+ * means that the transaction as a whole has failed and will need to be\n+ * rolled back.\n  */\n-void ref_transaction_delete(struct ref_transaction *transaction,\n-\t\t\t    const char *refname,\n-\t\t\t    const unsigned char *old_sha1,\n-\t\t\t    int flags, int have_old);\n+int ref_transaction_delete(struct ref_transaction *transaction,\n+\t\t\t   const char *refname,\n+\t\t\t   const unsigned char *old_sha1,\n+\t\t\t   int flags, int have_old,\n+\t\t\t   struct strbuf *err);\n \n /*\n  * Commit all of the changes that have been queued in transaction, as\n-- \n2.0.1.442.g7fe6834.dirty\n"},{"id":"246122","messageId":"1405467258-24102-4-git-send-email-sahlberg@google.com","threadId":"37136","inReplyTo":"1405467258-24102-1-git-send-email-sahlberg@google.com","subject":"[PATCH 03/20] refs.c: make ref_transaction_begin take an err argument","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-07-15T23:34:01Z","receivedAt":"2014-07-15T23:34:01Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Add an err argument to _begin so that on non-fatal failures in future ref\nbackends we can report a nice error back to the caller.\nWhile _begin can currently never fail for other reasons than OOM, in which\ncase we die() anyway, we may add other types of backends in the future.\nFor example, a hypothetical MySQL backend could fail in _begin with\n\"Can not connect to MySQL server. No route to host\".\n\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\n---\n builtin/update-ref.c | 2 +-\n refs.c               | 2 +-\n refs.h               | 2 +-\n 3 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex 7c9c248..c6ad0be 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -365,7 +365,7 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)\n \t\tdie(\"Refusing to perform update with empty message.\");\n \n \tif (read_stdin) {\n-\t\ttransaction = ref_transaction_begin();\n+\t\ttransaction = ref_transaction_begin(&err);\n \t\tif (delete || no_deref || argc > 0)\n \t\t\tusage_with_options(git_update_ref_usage, options);\n \t\tif (end_null)\ndiff --git a/refs.c b/refs.c\nindex 40f04f4..9cb7908 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3397,7 +3397,7 @@ struct ref_transaction {\n \tsize_t nr;\n };\n \n-struct ref_transaction *ref_transaction_begin(void)\n+struct ref_transaction *ref_transaction_begin(struct strbuf *err)\n {\n \treturn xcalloc(1, sizeof(struct ref_transaction));\n }\ndiff --git a/refs.h b/refs.h\nindex 71389a1..3f37c65 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -262,7 +262,7 @@ enum action_on_err {\n  * Begin a reference transaction.  The reference transaction must\n  * be freed by calling ref_transaction_free().\n  */\n-struct ref_transaction *ref_transaction_begin(void);\n+struct ref_transaction *ref_transaction_begin(struct strbuf *err);\n \n /*\n  * The following functions add a reference check or update to a\n-- \n2.0.1.442.g7fe6834.dirty\n"},{"id":"246137","messageId":"1405467258-24102-5-git-send-email-sahlberg@google.com","threadId":"37136","inReplyTo":"1405467258-24102-1-git-send-email-sahlberg@google.com","subject":"[PATCH 04/20] refs.c: add transaction.status and track OPEN/CLOSED/ERROR","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-07-15T23:34:02Z","receivedAt":"2014-07-15T23:34:02Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Track the state of a transaction in a new state field. Check the field for\nsanity, i.e. that state must be OPEN when _commit/_create/_delete or\n_update is called or else die(BUG:...)\n\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\n---\n refs.c | 39 ++++++++++++++++++++++++++++++++++++++-\n 1 file changed, 38 insertions(+), 1 deletion(-)\n\ndiff --git a/refs.c b/refs.c\nindex 9cb7908..d015285 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3387,6 +3387,25 @@ struct ref_update {\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: If an open transaction is successfully committed the state will\n+ *         change to CLOSED. No further changes can be made to a CLOSED\n+ *         transaction.\n+ *         CLOSED means that all updates have been successfully committed and\n+ *         the only thing that remains is to free the completed transaction.\n+ * ERROR:  The transaction has failed and is no longer committable.\n+ *         No further changes can be made to a CLOSED transaction and it must\n+ *         be rolled back using transaction_free.\n+ */\n+enum ref_transaction_state {\n+\tREF_TRANSACTION_OPEN   = 0,\n+\tREF_TRANSACTION_CLOSED = 1,\n+\tREF_TRANSACTION_ERROR  = 2,\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@@ -3395,6 +3414,7 @@ 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@@ -3437,6 +3457,9 @@ int ref_transaction_update(struct ref_transaction *transaction,\n {\n \tstruct ref_update *update;\n \n+\tif (transaction->state != REF_TRANSACTION_OPEN)\n+\t\tdie(\"BUG: update called for transaction that is not open\");\n+\n \tif (have_old && !old_sha1)\n \t\tdie(\"BUG: have_old is true but old_sha1 is NULL\");\n \n@@ -3457,6 +3480,9 @@ int ref_transaction_create(struct ref_transaction *transaction,\n {\n \tstruct ref_update *update;\n \n+\tif (transaction->state != REF_TRANSACTION_OPEN)\n+\t\tdie(\"BUG: create called for transaction that is not open\");\n+\n \tif (!new_sha1 || is_null_sha1(new_sha1))\n \t\tdie(\"BUG: create ref with null new_sha1\");\n \n@@ -3477,6 +3503,9 @@ int ref_transaction_delete(struct ref_transaction *transaction,\n {\n \tstruct ref_update *update;\n \n+\tif (transaction->state != REF_TRANSACTION_OPEN)\n+\t\tdie(\"BUG: delete called for transaction that is not open\");\n+\n \tif (have_old && !old_sha1)\n \t\tdie(\"BUG: have_old is true but old_sha1 is NULL\");\n \n@@ -3532,8 +3561,13 @@ int ref_transaction_commit(struct ref_transaction *transaction,\n \tint n = transaction->nr;\n \tstruct ref_update **updates = transaction->updates;\n \n-\tif (!n)\n+\tif (transaction->state != REF_TRANSACTION_OPEN)\n+\t\tdie(\"BUG: commit called for transaction that is not open\");\n+\n+\tif (!n) {\n+\t\ttransaction->state = REF_TRANSACTION_CLOSED;\n \t\treturn 0;\n+\t}\n \n \t/* Allocate work space */\n \tdelnames = xmalloc(sizeof(*delnames) * n);\n@@ -3595,6 +3629,9 @@ int ref_transaction_commit(struct ref_transaction *transaction,\n \tclear_loose_ref_cache(&ref_cache);\n \n cleanup:\n+\ttransaction->state = ret ? REF_TRANSACTION_ERROR\n+\t\t: REF_TRANSACTION_CLOSED;\n+\n \tfor (i = 0; i < n; i++)\n \t\tif (updates[i]->lock)\n \t\t\tunlock_ref(updates[i]->lock);\n-- \n2.0.1.442.g7fe6834.dirty\n"},{"id":"246124","messageId":"1405467258-24102-6-git-send-email-sahlberg@google.com","threadId":"37136","inReplyTo":"1405467258-24102-1-git-send-email-sahlberg@google.com","subject":"[PATCH 05/20] tag.c: use ref transactions when doing updates","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-07-15T23:34:03Z","receivedAt":"2014-07-15T23:34:03Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Change tag.c to use ref transactions for all ref updates.\n\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\n---\n builtin/tag.c | 15 +++++++++------\n 1 file changed, 9 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex c6e8a71..1aa88a2 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -548,7 +548,6 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \tstruct strbuf ref = STRBUF_INIT;\n \tunsigned char object[20], prev[20];\n \tconst char *object_ref, *tag;\n-\tstruct ref_lock *lock;\n \tstruct create_tag_options opt;\n \tchar *cleanup_arg = NULL;\n \tint annotate = 0, force = 0, lines = -1;\n@@ -556,6 +555,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \tconst char *msgfile = NULL, *keyid = NULL;\n \tstruct msg_arg msg = { 0, STRBUF_INIT };\n \tstruct commit_list *with_commit = NULL;\n+\tstruct ref_transaction *transaction;\n+\tstruct strbuf err = STRBUF_INIT;\n \tstruct option options[] = {\n \t\tOPT_CMDMODE('l', \"list\", &cmdmode, N_(\"list tag names\"), 'l'),\n \t\t{ OPTION_INTEGER, 'n', NULL, &lines, N_(\"n\"),\n@@ -701,11 +702,13 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \tif (annotate)\n \t\tcreate_tag(object, tag, &buf, &opt, prev, object);\n \n-\tlock = lock_any_ref_for_update(ref.buf, prev, 0, NULL);\n-\tif (!lock)\n-\t\tdie(_(\"%s: cannot lock the ref\"), ref.buf);\n-\tif (write_ref_sha1(lock, object, NULL) < 0)\n-\t\tdie(_(\"%s: cannot update the ref\"), ref.buf);\n+\ttransaction = ref_transaction_begin(&err);\n+\tif (!transaction ||\n+\t    ref_transaction_update(transaction, ref.buf, object, prev,\n+\t\t\t\t   0, 1, &err) ||\n+\t    ref_transaction_commit(transaction, NULL, &err))\n+\t\tdie(\"%s\", err.buf);\n+\tref_transaction_free(transaction);\n \tif (force && !is_null_sha1(prev) && hashcmp(prev, object))\n \t\tprintf(_(\"Updated tag '%s' (was %s)\\n\"), tag, find_unique_abbrev(prev, DEFAULT_ABBREV));\n \n-- \n2.0.1.442.g7fe6834.dirty\n"},{"id":"246127","messageId":"1405467258-24102-7-git-send-email-sahlberg@google.com","threadId":"37136","inReplyTo":"1405467258-24102-1-git-send-email-sahlberg@google.com","subject":"[PATCH 06/20] replace.c: use the ref transaction functions for updates","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-07-15T23:34:04Z","receivedAt":"2014-07-15T23:34:04Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Update replace.c to use ref transactions for updates.\n\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\n---\n builtin/replace.c | 15 +++++++++------\n 1 file changed, 9 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/replace.c b/builtin/replace.c\nindex 1bb491d..7528f3d 100644\n--- a/builtin/replace.c\n+++ b/builtin/replace.c\n@@ -153,7 +153,8 @@ static int replace_object_sha1(const char *object_ref,\n \tunsigned char prev[20];\n \tenum object_type obj_type, repl_type;\n \tchar ref[PATH_MAX];\n-\tstruct ref_lock *lock;\n+\tstruct ref_transaction *transaction;\n+\tstruct strbuf err = STRBUF_INIT;\n \n \tobj_type = sha1_object_info(object, NULL);\n \trepl_type = sha1_object_info(repl, NULL);\n@@ -166,12 +167,14 @@ static int replace_object_sha1(const char *object_ref,\n \n \tcheck_ref_valid(object, prev, ref, sizeof(ref), force);\n \n-\tlock = lock_any_ref_for_update(ref, prev, 0, NULL);\n-\tif (!lock)\n-\t\tdie(\"%s: cannot lock the ref\", ref);\n-\tif (write_ref_sha1(lock, repl, NULL) < 0)\n-\t\tdie(\"%s: cannot update the ref\", ref);\n+\ttransaction = ref_transaction_begin(&err);\n+\tif (!transaction ||\n+\t    ref_transaction_update(transaction, ref, repl, prev,\n+\t\t\t\t   0, !is_null_sha1(prev), &err) ||\n+\t    ref_transaction_commit(transaction, NULL, &err))\n+\t\tdie(\"%s\", err.buf);\n \n+\tref_transaction_free(transaction);\n \treturn 0;\n }\n \n-- \n2.0.1.442.g7fe6834.dirty\n"},{"id":"246136","messageId":"1405467258-24102-8-git-send-email-sahlberg@google.com","threadId":"37136","inReplyTo":"1405467258-24102-1-git-send-email-sahlberg@google.com","subject":"[PATCH 07/20] commit.c: use ref transactions for updates","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-07-15T23:34:05Z","receivedAt":"2014-07-15T23:34:05Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Change commit.c to use ref transactions for all ref updates.\nMake sure we pass a NULL pointer to ref_transaction_update if have_old\nis false.\n\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\n---\n builtin/commit.c | 24 +++++++++++-------------\n 1 file changed, 11 insertions(+), 13 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 5e2221c..668fa6a 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1627,11 +1627,12 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \tconst char *index_file, *reflog_msg;\n \tchar *nl;\n \tunsigned char sha1[20];\n-\tstruct ref_lock *ref_lock;\n \tstruct commit_list *parents = NULL, **pptr = &parents;\n \tstruct stat statbuf;\n \tstruct commit *current_head = NULL;\n \tstruct commit_extra_header *extra = NULL;\n+\tstruct ref_transaction *transaction;\n+\tstruct strbuf err = STRBUF_INIT;\n \n \tif (argc == 2 && !strcmp(argv[1], \"-h\"))\n \t\tusage_with_options(builtin_commit_usage, builtin_commit_options);\n@@ -1753,16 +1754,6 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \tstrbuf_release(&author_ident);\n \tfree_commit_extra_headers(extra);\n \n-\tref_lock = lock_any_ref_for_update(\"HEAD\",\n-\t\t\t\t\t   !current_head\n-\t\t\t\t\t   ? NULL\n-\t\t\t\t\t   : current_head->object.sha1,\n-\t\t\t\t\t   0, NULL);\n-\tif (!ref_lock) {\n-\t\trollback_index_files();\n-\t\tdie(_(\"cannot lock HEAD ref\"));\n-\t}\n-\n \tnl = strchr(sb.buf, '\\n');\n \tif (nl)\n \t\tstrbuf_setlen(&sb, nl + 1 - sb.buf);\n@@ -1771,10 +1762,17 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \tstrbuf_insert(&sb, 0, reflog_msg, strlen(reflog_msg));\n \tstrbuf_insert(&sb, strlen(reflog_msg), \": \", 2);\n \n-\tif (write_ref_sha1(ref_lock, sha1, sb.buf) < 0) {\n+\ttransaction = ref_transaction_begin(&err);\n+\tif (!transaction ||\n+\t    ref_transaction_update(transaction, \"HEAD\", sha1,\n+\t\t\t\t   current_head ?\n+\t\t\t\t   current_head->object.sha1 : NULL,\n+\t\t\t\t   0, !!current_head, &err) ||\n+\t    ref_transaction_commit(transaction, sb.buf, &err)) {\n \t\trollback_index_files();\n-\t\tdie(_(\"cannot update HEAD ref\"));\n+\t\tdie(\"%s\", err.buf);\n \t}\n+\tref_transaction_free(transaction);\n \n \tunlink(git_path(\"CHERRY_PICK_HEAD\"));\n \tunlink(git_path(\"REVERT_HEAD\"));\n-- \n2.0.1.442.g7fe6834.dirty\n"},{"id":"246125","messageId":"1405467258-24102-9-git-send-email-sahlberg@google.com","threadId":"37136","inReplyTo":"1405467258-24102-1-git-send-email-sahlberg@google.com","subject":"[PATCH 08/20] sequencer.c: use ref transactions for all ref updates","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-07-15T23:34:06Z","receivedAt":"2014-07-15T23:34:06Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Change to use ref transactions for all updates to refs.\n\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\n---\n sequencer.c | 27 ++++++++++++++++++---------\n 1 file changed, 18 insertions(+), 9 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex 9230474..cf17c69 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -272,23 +272,32 @@ static int error_dirty_index(struct replay_opts *opts)\n static int fast_forward_to(const unsigned char *to, const unsigned char *from,\n \t\t\tint unborn, struct replay_opts *opts)\n {\n-\tstruct ref_lock *ref_lock;\n+\tstruct ref_transaction *transaction;\n \tstruct strbuf sb = STRBUF_INIT;\n-\tint ret;\n+\tstruct strbuf err = STRBUF_INIT;\n \n \tread_cache();\n \tif (checkout_fast_forward(from, to, 1))\n-\t\texit(128); /* the callee should have complained already */\n-\tref_lock = lock_any_ref_for_update(\"HEAD\", unborn ? null_sha1 : from,\n-\t\t\t\t\t   0, NULL);\n-\tif (!ref_lock)\n-\t\treturn error(_(\"Failed to lock HEAD during fast_forward_to\"));\n+\t\texit(1); /* the callee should have complained already */\n \n \tstrbuf_addf(&sb, \"%s: fast-forward\", action_name(opts));\n-\tret = write_ref_sha1(ref_lock, to, sb.buf);\n+\n+\ttransaction = ref_transaction_begin(&err);\n+\tif (!transaction ||\n+\t    ref_transaction_update(transaction, \"HEAD\",\n+\t\t\t\t   to, unborn ? null_sha1 : from,\n+\t\t\t\t   0, 1, &err) ||\n+\t    ref_transaction_commit(transaction, sb.buf, &err)) {\n+\t\tref_transaction_free(transaction);\n+\t\terror(\"%s\", err.buf);\n+\t\tstrbuf_release(&sb);\n+\t\tstrbuf_release(&err);\n+\t\treturn -1;\n+\t}\n \n \tstrbuf_release(&sb);\n-\treturn ret;\n+\tref_transaction_free(transaction);\n+\treturn 0;\n }\n \n static int do_recursive_merge(struct commit *base, struct commit *next,\n-- \n2.0.1.442.g7fe6834.dirty\n"},{"id":"246138","messageId":"1405467258-24102-10-git-send-email-sahlberg@google.com","threadId":"37136","inReplyTo":"1405467258-24102-1-git-send-email-sahlberg@google.com","subject":"[PATCH 09/20] fast-import.c: change update_branch to use ref transactions","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-07-15T23:34:07Z","receivedAt":"2014-07-15T23:34:07Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Change update_branch() to use ref transactions for updates.\n\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\n---\n fast-import.c | 24 ++++++++++++++----------\n 1 file changed, 14 insertions(+), 10 deletions(-)\n\ndiff --git a/fast-import.c b/fast-import.c\nindex 6707a66..d5206ee 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -1679,8 +1679,9 @@ found_entry:\n static int update_branch(struct branch *b)\n {\n \tstatic const char *msg = \"fast-import\";\n-\tstruct ref_lock *lock;\n+\tstruct ref_transaction *transaction;\n \tunsigned char old_sha1[20];\n+\tstruct strbuf err = STRBUF_INIT;\n \n \tif (read_ref(b->name, old_sha1))\n \t\thashclr(old_sha1);\n@@ -1689,29 +1690,32 @@ static int update_branch(struct branch *b)\n \t\t\tdelete_ref(b->name, old_sha1, 0);\n \t\treturn 0;\n \t}\n-\tlock = lock_any_ref_for_update(b->name, old_sha1, 0, NULL);\n-\tif (!lock)\n-\t\treturn error(\"Unable to lock %s\", b->name);\n \tif (!force_update && !is_null_sha1(old_sha1)) {\n \t\tstruct commit *old_cmit, *new_cmit;\n \n \t\told_cmit = lookup_commit_reference_gently(old_sha1, 0);\n \t\tnew_cmit = lookup_commit_reference_gently(b->sha1, 0);\n-\t\tif (!old_cmit || !new_cmit) {\n-\t\t\tunlock_ref(lock);\n+\t\tif (!old_cmit || !new_cmit)\n \t\t\treturn error(\"Branch %s is missing commits.\", b->name);\n-\t\t}\n \n \t\tif (!in_merge_bases(old_cmit, new_cmit)) {\n-\t\t\tunlock_ref(lock);\n \t\t\twarning(\"Not updating %s\"\n \t\t\t\t\" (new tip %s does not contain %s)\",\n \t\t\t\tb->name, sha1_to_hex(b->sha1), sha1_to_hex(old_sha1));\n \t\t\treturn -1;\n \t\t}\n \t}\n-\tif (write_ref_sha1(lock, b->sha1, msg) < 0)\n-\t\treturn error(\"Unable to update %s\", b->name);\n+\ttransaction = ref_transaction_begin(&err);\n+\tif (!transaction ||\n+\t    ref_transaction_update(transaction, b->name, b->sha1, old_sha1,\n+\t\t\t\t   0, 1, &err) ||\n+\t    ref_transaction_commit(transaction, msg, &err)) {\n+\t\tref_transaction_free(transaction);\n+\t\terror(\"%s\", err.buf);\n+\t\tstrbuf_release(&err);\n+\t\treturn -1;\n+\t}\n+\tref_transaction_free(transaction);\n \treturn 0;\n }\n \n-- \n2.0.1.442.g7fe6834.dirty\n"},{"id":"246129","messageId":"1405467258-24102-11-git-send-email-sahlberg@google.com","threadId":"37136","inReplyTo":"1405467258-24102-1-git-send-email-sahlberg@google.com","subject":"[PATCH 10/20] branch.c: use ref transaction for all ref updates","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-07-15T23:34:08Z","receivedAt":"2014-07-15T23:34:08Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Change create_branch to use a ref transaction when creating the new branch.\n\nThis also fixes a race condition in the old code where two concurrent\ncreate_branch could race since the lock_any_ref_for_update/write_ref_sha1\ndid not protect against the ref already existing. I.e. one thread could end up\noverwriting a branch even if the forcing flag is false.\n\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\n---\n branch.c | 30 ++++++++++++++++--------------\n 1 file changed, 16 insertions(+), 14 deletions(-)\n\ndiff --git a/branch.c b/branch.c\nindex 660097b..c1eae00 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -226,7 +226,6 @@ void create_branch(const char *head,\n \t\t   int force, int reflog, int clobber_head,\n \t\t   int quiet, enum branch_track track)\n {\n-\tstruct ref_lock *lock = NULL;\n \tstruct commit *commit;\n \tunsigned char sha1[20];\n \tchar *real_ref, msg[PATH_MAX + 20];\n@@ -285,15 +284,6 @@ void create_branch(const char *head,\n \t\tdie(_(\"Not a valid branch point: '%s'.\"), start_name);\n \thashcpy(sha1, commit->object.sha1);\n \n-\tif (!dont_change_ref) {\n-\t\tlock = lock_any_ref_for_update(ref.buf, NULL, 0, NULL);\n-\t\tif (!lock)\n-\t\t\tdie_errno(_(\"Failed to lock ref for update\"));\n-\t}\n-\n-\tif (reflog)\n-\t\tlog_all_ref_updates = 1;\n-\n \tif (forcing)\n \t\tsnprintf(msg, sizeof msg, \"branch: Reset to %s\",\n \t\t\t start_name);\n@@ -301,13 +291,25 @@ void create_branch(const char *head,\n \t\tsnprintf(msg, sizeof msg, \"branch: Created from %s\",\n \t\t\t start_name);\n \n+\tif (reflog)\n+\t\tlog_all_ref_updates = 1;\n+\n+\tif (!dont_change_ref) {\n+\t\tstruct ref_transaction *transaction;\n+\t\tstruct strbuf err = STRBUF_INIT;\n+\n+\t\ttransaction = ref_transaction_begin(&err);\n+\t\tif (!transaction ||\n+\t\t    ref_transaction_update(transaction, ref.buf, sha1,\n+\t\t\t\t\t   null_sha1, 0, !forcing, &err) ||\n+\t\t    ref_transaction_commit(transaction, msg, &err))\n+\t\t\tdie(\"%s\", err.buf);\n+\t\tref_transaction_free(transaction);\n+\t}\n+\n \tif (real_ref && track)\n \t\tsetup_tracking(ref.buf + 11, real_ref, track, quiet);\n \n-\tif (!dont_change_ref)\n-\t\tif (write_ref_sha1(lock, sha1, msg) < 0)\n-\t\t\tdie_errno(_(\"Failed to write ref\"));\n-\n \tstrbuf_release(&ref);\n \tfree(real_ref);\n }\n-- \n2.0.1.442.g7fe6834.dirty\n"},{"id":"246130","messageId":"1405467258-24102-12-git-send-email-sahlberg@google.com","threadId":"37136","inReplyTo":"1405467258-24102-1-git-send-email-sahlberg@google.com","subject":"[PATCH 11/20] refs.c: change update_ref to use a transaction","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-07-15T23:34:09Z","receivedAt":"2014-07-15T23:34:09Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Change the update_ref helper function to use a ref transaction internally.\n\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\n---\n refs.c | 28 ++++++++++++++++++++++++----\n 1 file changed, 24 insertions(+), 4 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex d015285..ff4e799 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3523,11 +3523,31 @@ int update_ref(const char *action, const char *refname,\n \t       const unsigned char *sha1, const unsigned char *oldval,\n \t       int flags, enum action_on_err onerr)\n {\n-\tstruct ref_lock *lock;\n-\tlock = update_ref_lock(refname, oldval, flags, NULL, onerr);\n-\tif (!lock)\n+\tstruct ref_transaction *t;\n+\tstruct strbuf err = STRBUF_INIT;\n+\n+\tt = ref_transaction_begin(&err);\n+\tif (!t ||\n+\t    ref_transaction_update(t, refname, sha1, oldval, flags,\n+\t\t\t\t   !!oldval, &err) ||\n+\t    ref_transaction_commit(t, action, &err)) {\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+\t\t\tbreak;\n+\t\tcase UPDATE_REFS_DIE_ON_ERR:\n+\t\t\tdie(str, refname, err.buf);\n+\t\t\tbreak;\n+\t\tcase UPDATE_REFS_QUIET_ON_ERR:\n+\t\t\tbreak;\n+\t\t}\n+\t\tstrbuf_release(&err);\n \t\treturn 1;\n-\treturn update_ref_write(action, refname, sha1, lock, NULL, onerr);\n+\t}\n+\treturn 0;\n }\n \n static int ref_update_compare(const void *r1, const void *r2)\n-- \n2.0.1.442.g7fe6834.dirty\n"},{"id":"246135","messageId":"1405467258-24102-13-git-send-email-sahlberg@google.com","threadId":"37136","inReplyTo":"1405467258-24102-1-git-send-email-sahlberg@google.com","subject":"[PATCH 12/20] receive-pack.c: use a reference transaction for updating the refs","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-07-15T23:34:10Z","receivedAt":"2014-07-15T23:34:10Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Wrap all the ref updates inside a transaction.\n\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\n---\n builtin/receive-pack.c | 96 +++++++++++++++++++++++++++++++++-----------------\n 1 file changed, 63 insertions(+), 33 deletions(-)\n\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex c323081..91099ad 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -194,7 +194,7 @@ static void write_head_info(void)\n \n struct command {\n \tstruct command *next;\n-\tconst char *error_string;\n+\tchar *error_string;\n \tunsigned int skip_update:1,\n \t\t     did_not_exist:1;\n \tint index;\n@@ -468,19 +468,18 @@ static int update_shallow_ref(struct command *cmd, struct shallow_info *si)\n \treturn 0;\n }\n \n-static const char *update(struct command *cmd, struct shallow_info *si)\n+static char *update(struct command *cmd, struct shallow_info *si)\n {\n \tconst char *name = cmd->ref_name;\n \tstruct strbuf namespaced_name_buf = STRBUF_INIT;\n \tconst char *namespaced_name;\n \tunsigned char *old_sha1 = cmd->old_sha1;\n \tunsigned char *new_sha1 = cmd->new_sha1;\n-\tstruct ref_lock *lock;\n \n \t/* only refs/... are allowed */\n \tif (!starts_with(name, \"refs/\") || check_refname_format(name + 5, 0)) {\n \t\trp_error(\"refusing to create funny ref '%s' remotely\", name);\n-\t\treturn \"funny refname\";\n+\t\treturn xstrdup(\"funny refname\");\n \t}\n \n \tstrbuf_addf(&namespaced_name_buf, \"%s%s\", get_git_namespace(), name);\n@@ -498,20 +497,20 @@ static const char *update(struct command *cmd, struct shallow_info *si)\n \t\t\trp_error(\"refusing to update checked out branch: %s\", name);\n \t\t\tif (deny_current_branch == DENY_UNCONFIGURED)\n \t\t\t\trefuse_unconfigured_deny();\n-\t\t\treturn \"branch is currently checked out\";\n+\t\t\treturn xstrdup(\"branch is currently checked out\");\n \t\t}\n \t}\n \n \tif (!is_null_sha1(new_sha1) && !has_sha1_file(new_sha1)) {\n \t\terror(\"unpack should have generated %s, \"\n \t\t      \"but I can't find it!\", sha1_to_hex(new_sha1));\n-\t\treturn \"bad pack\";\n+\t\treturn xstrdup(\"bad pack\");\n \t}\n \n \tif (!is_null_sha1(old_sha1) && is_null_sha1(new_sha1)) {\n \t\tif (deny_deletes && starts_with(name, \"refs/heads/\")) {\n \t\t\trp_error(\"denying ref deletion for %s\", name);\n-\t\t\treturn \"deletion prohibited\";\n+\t\t\treturn xstrdup(\"deletion prohibited\");\n \t\t}\n \n \t\tif (!strcmp(namespaced_name, head_name)) {\n@@ -526,7 +525,7 @@ static const char *update(struct command *cmd, struct shallow_info *si)\n \t\t\t\tif (deny_delete_current == DENY_UNCONFIGURED)\n \t\t\t\t\trefuse_unconfigured_deny_delete_current();\n \t\t\t\trp_error(\"refusing to delete the current branch: %s\", name);\n-\t\t\t\treturn \"deletion of the current branch prohibited\";\n+\t\t\t\treturn xstrdup(\"deletion of the current branch prohibited\");\n \t\t\t}\n \t\t}\n \t}\n@@ -544,19 +543,19 @@ static const char *update(struct command *cmd, struct shallow_info *si)\n \t\t    old_object->type != OBJ_COMMIT ||\n \t\t    new_object->type != OBJ_COMMIT) {\n \t\t\terror(\"bad sha1 objects for %s\", name);\n-\t\t\treturn \"bad ref\";\n+\t\t\treturn xstrdup(\"bad ref\");\n \t\t}\n \t\told_commit = (struct commit *)old_object;\n \t\tnew_commit = (struct commit *)new_object;\n \t\tif (!in_merge_bases(old_commit, new_commit)) {\n \t\t\trp_error(\"denying non-fast-forward %s\"\n \t\t\t\t \" (you should pull first)\", name);\n-\t\t\treturn \"non-fast-forward\";\n+\t\t\treturn xstrdup(\"non-fast-forward\");\n \t\t}\n \t}\n \tif (run_update_hook(cmd)) {\n \t\trp_error(\"hook declined to update %s\", name);\n-\t\treturn \"hook declined\";\n+\t\treturn xstrdup(\"hook declined\");\n \t}\n \n \tif (is_null_sha1(new_sha1)) {\n@@ -571,24 +570,32 @@ static const char *update(struct command *cmd, struct shallow_info *si)\n \t\t}\n \t\tif (delete_ref(namespaced_name, old_sha1, 0)) {\n \t\t\trp_error(\"failed to delete %s\", name);\n-\t\t\treturn \"failed to delete\";\n+\t\t\treturn xstrdup(\"failed to delete\");\n \t\t}\n \t\treturn NULL; /* good */\n \t}\n \telse {\n+\t\tstruct strbuf err = STRBUF_INIT;\n+\t\tstruct ref_transaction *transaction;\n+\n \t\tif (shallow_update && si->shallow_ref[cmd->index] &&\n \t\t    update_shallow_ref(cmd, si))\n-\t\t\treturn \"shallow error\";\n-\n-\t\tlock = lock_any_ref_for_update(namespaced_name, old_sha1,\n-\t\t\t\t\t       0, NULL);\n-\t\tif (!lock) {\n-\t\t\trp_error(\"failed to lock %s\", name);\n-\t\t\treturn \"failed to lock\";\n-\t\t}\n-\t\tif (write_ref_sha1(lock, new_sha1, \"push\")) {\n-\t\t\treturn \"failed to write\"; /* error() already called */\n+\t\t\treturn xstrdup(\"shallow error\");\n+\n+\t\ttransaction = ref_transaction_begin(&err);\n+\t\tif (!transaction ||\n+\t\t    ref_transaction_update(transaction, namespaced_name,\n+\t\t\t\t\t   new_sha1, old_sha1, 0, 1, &err) ||\n+\t\t    ref_transaction_commit(transaction, \"push\", &err)) {\n+\t\t\tchar *str = strbuf_detach(&err, NULL);\n+\t\t\tref_transaction_free(transaction);\n+\n+\t\t\trp_error(\"%s\", str);\n+\t\t\treturn str;\n \t\t}\n+\n+\t\tref_transaction_free(transaction);\n+\t\tstrbuf_release(&err);\n \t\treturn NULL; /* good */\n \t}\n }\n@@ -647,6 +654,9 @@ static void check_aliased_update(struct command *cmd, struct string_list *list)\n \tchar cmd_oldh[41], cmd_newh[41], dst_oldh[41], dst_newh[41];\n \tint flag;\n \n+\tif (cmd->error_string)\n+\t\tdie(\"BUG: check_aliased_update called with failed cmd\");\n+\n \tstrbuf_addf(&buf, \"%s%s\", get_git_namespace(), cmd->ref_name);\n \tdst_name = resolve_ref_unsafe(buf.buf, sha1, 0, &flag);\n \tstrbuf_release(&buf);\n@@ -658,7 +668,7 @@ static void check_aliased_update(struct command *cmd, struct string_list *list)\n \tif (!dst_name) {\n \t\trp_error(\"refusing update to broken symref '%s'\", cmd->ref_name);\n \t\tcmd->skip_update = 1;\n-\t\tcmd->error_string = \"broken symref\";\n+\t\tcmd->error_string = xstrdup(\"broken symref\");\n \t\treturn;\n \t}\n \n@@ -684,8 +694,9 @@ static void check_aliased_update(struct command *cmd, struct string_list *list)\n \t\t cmd->ref_name, cmd_oldh, cmd_newh,\n \t\t dst_cmd->ref_name, dst_oldh, dst_newh);\n \n-\tcmd->error_string = dst_cmd->error_string =\n-\t\t\"inconsistent aliased update\";\n+\tcmd->error_string = xstrdup(\"inconsistent aliased update\");\n+\tfree(dst_cmd->error_string);\n+\tdst_cmd->error_string = xstrdup(\"inconsistent aliased update\");\n }\n \n static void check_aliased_updates(struct command *commands)\n@@ -733,7 +744,9 @@ static void set_connectivity_errors(struct command *commands,\n \t\tif (!check_everything_connected(command_singleton_iterator,\n \t\t\t\t\t\t0, &singleton))\n \t\t\tcontinue;\n-\t\tcmd->error_string = \"missing necessary objects\";\n+\t\tif (cmd->error_string)  /* can't happen */\n+\t\t\tcontinue;\n+\t\tcmd->error_string = xstrdup(\"missing necessary objects\");\n \t}\n }\n \n@@ -770,9 +783,9 @@ static void reject_updates_to_hidden(struct command *commands)\n \t\tif (cmd->error_string || !ref_is_hidden(cmd->ref_name))\n \t\t\tcontinue;\n \t\tif (is_null_sha1(cmd->new_sha1))\n-\t\t\tcmd->error_string = \"deny deleting a hidden ref\";\n+\t\t\tcmd->error_string = xstrdup(\"deny deleting a hidden ref\");\n \t\telse\n-\t\t\tcmd->error_string = \"deny updating a hidden ref\";\n+\t\t\tcmd->error_string = xstrdup(\"deny updating a hidden ref\");\n \t}\n }\n \n@@ -786,8 +799,11 @@ static void execute_commands(struct command *commands,\n \tstruct iterate_data data;\n \n \tif (unpacker_error) {\n-\t\tfor (cmd = commands; cmd; cmd = cmd->next)\n-\t\t\tcmd->error_string = \"unpacker error\";\n+\t\tfor (cmd = commands; cmd; cmd = cmd->next) {\n+\t\t\tif (cmd->error_string)  /* can't happen */\n+\t\t\t\tcontinue;\n+\t\t\tcmd->error_string = xstrdup(\"unpacker error\");\n+\t\t}\n \t\treturn;\n \t}\n \n@@ -800,8 +816,9 @@ static void execute_commands(struct command *commands,\n \n \tif (run_receive_hook(commands, \"pre-receive\", 0)) {\n \t\tfor (cmd = commands; cmd; cmd = cmd->next) {\n-\t\t\tif (!cmd->error_string)\n-\t\t\t\tcmd->error_string = \"pre-receive hook declined\";\n+\t\t\tif (cmd->error_string)\n+\t\t\t\tcontinue;\n+\t\t\tcmd->error_string = xstrdup(\"pre-receive hook declined\");\n \t\t}\n \t\treturn;\n \t}\n@@ -1079,7 +1096,8 @@ static void update_shallow_info(struct command *commands,\n \t\tif (is_null_sha1(cmd->new_sha1))\n \t\t\tcontinue;\n \t\tif (ref_status[cmd->index]) {\n-\t\t\tcmd->error_string = \"shallow update not allowed\";\n+\t\t\tfree(cmd->error_string);\n+\t\t\tcmd->error_string = xstrdup(\"shallow update not allowed\");\n \t\t\tcmd->skip_update = 1;\n \t\t}\n \t}\n@@ -1120,6 +1138,17 @@ static int delete_only(struct command *commands)\n \treturn 1;\n }\n \n+static void free_commands(struct command *commands)\n+{\n+\twhile (commands) {\n+\t\tstruct command *next = commands->next;\n+\n+\t\tfree(commands->error_string);\n+\t\tfree(commands);\n+\t\tcommands = next;\n+\t}\n+}\n+\n int cmd_receive_pack(int argc, const char **argv, const char *prefix)\n {\n \tint advertise_refs = 0;\n@@ -1215,5 +1244,6 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)\n \t\tpacket_flush(1);\n \tsha1_array_clear(&shallow);\n \tsha1_array_clear(&ref);\n+\tfree_commands(commands);\n \treturn 0;\n }\n-- \n2.0.1.442.g7fe6834.dirty\n"},{"id":"246131","messageId":"1405467258-24102-14-git-send-email-sahlberg@google.com","threadId":"37136","inReplyTo":"1405467258-24102-1-git-send-email-sahlberg@google.com","subject":"[PATCH 13/20] fast-import.c: use a ref transaction when dumping tags","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-07-15T23:34:11Z","receivedAt":"2014-07-15T23:34:11Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\n---\n fast-import.c | 29 +++++++++++++++++++++++------\n 1 file changed, 23 insertions(+), 6 deletions(-)\n\ndiff --git a/fast-import.c b/fast-import.c\nindex d5206ee..a95e1be 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -1734,15 +1734,32 @@ static void dump_tags(void)\n {\n \tstatic const char *msg = \"fast-import\";\n \tstruct tag *t;\n-\tstruct ref_lock *lock;\n-\tchar ref_name[PATH_MAX];\n+\tstruct strbuf ref_name = STRBUF_INIT;\n+\tstruct strbuf err = STRBUF_INIT;\n+\tstruct ref_transaction *transaction;\n \n+\ttransaction = ref_transaction_begin(&err);\n+\tif (!transaction) {\n+\t\tfailure |= error(\"%s\", err.buf);\n+\t\tgoto cleanup;\n+\t}\n \tfor (t = first_tag; t; t = t->next_tag) {\n-\t\tsprintf(ref_name, \"tags/%s\", t->name);\n-\t\tlock = lock_ref_sha1(ref_name, NULL);\n-\t\tif (!lock || write_ref_sha1(lock, t->sha1, msg) < 0)\n-\t\t\tfailure |= error(\"Unable to update %s\", ref_name);\n+\t\tstrbuf_reset(&ref_name);\n+\t\tstrbuf_addf(&ref_name, \"refs/tags/%s\", t->name);\n+\n+\t\tif (ref_transaction_update(transaction, ref_name.buf, t->sha1,\n+\t\t\t\t\t   NULL, 0, 0, &err)) {\n+\t\t\tfailure |= error(\"%s\", err.buf);\n+\t\t\tgoto cleanup;\n+\t\t}\n \t}\n+\tif (ref_transaction_commit(transaction, msg, &err))\n+\t\tfailure |= error(\"%s\", err.buf);\n+\n+ cleanup:\n+\tref_transaction_free(transaction);\n+\tstrbuf_release(&ref_name);\n+\tstrbuf_release(&err);\n }\n \n static void dump_marks_helper(FILE *f,\n-- \n2.0.1.442.g7fe6834.dirty\n"},{"id":"246126","messageId":"1405467258-24102-15-git-send-email-sahlberg@google.com","threadId":"37136","inReplyTo":"1405467258-24102-1-git-send-email-sahlberg@google.com","subject":"[PATCH 14/20] walker.c: use ref transaction for ref updates","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-07-15T23:34:12Z","receivedAt":"2014-07-15T23:34:12Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Switch to using ref transactions in walker_fetch(). As part of the refactoring\nto use ref transactions we also fix a potential memory leak where in the\noriginal code if write_ref_sha1() would fail we would end up returning from\nthe function without free()ing the msg string.\n\nNote that this function is only called when fetching from a remote HTTP\nrepository onto the local (most of the time single-user) repository which\nlikely means that the type of collisions that the previous locking would\nprotect against and cause the fetch to fail for are even more rare.\n\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\n---\n walker.c | 59 +++++++++++++++++++++++++++++++++++------------------------\n 1 file changed, 35 insertions(+), 24 deletions(-)\n\ndiff --git a/walker.c b/walker.c\nindex 1dd86b8..60d9f9e 100644\n--- a/walker.c\n+++ b/walker.c\n@@ -251,39 +251,36 @@ void walker_targets_free(int targets, char **target, const char **write_ref)\n int walker_fetch(struct walker *walker, int targets, char **target,\n \t\t const char **write_ref, const char *write_ref_log_details)\n {\n-\tstruct ref_lock **lock = xcalloc(targets, sizeof(struct ref_lock *));\n+\tstruct strbuf ref_name = STRBUF_INIT;\n+\tstruct strbuf err = STRBUF_INIT;\n+\tstruct ref_transaction *transaction = NULL;\n \tunsigned char *sha1 = xmalloc(targets * 20);\n-\tchar *msg;\n-\tint ret;\n+\tchar *msg = NULL;\n \tint i;\n \n \tsave_commit_buffer = 0;\n \n-\tfor (i = 0; i < targets; i++) {\n-\t\tif (!write_ref || !write_ref[i])\n-\t\t\tcontinue;\n-\n-\t\tlock[i] = lock_ref_sha1(write_ref[i], NULL);\n-\t\tif (!lock[i]) {\n-\t\t\terror(\"Can't lock ref %s\", write_ref[i]);\n-\t\t\tgoto unlock_and_fail;\n+\tif (write_ref) {\n+\t\ttransaction = ref_transaction_begin(&err);\n+\t\tif (!transaction) {\n+\t\t\terror(\"%s\", err.buf);\n+\t\t\tgoto rollback_and_fail;\n \t\t}\n \t}\n-\n \tif (!walker->get_recover)\n \t\tfor_each_ref(mark_complete, NULL);\n \n \tfor (i = 0; i < targets; i++) {\n \t\tif (interpret_target(walker, target[i], &sha1[20 * i])) {\n \t\t\terror(\"Could not interpret response from server '%s' as something to pull\", target[i]);\n-\t\t\tgoto unlock_and_fail;\n+\t\t\tgoto rollback_and_fail;\n \t\t}\n \t\tif (process(walker, lookup_unknown_object(&sha1[20 * i])))\n-\t\t\tgoto unlock_and_fail;\n+\t\t\tgoto rollback_and_fail;\n \t}\n \n \tif (loop(walker))\n-\t\tgoto unlock_and_fail;\n+\t\tgoto rollback_and_fail;\n \n \tif (write_ref_log_details) {\n \t\tmsg = xmalloc(strlen(write_ref_log_details) + 12);\n@@ -294,19 +291,33 @@ int walker_fetch(struct walker *walker, int targets, char **target,\n \tfor (i = 0; i < targets; i++) {\n \t\tif (!write_ref || !write_ref[i])\n \t\t\tcontinue;\n-\t\tret = write_ref_sha1(lock[i], &sha1[20 * i], msg ? msg : \"fetch (unknown)\");\n-\t\tlock[i] = NULL;\n-\t\tif (ret)\n-\t\t\tgoto unlock_and_fail;\n+\t\tstrbuf_reset(&ref_name);\n+\t\tstrbuf_addf(&ref_name, \"refs/%s\", write_ref[i]);\n+\t\tif (ref_transaction_update(transaction, ref_name.buf,\n+\t\t\t\t\t   &sha1[20 * i], NULL, 0, 0,\n+\t\t\t\t\t   &err)) {\n+\t\t\terror(\"%s\", err.buf);\n+\t\t\tgoto rollback_and_fail;\n+\t\t}\n+\t}\n+\tif (write_ref) {\n+\t\tif (ref_transaction_commit(transaction,\n+\t\t\t\t\t   msg ? msg : \"fetch (unknown)\",\n+\t\t\t\t\t   &err)) {\n+\t\t\terror(\"%s\", err.buf);\n+\t\t\tgoto rollback_and_fail;\n+\t\t}\n+\t\tref_transaction_free(transaction);\n \t}\n-\tfree(msg);\n \n+\tfree(msg);\n \treturn 0;\n \n-unlock_and_fail:\n-\tfor (i = 0; i < targets; i++)\n-\t\tif (lock[i])\n-\t\t\tunlock_ref(lock[i]);\n+rollback_and_fail:\n+\tref_transaction_free(transaction);\n+\tfree(msg);\n+\tstrbuf_release(&err);\n+\tstrbuf_release(&ref_name);\n \n \treturn -1;\n }\n-- \n2.0.1.442.g7fe6834.dirty\n"},{"id":"246132","messageId":"1405467258-24102-16-git-send-email-sahlberg@google.com","threadId":"37136","inReplyTo":"1405467258-24102-1-git-send-email-sahlberg@google.com","subject":"[PATCH 15/20] refs.c: make lock_ref_sha1 static","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-07-15T23:34:13Z","receivedAt":"2014-07-15T23:34:13Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"No external callers reference lock_ref_sha1 any more so lets declare it static.\n\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\n---\n refs.c | 2 +-\n refs.h | 6 ------\n 2 files changed, 1 insertion(+), 7 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex ff4e799..10cea87 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2170,7 +2170,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,\n \treturn NULL;\n }\n \n-struct ref_lock *lock_ref_sha1(const char *refname, const unsigned char *old_sha1)\n+static struct ref_lock *lock_ref_sha1(const char *refname, const unsigned char *old_sha1)\n {\n \tchar refpath[PATH_MAX];\n \tif (check_refname_format(refname, 0))\ndiff --git a/refs.h b/refs.h\nindex 3f37c65..65dd593 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -170,12 +170,6 @@ extern int ref_exists(const char *);\n  */\n extern int peel_ref(const char *refname, unsigned char *sha1);\n \n-/*\n- * Locks a \"refs/\" ref returning the lock on success and NULL on failure.\n- * On failure errno is set to something meaningful.\n- */\n-extern struct ref_lock *lock_ref_sha1(const char *refname, const unsigned char *old_sha1);\n-\n /** Locks any ref (for 'HEAD' type refs). */\n #define REF_NODEREF\t0x01\n /* errno is set to something meaningful on failure */\n-- \n2.0.1.442.g7fe6834.dirty\n"},{"id":"246141","messageId":"1405467258-24102-17-git-send-email-sahlberg@google.com","threadId":"37136","inReplyTo":"1405467258-24102-1-git-send-email-sahlberg@google.com","subject":"[PATCH 16/20] refs.c: remove the update_ref_lock function","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-07-15T23:34:14Z","receivedAt":"2014-07-15T23:34:14Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Since we now only call update_ref_lock with onerr==QUIET_ON_ERR we no longer\nneed this function and can replace it with just calling lock_any_ref_for_update\ndirectly.\n\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\n---\n refs.c | 30 ++++++------------------------\n 1 file changed, 6 insertions(+), 24 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 10cea87..091c343 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3333,24 +3333,6 @@ int for_each_reflog(each_ref_fn fn, void *cb_data)\n \treturn retval;\n }\n \n-static struct ref_lock *update_ref_lock(const char *refname,\n-\t\t\t\t\tconst unsigned char *oldval,\n-\t\t\t\t\tint flags, int *type_p,\n-\t\t\t\t\tenum action_on_err onerr)\n-{\n-\tstruct ref_lock *lock;\n-\tlock = lock_any_ref_for_update(refname, oldval, flags, type_p);\n-\tif (!lock) {\n-\t\tconst char *str = \"Cannot lock the ref '%s'.\";\n-\t\tswitch (onerr) {\n-\t\tcase UPDATE_REFS_MSG_ON_ERR: error(str, refname); break;\n-\t\tcase UPDATE_REFS_DIE_ON_ERR: die(str, refname); break;\n-\t\tcase UPDATE_REFS_QUIET_ON_ERR: break;\n-\t\t}\n-\t}\n-\treturn lock;\n-}\n-\n static int update_ref_write(const char *action, const char *refname,\n \t\t\t    const unsigned char *sha1, struct ref_lock *lock,\n \t\t\t    struct strbuf *err, enum action_on_err onerr)\n@@ -3602,12 +3584,12 @@ int ref_transaction_commit(struct ref_transaction *transaction,\n \tfor (i = 0; i < n; i++) {\n \t\tstruct ref_update *update = updates[i];\n \n-\t\tupdate->lock = update_ref_lock(update->refname,\n-\t\t\t\t\t       (update->have_old ?\n-\t\t\t\t\t\tupdate->old_sha1 : NULL),\n-\t\t\t\t\t       update->flags,\n-\t\t\t\t\t       &update->type,\n-\t\t\t\t\t       UPDATE_REFS_QUIET_ON_ERR);\n+\t\tupdate->lock = lock_any_ref_for_update(update->refname,\n+\t\t\t\t\t\t       (update->have_old ?\n+\t\t\t\t\t\t\tupdate->old_sha1 :\n+\t\t\t\t\t\t\tNULL),\n+\t\t\t\t\t\t       update->flags,\n+\t\t\t\t\t\t       &update->type);\n \t\tif (!update->lock) {\n \t\t\tif (err)\n \t\t\t\tstrbuf_addf(err, \"Cannot lock the ref '%s'.\",\n-- \n2.0.1.442.g7fe6834.dirty\n"},{"id":"246123","messageId":"1405467258-24102-18-git-send-email-sahlberg@google.com","threadId":"37136","inReplyTo":"1405467258-24102-1-git-send-email-sahlberg@google.com","subject":"[PATCH 17/20] refs.c: remove the update_ref_write function","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-07-15T23:34:15Z","receivedAt":"2014-07-15T23:34:15Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Since we only call update_ref_write from a single place and we only call it\nwith onerr==QUIET_ON_ERR we can just as well get rid of it and just call\nwrite_ref_sha1 directly. This changes the return status for _commit from\n1 to -1 on failures when writing to the ref. Eventually we will want\n_commit to start returning more detailed error conditions than the current\nsimple success/failure. For example if the commit failed due to name\nconflicts etc.\n\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\n---\n refs.c | 35 +++++++++--------------------------\n 1 file changed, 9 insertions(+), 26 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 091c343..27c629f 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3333,25 +3333,6 @@ int for_each_reflog(each_ref_fn fn, void *cb_data)\n \treturn retval;\n }\n \n-static int update_ref_write(const char *action, const char *refname,\n-\t\t\t    const unsigned char *sha1, struct ref_lock *lock,\n-\t\t\t    struct strbuf *err, enum action_on_err onerr)\n-{\n-\tif (write_ref_sha1(lock, sha1, action) < 0) {\n-\t\tconst char *str = \"Cannot update the ref '%s'.\";\n-\t\tif (err)\n-\t\t\tstrbuf_addf(err, str, refname);\n-\n-\t\tswitch (onerr) {\n-\t\tcase UPDATE_REFS_MSG_ON_ERR: error(str, refname); break;\n-\t\tcase UPDATE_REFS_DIE_ON_ERR: die(str, refname); break;\n-\t\tcase UPDATE_REFS_QUIET_ON_ERR: break;\n-\t\t}\n-\t\treturn 1;\n-\t}\n-\treturn 0;\n-}\n-\n /**\n  * Information needed for a single ref update.  Set new_sha1 to the\n  * new value or to zero to delete the ref.  To check the old value\n@@ -3604,14 +3585,16 @@ int ref_transaction_commit(struct ref_transaction *transaction,\n \t\tstruct ref_update *update = updates[i];\n \n \t\tif (!is_null_sha1(update->new_sha1)) {\n-\t\t\tret = update_ref_write(msg,\n-\t\t\t\t\t       update->refname,\n-\t\t\t\t\t       update->new_sha1,\n-\t\t\t\t\t       update->lock, err,\n-\t\t\t\t\t       UPDATE_REFS_QUIET_ON_ERR);\n-\t\t\tupdate->lock = NULL; /* freed by update_ref_write */\n-\t\t\tif (ret)\n+\t\t\tret = write_ref_sha1(update->lock, update->new_sha1,\n+\t\t\t\t\t     msg);\n+\t\t\tupdate->lock = NULL; /* freed by write_ref_sha1 */\n+\t\t\tif (ret) {\n+\t\t\t\tconst char *str = \"Cannot update the ref '%s'.\";\n+\n+\t\t\t\tif (err)\n+\t\t\t\t\tstrbuf_addf(err, str, update->refname);\n \t\t\t\tgoto cleanup;\n+\t\t\t}\n \t\t}\n \t}\n \n-- \n2.0.1.442.g7fe6834.dirty\n"},{"id":"246133","messageId":"1405467258-24102-19-git-send-email-sahlberg@google.com","threadId":"37136","inReplyTo":"1405467258-24102-1-git-send-email-sahlberg@google.com","subject":"[PATCH 18/20] refs.c: remove lock_ref_sha1","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-07-15T23:34:16Z","receivedAt":"2014-07-15T23:34:16Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"lock_ref_sha1 was only called from one place in refc.c and only provided\na check that the refname was sane before adding back the initial \"refs/\"\npart of the ref path name, the initial \"refs/\" that this caller had already\nstripped off before calling lock_ref_sha1.\n\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\n---\n refs.c | 15 +++++----------\n 1 file changed, 5 insertions(+), 10 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 27c629f..dbf6af9 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2170,15 +2170,6 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,\n \treturn NULL;\n }\n \n-static struct ref_lock *lock_ref_sha1(const char *refname, const unsigned char *old_sha1)\n-{\n-\tchar refpath[PATH_MAX];\n-\tif (check_refname_format(refname, 0))\n-\t\treturn NULL;\n-\tstrcpy(refpath, mkpath(\"refs/%s\", refname));\n-\treturn lock_ref_sha1_basic(refpath, old_sha1, 0, NULL);\n-}\n-\n struct ref_lock *lock_any_ref_for_update(const char *refname,\n \t\t\t\t\t const unsigned char *old_sha1,\n \t\t\t\t\t int flags, int *type_p)\n@@ -2388,8 +2379,12 @@ static void try_remove_empty_parents(char *name)\n /* make sure nobody touched the ref, and unlink */\n static void prune_ref(struct ref_to_prune *r)\n {\n-\tstruct ref_lock *lock = lock_ref_sha1(r->name + 5, r->sha1);\n+\tstruct ref_lock *lock;\n+\n+\tif (check_refname_format(r->name + 5, 0))\n+\t\treturn;\n \n+\tlock = lock_ref_sha1_basic(r->name, r->sha1, 0, NULL);\n \tif (lock) {\n \t\tunlink_or_warn(git_path(\"%s\", r->name));\n \t\tunlock_ref(lock);\n-- \n2.0.1.442.g7fe6834.dirty\n"},{"id":"246128","messageId":"1405467258-24102-20-git-send-email-sahlberg@google.com","threadId":"37136","inReplyTo":"1405467258-24102-1-git-send-email-sahlberg@google.com","subject":"[PATCH 19/20] refs.c: make prune_ref use a transaction to delete the ref","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-07-15T23:34:17Z","receivedAt":"2014-07-15T23:34:17Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Change prune_ref to delete the ref using a ref transaction. To do this we also\nneed to add a new flag REF_ISPRUNING that will tell the transaction that we\ndo not want to delete this ref from the packed refs. This flag is private to\nrefs.c and not exposed to external callers.\n\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\n---\n refs.c | 27 ++++++++++++++++++++-------\n refs.h | 14 ++++++++++++--\n 2 files changed, 32 insertions(+), 9 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex dbf6af9..186df37 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -25,6 +25,11 @@ static unsigned char refname_disposition[256] = {\n };\n \n /*\n+ * Used as a flag to ref_transaction_delete when a loose ref is being\n+ * pruned.\n+ */\n+#define REF_ISPRUNING\t0x0100\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@@ -2379,17 +2384,24 @@ static void try_remove_empty_parents(char *name)\n /* make sure nobody touched the ref, and unlink */\n static void prune_ref(struct ref_to_prune *r)\n {\n-\tstruct ref_lock *lock;\n+\tstruct ref_transaction *transaction;\n+\tstruct strbuf err = STRBUF_INIT;\n \n \tif (check_refname_format(r->name + 5, 0))\n \t\treturn;\n \n-\tlock = lock_ref_sha1_basic(r->name, r->sha1, 0, NULL);\n-\tif (lock) {\n-\t\tunlink_or_warn(git_path(\"%s\", r->name));\n-\t\tunlock_ref(lock);\n-\t\ttry_remove_empty_parents(r->name);\n+\ttransaction = ref_transaction_begin(&err);\n+\tif (!transaction ||\n+\t    ref_transaction_delete(transaction, r->name, r->sha1,\n+\t\t\t\t   REF_ISPRUNING, 1, &err) ||\n+\t    ref_transaction_commit(transaction, NULL, &err)) {\n+\t\tref_transaction_free(transaction);\n+\t\terror(\"%s\", err.buf);\n+\t\tstrbuf_release(&err);\n+\t\treturn;\n \t}\n+\tref_transaction_free(transaction);\n+\ttry_remove_empty_parents(r->name);\n }\n \n static void prune_refs(struct ref_to_prune *r)\n@@ -3598,8 +3610,9 @@ int ref_transaction_commit(struct ref_transaction *transaction,\n \t\tstruct ref_update *update = updates[i];\n \n \t\tif (update->lock) {\n-\t\t\tdelnames[delnum++] = update->lock->ref_name;\n \t\t\tret |= delete_ref_loose(update->lock, update->type);\n+\t\t\tif (!(update->flags & REF_ISPRUNING))\n+\t\t\t\tdelnames[delnum++] = update->lock->ref_name;\n \t\t}\n \t}\n \ndiff --git a/refs.h b/refs.h\nindex 65dd593..aad846c 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -170,9 +170,19 @@ extern int ref_exists(const char *);\n  */\n extern int peel_ref(const char *refname, unsigned char *sha1);\n \n-/** Locks any ref (for 'HEAD' type refs). */\n+/*\n+ * Flags controlling lock_any_ref_for_update(), ref_transaction_update(),\n+ * ref_transaction_create(), etc.\n+ * REF_NODEREF: act on the ref directly, instead of dereferencing\n+ *              symbolic references.\n+ *\n+ * Flags >= 0x100 are reserved for internal use.\n+ */\n #define REF_NODEREF\t0x01\n-/* errno is set to something meaningful on failure */\n+/*\n+ * Locks any ref (for 'HEAD' type refs) and sets errno to something\n+ * meaningful on failure.\n+ */\n extern struct ref_lock *lock_any_ref_for_update(const char *refname,\n \t\t\t\t\t\tconst unsigned char *old_sha1,\n \t\t\t\t\t\tint flags, int *type_p);\n-- \n2.0.1.442.g7fe6834.dirty\n"},{"id":"246134","messageId":"1405467258-24102-21-git-send-email-sahlberg@google.com","threadId":"37136","inReplyTo":"1405467258-24102-1-git-send-email-sahlberg@google.com","subject":"[PATCH 20/20] refs.c: make delete_ref use a transaction","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-07-15T23:34:18Z","receivedAt":"2014-07-15T23:34:18Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Change delete_ref to use a ref transaction for the deletion. At the same time\nsince we no longer have any callers of repack_without_ref we can now delete\nthis function.\n\nChange delete_ref to return 0 on success and 1 on failure instead of the\nprevious 0 on success either 1 or -1 on failure.\n\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Ronnie Sahlberg <sahlberg@google.com>\n---\n refs.c | 34 +++++++++++++---------------------\n 1 file changed, 13 insertions(+), 21 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 186df37..0017d9c 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2544,11 +2544,6 @@ int repack_without_refs(const char **refnames, int n, struct strbuf *err)\n \treturn ret;\n }\n \n-static int repack_without_ref(const char *refname)\n-{\n-\treturn repack_without_refs(&refname, 1, NULL);\n-}\n-\n static int delete_ref_loose(struct ref_lock *lock, int flag)\n {\n \tif (!(flag & REF_ISPACKED) || flag & REF_ISSYMREF) {\n@@ -2566,24 +2561,21 @@ static int delete_ref_loose(struct ref_lock *lock, int flag)\n \n int delete_ref(const char *refname, const unsigned char *sha1, int delopt)\n {\n-\tstruct ref_lock *lock;\n-\tint ret = 0, flag = 0;\n+\tstruct ref_transaction *transaction;\n+\tstruct strbuf err = STRBUF_INIT;\n \n-\tlock = lock_ref_sha1_basic(refname, sha1, delopt, &flag);\n-\tif (!lock)\n+\ttransaction = ref_transaction_begin(&err);\n+\tif (!transaction ||\n+\t    ref_transaction_delete(transaction, refname, sha1, delopt,\n+\t\t\t\t   sha1 && !is_null_sha1(sha1), &err) ||\n+\t    ref_transaction_commit(transaction, NULL, &err)) {\n+\t\terror(\"%s\", err.buf);\n+\t\tref_transaction_free(transaction);\n+\t\tstrbuf_release(&err);\n \t\treturn 1;\n-\tret |= delete_ref_loose(lock, flag);\n-\n-\t/* removing the loose one could have resurrected an earlier\n-\t * packed one.  Also, if it was not loose we need to repack\n-\t * without it.\n-\t */\n-\tret |= repack_without_ref(lock->ref_name);\n-\n-\tunlink_or_warn(git_path(\"logs/%s\", lock->ref_name));\n-\tclear_loose_ref_cache(&ref_cache);\n-\tunlock_ref(lock);\n-\treturn ret;\n+\t}\n+\tref_transaction_free(transaction);\n+\treturn 0;\n }\n \n /*\n-- \n2.0.1.442.g7fe6834.dirty\n"},{"id":"246139","messageId":"CAL=YDW=At4isTCtwdeGZ-v3cS=vbGyC7aQ-CwAEzxKN0c_3Jyg@mail.gmail.com","threadId":"37136","inReplyTo":"1405467258-24102-1-git-send-email-sahlberg@google.com","subject":"Re: [PATCH 00/20] ref transactions part 2","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-07-15T23:37:44Z","receivedAt":"2014-07-15T23:37:44Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Hi Michael,\n\nHere is the next set of 20 patches of those you already reviewed.\nI cut this patch off just before patch #40 in the previous series.\n\nThese are the patches numbered 20-39 in the original series.\nI think I have addressed your concerns here so If you could take a quick look\nand hopefully bless them that would be awesome!\n\n\nThanks\nronnie sahlberg\n\nOn Tue, Jul 15, 2014 at 4:33 PM, Ronnie Sahlberg <sahlberg@google.com> wrote:\n> This is the next 20 patches from my originally big patch series and follow\n> the previous 19 patches that is now in juns tree.\n> These patches were numbered 20-39 in the original 48-patch series.\n>\n> Changes since these patches were in the original series:\n>\n> - Addressing concerns from mhagger's review\n>\n>\n> Ronnie Sahlberg (20):\n>   refs.c: change ref_transaction_create to do error checking and return\n>     status\n>   refs.c: update ref_transaction_delete to check for error and return\n>     status\n>   refs.c: make ref_transaction_begin take an err argument\n>   refs.c: add transaction.status and track OPEN/CLOSED/ERROR\n>   tag.c: use ref transactions when doing updates\n>   replace.c: use the ref transaction functions for updates\n>   commit.c: use ref transactions for updates\n>   sequencer.c: use ref transactions for all ref updates\n>   fast-import.c: change update_branch to use ref transactions\n>   branch.c: use ref transaction for all ref updates\n>   refs.c: change update_ref to use a transaction\n>   receive-pack.c: use a reference transaction for updating the refs\n>   fast-import.c: use a ref transaction when dumping tags\n>   walker.c: use ref transaction for ref updates\n>   refs.c: make lock_ref_sha1 static\n>   refs.c: remove the update_ref_lock function\n>   refs.c: remove the update_ref_write function\n>   refs.c: remove lock_ref_sha1\n>   refs.c: make prune_ref use a transaction to delete the ref\n>   refs.c: make delete_ref use a transaction\n>\n>  branch.c               |  30 +++---\n>  builtin/commit.c       |  24 +++--\n>  builtin/receive-pack.c |  96 +++++++++++++-------\n>  builtin/replace.c      |  15 +--\n>  builtin/tag.c          |  15 +--\n>  builtin/update-ref.c   |  11 ++-\n>  fast-import.c          |  53 +++++++----\n>  refs.c                 | 242 ++++++++++++++++++++++++++++---------------------\n>  refs.h                 |  78 ++++++++++++----\n>  sequencer.c            |  27 ++++--\n>  walker.c               |  59 +++++++-----\n>  11 files changed, 403 insertions(+), 247 deletions(-)\n>\n> --\n> 2.0.1.442.g7fe6834.dirty\n>\n"},{"id":"246193","messageId":"xmqqegxlq799.fsf@gitster.dls.corp.google.com","threadId":"37136","inReplyTo":"CAL=YDW=At4isTCtwdeGZ-v3cS=vbGyC7aQ-CwAEzxKN0c_3Jyg@mail.gmail.com","subject":"Re: [PATCH 00/20] ref transactions part 2","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-07-16T22:16:33Z","receivedAt":"2014-07-16T22:16:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ronnie Sahlberg <sahlberg@google.com> writes:\n\n> On Tue, Jul 15, 2014 at 4:33 PM, Ronnie Sahlberg <sahlberg@google.com> wrote:\n>> This is the next 20 patches from my originally big patch series and follow\n>> the previous 19 patches that is now in juns tree.\n>> These patches were numbered 20-39 in the original 48-patch series.\n>>\n>> Changes since these patches were in the original series:\n>>\n>> - Addressing concerns from mhagger's review\n\nOne patch in the series did not apply cleanly on top of the tip of\nthe previous series (now queued as rs/ref-transaction-0) and I had\nto wiggle it.  Please check the result (queued as three topics, this\none is rs/ref-transaction-1 which is built on the abovementioned\n\"-0\", and the remainder from the previous round is rebased on \"-1\"\nas rs/ref-transaction), all of which are queued on 'jch' (which is\npart of 'pu').\n\nThanks.\n"},{"id":"246208","messageId":"CAL=YDWnQpLxM1TJ3dja+hGGNd1ckFUJHz83Z=WsOcfo24PT0Vg@mail.gmail.com","threadId":"37136","inReplyTo":"xmqqegxlq799.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 00/20] ref transactions part 2","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-07-16T22:52:10Z","receivedAt":"2014-07-16T22:52:10Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"I had a look at the changes in origin/pu and they look sane to me.\n\nmake test   passes all tests too.\n\n\nregards\nronnie sahlberg\n\n\nOn Wed, Jul 16, 2014 at 3:16 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Ronnie Sahlberg <sahlberg@google.com> writes:\n>\n>> On Tue, Jul 15, 2014 at 4:33 PM, Ronnie Sahlberg <sahlberg@google.com> wrote:\n>>> This is the next 20 patches from my originally big patch series and follow\n>>> the previous 19 patches that is now in juns tree.\n>>> These patches were numbered 20-39 in the original 48-patch series.\n>>>\n>>> Changes since these patches were in the original series:\n>>>\n>>> - Addressing concerns from mhagger's review\n>\n> One patch in the series did not apply cleanly on top of the tip of\n> the previous series (now queued as rs/ref-transaction-0) and I had\n> to wiggle it.  Please check the result (queued as three topics, this\n> one is rs/ref-transaction-1 which is built on the abovementioned\n> \"-0\", and the remainder from the previous round is rebased on \"-1\"\n> as rs/ref-transaction), all of which are queued on 'jch' (which is\n> part of 'pu').\n>\n> Thanks.\n"}]}