{"thread":{"id":"36672","subject":"[PATCH v9 04/44] refs.c: add an err argument to repack_without_refs","startedAt":"2014-05-15T23:14:57Z","lastAt":"2014-05-15T23:15:41Z","messageCount":45,"participants":["Ronnie Sahlberg"],"isPatch":true,"patchVersion":9,"patchTotal":44},"messages":[{"id":"241845","messageId":"1400195741-22996-1-git-send-email-sahlberg@google.com","threadId":"36672","inReplyTo":null,"subject":"[PATCH v9 00/44] Use ref transactions for all ref updates","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-15T23:14:57Z","receivedAt":"2014-05-15T23:14:57Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"This patch series can also be found at\nhttps://github.com/rsahlberg/git/tree/ref-transactions\n\n\nThis patch series is based on next and expands on the transaction API. It\nconverts all ref updates, inside refs.c as well as external, to use the\ntransaction API for updates. This makes most of the ref updates to become\natomic when there are failures locking or writing to a ref.\n\nThis version completes the work to convert all ref updates to use transactions.\nNow that all updates are through transactions I will start working on\ncleaning up the reading of refs and to create an api for managing reflogs but\nall that will go in a different patch series.\n\nVersion 9:\n - Lots of updates to error messages based on JN's review.\n\nVersion 8:\n - Updates after review by JN\n - Improve commit messages\n - Add a patch that adds an err argument to repack_without_refs\n - Add a patch that adds an err argument to delete_loose_ref\n - Better document that a _update/_delete/_create failure means the whole\n transaction has failed and needs rollback.\n\nVersion 7:\n - Updated commit messages per JNs review comments.\n - Changed REF_ISPRUNING and REF_ISPACKONLY to be private flags and not\n exposed through refs.h\n\nVersion 6:\n - Convert all updates in refs.c to use transactions too.\n\nVersion 5:\n - Reword commit messages for having _create/_delete/_update returning\n success/failure. There are no conditions yet that return an error from\n these failures but there will be in the future. So we still check the\n return from these functions in the callers in preparation for this.\n - Don't leak memory by just passing a strbuf_detach() pointer to functions.\n Use .buf and explicitely strbuf_release the data afterwards.\n - Remove the function update_ref_lock.\n - Remove the function update_ref_write.\n - Track transaction status and die(BUG:) if we call _create/_delete/_update/\n _commit for a transaction that is not OPEN.\n\nVersion 4:\n - Rename patch series from \"Use ref transactions from most callers\" to\n \"Use ref transactions for all ref updates\".\n - Convert all external ref writes to use transactions and make write_ref_sha1\n and lock_ref_sha1 static functions.\n - Change the ref commit and free handling so we no longer pass pointer to\n pointer to _commit. _commit no longer frees the transaction. The caller\n MUST call _free itself.\n - Change _commit to take a strbuf pointer instead of a char* for error\n reporting back to the caller.\n - Re-add the walker patch after fixing it.\n\nVersion 3:\n - Remove the walker patch for now. Walker needs more complex solution\n so defer it until the basics are done.\n - Remove the onerr argument to ref_transaction_commit(). All callers\n that need to die() on error now have to do this explicitely.\n - Pass an error string from ref_transaction_commit() back to the callers\n so that they can craft a nice error message upon failures.\n - Make ref_transaction_rollback() accept NULL as argument.\n - Change ref_transaction_commit() to take a pointer to pointer argument for\n the transaction and have it clear the callers pointer to NULL when\n invoked. This allows for much nicer handling of transaction rollback on\n failure.\n\nVersion 2:\n - Add a patch to ref_transaction_commit to make it honor onerr even if the\n error triggered in ref_Transaction_commit itself rather than in a call\n to other functions (that already honor onerr).\n - Add a patch to make the update_ref() helper function use transactions\n internally.\n - Change ref_transaction_update to die() instead of error() if we pass\n if a NULL old_sha1 but have have_old == true.\n - Change ref_transaction_create to die() instead of error() if new_sha1\n is false but we pass it a null_sha1.\n - Change ref_transaction_delete die() instead of error() if we pass\n if a NULL old_sha1 but have have_old == true.\n - Change several places to do if(!transaction || ref_transaction_update()\n || ref_Transaction_commit()) die(generic-message) instead of checking each\n step separately and having a different message for each failure.\n Most users are likely not interested in what step of the transaction\n failed and only whether it failed or not.\n - Change commit.c to only pass a pointer to ref_transaction_update\n iff current_head is non-NULL.\n The previous patch used to compute a garbage pointer for\n current_head->object.sha1 and relied on the fact that ref_transaction_update\n would not try to dereference this pointer if !!current_head was 0.\n - Updated commit message for the walker_fetch change to try to justify why\n the change in locking semantics should not be harmful.\n\n\nRonnie Sahlberg (44):\n refs.c: constify the sha arguments for\n ref_transaction_create|delete|update\n refs.c: allow passing NULL to ref_transaction_free\n refs.c: add a strbuf argument to ref_transaction_commit for error\n logging\n refs.c: add an err argument to repack_without_refs\n refs.c: make ref_update_reject_duplicates take a strbuf argument for\n errors\n refs.c: add an err argument to delete_ref_loose\n refs.c: make update_ref_write update a strbuf on failure\n update-ref.c: log transaction error from the update_ref\n refs.c: remove the onerr argument to ref_transaction_commit\n refs.c: change ref_transaction_update() to do error checking and\n return status\n refs.c: change ref_transaction_create to do error checking and return\n status\n refs.c: ref_transaction_delete to check for error and return status\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 refs.c: free the transaction before returning when number of updates\n is 0\n refs.c: ref_transaction_commit should not free the transaction\n fetch.c: clear errno before calling functions that might set it\n fetch.c: change s_update_ref to use a ref transaction\n fetch.c: use a single ref transaction for all ref updates\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 write_ref_sha1 static\n refs.c: make lock_ref_sha1 static\n refs.c: add transaction.status and track OPEN/CLOSED/ERROR\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 refs.c: pass the ref log message to _create/delete/update instead of\n _commit\n refs.c: pass NULL as *flags to read_ref_full\n refs.c: pack all refs before we start to rename a ref\n refs.c: move the check for valid refname to lock_ref_sha1_basic\n refs.c: call lock_ref_sha1_basic directly from commit\n refs.c: add a new flag for transaction delete for refs we know are\n packed only\n refs.c: pass a skip list to name_conflict_fn\n refs.c: make rename_ref use a transaction\n refs.c: remove forward declaration of write_ref_sha1\n\n branch.c | 30 ++--\n builtin/commit.c | 24 ++-\n builtin/fetch.c | 29 +--\n builtin/receive-pack.c | 21 +--\n builtin/replace.c | 15 +-\n builtin/tag.c | 15 +-\n builtin/update-ref.c | 32 ++--\n cache.h | 2 +\n fast-import.c | 42 +++--\n lockfile.c | 17 +-\n refs.c | 475 ++++++++++++++++++++++++++++++-------------------\n refs.h | 60 ++++---\n sequencer.c | 24 ++-\n t/t3200-branch.sh | 2 +-\n walker.c | 51 +++---\n 15 files changed, 501 insertions(+), 338 deletions(-)\n\n-- \n2.0.0.rc3.477.gffe78a2\n"},{"id":"241804","messageId":"1400195741-22996-2-git-send-email-sahlberg@google.com","threadId":"36672","inReplyTo":"1400195741-22996-1-git-send-email-sahlberg@google.com","subject":"[PATCH v9 01/44] refs.c: constify the sha arguments for ref_transaction_create|delete|update","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-15T23:14:58Z","receivedAt":"2014-05-15T23:14:58Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"ref_transaction_create|delete|update has no need to modify the sha1\narguments passed to it so it should use const unsigned char* instead\nof unsigned char*.\n\nSome functions, such as fast_forward_to(), already have its old/new\nsha1 arguments as consts. This function will at some point need to\nuse ref_transaction_update() in which case this change is required.\n\nReviewed-by: Jonathan Nieder \nSigned-off-by: Ronnie Sahlberg \n---\n refs.c | 7 ++++---\n refs.h | 7 ++++---\n 2 files changed, 8 insertions(+), 6 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 6898263..2114748 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3338,7 +3338,8 @@ static struct ref_update *add_update(struct ref_transaction *transaction,\n \n void ref_transaction_update(struct ref_transaction *transaction,\n \t\t\t const char *refname,\n-\t\t\t unsigned char *new_sha1, unsigned char *old_sha1,\n+\t\t\t const unsigned char *new_sha1,\n+\t\t\t const unsigned char *old_sha1,\n \t\t\t int flags, int have_old)\n {\n \tstruct ref_update *update = add_update(transaction, refname);\n@@ -3352,7 +3353,7 @@ void ref_transaction_update(struct ref_transaction *transaction,\n \n void ref_transaction_create(struct ref_transaction *transaction,\n \t\t\t const char *refname,\n-\t\t\t unsigned char *new_sha1,\n+\t\t\t const unsigned char *new_sha1,\n \t\t\t int flags)\n {\n \tstruct ref_update *update = add_update(transaction, refname);\n@@ -3366,7 +3367,7 @@ void ref_transaction_create(struct ref_transaction *transaction,\n \n void ref_transaction_delete(struct ref_transaction *transaction,\n \t\t\t const char *refname,\n-\t\t\t unsigned char *old_sha1,\n+\t\t\t const unsigned char *old_sha1,\n \t\t\t int flags, int have_old)\n {\n \tstruct ref_update *update = add_update(transaction, refname);\ndiff --git a/refs.h b/refs.h\nindex 09ff483..6d97700 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -245,7 +245,8 @@ void ref_transaction_rollback(struct ref_transaction *transaction);\n */\n void ref_transaction_update(struct ref_transaction *transaction,\n \t\t\t const char *refname,\n-\t\t\t unsigned char *new_sha1, unsigned char *old_sha1,\n+\t\t\t const unsigned char *new_sha1,\n+\t\t\t const unsigned char *old_sha1,\n \t\t\t int flags, int have_old);\n \n /*\n@@ -256,7 +257,7 @@ void ref_transaction_update(struct ref_transaction *transaction,\n */\n void ref_transaction_create(struct ref_transaction *transaction,\n \t\t\t const char *refname,\n-\t\t\t unsigned char *new_sha1,\n+\t\t\t const unsigned char *new_sha1,\n \t\t\t int flags);\n \n /*\n@@ -266,7 +267,7 @@ void ref_transaction_create(struct ref_transaction *transaction,\n */\n void ref_transaction_delete(struct ref_transaction *transaction,\n \t\t\t const char *refname,\n-\t\t\t unsigned char *old_sha1,\n+\t\t\t const unsigned char *old_sha1,\n \t\t\t int flags, int have_old);\n \n /*\n-- \n2.0.0.rc3.477.gffe78a2\n"},{"id":"241846","messageId":"1400195741-22996-3-git-send-email-sahlberg@google.com","threadId":"36672","inReplyTo":"1400195741-22996-1-git-send-email-sahlberg@google.com","subject":"[PATCH v9 02/44] refs.c: allow passing NULL to ref_transaction_free","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-15T23:14:59Z","receivedAt":"2014-05-15T23:14:59Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Allow ref_transaction_free(NULL) and hence ref_transaction_rollback(NULL)\nas no-ops. This makes ref_transaction_rollback easier to use and more similar\nto plain 'free'.\n\nIn particular, it lets us rollback unconditionally as part of cleanup code\nafter setting 'transaction = NULL' if a transaction has been committed or\nrolled back already.\n\nThis allows us to write code like\n if ( (!transaction ||\n ref_transaction_update(...)) ||\n (ref_transaction_commit(...) && !(transaction = NULL)) {\n ref_transaction_rollback(transaction);\n ...\n }\n\nReviewed-by: Jonathan Nieder \nSigned-off-by: Ronnie Sahlberg \n---\n refs.c | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/refs.c b/refs.c\nindex 2114748..88d73c8 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3312,6 +3312,9 @@ static void ref_transaction_free(struct ref_transaction *transaction)\n {\n \tint i;\n \n+\tif (!transaction)\n+\t\treturn;\n+\n \tfor (i = 0; i < transaction->nr; i++)\n \t\tfree(transaction->updates[i]);\n \n-- \n2.0.0.rc3.477.gffe78a2\n"},{"id":"241839","messageId":"1400195741-22996-4-git-send-email-sahlberg@google.com","threadId":"36672","inReplyTo":"1400195741-22996-1-git-send-email-sahlberg@google.com","subject":"[PATCH v9 03/44] refs.c: add a strbuf argument to ref_transaction_commit for error logging","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-15T23:15:00Z","receivedAt":"2014-05-15T23:15:00Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Add a strbuf argument to _commit so that we can pass an error string back to\nthe caller. So that we can do error logging from the caller instead of from\n_commit.\n\nLonger term plan is to first convert all callers to use onerr==QUIET_ON_ERR\nand craft any log messages from the callers themselves and finally remove the\nonerr argument completely.\n\nReviewed-by: Jonathan Nieder \nSigned-off-by: Ronnie Sahlberg \n---\n builtin/update-ref.c | 2 +-\n refs.c | 6 +++++-\n refs.h | 5 ++++-\n 3 files changed, 10 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex 405267f..aaa06aa 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -367,7 +367,7 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)\n \t\tif (end_null)\n \t\t\tline_termination = '\\0';\n \t\tupdate_refs_stdin();\n-\t\tret = ref_transaction_commit(transaction, msg,\n+\t\tret = ref_transaction_commit(transaction, msg, NULL,\n \t\t\t\t\t UPDATE_REFS_DIE_ON_ERR);\n \t\treturn ret;\n \t}\ndiff --git a/refs.c b/refs.c\nindex 88d73c8..1a7f9d9 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3423,7 +3423,8 @@ static int ref_update_reject_duplicates(struct ref_update **updates, int n,\n }\n \n int ref_transaction_commit(struct ref_transaction *transaction,\n-\t\t\t const char *msg, enum action_on_err onerr)\n+\t\t\t const char *msg, struct strbuf *err,\n+\t\t\t enum action_on_err onerr)\n {\n \tint ret = 0, delnum = 0, i;\n \tconst char **delnames;\n@@ -3452,6 +3453,9 @@ int ref_transaction_commit(struct ref_transaction *transaction,\n \t\t\t\t\t update->flags,\n \t\t\t\t\t &update->type, onerr);\n \t\tif (!update->lock) {\n+\t\t\tif (err)\n+\t\t\t\tstrbuf_addf(err, \"Cannot lock the ref '%s'.\",\n+\t\t\t\t\t update->refname);\n \t\t\tret = 1;\n \t\t\tgoto cleanup;\n \t\t}\ndiff --git a/refs.h b/refs.h\nindex 6d97700..25ae110 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -274,9 +274,12 @@ 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. The ref_transaction is freed by this function.\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, enum action_on_err onerr);\n+\t\t\t const char *msg, struct strbuf *err,\n+\t\t\t enum action_on_err onerr);\n \n /** Lock a ref and then write its file */\n int update_ref(const char *action, const char *refname,\n-- \n2.0.0.rc3.477.gffe78a2\n"},{"id":"241803","messageId":"1400195741-22996-5-git-send-email-sahlberg@google.com","threadId":"36672","inReplyTo":"1400195741-22996-1-git-send-email-sahlberg@google.com","subject":"[PATCH v9 04/44] refs.c: add an err argument to repack_without_refs","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-15T23:15:01Z","receivedAt":"2014-05-15T23:15:01Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Update repack_without_refs to take an err argument and update it if there\nis a failure. Pass the err variable from ref_transaction_commit to this\nfunction so that callers can print a meaningful error message if _commit\nfails due to a problem in repack_without_refs.\n\nSigned-off-by: Ronnie Sahlberg \n---\n cache.h | 2 ++\n lockfile.c | 17 ++++++++++-------\n refs.c | 25 +++++++++++++++++++------\n 3 files changed, 31 insertions(+), 13 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 8c6cdc2..48548d6 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -559,6 +559,8 @@ struct lock_file {\n #define LOCK_DIE_ON_ERROR 1\n #define LOCK_NODEREF 2\n extern int unable_to_lock_error(const char *path, int err);\n+extern void unable_to_lock_strbuf(const char *path, int err,\n+\t\t\t\t struct strbuf *buf);\n extern NORETURN void unable_to_lock_index_die(const char *path, int err);\n extern int hold_lock_file_for_update(struct lock_file *, const char *path, int);\n extern int hold_lock_file_for_append(struct lock_file *, const char *path, int);\ndiff --git a/lockfile.c b/lockfile.c\nindex 8fbcb6a..67e9bb5 100644\n--- a/lockfile.c\n+++ b/lockfile.c\n@@ -157,9 +157,8 @@ static int lock_file(struct lock_file *lk, const char *path, int flags)\n \treturn lk->fd;\n }\n \n-static char *unable_to_lock_message(const char *path, int err)\n+void unable_to_lock_strbuf(const char *path, int err, struct strbuf *buf)\n {\n-\tstruct strbuf buf = STRBUF_INIT;\n \n \tif (err == EEXIST) {\n \t\tstrbuf_addf(&buf, \"Unable to create '%s.lock': %s.\\n\\n\"\n@@ -170,20 +169,24 @@ static char *unable_to_lock_message(const char *path, int err)\n \t} else\n \t\tstrbuf_addf(&buf, \"Unable to create '%s.lock': %s\",\n \t\t\t absolute_path(path), strerror(err));\n-\treturn strbuf_detach(&buf, NULL);\n }\n \n int unable_to_lock_error(const char *path, int err)\n {\n-\tchar *msg = unable_to_lock_message(path, err);\n-\terror(\"%s\", msg);\n-\tfree(msg);\n+\tstruct strbuf buf = STRBUF_INIT;\n+\n+\tunable_to_lock_strbuf(path, err, &buf);\n+\terror(\"%s\", buf.buf);\n+\tstrbuf_release(&buf);\n \treturn -1;\n }\n \n NORETURN void unable_to_lock_index_die(const char *path, int err)\n {\n-\tdie(\"%s\", unable_to_lock_message(path, err));\n+\tstruct strbuf buf = STRBUF_INIT;\n+\n+\tunable_to_lock_strbuf(path, err, &buf);\n+\tdie(\"%s\", buf.buf);\n }\n \n int hold_lock_file_for_update(struct lock_file *lk, const char *path, int flags)\ndiff --git a/refs.c b/refs.c\nindex 1a7f9d9..66938d7 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2208,6 +2208,7 @@ int commit_packed_refs(void)\n \tstruct packed_ref_cache *packed_ref_cache =\n \t\tget_packed_ref_cache(&ref_cache);\n \tint error = 0;\n+\tint save_errno;\n \n \tif (!packed_ref_cache->lock)\n \t\tdie(\"internal error: packed-refs not locked\");\n@@ -2217,10 +2218,13 @@ int commit_packed_refs(void)\n \tdo_for_each_entry_in_dir(get_packed_ref_dir(packed_ref_cache),\n \t\t\t\t 0, write_packed_entry_fn,\n \t\t\t\t &packed_ref_cache->lock->fd);\n-\tif (commit_lock_file(packed_ref_cache->lock))\n+\tif (commit_lock_file(packed_ref_cache->lock)) {\n+\t\tsave_errno = errno;\n \t\terror = -1;\n+\t}\n \tpacked_ref_cache->lock = NULL;\n \trelease_packed_ref_cache(packed_ref_cache);\n+\terrno = save_errno;\n \treturn error;\n }\n \n@@ -2427,12 +2431,12 @@ static int curate_packed_ref_fn(struct ref_entry *entry, void *cb_data)\n \treturn 0;\n }\n \n-static int repack_without_refs(const char **refnames, int n)\n+static int repack_without_refs(const char **refnames, int n, struct strbuf *err)\n {\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, removed = 0;\n+\tint i, ret, removed = 0;\n \n \t/* Look for a packed ref */\n \tfor (i = 0; i < n; i++)\n@@ -2444,6 +2448,11 @@ static int repack_without_refs(const char **refnames, int n)\n \t\treturn 0; /* no refname exists in packed refs */\n \n \tif (lock_packed_refs(0)) {\n+\t\tif (err) {\n+\t\t\tunable_to_lock_strbuf(git_path(\"packed-refs\"), errno,\n+\t\t\t\t\t err);\n+\t\t\treturn 1;\n+\t\t}\n \t\tunable_to_lock_error(git_path(\"packed-refs\"), errno);\n \t\treturn error(\"cannot delete '%s' from packed refs\", refnames[i]);\n \t}\n@@ -2470,12 +2479,16 @@ static int repack_without_refs(const char **refnames, int n)\n \t}\n \n \t/* Write what remains */\n-\treturn commit_packed_refs();\n+\tret = commit_packed_refs();\n+\tif (ret && err)\n+\t\tstrbuf_addf(err, \"unable to overwrite old ref-pack file: %s\",\n+\t\t\t strerror(errno));\n+\treturn ret;\n }\n \n static int repack_without_ref(const char *refname)\n {\n-\treturn repack_without_refs(&refname, 1);\n+\treturn repack_without_refs(&refname, 1, NULL);\n }\n \n static int delete_ref_loose(struct ref_lock *lock, int flag)\n@@ -3486,7 +3499,7 @@ int ref_transaction_commit(struct ref_transaction *transaction,\n \t\t}\n \t}\n \n-\tret |= repack_without_refs(delnames, delnum);\n+\tret |= repack_without_refs(delnames, delnum, err);\n \tfor (i = 0; i < delnum; i++)\n \t\tunlink_or_warn(git_path(\"logs/%s\", delnames[i]));\n \tclear_loose_ref_cache(&ref_cache);\n-- \n2.0.0.rc3.477.gffe78a2\n"},{"id":"241844","messageId":"1400195741-22996-6-git-send-email-sahlberg@google.com","threadId":"36672","inReplyTo":"1400195741-22996-1-git-send-email-sahlberg@google.com","subject":"[PATCH v9 05/44] refs.c: make ref_update_reject_duplicates take a strbuf argument for errors","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-15T23:15:02Z","receivedAt":"2014-05-15T23:15:02Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Make ref_update_reject_duplicates return any error that occurs through a\nnew strbuf argument. This means that when a transaction commit fails in\nthis function we will now be able to pass a helpful error message back to the\ncaller.\n\nReviewed-by: Jonathan Nieder \nSigned-off-by: Ronnie Sahlberg \n---\n refs.c | 6 +++++-\n 1 file changed, 5 insertions(+), 1 deletion(-)\n\ndiff --git a/refs.c b/refs.c\nindex 66938d7..4ebf130 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3415,6 +3415,7 @@ static int ref_update_compare(const void *r1, const void *r2)\n }\n \n static int ref_update_reject_duplicates(struct ref_update **updates, int n,\n+\t\t\t\t\tstruct strbuf *err,\n \t\t\t\t\tenum action_on_err onerr)\n {\n \tint i;\n@@ -3422,6 +3423,9 @@ static int ref_update_reject_duplicates(struct ref_update **updates, int n,\n \t\tif (!strcmp(updates[i - 1]->refname, updates[i]->refname)) {\n \t\t\tconst char *str =\n \t\t\t\t\"Multiple updates for ref '%s' not allowed.\";\n+\t\t\tif (err)\n+\t\t\t\tstrbuf_addf(err, str, updates[i]->refname);\n+\n \t\t\tswitch (onerr) {\n \t\t\tcase UPDATE_REFS_MSG_ON_ERR:\n \t\t\t\terror(str, updates[i]->refname); break;\n@@ -3452,7 +3456,7 @@ int ref_transaction_commit(struct ref_transaction *transaction,\n \n \t/* Copy, sort, and reject duplicate refs */\n \tqsort(updates, n, sizeof(*updates), ref_update_compare);\n-\tret = ref_update_reject_duplicates(updates, n, onerr);\n+\tret = ref_update_reject_duplicates(updates, n, err, onerr);\n \tif (ret)\n \t\tgoto cleanup;\n \n-- \n2.0.0.rc3.477.gffe78a2\n"},{"id":"241812","messageId":"1400195741-22996-7-git-send-email-sahlberg@google.com","threadId":"36672","inReplyTo":"1400195741-22996-1-git-send-email-sahlberg@google.com","subject":"[PATCH v9 06/44] refs.c: add an err argument to delete_ref_loose","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-15T23:15:03Z","receivedAt":"2014-05-15T23:15:03Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Add an err argument to delete_loose_ref so that we can pass a descriptive\nerror string back to the caller. Pass the err argument from transaction\ncommit to this function so that transaction users will have a nice error\nstring if the transaction failed due to delete_loose_ref.\n\nAdd a new function unlink_or_err that we can call from delete_ref_loose. This\nfunction is similar to unlink_or_warn except that we can pass it an err\nargument. If err is non-NULL the function will populate err instead of\nprinting a warning().\n\nSigned-off-by: Ronnie Sahlberg \n---\n refs.c | 39 +++++++++++++++++++++++++++++++++------\n 1 file changed, 33 insertions(+), 6 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 4ebf130..e4d35c9 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2491,17 +2491,43 @@ static int repack_without_ref(const char *refname)\n \treturn repack_without_refs(&refname, 1, NULL);\n }\n \n-static int delete_ref_loose(struct ref_lock *lock, int flag)\n+static int add_err_if_unremovable(const char *op, const char *file,\n+\t\t\t\t struct strbuf *e, int rc)\n+{\n+\tint err = errno;\n+\tif (rc < 0 && err != ENOENT) {\n+\t\tstrbuf_addf(e, \"unable to %s %s: %s\",\n+\t\t\t op, file, strerror(errno));\n+\t\terrno = err;\n+\t}\n+\treturn rc;\n+}\n+\n+static int unlink_or_err(const char *file, struct strbuf *err)\n+{\n+\tif (err)\n+\t\treturn add_err_if_unremovable(\"unlink\", file, err,\n+\t\t\t\t\t unlink(file));\n+\telse\n+\t\treturn unlink_or_warn(file);\n+}\n+\n+static int delete_ref_loose(struct ref_lock *lock, int flag, struct strbuf *err)\n {\n \tif (!(flag & REF_ISPACKED) || flag & REF_ISSYMREF) {\n \t\t/* loose */\n-\t\tint err, i = strlen(lock->lk->filename) - 5; /* .lock */\n+\t\tint res, i = strlen(lock->lk->filename) - 5; /* .lock */\n \n \t\tlock->lk->filename[i] = 0;\n-\t\terr = unlink_or_warn(lock->lk->filename);\n+\t\tres = unlink_or_err(lock->lk->filename, err);\n \t\tlock->lk->filename[i] = '.';\n-\t\tif (err && errno != ENOENT)\n+\t\tif (res && errno != ENOENT) {\n+\t\t\tif (err)\n+\t\t\t\tstrbuf_addf(err,\n+\t\t\t\t\t \"failed to delete loose ref '%s'\",\n+\t\t\t\t\t lock->lk->filename);\n \t\t\treturn 1;\n+\t\t}\n \t}\n \treturn 0;\n }\n@@ -2514,7 +2540,7 @@ int delete_ref(const char *refname, const unsigned char *sha1, int delopt)\n \tlock = lock_ref_sha1_basic(refname, sha1, delopt, &flag);\n \tif (!lock)\n \t\treturn 1;\n-\tret |= delete_ref_loose(lock, flag);\n+\tret |= delete_ref_loose(lock, flag, NULL);\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@@ -3499,7 +3525,8 @@ int ref_transaction_commit(struct ref_transaction *transaction,\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\tret |= delete_ref_loose(update->lock, update->type,\n+\t\t\t\t\t\terr);\n \t\t}\n \t}\n \n-- \n2.0.0.rc3.477.gffe78a2\n"},{"id":"241843","messageId":"1400195741-22996-8-git-send-email-sahlberg@google.com","threadId":"36672","inReplyTo":"1400195741-22996-1-git-send-email-sahlberg@google.com","subject":"[PATCH v9 07/44] refs.c: make update_ref_write update a strbuf on failure","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-15T23:15:04Z","receivedAt":"2014-05-15T23:15:04Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Change update_ref_write to also update an error strbuf on failure.\nThis makes the error available to ref_transaction_commit callers if the\ntransaction failed due to update_ref_sha1/write_ref_sha1 failures.\n\nReviewed-by: Jonathan Nieder \nSigned-off-by: Ronnie Sahlberg \n---\n refs.c | 9 ++++++---\n 1 file changed, 6 insertions(+), 3 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex e4d35c9..e8756fd 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3301,10 +3301,13 @@ static struct ref_lock *update_ref_lock(const char *refname,\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 enum action_on_err onerr)\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@@ -3430,7 +3433,7 @@ int update_ref(const char *action, const char *refname,\n \tlock = update_ref_lock(refname, oldval, flags, NULL, onerr);\n \tif (!lock)\n \t\treturn 1;\n-\treturn update_ref_write(action, refname, sha1, lock, onerr);\n+\treturn update_ref_write(action, refname, sha1, lock, NULL, onerr);\n }\n \n static int ref_update_compare(const void *r1, const void *r2)\n@@ -3512,7 +3515,7 @@ int ref_transaction_commit(struct ref_transaction *transaction,\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, onerr);\n+\t\t\t\t\t update->lock, err, onerr);\n \t\t\tupdate->lock = NULL; /* freed by update_ref_write */\n \t\t\tif (ret)\n \t\t\t\tgoto cleanup;\n-- \n2.0.0.rc3.477.gffe78a2\n"},{"id":"241847","messageId":"1400195741-22996-9-git-send-email-sahlberg@google.com","threadId":"36672","inReplyTo":"1400195741-22996-1-git-send-email-sahlberg@google.com","subject":"[PATCH v9 08/44] update-ref.c: log transaction error from the update_ref","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-15T23:15:05Z","receivedAt":"2014-05-15T23:15:05Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Call ref_transaction_commit with QUIET_ON_ERR and use the strbuf that is\nreturned to print a log message if/after the transaction fails.\n\nReviewed-by: Jonathan Nieder \nSigned-off-by: Ronnie Sahlberg \n---\n builtin/update-ref.c | 10 +++++-----\n 1 file changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex aaa06aa..207e24d 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -342,6 +342,7 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)\n \tconst char *refname, *oldval, *msg = NULL;\n \tunsigned char sha1[20], oldsha1[20];\n \tint delete = 0, no_deref = 0, read_stdin = 0, end_null = 0, flags = 0;\n+\tstruct strbuf err = STRBUF_INIT;\n \tstruct option options[] = {\n \t\tOPT_STRING( 'm', NULL, &msg, N_(\"reason\"), N_(\"reason of the update\")),\n \t\tOPT_BOOL('d', NULL, &delete, N_(\"delete the reference\")),\n@@ -359,17 +360,16 @@ 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\tint ret;\n \t\ttransaction = ref_transaction_begin();\n-\n \t\tif (delete || no_deref || argc > 0)\n \t\t\tusage_with_options(git_update_ref_usage, options);\n \t\tif (end_null)\n \t\t\tline_termination = '\\0';\n \t\tupdate_refs_stdin();\n-\t\tret = ref_transaction_commit(transaction, msg, NULL,\n-\t\t\t\t\t UPDATE_REFS_DIE_ON_ERR);\n-\t\treturn ret;\n+\t\tif (ref_transaction_commit(transaction, msg, &err,\n+\t\t\t\t\t UPDATE_REFS_QUIET_ON_ERR))\n+\t\t\tdie(\"%s\", err.buf);\n+\t\treturn 0;\n \t}\n \n \tif (end_null)\n-- \n2.0.0.rc3.477.gffe78a2\n"},{"id":"241811","messageId":"1400195741-22996-10-git-send-email-sahlberg@google.com","threadId":"36672","inReplyTo":"1400195741-22996-1-git-send-email-sahlberg@google.com","subject":"[PATCH v9 09/44] refs.c: remove the onerr argument to ref_transaction_commit","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-15T23:15:06Z","receivedAt":"2014-05-15T23:15:06Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Since all callers now use QUIET_ON_ERR we no longer need to provide an onerr\nargument any more. Remove the onerr argument from the ref_transaction_commit\nsignature.\n\nReviewed-by: Jonathan Nieder \nSigned-off-by: Ronnie Sahlberg \n---\n builtin/update-ref.c | 3 +--\n refs.c | 22 +++++++---------------\n refs.h | 3 +--\n 3 files changed, 9 insertions(+), 19 deletions(-)\n\ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex 207e24d..2bef2a0 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -366,8 +366,7 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)\n \t\tif (end_null)\n \t\t\tline_termination = '\\0';\n \t\tupdate_refs_stdin();\n-\t\tif (ref_transaction_commit(transaction, msg, &err,\n-\t\t\t\t\t UPDATE_REFS_QUIET_ON_ERR))\n+\t\tif (ref_transaction_commit(transaction, msg, &err))\n \t\t\tdie(\"%s\", err.buf);\n \t\treturn 0;\n \t}\ndiff --git a/refs.c b/refs.c\nindex e8756fd..3eb73a3 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3444,8 +3444,7 @@ static int ref_update_compare(const void *r1, const void *r2)\n }\n \n static int ref_update_reject_duplicates(struct ref_update **updates, int n,\n-\t\t\t\t\tstruct strbuf *err,\n-\t\t\t\t\tenum action_on_err onerr)\n+\t\t\t\t\tstruct strbuf *err)\n {\n \tint i;\n \tfor (i = 1; i < n; i++)\n@@ -3455,22 +3454,13 @@ static int ref_update_reject_duplicates(struct ref_update **updates, int n,\n \t\t\tif (err)\n \t\t\t\tstrbuf_addf(err, str, updates[i]->refname);\n \n-\t\t\tswitch (onerr) {\n-\t\t\tcase UPDATE_REFS_MSG_ON_ERR:\n-\t\t\t\terror(str, updates[i]->refname); break;\n-\t\t\tcase UPDATE_REFS_DIE_ON_ERR:\n-\t\t\t\tdie(str, updates[i]->refname); break;\n-\t\t\tcase UPDATE_REFS_QUIET_ON_ERR:\n-\t\t\t\tbreak;\n-\t\t\t}\n \t\t\treturn 1;\n \t\t}\n \treturn 0;\n }\n \n int ref_transaction_commit(struct ref_transaction *transaction,\n-\t\t\t const char *msg, struct strbuf *err,\n-\t\t\t enum action_on_err onerr)\n+\t\t\t const char *msg, struct strbuf *err)\n {\n \tint ret = 0, delnum = 0, i;\n \tconst char **delnames;\n@@ -3485,7 +3475,7 @@ int ref_transaction_commit(struct ref_transaction *transaction,\n \n \t/* Copy, sort, and reject duplicate refs */\n \tqsort(updates, n, sizeof(*updates), ref_update_compare);\n-\tret = ref_update_reject_duplicates(updates, n, err, onerr);\n+\tret = ref_update_reject_duplicates(updates, n, err);\n \tif (ret)\n \t\tgoto cleanup;\n \n@@ -3497,7 +3487,8 @@ int ref_transaction_commit(struct ref_transaction *transaction,\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, onerr);\n+\t\t\t\t\t &update->type,\n+\t\t\t\t\t UPDATE_REFS_QUIET_ON_ERR);\n \t\tif (!update->lock) {\n \t\t\tif (err)\n \t\t\t\tstrbuf_addf(err, \"Cannot lock the ref '%s'.\",\n@@ -3515,7 +3506,8 @@ int ref_transaction_commit(struct ref_transaction *transaction,\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, onerr);\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\t\tgoto cleanup;\ndiff --git a/refs.h b/refs.h\nindex 25ae110..555ee59 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -278,8 +278,7 @@ void ref_transaction_delete(struct ref_transaction *transaction,\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-\t\t\t enum action_on_err onerr);\n+\t\t\t const char *msg, struct strbuf *err);\n \n /** Lock a ref and then write its file */\n int update_ref(const char *action, const char *refname,\n-- \n2.0.0.rc3.477.gffe78a2\n"},{"id":"241836","messageId":"1400195741-22996-11-git-send-email-sahlberg@google.com","threadId":"36672","inReplyTo":"1400195741-22996-1-git-send-email-sahlberg@google.com","subject":"[PATCH v9 10/44] refs.c: change ref_transaction_update() to do error checking and return status","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-15T23:15:07Z","receivedAt":"2014-05-15T23:15:07Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Update ref_transaction_update() do some basic error checking and return\nnon-zero on error. Update all callers to check ref_transaction_update() for\nerror. There are currently no conditions in _update that will return error but\nthere will be in the future. Add an err argument that will be updated on\nfailure.\n\nAlso check for BUGs during update and die(BUG:...) if we are calling\n_update with have_old but the old_sha1 pointer is NULL.\n\nSigned-off-by: Ronnie Sahlberg \n---\n builtin/update-ref.c | 12 +++++++-----\n refs.c | 18 ++++++++++++------\n refs.h | 14 +++++++++-----\n 3 files changed, 28 insertions(+), 16 deletions(-)\n\ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex 2bef2a0..9f328b2 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -16,6 +16,7 @@ static struct ref_transaction *transaction;\n \n static char line_termination = '\\n';\n static int update_flags;\n+static struct strbuf err = STRBUF_INIT;\n \n /*\n * Parse one whitespace- or NUL-terminated, possibly C-quoted argument\n@@ -197,8 +198,9 @@ static const char *parse_cmd_update(struct strbuf *input, const char *next)\n \tif (*next != line_termination)\n \t\tdie(\"update %s: extra input: %s\", refname, next);\n \n-\tref_transaction_update(transaction, refname, new_sha1, old_sha1,\n-\t\t\t update_flags, have_old);\n+\tif (ref_transaction_update(transaction, refname, new_sha1, old_sha1,\n+\t\t\t\t update_flags, have_old, &err))\n+\t\tdie(\"update %s: failed: %s\", refname, err.buf);\n \n \tupdate_flags = 0;\n \tfree(refname);\n@@ -286,8 +288,9 @@ static const char *parse_cmd_verify(struct strbuf *input, const char *next)\n \tif (*next != line_termination)\n \t\tdie(\"verify %s: extra input: %s\", refname, next);\n \n-\tref_transaction_update(transaction, refname, new_sha1, old_sha1,\n-\t\t\t update_flags, have_old);\n+\tif (ref_transaction_update(transaction, refname, new_sha1, old_sha1,\n+\t\t\t\t update_flags, have_old, &err))\n+\t\tdie(\"verify %s: %s\", refname, err.buf);\n \n \tupdate_flags = 0;\n \tfree(refname);\n@@ -342,7 +345,6 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)\n \tconst char *refname, *oldval, *msg = NULL;\n \tunsigned char sha1[20], oldsha1[20];\n \tint delete = 0, no_deref = 0, read_stdin = 0, end_null = 0, flags = 0;\n-\tstruct strbuf err = STRBUF_INIT;\n \tstruct option options[] = {\n \t\tOPT_STRING( 'm', NULL, &msg, N_(\"reason\"), N_(\"reason of the update\")),\n \t\tOPT_BOOL('d', NULL, &delete, N_(\"delete the reference\")),\ndiff --git a/refs.c b/refs.c\nindex 3eb73a3..3c9b981 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3381,19 +3381,25 @@ static struct ref_update *add_update(struct ref_transaction *transaction,\n \treturn update;\n }\n \n-void ref_transaction_update(struct ref_transaction *transaction,\n-\t\t\t const char *refname,\n-\t\t\t const unsigned char *new_sha1,\n-\t\t\t const unsigned char *old_sha1,\n-\t\t\t int flags, int have_old)\n+int ref_transaction_update(struct ref_transaction *transaction,\n+\t\t\t const char *refname,\n+\t\t\t const unsigned char *new_sha1,\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 \thashcpy(update->new_sha1, new_sha1);\n \tupdate->flags = flags;\n \tupdate->have_old = have_old;\n \tif (have_old)\n \t\thashcpy(update->old_sha1, old_sha1);\n+\treturn 0;\n }\n \n void ref_transaction_create(struct ref_transaction *transaction,\ndiff --git a/refs.h b/refs.h\nindex 555ee59..11d07ee 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -242,12 +242,16 @@ void ref_transaction_rollback(struct ref_transaction *transaction);\n * be deleted. If have_old is true, then old_sha1 holds the value\n * that the reference should have had before the update, or zeros if\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 */\n-void ref_transaction_update(struct ref_transaction *transaction,\n-\t\t\t const char *refname,\n-\t\t\t const unsigned char *new_sha1,\n-\t\t\t const unsigned char *old_sha1,\n-\t\t\t int flags, int have_old);\n+int ref_transaction_update(struct ref_transaction *transaction,\n+\t\t\t const char *refname,\n+\t\t\t const unsigned char *new_sha1,\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 * Add a reference creation to transaction. new_sha1 is the value\n-- \n2.0.0.rc3.477.gffe78a2\n"},{"id":"241809","messageId":"1400195741-22996-12-git-send-email-sahlberg@google.com","threadId":"36672","inReplyTo":"1400195741-22996-1-git-send-email-sahlberg@google.com","subject":"[PATCH v9 11/44] refs.c: change ref_transaction_create to do error checking and return status","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-15T23:15:08Z","receivedAt":"2014-05-15T23:15:08Z","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\nSigned-off-by: Ronnie Sahlberg \n---\n builtin/update-ref.c | 4 +++-\n refs.c | 18 ++++++++++++------\n refs.h | 12 ++++++++----\n 3 files changed, 23 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex 9f328b2..e9c216e 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, NULL))\n+\t\tdie(\"cannot create ref '%s'\", refname);\n \n \tupdate_flags = 0;\n \tfree(refname);\ndiff --git a/refs.c b/refs.c\nindex 3c9b981..344e27a 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3402,18 +3402,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 11d07ee..0af5ba7 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -258,11 +258,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. On failure the err buffer will be updated.\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-- \n2.0.0.rc3.477.gffe78a2\n"},{"id":"241837","messageId":"1400195741-22996-13-git-send-email-sahlberg@google.com","threadId":"36672","inReplyTo":"1400195741-22996-1-git-send-email-sahlberg@google.com","subject":"[PATCH v9 12/44] refs.c: ref_transaction_delete to check for error and return status","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-15T23:15:09Z","receivedAt":"2014-05-15T23:15:09Z","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\nSigned-off-by: Ronnie Sahlberg \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 e9c216e..cdb71a8 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, NULL))\n+\t\tdie(\"failed transaction delete for %s\", refname);\n \n \tupdate_flags = 0;\n \tfree(refname);\ndiff --git a/refs.c b/refs.c\nindex 344e27a..7276b9f 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3422,19 +3422,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 0af5ba7..2c2e694 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -272,11 +272,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. On failure the err buffer will be updated.\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.0.rc3.477.gffe78a2\n"},{"id":"241838","messageId":"1400195741-22996-14-git-send-email-sahlberg@google.com","threadId":"36672","inReplyTo":"1400195741-22996-1-git-send-email-sahlberg@google.com","subject":"[PATCH v9 13/44] tag.c: use ref transactions when doing updates","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-15T23:15:10Z","receivedAt":"2014-05-15T23:15:10Z","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\nSigned-off-by: Ronnie Sahlberg \n---\n builtin/tag.c | 14 ++++++++------\n 1 file changed, 8 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex c6e8a71..b05f9a5 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,12 @@ 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();\n+\tif (!transaction ||\n+\t ref_transaction_update(transaction, ref.buf, object, 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 \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.0.rc3.477.gffe78a2\n"},{"id":"241842","messageId":"1400195741-22996-15-git-send-email-sahlberg@google.com","threadId":"36672","inReplyTo":"1400195741-22996-1-git-send-email-sahlberg@google.com","subject":"[PATCH v9 14/44] replace.c: use the ref transaction functions for updates","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-15T23:15:11Z","receivedAt":"2014-05-15T23:15:11Z","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\nSigned-off-by: Ronnie Sahlberg \n---\n builtin/replace.c | 14 ++++++++------\n 1 file changed, 8 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/replace.c b/builtin/replace.c\nindex 3da1bae..e8932cd 100644\n--- a/builtin/replace.c\n+++ b/builtin/replace.c\n@@ -133,7 +133,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 \tif (snprintf(ref, sizeof(ref),\n \t\t \"refs/replace/%s\",\n@@ -156,11 +157,12 @@ static int replace_object_sha1(const char *object_ref,\n \telse if (!force)\n \t\tdie(\"replace ref '%s' already exists\", ref);\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();\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 \treturn 0;\n }\n-- \n2.0.0.rc3.477.gffe78a2\n"},{"id":"241848","messageId":"1400195741-22996-16-git-send-email-sahlberg@google.com","threadId":"36672","inReplyTo":"1400195741-22996-1-git-send-email-sahlberg@google.com","subject":"[PATCH v9 15/44] commit.c: use ref transactions for updates","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-15T23:15:12Z","receivedAt":"2014-05-15T23:15:12Z","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 \nSigned-off-by: Ronnie Sahlberg \n---\n builtin/commit.c | 23 ++++++++++-------------\n 1 file changed, 10 insertions(+), 13 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex d28505a..c429216 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1581,11 +1581,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@@ -1707,16 +1708,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@@ -1725,9 +1716,15 @@ 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();\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 \n \tunlink(git_path(\"CHERRY_PICK_HEAD\"));\n-- \n2.0.0.rc3.477.gffe78a2\n"},{"id":"241810","messageId":"1400195741-22996-17-git-send-email-sahlberg@google.com","threadId":"36672","inReplyTo":"1400195741-22996-1-git-send-email-sahlberg@google.com","subject":"[PATCH v9 16/44] sequencer.c: use ref transactions for all ref updates","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-15T23:15:13Z","receivedAt":"2014-05-15T23:15:13Z","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\nSigned-off-by: Ronnie Sahlberg \n---\n sequencer.c | 24 ++++++++++++++++--------\n 1 file changed, 16 insertions(+), 8 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex 0a80c58..e6b3bbc 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -272,23 +272,31 @@ 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(1); /* 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 \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();\n+\tif (!transaction ||\n+\t ref_transaction_update(transaction, \"HEAD\", to, from,\n+\t\t\t\t 0, !unborn, &err) ||\n+\t (ref_transaction_commit(transaction, sb.buf, &err) &&\n+\t !(transaction = NULL))) {\n+\t\tref_transaction_rollback(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+\treturn 0;\n }\n \n static int do_recursive_merge(struct commit *base, struct commit *next,\n-- \n2.0.0.rc3.477.gffe78a2\n"},{"id":"241807","messageId":"1400195741-22996-18-git-send-email-sahlberg@google.com","threadId":"36672","inReplyTo":"1400195741-22996-1-git-send-email-sahlberg@google.com","subject":"[PATCH v9 17/44] fast-import.c: change update_branch to use ref transactions","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-15T23:15:14Z","receivedAt":"2014-05-15T23:15:14Z","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\nSigned-off-by: Ronnie Sahlberg \n---\n fast-import.c | 25 +++++++++++++++----------\n 1 file changed, 15 insertions(+), 10 deletions(-)\n\ndiff --git a/fast-import.c b/fast-import.c\nindex 6707a66..bac799c 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -1679,39 +1679,44 @@ 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+\n \tif (is_null_sha1(b->sha1)) {\n \t\tif (b->delete)\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();\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 !(transaction = NULL))) {\n+\t\tref_transaction_rollback(transaction);\n+\t\terror(\"%s\", err.buf);\n+\t\tstrbuf_release(&err);\n+\t\treturn -1;\n+\t}\n \treturn 0;\n }\n \n-- \n2.0.0.rc3.477.gffe78a2\n"},{"id":"241805","messageId":"1400195741-22996-19-git-send-email-sahlberg@google.com","threadId":"36672","inReplyTo":"1400195741-22996-1-git-send-email-sahlberg@google.com","subject":"[PATCH v9 18/44] branch.c: use ref transaction for all ref updates","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-15T23:15:15Z","receivedAt":"2014-05-15T23:15:15Z","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.\nref_transaction_create will check that the ref does not already exist and fail\notherwise meaning that we no longer need to keep a lock on the ref during the\nsetup_tracking. This simplifies the code since we can now do the transaction\nin one single step.\n\nIf the forcing flag is false then use ref_transaction_create since this will\nfail if the ref already exist. Otherwise use ref_transaction_update.\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 \nSigned-off-by: Ronnie Sahlberg \n---\n branch.c | 29 +++++++++++++++--------------\n 1 file changed, 15 insertions(+), 14 deletions(-)\n\ndiff --git a/branch.c b/branch.c\nindex 660097b..8bf7588 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,24 @@ 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();\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}\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.0.rc3.477.gffe78a2\n"},{"id":"241841","messageId":"1400195741-22996-20-git-send-email-sahlberg@google.com","threadId":"36672","inReplyTo":"1400195741-22996-1-git-send-email-sahlberg@google.com","subject":"[PATCH v9 19/44] refs.c: change update_ref to use a transaction","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-15T23:15:16Z","receivedAt":"2014-05-15T23:15:16Z","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\nSigned-off-by: Ronnie Sahlberg \n---\n refs.c | 25 +++++++++++++++++++++----\n 1 file changed, 21 insertions(+), 4 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 7276b9f..5e82695 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3447,11 +3447,28 @@ 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();\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) && !(t = NULL))) {\n+\t\tconst char *str = \"update_ref failed for ref '%s': %s\";\n+\n+\t\tref_transaction_rollback(t);\n+\t\tswitch (onerr) {\n+\t\tcase UPDATE_REFS_MSG_ON_ERR:\n+\t\t\terror(str, refname, err.buf); break;\n+\t\tcase UPDATE_REFS_DIE_ON_ERR:\n+\t\t\tdie(str, refname, err.buf); break;\n+\t\tcase UPDATE_REFS_QUIET_ON_ERR: break;\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.0.rc3.477.gffe78a2\n"},{"id":"241831","messageId":"1400195741-22996-21-git-send-email-sahlberg@google.com","threadId":"36672","inReplyTo":"1400195741-22996-1-git-send-email-sahlberg@google.com","subject":"[PATCH v9 20/44] refs.c: free the transaction before returning when number of updates is 0","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-15T23:15:17Z","receivedAt":"2014-05-15T23:15:17Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"We have to free the transaction before returning in the early check for\n'return early if number of updates == 0' or else the following code would\ncreate a memory leak with the transaction never being freed :\n t = ref_transaction_begin()\n ref_transaction_commit(t)\n\nSigned-off-by: Ronnie Sahlberg \n---\n refs.c | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/refs.c b/refs.c\nindex 5e82695..b4c7d04 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3502,8 +3502,10 @@ 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 (!n) {\n+\t\tref_transaction_free(transaction);\n \t\treturn 0;\n+\t}\n \n \t/* Allocate work space */\n \tdelnames = xmalloc(sizeof(*delnames) * n);\n-- \n2.0.0.rc3.477.gffe78a2\n"},{"id":"241808","messageId":"1400195741-22996-22-git-send-email-sahlberg@google.com","threadId":"36672","inReplyTo":"1400195741-22996-1-git-send-email-sahlberg@google.com","subject":"[PATCH v9 21/44] refs.c: ref_transaction_commit should not free the transaction","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-15T23:15:18Z","receivedAt":"2014-05-15T23:15:18Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Change ref_transaction_commit so that it does not free the transaction.\nInstead require that a caller will end a transaction by either calling\nref_transaction_rollback or ref_transaction_free.\n\nBy having the transaction object remaining valid after _commit returns allows\nus to write much nicer code and still be able to call ref_transaction_rollback\nsafely. Instead of this horribleness\n\tt = ref_transaction_begin();\n\tif ((!t ||\n\t ref_transaction_update(t, refname, sha1, oldval, flags,\n\t\t\t\t !!oldval)) ||\n\t (ref_transaction_commit(t, action, &err) && !(t = NULL))) {\n\t\tref_transaction_rollback(t);\n\nwe can now just do the much nicer\n\tt = ref_transaction_begin();\n\tif (!t ||\n\t ref_transaction_update(t, refname, sha1, oldval, flags,\n\t\t\t\t !!oldval) ||\n\t ref_transaction_commit(&t, action, &err)) {\n\t\tref_transaction_rollback(t);\n\t\t... die/return ...\n\n\tref_transaction_free(transaction);\n\nSigned-off-by: Ronnie Sahlberg \n---\n branch.c | 1 +\n builtin/commit.c | 1 +\n builtin/replace.c | 1 +\n builtin/tag.c | 1 +\n builtin/update-ref.c | 1 +\n fast-import.c | 4 ++--\n refs.c | 14 ++++++--------\n refs.h | 14 +++++++++-----\n sequencer.c | 4 ++--\n 9 files changed, 24 insertions(+), 17 deletions(-)\n\ndiff --git a/branch.c b/branch.c\nindex 8bf7588..f78a28b 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -304,6 +304,7 @@ void create_branch(const char *head,\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)\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex c429216..6b888f2 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1726,6 +1726,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t\trollback_index_files();\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\"));\ndiff --git a/builtin/replace.c b/builtin/replace.c\nindex e8932cd..af7f72d 100644\n--- a/builtin/replace.c\n+++ b/builtin/replace.c\n@@ -164,6 +164,7 @@ static int replace_object_sha1(const char *object_ref,\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 \ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex b05f9a5..30b471c 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -708,6 +708,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\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+\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 \ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex cdb71a8..4c2145e 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -373,6 +373,7 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)\n \t\tupdate_refs_stdin();\n \t\tif (ref_transaction_commit(transaction, msg, &err))\n \t\t\tdie(\"%s\", err.buf);\n+\t\tref_transaction_free(transaction);\n \t\treturn 0;\n \t}\n \ndiff --git a/fast-import.c b/fast-import.c\nindex bac799c..6b186ce 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -1710,13 +1710,13 @@ static int update_branch(struct branch *b)\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 !(transaction = NULL))) {\n+\t ref_transaction_commit(transaction, msg, &err)) {\n \t\tref_transaction_rollback(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 \ndiff --git a/refs.c b/refs.c\nindex b4c7d04..016206b 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3350,7 +3350,7 @@ struct ref_transaction *ref_transaction_begin(void)\n \treturn xcalloc(1, sizeof(struct ref_transaction));\n }\n \n-static void ref_transaction_free(struct ref_transaction *transaction)\n+void ref_transaction_free(struct ref_transaction *transaction)\n {\n \tint i;\n \n@@ -3451,10 +3451,10 @@ int update_ref(const char *action, const char *refname,\n \tstruct strbuf err = STRBUF_INIT;\n \n \tt = ref_transaction_begin();\n-\tif ((!t ||\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) && !(t = NULL))) {\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_rollback(t);\n@@ -3468,6 +3468,7 @@ int update_ref(const char *action, const char *refname,\n \t\tstrbuf_release(&err);\n \t\treturn 1;\n \t}\n+\tref_transaction_free(t);\n \treturn 0;\n }\n \n@@ -3502,10 +3503,8 @@ 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-\t\tref_transaction_free(transaction);\n+\tif (!n)\n \t\treturn 0;\n-\t}\n \n \t/* Allocate work space */\n \tdelnames = xmalloc(sizeof(*delnames) * n);\n@@ -3572,7 +3571,6 @@ cleanup:\n \t\tif (updates[i]->lock)\n \t\t\tunlock_ref(updates[i]->lock);\n \tfree(delnames);\n-\tref_transaction_free(transaction);\n \treturn ret;\n }\n \ndiff --git a/refs.h b/refs.h\nindex 2c2e694..608ca10 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -216,8 +216,8 @@ enum action_on_err {\n \n /*\n * Begin a reference transaction. The reference transaction must\n- * eventually be commited using ref_transaction_commit() or rolled\n- * back using ref_transaction_rollback().\n+ * eventually be freed by either calling ref_transaction_rollback()\n+ * or ref_transaction_free().\n */\n struct ref_transaction *ref_transaction_begin(void);\n \n@@ -285,13 +285,17 @@ int ref_transaction_delete(struct ref_transaction *transaction,\n /*\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. The ref_transaction is freed by this function.\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+ * problem. If err is non-NULL we will add an error string to it to explain\n+ * why 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 \n+/*\n+ * Free an existing transaction.\n+ */\n+void ref_transaction_free(struct ref_transaction *transaction);\n+\n /** Lock a ref and then write its file */\n int update_ref(const char *action, const char *refname,\n \t\tconst unsigned char *sha1, const unsigned char *oldval,\ndiff --git a/sequencer.c b/sequencer.c\nindex e6b3bbc..f6f1536 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -286,14 +286,14 @@ static int fast_forward_to(const unsigned char *to, const unsigned char *from,\n \tif (!transaction ||\n \t ref_transaction_update(transaction, \"HEAD\", to, from,\n \t\t\t\t 0, !unborn, &err) ||\n-\t (ref_transaction_commit(transaction, sb.buf, &err) &&\n-\t !(transaction = NULL))) {\n+\t ref_transaction_commit(transaction, sb.buf, &err)) {\n \t\tref_transaction_rollback(transaction);\n \t\terror(\"%s\", err.buf);\n \t\tstrbuf_release(&sb);\n \t\tstrbuf_release(&err);\n \t\treturn -1;\n \t}\n+\tref_transaction_free(transaction);\n \n \tstrbuf_release(&sb);\n \treturn 0;\n-- \n2.0.0.rc3.477.gffe78a2\n"},{"id":"241806","messageId":"1400195741-22996-23-git-send-email-sahlberg@google.com","threadId":"36672","inReplyTo":"1400195741-22996-1-git-send-email-sahlberg@google.com","subject":"[PATCH v9 22/44] fetch.c: clear errno before calling functions that might set it","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-15T23:15:19Z","receivedAt":"2014-05-15T23:15:19Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"In s_update_ref there are two calls that when they fail we return an error\nbased on the errno value. In particular we want to return a specific error\nif ENOTDIR happened. Both these functions do have failure modes where they\nmay return an error without updating errno, in which case a previous and\nunrelated ENOTDIR may cause us to return the wrong error. Clear errno before\ncalling any functions if we check errno afterwards.\n\nAlso skip initializing a static variable to 0. Statics live in .bss and\nare all automatically initialized to 0.\n\nSigned-off-by: Ronnie Sahlberg \n---\n builtin/fetch.c | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex 55f457c..a93c893 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -44,7 +44,7 @@ static struct transport *gtransport;\n static struct transport *gsecondary;\n static const char *submodule_prefix = \"\";\n static const char *recurse_submodules_default;\n-static int shown_url = 0;\n+static int shown_url;\n \n static int option_parse_recurse_submodules(const struct option *opt,\n \t\t\t\t const char *arg, int unset)\n@@ -382,6 +382,8 @@ static int s_update_ref(const char *action,\n \tif (!rla)\n \t\trla = default_rla.buf;\n \tsnprintf(msg, sizeof(msg), \"%s: %s\", rla, action);\n+\n+\terrno = 0;\n \tlock = lock_any_ref_for_update(ref->name,\n \t\t\t\t check_old ? ref->old_sha1 : NULL,\n \t\t\t\t 0, NULL);\n-- \n2.0.0.rc3.477.gffe78a2\n"},{"id":"241840","messageId":"1400195741-22996-24-git-send-email-sahlberg@google.com","threadId":"36672","inReplyTo":"1400195741-22996-1-git-send-email-sahlberg@google.com","subject":"[PATCH v9 23/44] fetch.c: change s_update_ref to use a ref transaction","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-15T23:15:20Z","receivedAt":"2014-05-15T23:15:20Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Change s_update_ref to use a ref transaction for the ref update.\n\nSigned-off-by: Ronnie Sahlberg \n---\n builtin/fetch.c | 17 +++++++++--------\n 1 file changed, 9 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex a93c893..c9b5954 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -375,7 +375,7 @@ static int s_update_ref(const char *action,\n {\n \tchar msg[1024];\n \tchar *rla = getenv(\"GIT_REFLOG_ACTION\");\n-\tstatic struct ref_lock *lock;\n+\tstruct ref_transaction *transaction;\n \n \tif (dry_run)\n \t\treturn 0;\n@@ -384,15 +384,16 @@ static int s_update_ref(const char *action,\n \tsnprintf(msg, sizeof(msg), \"%s: %s\", rla, action);\n \n \terrno = 0;\n-\tlock = lock_any_ref_for_update(ref->name,\n-\t\t\t\t check_old ? ref->old_sha1 : NULL,\n-\t\t\t\t 0, NULL);\n-\tif (!lock)\n-\t\treturn errno == ENOTDIR ? STORE_REF_ERROR_DF_CONFLICT :\n-\t\t\t\t\t STORE_REF_ERROR_OTHER;\n-\tif (write_ref_sha1(lock, ref->new_sha1, msg) < 0)\n+\ttransaction = ref_transaction_begin();\n+\tif (!transaction ||\n+\t ref_transaction_update(transaction, ref->name, ref->new_sha1,\n+\t\t\t\t ref->old_sha1, 0, check_old, NULL) ||\n+\t ref_transaction_commit(transaction, msg, NULL)) {\n+\t\tref_transaction_rollback(transaction);\n \t\treturn errno == ENOTDIR ? STORE_REF_ERROR_DF_CONFLICT :\n \t\t\t\t\t STORE_REF_ERROR_OTHER;\n+\t}\n+\tref_transaction_free(transaction);\n \treturn 0;\n }\n \n-- \n2.0.0.rc3.477.gffe78a2\n"},{"id":"241826","messageId":"1400195741-22996-25-git-send-email-sahlberg@google.com","threadId":"36672","inReplyTo":"1400195741-22996-1-git-send-email-sahlberg@google.com","subject":"[PATCH v9 24/44] fetch.c: use a single ref transaction for all ref updates","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-15T23:15:21Z","receivedAt":"2014-05-15T23:15:21Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Change store_updated_refs to use a single ref transaction for all refs that\nare updated during the fetch. This makes the fetch more atomic when update\nfailures occur.\n\nSince ref update failures will now no longer occur in the code path for\nupdating a single ref in s_update_ref, we no longer have as detailed error\nmessage logging the exact reference and the ref log action as in the old cod\nInstead since we fail the entire transaction we log a much more generic\nmessage. But since we commit the transaction using MSG_ON_ERR we will log\nan error containing the ref name if either locking of writing the ref would\nso the regression in the log message is minor.\n\nThis will also change the order in which errors are checked for and logged\nwhich may alter which error will be logged if there are multiple errors\noccuring during a fetch.\n\nFor example, assume we have a fetch for two refs that both would fail.\nWhere the first ref would fail with ENOTDIR due to a directory in the ref\npath not existing, and the second ref in the fetch would fail due to\nthe check in update_logical_ref():\n if (current_branch &&\n !strcmp(ref->name, current_branch->name) &&\n !(update_head_ok || is_bare_repository()) &&\n !is_null_sha1(ref->old_sha1)) {\n /*\n * If this is the head, and it's not okay to update\n * the head, and the old value of the head isn't empty...\n */\n\nIn the old code since we would update the refs one ref at a time we would\nfirst fail the ENOTDIR and then fail the second update of HEAD as well.\nBut since the first ref failed with ENOTDIR we would eventually fail the who\nfetch with STORE_REF_ERROR_DF_CONFLICT\n\nIn the new code, since we defer committing the transaction until all refs\nhave been processed, we would now detect that the second ref was bad and\nrollback the transaction before we would even try start writing the update t\ndisk and thus we would not return STORE_REF_ERROR_DF_CONFLICT for this case.\n\nI think this new behaviour is more correct, since if there was a problem\nwe would not even try to commit the transaction but need to highlight this\nchange in how/what errors are reported.\nThis change in what error is returned only occurs if there are multiple\nrefs that fail to update and only some, but not all, of them fail due to\nENOTDIR.\n\nSigned-off-by: Ronnie Sahlberg \n---\n builtin/fetch.c | 34 ++++++++++++++++------------------\n 1 file changed, 16 insertions(+), 18 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex c9b5954..5b0cc31 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -45,6 +45,7 @@ static struct transport *gsecondary;\n static const char *submodule_prefix = \"\";\n static const char *recurse_submodules_default;\n static int shown_url;\n+static struct ref_transaction *transaction;\n \n static int option_parse_recurse_submodules(const struct option *opt,\n \t\t\t\t const char *arg, int unset)\n@@ -373,27 +374,13 @@ static int s_update_ref(const char *action,\n \t\t\tstruct ref *ref,\n \t\t\tint check_old)\n {\n-\tchar msg[1024];\n-\tchar *rla = getenv(\"GIT_REFLOG_ACTION\");\n-\tstruct ref_transaction *transaction;\n-\n \tif (dry_run)\n \t\treturn 0;\n-\tif (!rla)\n-\t\trla = default_rla.buf;\n-\tsnprintf(msg, sizeof(msg), \"%s: %s\", rla, action);\n \n-\terrno = 0;\n-\ttransaction = ref_transaction_begin();\n-\tif (!transaction ||\n-\t ref_transaction_update(transaction, ref->name, ref->new_sha1,\n-\t\t\t\t ref->old_sha1, 0, check_old, NULL) ||\n-\t ref_transaction_commit(transaction, msg, NULL)) {\n-\t\tref_transaction_rollback(transaction);\n-\t\treturn errno == ENOTDIR ? STORE_REF_ERROR_DF_CONFLICT :\n-\t\t\t\t\t STORE_REF_ERROR_OTHER;\n-\t}\n-\tref_transaction_free(transaction);\n+\tif (ref_transaction_update(transaction, ref->name, ref->new_sha1,\n+\t\t\t\t ref->old_sha1, 0, check_old, NULL))\n+\t\treturn STORE_REF_ERROR_OTHER;\n+\n \treturn 0;\n }\n \n@@ -565,6 +552,13 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,\n \t\tgoto abort;\n \t}\n \n+\terrno = 0;\n+\ttransaction = ref_transaction_begin();\n+\tif (!transaction) {\n+\t\trc = error(_(\"cannot start ref transaction\\n\"));\n+\t\tgoto abort;\n+\t}\n+\n \t/*\n \t * We do a pass for each fetch_head_status type in their enum order, so\n \t * merged entries are written before not-for-merge. That lets readers\n@@ -676,6 +670,10 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,\n \t\t\t}\n \t\t}\n \t}\n+\tif (ref_transaction_commit(transaction, \"fetch_ref transaction\", NULL))\n+\t\trc |= errno == ENOTDIR ? STORE_REF_ERROR_DF_CONFLICT :\n+\t\t STORE_REF_ERROR_OTHER;\n+\tref_transaction_free(transaction);\n \n \tif (rc & STORE_REF_ERROR_DF_CONFLICT)\n \t\terror(_(\"some local refs could not be updated; try running\\n\"\n-- \n2.0.0.rc3.477.gffe78a2\n"},{"id":"241833","messageId":"1400195741-22996-26-git-send-email-sahlberg@google.com","threadId":"36672","inReplyTo":"1400195741-22996-1-git-send-email-sahlberg@google.com","subject":"[PATCH v9 25/44] receive-pack.c: use a reference transaction for updating the refs","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-15T23:15:22Z","receivedAt":"2014-05-15T23:15:22Z","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 to make the update atomic.\n\nSigned-off-by: Ronnie Sahlberg \n---\n builtin/receive-pack.c | 20 ++++++++++----------\n 1 file changed, 10 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex c323081..5534138 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -46,6 +46,8 @@ static void *head_name_to_free;\n static int sent_capabilities;\n static int shallow_update;\n static const char *alt_shallow_file;\n+static struct strbuf err = STRBUF_INIT;\n+static struct ref_transaction *transaction;\n \n static enum deny_action parse_deny_action(const char *var, const char *value)\n {\n@@ -475,7 +477,6 @@ static const char *update(struct command *cmd, struct shallow_info *si)\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@@ -580,15 +581,9 @@ static const char *update(struct command *cmd, struct shallow_info *si)\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}\n+\t\tif (ref_transaction_update(transaction, namespaced_name,\n+\t\t\t\t\t new_sha1, old_sha1, 0, 1, &err))\n+\t\t\treturn \"failed to update\";\n \t\treturn NULL; /* good */\n \t}\n }\n@@ -812,6 +807,7 @@ static void execute_commands(struct command *commands,\n \thead_name = head_name_to_free = resolve_refdup(\"HEAD\", sha1, 0, NULL);\n \n \tchecked_connectivity = 1;\n+\ttransaction = ref_transaction_begin();\n \tfor (cmd = commands; cmd; cmd = cmd->next) {\n \t\tif (cmd->error_string)\n \t\t\tcontinue;\n@@ -827,6 +823,10 @@ static void execute_commands(struct command *commands,\n \t\t\tchecked_connectivity = 0;\n \t\t}\n \t}\n+\tif (ref_transaction_commit(transaction, \"push\", &err))\n+\t\terror(\"%s\", err.buf);\n+\tref_transaction_free(transaction);\n+\tstrbuf_release(&err);\n \n \tif (shallow_update && !checked_connectivity)\n \t\terror(\"BUG: run 'git fsck' for safety.\\n\"\n-- \n2.0.0.rc3.477.gffe78a2\n"},{"id":"241828","messageId":"1400195741-22996-27-git-send-email-sahlberg@google.com","threadId":"36672","inReplyTo":"1400195741-22996-1-git-send-email-sahlberg@google.com","subject":"[PATCH v9 26/44] fast-import.c: use a ref transaction when dumping tags","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-15T23:15:23Z","receivedAt":"2014-05-15T23:15:23Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Signed-off-by: Ronnie Sahlberg \n---\n fast-import.c | 17 ++++++++++++-----\n 1 file changed, 12 insertions(+), 5 deletions(-)\n\ndiff --git a/fast-import.c b/fast-import.c\nindex 6b186ce..6ef5cec 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -1735,15 +1735,22 @@ 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 err = STRBUF_INIT;\n+\tstruct ref_transaction *transaction;\n \n+\ttransaction = ref_transaction_begin();\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\tsprintf(ref_name, \"refs/tags/%s\", t->name);\n+\n+\t\tif (ref_transaction_update(transaction, ref_name, t->sha1,\n+\t\t\t\t\t NULL, 0, 0, &err))\n+\t\t\tfailure |= error(\"%s\", err.buf);\n \t}\n+\tif (failure || ref_transaction_commit(transaction, msg, &err))\n+\t\tfailure |= error(\"%s\", err.buf);\n+\tref_transaction_free(transaction);\n+\tstrbuf_release(&err);\n }\n \n static void dump_marks_helper(FILE *f,\n-- \n2.0.0.rc3.477.gffe78a2\n"},{"id":"241817","messageId":"1400195741-22996-28-git-send-email-sahlberg@google.com","threadId":"36672","inReplyTo":"1400195741-22996-1-git-send-email-sahlberg@google.com","subject":"[PATCH v9 27/44] walker.c: use ref transaction for ref updates","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-15T23:15:24Z","receivedAt":"2014-05-15T23:15:24Z","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\nThis changes the locking slightly for walker_fetch. Previously the code would\nlock all refs before writing them but now we do not lock the refs until the\ncommit stage. There is thus a very short window where changes could be done\nlocally during the fetch which would be overwritten when the fetch completes\nand commits its transaction. But this window should be reasonably short.\nEven if this race does trigger, since both the old code and the new code\njust overwrites the refs to the new values without checking or comparing\nthem with the previous value, this is not too dissimilar to a similar scenario\nwhere you first do a ref change locally and then later do a fetch that\noverwrites the local change. With this in mind I do not see the change in\nlocking semantics to be critical.\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 collissions that the previous locking would\nprotect against and cause the fetch to fail for to be even more rare.\n\nSigned-off-by: Ronnie Sahlberg \n---\n walker.c | 51 ++++++++++++++++++++++++++-------------------------\n 1 file changed, 26 insertions(+), 25 deletions(-)\n\ndiff --git a/walker.c b/walker.c\nindex 1dd86b8..cbe5686 100644\n--- a/walker.c\n+++ b/walker.c\n@@ -251,24 +251,18 @@ 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+\tchar ref_name[PATH_MAX];\n+\tstruct strbuf err = STRBUF_INIT;\n+\tstruct ref_transaction *transaction;\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-\t\t}\n-\t}\n+\ttransaction = ref_transaction_begin();\n+\tif (!transaction)\n+\t\treturn -1;\n \n \tif (!walker->get_recover)\n \t\tfor_each_ref(mark_complete, NULL);\n@@ -276,14 +270,14 @@ int walker_fetch(struct walker *walker, int targets, char **target,\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 +288,26 @@ 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\tsprintf(ref_name, \"refs/%s\", write_ref[i]);\n+\t\tif (ref_transaction_update(transaction, ref_name,\n+\t\t\t\t\t &sha1[20 * i], NULL,\n+\t\t\t\t\t 0, 0, &err))\n+\t\t\tgoto rollback_and_fail;\n+\t}\n+\n+\tif (ref_transaction_commit(transaction, msg ? msg : \"fetch (unknown)\",\n+\t\t\t\t &err)) {\n+\t\terror(\"%s\", err.buf);\n+\t\tgoto rollback_and_fail;\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+\tfree(msg);\n+\tstrbuf_release(&err);\n+\tref_transaction_free(transaction);\n \n \treturn -1;\n }\n-- \n2.0.0.rc3.477.gffe78a2\n"},{"id":"241829","messageId":"1400195741-22996-29-git-send-email-sahlberg@google.com","threadId":"36672","inReplyTo":"1400195741-22996-1-git-send-email-sahlberg@google.com","subject":"[PATCH v9 28/44] refs.c: make write_ref_sha1 static","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-15T23:15:25Z","receivedAt":"2014-05-15T23:15:25Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"No external users call write_ref_sha1 any more so lets declare it static.\n\nSigned-off-by: Ronnie Sahlberg \n---\n refs.c | 4 +++-\n refs.h | 3 ---\n 2 files changed, 3 insertions(+), 4 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 016206b..6151c3c 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -251,6 +251,8 @@ struct ref_entry {\n };\n \n static void read_loose_refs(const char *dirname, struct ref_dir *dir);\n+static int write_ref_sha1(struct ref_lock *lock,\n+\t\t\t const unsigned char *sha1, const char *logmsg);\n \n static struct ref_dir *get_ref_dir(struct ref_entry *entry)\n {\n@@ -2833,7 +2835,7 @@ static int is_branch(const char *refname)\n \treturn !strcmp(refname, \"HEAD\") || starts_with(refname, \"refs/heads/\");\n }\n \n-int write_ref_sha1(struct ref_lock *lock,\n+static int write_ref_sha1(struct ref_lock *lock,\n \tconst unsigned char *sha1, const char *logmsg)\n {\n \tstatic char term = '\\n';\ndiff --git a/refs.h b/refs.h\nindex 608ca10..a45712c 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -150,9 +150,6 @@ extern int commit_ref(struct ref_lock *lock);\n /** Release any lock taken but not written. **/\n extern void unlock_ref(struct ref_lock *lock);\n \n-/** Writes sha1 into the ref specified by the lock. **/\n-extern int write_ref_sha1(struct ref_lock *lock, const unsigned char *sha1, const char *msg);\n-\n /** Setup reflog before using. **/\n int log_ref_setup(const char *refname, char *logfile, int bufsize);\n \n-- \n2.0.0.rc3.477.gffe78a2\n"},{"id":"241834","messageId":"1400195741-22996-30-git-send-email-sahlberg@google.com","threadId":"36672","inReplyTo":"1400195741-22996-1-git-send-email-sahlberg@google.com","subject":"[PATCH v9 29/44] refs.c: make lock_ref_sha1 static","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-15T23:15:26Z","receivedAt":"2014-05-15T23:15:26Z","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\nSigned-off-by: Ronnie Sahlberg \n---\n refs.c | 2 +-\n refs.h | 3 ---\n 2 files changed, 1 insertion(+), 4 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 6151c3c..733dfb3 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2126,7 +2126,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 a45712c..832d81b 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -132,9 +132,6 @@ extern int ref_exists(const char *);\n */\n extern int peel_ref(const char *refname, unsigned char *sha1);\n \n-/** Locks a \"refs/\" ref returning the lock on success and NULL on failure. **/\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 extern struct ref_lock *lock_any_ref_for_update(const char *refname,\n-- \n2.0.0.rc3.477.gffe78a2\n"},{"id":"241830","messageId":"1400195741-22996-31-git-send-email-sahlberg@google.com","threadId":"36672","inReplyTo":"1400195741-22996-1-git-send-email-sahlberg@google.com","subject":"[PATCH v9 30/44] refs.c: add transaction.status and track OPEN/CLOSED/ERROR","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-15T23:15:27Z","receivedAt":"2014-05-15T23:15:27Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Track the status of a transaction in a new status field. Check the field for\nsanity, i.e. that status must be OPEN when _commit/_create/_delete or\n_update is called or else die(BUG:...)\n\nSigned-off-by: Ronnie Sahlberg \n---\n refs.c | 31 ++++++++++++++++++++++++++++++-\n 1 file changed, 30 insertions(+), 1 deletion(-)\n\ndiff --git a/refs.c b/refs.c\nindex 733dfb3..3fb5cb8 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3336,6 +3336,12 @@ struct ref_update {\n \tconst char refname[FLEX_ARRAY];\n };\n \n+enum ref_transaction_status {\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@@ -3345,6 +3351,7 @@ struct ref_transaction {\n \tstruct ref_update **updates;\n \tsize_t alloc;\n \tsize_t nr;\n+\tenum ref_transaction_status status;\n };\n \n struct ref_transaction *ref_transaction_begin(void)\n@@ -3368,6 +3375,11 @@ void ref_transaction_free(struct ref_transaction *transaction)\n \n void ref_transaction_rollback(struct ref_transaction *transaction)\n {\n+\tif (!transaction)\n+\t\treturn;\n+\n+\ttransaction->status = REF_TRANSACTION_ERROR;\n+\n \tref_transaction_free(transaction);\n }\n \n@@ -3395,6 +3407,9 @@ int ref_transaction_update(struct ref_transaction *transaction,\n \tif (have_old && !old_sha1)\n \t\tdie(\"BUG: have_old is true but old_sha1 is NULL\");\n \n+\tif (transaction->status != REF_TRANSACTION_OPEN)\n+\t\tdie(\"BUG: update on transaction that is not open\");\n+\n \tupdate = add_update(transaction, refname);\n \thashcpy(update->new_sha1, new_sha1);\n \tupdate->flags = flags;\n@@ -3415,6 +3430,9 @@ int ref_transaction_create(struct ref_transaction *transaction,\n \tif (!new_sha1 || is_null_sha1(new_sha1))\n \t\tdie(\"BUG: create ref with null new_sha1\");\n \n+\tif (transaction->status != REF_TRANSACTION_OPEN)\n+\t\tdie(\"BUG: create on transaction that is not open\");\n+\n \tupdate = add_update(transaction, refname);\n \n \thashcpy(update->new_sha1, new_sha1);\n@@ -3435,6 +3453,9 @@ int ref_transaction_delete(struct ref_transaction *transaction,\n \tif (have_old && !old_sha1)\n \t\tdie(\"BUG: have_old is true but old_sha1 is NULL\");\n \n+\tif (transaction->status != REF_TRANSACTION_OPEN)\n+\t\tdie(\"BUG: delete on transaction that is not open\");\n+\n \tupdate = add_update(transaction, refname);\n \tupdate->flags = flags;\n \tupdate->have_old = have_old;\n@@ -3505,8 +3526,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->status != REF_TRANSACTION_OPEN)\n+\t\tdie(\"BUG: commit on transaction that is not open\");\n+\n+\tif (!n) {\n+\t\ttransaction->status = REF_TRANSACTION_CLOSED;\n \t\treturn 0;\n+\t}\n \n \t/* Allocate work space */\n \tdelnames = xmalloc(sizeof(*delnames) * n);\n@@ -3569,6 +3595,9 @@ int ref_transaction_commit(struct ref_transaction *transaction,\n \tclear_loose_ref_cache(&ref_cache);\n \n cleanup:\n+\ttransaction->status = ret ? REF_TRANSACTION_ERROR\n+\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.0.rc3.477.gffe78a2\n"},{"id":"241827","messageId":"1400195741-22996-32-git-send-email-sahlberg@google.com","threadId":"36672","inReplyTo":"1400195741-22996-1-git-send-email-sahlberg@google.com","subject":"[PATCH v9 31/44] refs.c: remove the update_ref_lock function","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-15T23:15:28Z","receivedAt":"2014-05-15T23:15:28Z","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\nSigned-off-by: Ronnie Sahlberg \n---\n refs.c | 30 ++++++------------------------\n 1 file changed, 6 insertions(+), 24 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 3fb5cb8..3b0d574 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3283,24 +3283,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@@ -3547,12 +3529,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.0.rc3.477.gffe78a2\n"},{"id":"241832","messageId":"1400195741-22996-33-git-send-email-sahlberg@google.com","threadId":"36672","inReplyTo":"1400195741-22996-1-git-send-email-sahlberg@google.com","subject":"[PATCH v9 32/44] refs.c: remove the update_ref_write function","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-15T23:15:29Z","receivedAt":"2014-05-15T23:15:29Z","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.\n\nSigned-off-by: Ronnie Sahlberg \n---\n refs.c | 35 +++++++++--------------------------\n 1 file changed, 9 insertions(+), 26 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 3b0d574..1857e61 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3283,25 +3283,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@@ -3549,14 +3530,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.0.rc3.477.gffe78a2\n"},{"id":"241818","messageId":"1400195741-22996-34-git-send-email-sahlberg@google.com","threadId":"36672","inReplyTo":"1400195741-22996-1-git-send-email-sahlberg@google.com","subject":"[PATCH v9 33/44] refs.c: remove lock_ref_sha1","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-15T23:15:30Z","receivedAt":"2014-05-15T23:15:30Z","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\nSigned-off-by: Ronnie Sahlberg \n---\n refs.c | 15 +++++----------\n 1 file changed, 5 insertions(+), 10 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 1857e61..99b110f 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2126,15 +2126,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@@ -2339,8 +2330,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.0.rc3.477.gffe78a2\n"},{"id":"241813","messageId":"1400195741-22996-35-git-send-email-sahlberg@google.com","threadId":"36672","inReplyTo":"1400195741-22996-1-git-send-email-sahlberg@google.com","subject":"[PATCH v9 34/44] refs.c: make prune_ref use a transaction to delete the ref","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-15T23:15:31Z","receivedAt":"2014-05-15T23:15:31Z","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\nSigned-off-by: Ronnie Sahlberg \n---\n refs.c | 27 ++++++++++++++++++++-------\n 1 file changed, 20 insertions(+), 7 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 99b110f..f88263e 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -29,6 +29,11 @@ static inline int bad_ref_char(int ch)\n \treturn 0;\n }\n \n+/** Used as a flag to ref_transaction_delete when a loose ref is beeing\n+ * pruned.\n+ */\n+#define REF_ISPRUNING\t0x0100\n+\n /*\n * Try to read one refname component from the front of refname. Return\n * the length of the component found, or -1 if the component is not\n@@ -2330,17 +2335,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();\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_rollback(transaction);\n+\t\twarning(\"prune_ref: %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@@ -3543,9 +3555,10 @@ 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\t\t\t\terr);\n+\t\t\tif (!(update->flags & REF_ISPRUNING))\n+\t\t\t\tdelnames[delnum++] = update->lock->ref_name;\n \t\t}\n \t}\n \n-- \n2.0.0.rc3.477.gffe78a2\n"},{"id":"241835","messageId":"1400195741-22996-36-git-send-email-sahlberg@google.com","threadId":"36672","inReplyTo":"1400195741-22996-1-git-send-email-sahlberg@google.com","subject":"[PATCH v9 35/44] refs.c: make delete_ref use a transaction","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-15T23:15:32Z","receivedAt":"2014-05-15T23:15:32Z","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\nSigned-off-by: Ronnie Sahlberg \n---\n refs.c | 31 ++++++++++---------------------\n 1 file changed, 10 insertions(+), 21 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex f88263e..7efb6bb 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2495,11 +2495,6 @@ static 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 add_err_if_unremovable(const char *op, const char *file,\n \t\t\t\t struct strbuf *e, int rc)\n {\n@@ -2543,24 +2538,18 @@ static int delete_ref_loose(struct ref_lock *lock, int flag, struct strbuf *err)\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 \n-\tlock = lock_ref_sha1_basic(refname, sha1, delopt, &flag);\n-\tif (!lock)\n+\ttransaction = ref_transaction_begin();\n+\tif (!transaction ||\n+\t ref_transaction_delete(transaction, refname, sha1, delopt,\n+\t\t\t\t sha1 && !is_null_sha1(sha1), NULL) ||\n+\t ref_transaction_commit(transaction, NULL, NULL)) {\n+\t\tref_transaction_rollback(transaction);\n \t\treturn 1;\n-\tret |= delete_ref_loose(lock, flag, NULL);\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.0.rc3.477.gffe78a2\n"},{"id":"241814","messageId":"1400195741-22996-37-git-send-email-sahlberg@google.com","threadId":"36672","inReplyTo":"1400195741-22996-1-git-send-email-sahlberg@google.com","subject":"[PATCH v9 36/44] refs.c: pass the ref log message to _create/delete/update instead of _commit","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-15T23:15:33Z","receivedAt":"2014-05-15T23:15:33Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Change the reference transactions so that we pass the reflog message\nthrough to the create/delete/update function instead of the commit message.\nThis allows for individual messages for each change in a multi ref\ntransaction.\n\nSigned-off-by: Ronnie Sahlberg \n---\n branch.c | 4 ++--\n builtin/commit.c | 4 ++--\n builtin/fetch.c | 10 ++++++++--\n builtin/receive-pack.c | 5 +++--\n builtin/replace.c | 4 ++--\n builtin/tag.c | 4 ++--\n builtin/update-ref.c | 13 +++++++------\n fast-import.c | 8 ++++----\n refs.c | 31 +++++++++++++++++++++----------\n refs.h | 5 ++++-\n sequencer.c | 4 ++--\n walker.c | 6 +++---\n 12 files changed, 60 insertions(+), 38 deletions(-)\n\ndiff --git a/branch.c b/branch.c\nindex f78a28b..6dfdc2e 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -301,8 +301,8 @@ void create_branch(const char *head,\n \t\ttransaction = ref_transaction_begin();\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\t\t\t null_sha1, 0, !forcing, msg, &err) ||\n+\t\t ref_transaction_commit(transaction, &err))\n \t\t\tdie(\"%s\", err.buf);\n \t\tref_transaction_free(transaction);\n \t}\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 6b888f2..b361a13 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1721,8 +1721,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\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\t\t\t 0, !!current_head, sb.buf, &err) ||\n+\t ref_transaction_commit(transaction, &err)) {\n \t\trollback_index_files();\n \t\tdie(\"%s\", err.buf);\n \t}\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex 5b0cc31..4603cb6 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -374,11 +374,17 @@ static int s_update_ref(const char *action,\n \t\t\tstruct ref *ref,\n \t\t\tint check_old)\n {\n+\tchar msg[1024];\n+\tchar *rla = getenv(\"GIT_REFLOG_ACTION\");\n+\n \tif (dry_run)\n \t\treturn 0;\n+\tif (!rla)\n+\t\trla = default_rla.buf;\n+\tsnprintf(msg, sizeof(msg), \"%s: %s\", rla, action);\n \n \tif (ref_transaction_update(transaction, ref->name, ref->new_sha1,\n-\t\t\t\t ref->old_sha1, 0, check_old, NULL))\n+\t\t\t\t ref->old_sha1, 0, check_old, msg, NULL))\n \t\treturn STORE_REF_ERROR_OTHER;\n \n \treturn 0;\n@@ -670,7 +676,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,\n \t\t\t}\n \t\t}\n \t}\n-\tif (ref_transaction_commit(transaction, \"fetch_ref transaction\", NULL))\n+\tif (ref_transaction_commit(transaction, NULL))\n \t\trc |= errno == ENOTDIR ? STORE_REF_ERROR_DF_CONFLICT :\n \t\t STORE_REF_ERROR_OTHER;\n \tref_transaction_free(transaction);\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex 5534138..324a220 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -582,7 +582,8 @@ static const char *update(struct command *cmd, struct shallow_info *si)\n \t\t\treturn \"shallow error\";\n \n \t\tif (ref_transaction_update(transaction, namespaced_name,\n-\t\t\t\t\t new_sha1, old_sha1, 0, 1, &err))\n+\t\t\t\t\t new_sha1, old_sha1, 0, 1, \"push\",\n+\t\t\t\t\t &err))\n \t\t\treturn \"failed to update\";\n \t\treturn NULL; /* good */\n \t}\n@@ -823,7 +824,7 @@ static void execute_commands(struct command *commands,\n \t\t\tchecked_connectivity = 0;\n \t\t}\n \t}\n-\tif (ref_transaction_commit(transaction, \"push\", &err))\n+\tif (ref_transaction_commit(transaction, &err))\n \t\terror(\"%s\", err.buf);\n \tref_transaction_free(transaction);\n \tstrbuf_release(&err);\ndiff --git a/builtin/replace.c b/builtin/replace.c\nindex af7f72d..4fa74c1 100644\n--- a/builtin/replace.c\n+++ b/builtin/replace.c\n@@ -160,8 +160,8 @@ static int replace_object_sha1(const char *object_ref,\n \ttransaction = ref_transaction_begin();\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\t\t\t 0, !is_null_sha1(prev), NULL, &err) ||\n+\t ref_transaction_commit(transaction, &err))\n \t\tdie(\"%s\", err.buf);\n \n \tref_transaction_free(transaction);\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 30b471c..8d39068 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -705,8 +705,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \ttransaction = ref_transaction_begin();\n \tif (!transaction ||\n \t ref_transaction_update(transaction, ref.buf, object, prev,\n-\t\t\t\t 0, !is_null_sha1(prev), &err) ||\n-\t ref_transaction_commit(transaction, NULL, &err))\n+\t\t\t\t 0, !is_null_sha1(prev), NULL, &err) ||\n+\t ref_transaction_commit(transaction, &err))\n \t\tdie(\"%s\", err.buf);\n \tref_transaction_free(transaction);\n \tif (force && !is_null_sha1(prev) && hashcmp(prev, object))\ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex 4c2145e..8f13d54 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -16,6 +16,7 @@ static struct ref_transaction *transaction;\n \n static char line_termination = '\\n';\n static int update_flags;\n+static const char *msg;\n static struct strbuf err = STRBUF_INIT;\n \n /*\n@@ -199,7 +200,7 @@ static const char *parse_cmd_update(struct strbuf *input, const char *next)\n \t\tdie(\"update %s: extra input: %s\", refname, next);\n \n \tif (ref_transaction_update(transaction, refname, new_sha1, old_sha1,\n-\t\t\t\t update_flags, have_old, &err))\n+\t\t\t\t update_flags, have_old, msg, &err))\n \t\tdie(\"update %s: failed: %s\", refname, err.buf);\n \n \tupdate_flags = 0;\n@@ -227,7 +228,7 @@ static const char *parse_cmd_create(struct strbuf *input, const char *next)\n \t\tdie(\"create %s: extra input: %s\", refname, next);\n \n \tif (ref_transaction_create(transaction, refname, new_sha1,\n-\t\t\t\t update_flags, NULL))\n+\t\t\t\t update_flags, msg, &err))\n \t\tdie(\"cannot create ref '%s'\", refname);\n \n \tupdate_flags = 0;\n@@ -259,7 +260,7 @@ static const char *parse_cmd_delete(struct strbuf *input, const char *next)\n \t\tdie(\"delete %s: extra input: %s\", refname, next);\n \n \tif (ref_transaction_delete(transaction, refname, old_sha1,\n-\t\t\t\t update_flags, have_old, NULL))\n+\t\t\t\t update_flags, have_old, msg, &err))\n \t\tdie(\"failed transaction delete for %s\", refname);\n \n \tupdate_flags = 0;\n@@ -292,7 +293,7 @@ static const char *parse_cmd_verify(struct strbuf *input, const char *next)\n \t\tdie(\"verify %s: extra input: %s\", refname, next);\n \n \tif (ref_transaction_update(transaction, refname, new_sha1, old_sha1,\n-\t\t\t\t update_flags, have_old, &err))\n+\t\t\t\t update_flags, have_old, msg, &err))\n \t\tdie(\"verify %s: %s\", refname, err.buf);\n \n \tupdate_flags = 0;\n@@ -345,7 +346,7 @@ static void update_refs_stdin(void)\n \n int cmd_update_ref(int argc, const char **argv, const char *prefix)\n {\n-\tconst char *refname, *oldval, *msg = NULL;\n+\tconst char *refname, *oldval;\n \tunsigned char sha1[20], oldsha1[20];\n \tint delete = 0, no_deref = 0, read_stdin = 0, end_null = 0, flags = 0;\n \tstruct option options[] = {\n@@ -371,7 +372,7 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)\n \t\tif (end_null)\n \t\t\tline_termination = '\\0';\n \t\tupdate_refs_stdin();\n-\t\tif (ref_transaction_commit(transaction, msg, &err))\n+\t\tif (ref_transaction_commit(transaction, &err))\n \t\t\tdie(\"%s\", err.buf);\n \t\tref_transaction_free(transaction);\n \t\treturn 0;\ndiff --git a/fast-import.c b/fast-import.c\nindex 6ef5cec..db511c3 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -1709,8 +1709,8 @@ static int update_branch(struct branch *b)\n \ttransaction = ref_transaction_begin();\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\t\t\t 0, 1, msg, &err) ||\n+\t ref_transaction_commit(transaction, &err)) {\n \t\tref_transaction_rollback(transaction);\n \t\terror(\"%s\", err.buf);\n \t\tstrbuf_release(&err);\n@@ -1744,10 +1744,10 @@ static void dump_tags(void)\n \t\tsprintf(ref_name, \"refs/tags/%s\", t->name);\n \n \t\tif (ref_transaction_update(transaction, ref_name, t->sha1,\n-\t\t\t\t\t NULL, 0, 0, &err))\n+\t\t\t\t\t NULL, 0, 0, msg, &err))\n \t\t\tfailure |= error(\"%s\", err.buf);\n \t}\n-\tif (failure || ref_transaction_commit(transaction, msg, &err))\n+\tif (failure || ref_transaction_commit(transaction, &err))\n \t\tfailure |= error(\"%s\", err.buf);\n \tref_transaction_free(transaction);\n \tstrbuf_release(&err);\ndiff --git a/refs.c b/refs.c\nindex 7efb6bb..5369a39 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2344,8 +2344,8 @@ static void prune_ref(struct ref_to_prune *r)\n \ttransaction = ref_transaction_begin();\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\t\t\t REF_ISPRUNING, 1, NULL, &err) ||\n+\t ref_transaction_commit(transaction, &err)) {\n \t\tref_transaction_rollback(transaction);\n \t\twarning(\"prune_ref: %s\", err.buf);\n \t\tstrbuf_release(&err);\n@@ -2543,8 +2543,8 @@ int delete_ref(const char *refname, const unsigned char *sha1, int delopt)\n \ttransaction = ref_transaction_begin();\n \tif (!transaction ||\n \t ref_transaction_delete(transaction, refname, sha1, delopt,\n-\t\t\t\t sha1 && !is_null_sha1(sha1), NULL) ||\n-\t ref_transaction_commit(transaction, NULL, NULL)) {\n+\t\t\t\t sha1 && !is_null_sha1(sha1), NULL, NULL) ||\n+\t ref_transaction_commit(transaction, NULL)) {\n \t\tref_transaction_rollback(transaction);\n \t\treturn 1;\n \t}\n@@ -3292,6 +3292,7 @@ struct ref_update {\n \tint have_old; /* 1 if old_sha1 is valid, 0 otherwise */\n \tstruct ref_lock *lock;\n \tint type;\n+\tconst char *msg;\n \tconst char refname[FLEX_ARRAY];\n };\n \n@@ -3325,9 +3326,10 @@ void ref_transaction_free(struct ref_transaction *transaction)\n \tif (!transaction)\n \t\treturn;\n \n-\tfor (i = 0; i < transaction->nr; i++)\n+\tfor (i = 0; i < transaction->nr; i++) {\n+\t free((char *)transaction->updates[i]->msg);\n \t\tfree(transaction->updates[i]);\n-\n+\t}\n \tfree(transaction->updates);\n \tfree(transaction);\n }\n@@ -3359,6 +3361,7 @@ int ref_transaction_update(struct ref_transaction *transaction,\n \t\t\t const unsigned char *new_sha1,\n \t\t\t const unsigned char *old_sha1,\n \t\t\t int flags, int have_old,\n+\t\t\t const char *msg,\n \t\t\t struct strbuf *err)\n {\n \tstruct ref_update *update;\n@@ -3375,6 +3378,8 @@ int ref_transaction_update(struct ref_transaction *transaction,\n \tupdate->have_old = have_old;\n \tif (have_old)\n \t\thashcpy(update->old_sha1, old_sha1);\n+\tif (msg)\n+\t\tupdate->msg = xstrdup(msg);\n \treturn 0;\n }\n \n@@ -3382,6 +3387,7 @@ 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 const char *msg,\n \t\t\t struct strbuf *err)\n {\n \tstruct ref_update *update;\n@@ -3398,6 +3404,8 @@ int ref_transaction_create(struct ref_transaction *transaction,\n \thashclr(update->old_sha1);\n \tupdate->flags = flags;\n \tupdate->have_old = 1;\n+\tif (msg)\n+\t\tupdate->msg = xstrdup(msg);\n \treturn 0;\n }\n \n@@ -3405,6 +3413,7 @@ 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 const char *msg,\n \t\t\t struct strbuf *err)\n {\n \tstruct ref_update *update;\n@@ -3422,6 +3431,8 @@ int ref_transaction_delete(struct ref_transaction *transaction,\n \t\tassert(!is_null_sha1(old_sha1));\n \t\thashcpy(update->old_sha1, old_sha1);\n \t}\n+\tif (msg)\n+\t\tupdate->msg = xstrdup(msg);\n \treturn 0;\n }\n \n@@ -3435,8 +3446,8 @@ int update_ref(const char *action, const char *refname,\n \tt = ref_transaction_begin();\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\t\t\t !!oldval, action, NULL) ||\n+\t ref_transaction_commit(t, &err)) {\n \t\tconst char *str = \"update_ref failed for ref '%s': %s\";\n \n \t\tref_transaction_rollback(t);\n@@ -3478,7 +3489,7 @@ static int ref_update_reject_duplicates(struct ref_update **updates, int n,\n }\n \n int ref_transaction_commit(struct ref_transaction *transaction,\n-\t\t\t const char *msg, struct strbuf *err)\n+\t\t\t struct strbuf *err)\n {\n \tint ret = 0, delnum = 0, i;\n \tconst char **delnames;\n@@ -3527,7 +3538,7 @@ int ref_transaction_commit(struct ref_transaction *transaction,\n \n \t\tif (!is_null_sha1(update->new_sha1)) {\n \t\t\tret = write_ref_sha1(update->lock, update->new_sha1,\n-\t\t\t\t\t msg);\n+\t\t\t\t\t update->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'.\";\ndiff --git a/refs.h b/refs.h\nindex 832d81b..a59fca8 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -245,6 +245,7 @@ int ref_transaction_update(struct ref_transaction *transaction,\n \t\t\t const unsigned char *new_sha1,\n \t\t\t const unsigned char *old_sha1,\n \t\t\t int flags, int have_old,\n+\t\t\t const char *msg,\n \t\t\t struct strbuf *err);\n \n /*\n@@ -260,6 +261,7 @@ 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 const char *msg,\n \t\t\t struct strbuf *err);\n \n /*\n@@ -274,6 +276,7 @@ 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 const char *msg,\n \t\t\t struct strbuf *err);\n \n /*\n@@ -283,7 +286,7 @@ int ref_transaction_delete(struct ref_transaction *transaction,\n * why 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+\t\t\t struct strbuf *err);\n \n /*\n * Free an existing transaction.\ndiff --git a/sequencer.c b/sequencer.c\nindex f6f1536..edad040 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -285,8 +285,8 @@ static int fast_forward_to(const unsigned char *to, const unsigned char *from,\n \ttransaction = ref_transaction_begin();\n \tif (!transaction ||\n \t ref_transaction_update(transaction, \"HEAD\", to, from,\n-\t\t\t\t 0, !unborn, &err) ||\n-\t ref_transaction_commit(transaction, sb.buf, &err)) {\n+\t\t\t\t 0, !unborn, sb.buf, &err) ||\n+\t ref_transaction_commit(transaction, &err)) {\n \t\tref_transaction_rollback(transaction);\n \t\terror(\"%s\", err.buf);\n \t\tstrbuf_release(&sb);\ndiff --git a/walker.c b/walker.c\nindex cbe5686..0403feb 100644\n--- a/walker.c\n+++ b/walker.c\n@@ -291,12 +291,12 @@ int walker_fetch(struct walker *walker, int targets, char **target,\n \t\tsprintf(ref_name, \"refs/%s\", write_ref[i]);\n \t\tif (ref_transaction_update(transaction, ref_name,\n \t\t\t\t\t &sha1[20 * i], NULL,\n-\t\t\t\t\t 0, 0, &err))\n+\t\t\t\t\t 0, 0,\n+\t\t\t\t\t msg ? msg : \"fetch (unknown)\", &err))\n \t\t\tgoto rollback_and_fail;\n \t}\n \n-\tif (ref_transaction_commit(transaction, msg ? msg : \"fetch (unknown)\",\n-\t\t\t\t &err)) {\n+\tif (ref_transaction_commit(transaction, &err)) {\n \t\terror(\"%s\", err.buf);\n \t\tgoto rollback_and_fail;\n \t}\n-- \n2.0.0.rc3.477.gffe78a2\n"},{"id":"241820","messageId":"1400195741-22996-38-git-send-email-sahlberg@google.com","threadId":"36672","inReplyTo":"1400195741-22996-1-git-send-email-sahlberg@google.com","subject":"[PATCH v9 37/44] refs.c: pass NULL as *flags to read_ref_full","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-15T23:15:34Z","receivedAt":"2014-05-15T23:15:34Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"We call read_ref_full with a pointer to flags from rename_ref but since\nwe never actually use the returned flags we can just pass NULL here instead.\n\nSigned-off-by: Ronnie Sahlberg \n---\n refs.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/refs.c b/refs.c\nindex 5369a39..e25345c 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2640,7 +2640,7 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms\n \t\tgoto rollback;\n \t}\n \n-\tif (!read_ref_full(newrefname, sha1, 1, &flag) &&\n+\tif (!read_ref_full(newrefname, sha1, 1, NULL) &&\n \t delete_ref(newrefname, sha1, REF_NODEREF)) {\n \t\tif (errno==EISDIR) {\n \t\t\tif (remove_empty_directories(git_path(\"%s\", newrefname))) {\n-- \n2.0.0.rc3.477.gffe78a2\n"},{"id":"241819","messageId":"1400195741-22996-39-git-send-email-sahlberg@google.com","threadId":"36672","inReplyTo":"1400195741-22996-1-git-send-email-sahlberg@google.com","subject":"[PATCH v9 38/44] refs.c: pack all refs before we start to rename a ref","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-15T23:15:35Z","receivedAt":"2014-05-15T23:15:35Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"This means that most loose refs will no longer be present after the rename\nwhich triggered a test failure since it assumes the file for an unrelated\nref would still be present after the rename.\n\nSigned-off-by: Ronnie Sahlberg \n---\n refs.c | 3 +++\n t/t3200-branch.sh | 2 +-\n 2 files changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/refs.c b/refs.c\nindex e25345c..fb36cba 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2635,6 +2635,9 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms\n \t\treturn error(\"unable to move logfile logs/%s to \"TMP_RENAMED_LOG\": %s\",\n \t\t\toldrefname, strerror(errno));\n \n+\tif (pack_refs(PACK_REFS_ALL | PACK_REFS_PRUNE))\n+\t\treturn error(\"unable to pack refs\");\n+\n \tif (delete_ref(oldrefname, orig_sha1, REF_NODEREF)) {\n \t\terror(\"unable to delete old %s\", oldrefname);\n \t\tgoto rollback;\ndiff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\nindex ac31b71..fafd38a 100755\n--- a/t/t3200-branch.sh\n+++ b/t/t3200-branch.sh\n@@ -289,7 +289,7 @@ test_expect_success 'renaming a symref is not allowed' '\n \tgit symbolic-ref refs/heads/master2 refs/heads/master &&\n \ttest_must_fail git branch -m master2 master3 &&\n \tgit symbolic-ref refs/heads/master2 &&\n-\ttest_path_is_file .git/refs/heads/master &&\n+\ttest_path_is_missing .git/refs/heads/master &&\n \ttest_path_is_missing .git/refs/heads/master3\n '\n \n-- \n2.0.0.rc3.477.gffe78a2\n"},{"id":"241815","messageId":"1400195741-22996-40-git-send-email-sahlberg@google.com","threadId":"36672","inReplyTo":"1400195741-22996-1-git-send-email-sahlberg@google.com","subject":"[PATCH v9 39/44] refs.c: move the check for valid refname to lock_ref_sha1_basic","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-15T23:15:36Z","receivedAt":"2014-05-15T23:15:36Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Move the check for check_refname_format from lock_any_ref_for_update\nto lock_ref_sha1_basic. At some later stage we will get rid of\nlock_any_ref_for_update completely.\n\nThis leaves lock_any_ref_for_updates as a no-op wrapper which could be removed.\nBut this wrapper is also called from an external caller and we will soon\nmake changes to the signature to lock_ref_sha1_basic that we do not want to\nexpose to that caller.\n\nSigned-off-by: Ronnie Sahlberg \n---\n refs.c | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex fb36cba..baa620f 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2044,6 +2044,9 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,\n \tint missing = 0;\n \tint attempts_remaining = 3;\n \n+\tif (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL))\n+\t\treturn NULL;\n+\n \tlock = xcalloc(1, sizeof(struct ref_lock));\n \tlock->lock_fd = -1;\n \n@@ -2135,8 +2138,6 @@ 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 {\n-\tif (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL))\n-\t\treturn NULL;\n \treturn lock_ref_sha1_basic(refname, old_sha1, flags, type_p);\n }\n \n-- \n2.0.0.rc3.477.gffe78a2\n"},{"id":"241816","messageId":"1400195741-22996-41-git-send-email-sahlberg@google.com","threadId":"36672","inReplyTo":"1400195741-22996-1-git-send-email-sahlberg@google.com","subject":"[PATCH v9 40/44] refs.c: call lock_ref_sha1_basic directly from commit","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-15T23:15:37Z","receivedAt":"2014-05-15T23:15:37Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Skip using the lock_any_ref_for_update wrapper and call lock_ref_sha1_basic\ndirectly from the commit function.\n\nSigned-off-by: Ronnie Sahlberg \n---\n refs.c | 12 ++++++------\n 1 file changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex baa620f..87113af 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3521,12 +3521,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 = 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\tupdate->lock = lock_ref_sha1_basic(update->refname,\n+\t\t\t\t\t\t (update->have_old ?\n+\t\t\t\t\t\t update->old_sha1 :\n+\t\t\t\t\t\t NULL),\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.0.rc3.477.gffe78a2\n"},{"id":"241825","messageId":"1400195741-22996-42-git-send-email-sahlberg@google.com","threadId":"36672","inReplyTo":"1400195741-22996-1-git-send-email-sahlberg@google.com","subject":"[PATCH v9 41/44] refs.c: add a new flag for transaction delete for refs we know are packed only","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-15T23:15:38Z","receivedAt":"2014-05-15T23:15:38Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Add a new flag REF_ISPACKONLY that we can use in ref_transaction_delete.\nThis flag indicates that the ref does not exist as a loose ref andf only as\na packed ref. If this is the case we then change the commit code so that\nwe skip taking out a lock file and we skip calling delete_ref_loose.\nCheck for this flag and die(BUG:...) if used with _update or _create.\n\nAt the start of the transaction, before we even start locking any refs,\nwe add all such REF_ISPACKONLY refs to delnames so that we have a list of\nall pack only refs that we will be deleting during this transaction.\n\nSigned-off-by: Ronnie Sahlberg \n---\n refs.c | 23 +++++++++++++++++++++++\n 1 file changed, 23 insertions(+)\n\ndiff --git a/refs.c b/refs.c\nindex 87113af..6299b3d 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -33,6 +33,10 @@ static inline int bad_ref_char(int ch)\n * pruned.\n */\n #define REF_ISPRUNING\t0x0100\n+/** Deletion of a ref that only exists as a packed ref in which case we do not\n+ * need to lock the loose ref during the transaction.\n+ */\n+#define REF_ISPACKONLY\t0x0200\n \n /*\n * Try to read one refname component from the front of refname. Return\n@@ -3376,6 +3380,9 @@ int ref_transaction_update(struct ref_transaction *transaction,\n \tif (transaction->status != REF_TRANSACTION_OPEN)\n \t\tdie(\"BUG: update on transaction that is not open\");\n \n+\tif (flags & REF_ISPACKONLY)\n+\t\tdie(\"BUG: REF_ISPACKONLY can not be used with updates\");\n+\n \tupdate = add_update(transaction, refname);\n \thashcpy(update->new_sha1, new_sha1);\n \tupdate->flags = flags;\n@@ -3402,6 +3409,9 @@ int ref_transaction_create(struct ref_transaction *transaction,\n \tif (transaction->status != REF_TRANSACTION_OPEN)\n \t\tdie(\"BUG: create on transaction that is not open\");\n \n+\tif (flags & REF_ISPACKONLY)\n+\t\tdie(\"BUG: REF_ISPACKONLY can not be used with creates\");\n+\n \tupdate = add_update(transaction, refname);\n \n \thashcpy(update->new_sha1, new_sha1);\n@@ -3517,10 +3527,20 @@ int ref_transaction_commit(struct ref_transaction *transaction,\n \tif (ret)\n \t\tgoto cleanup;\n \n+\tfor (i = 0; i < n; i++) {\n+\t\tstruct ref_update *update = updates[i];\n+\n+\t\tif (update->flags & REF_ISPACKONLY)\n+\t\t\tdelnames[delnum++] = update->refname;\n+\t}\n+\n \t/* Acquire all locks while verifying old values */\n \tfor (i = 0; i < n; i++) {\n \t\tstruct ref_update *update = updates[i];\n \n+\t\tif (update->flags & REF_ISPACKONLY)\n+\t\t\tcontinue;\n+\n \t\tupdate->lock = lock_ref_sha1_basic(update->refname,\n \t\t\t\t\t\t (update->have_old ?\n \t\t\t\t\t\t update->old_sha1 :\n@@ -3558,6 +3578,9 @@ 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\tif (update->flags & REF_ISPACKONLY)\n+\t\t\tcontinue;\n+\n \t\tif (update->lock) {\n \t\t\tret |= delete_ref_loose(update->lock, update->type,\n \t\t\t\t\t\terr);\n-- \n2.0.0.rc3.477.gffe78a2\n"},{"id":"241821","messageId":"1400195741-22996-43-git-send-email-sahlberg@google.com","threadId":"36672","inReplyTo":"1400195741-22996-1-git-send-email-sahlberg@google.com","subject":"[PATCH v9 42/44] refs.c: pass a skip list to name_conflict_fn","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-15T23:15:39Z","receivedAt":"2014-05-15T23:15:39Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Allow passing a list of refs to skip checking to name_conflict_fn.\nThere are some conditions where we want to allow a temporary conflict and skip\nchecking those refs. For example if we have a transaction that\n1, guarantees that m is a packed refs and there is no loose ref for m\n2, the transaction will delete m from the packed ref\n3, the transaction will create conflicting m/m\n\nFor this case we want to be able to lock and create m/m since we know that the\nconflict is only transient. I.e. the conflict will be automatically resolved\nby the transaction when it deletes m.\n\nSigned-off-by: Ronnie Sahlberg \n---\n refs.c | 43 +++++++++++++++++++++++++++++++++----------\n 1 file changed, 33 insertions(+), 10 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 6299b3d..c471811 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -798,11 +798,19 @@ struct name_conflict_cb {\n \tconst char *refname;\n \tconst char *oldrefname;\n \tconst char *conflicting_refname;\n+\tconst char **skip;\n+\tint skipnum;\n };\n \n static int name_conflict_fn(struct ref_entry *entry, void *cb_data)\n {\n \tstruct name_conflict_cb *data = (struct name_conflict_cb *)cb_data;\n+\tint i;\n+\tfor(i = 0; i < data->skipnum; i++) {\n+\t\tif (!strcmp(entry->name, data->skip[i])) {\n+\t\t\treturn 0;\n+\t\t}\n+\t}\n \tif (data->oldrefname && !strcmp(data->oldrefname, entry->name))\n \t\treturn 0;\n \tif (names_conflict(data->refname, entry->name)) {\n@@ -817,15 +825,21 @@ static int name_conflict_fn(struct ref_entry *entry, void *cb_data)\n * conflicting with the name of an existing reference in dir. If\n * oldrefname is non-NULL, ignore potential conflicts with oldrefname\n * (e.g., because oldrefname is scheduled for deletion in the same\n- * operation).\n+ * operation). skip contains a list of refs we want to skip checking for\n+ * conflicts with. Refs may be skipped due to us knowing that it will\n+ * be deleted later during a transaction that deletes one reference and then\n+ * creates a new conflicting reference. For example a rename from m to m/m.\n */\n static int is_refname_available(const char *refname, const char *oldrefname,\n-\t\t\t\tstruct ref_dir *dir)\n+\t\t\t\tstruct ref_dir *dir,\n+\t\t\t\tconst char **skip, int skipnum)\n {\n \tstruct name_conflict_cb data;\n \tdata.refname = refname;\n \tdata.oldrefname = oldrefname;\n \tdata.conflicting_refname = NULL;\n+\tdata.skip = skip;\n+\tdata.skipnum = skipnum;\n \n \tsort_ref_dir(dir);\n \tif (do_for_each_entry_in_dir(dir, 0, name_conflict_fn, &data)) {\n@@ -2037,7 +2051,8 @@ int dwim_log(const char *str, int len, unsigned char *sha1, char **log)\n \n static struct ref_lock *lock_ref_sha1_basic(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+\t\t\t\t\t int flags, int *type_p,\n+\t\t\t\t\t const char **skip, int skipnum)\n {\n \tchar *ref_file;\n \tconst char *orig_refname = refname;\n@@ -2084,7 +2099,9 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,\n \t * name is a proper prefix of our refname.\n \t */\n \tif (missing &&\n-\t !is_refname_available(refname, NULL, get_packed_refs(&ref_cache))) {\n+\t !is_refname_available(refname, NULL,\n+\t\t\t\t get_packed_refs(&ref_cache),\n+\t\t\t\t skip, skipnum)) {\n \t\tlast_errno = ENOTDIR;\n \t\tgoto error_return;\n \t}\n@@ -2142,7 +2159,7 @@ 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 {\n-\treturn lock_ref_sha1_basic(refname, old_sha1, flags, type_p);\n+\treturn lock_ref_sha1_basic(refname, old_sha1, flags, type_p, NULL, 0);\n }\n \n /*\n@@ -2620,6 +2637,9 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms\n \tint log = !lstat(git_path(\"logs/%s\", oldrefname), &loginfo);\n \tconst char *symref = NULL;\n \n+\tif (!strcmp(oldrefname, newrefname))\n+\t\treturn 0;\n+\n \tif (log && S_ISLNK(loginfo.st_mode))\n \t\treturn error(\"reflog for %s is a symlink\", oldrefname);\n \n@@ -2630,10 +2650,12 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms\n \tif (!symref)\n \t\treturn error(\"refname %s not found\", oldrefname);\n \n-\tif (!is_refname_available(newrefname, oldrefname, get_packed_refs(&ref_cache)))\n+\tif (!is_refname_available(newrefname, oldrefname,\n+\t\t\t\t get_packed_refs(&ref_cache), NULL, 0))\n \t\treturn 1;\n \n-\tif (!is_refname_available(newrefname, oldrefname, get_loose_refs(&ref_cache)))\n+\tif (!is_refname_available(newrefname, oldrefname,\n+\t\t\t\t get_loose_refs(&ref_cache), NULL, 0))\n \t\treturn 1;\n \n \tif (log && rename(git_path(\"logs/%s\", oldrefname), git_path(TMP_RENAMED_LOG)))\n@@ -2666,7 +2688,7 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms\n \n \tlogmoved = log;\n \n-\tlock = lock_ref_sha1_basic(newrefname, NULL, 0, NULL);\n+\tlock = lock_ref_sha1_basic(newrefname, NULL, 0, NULL, NULL, 0);\n \tif (!lock) {\n \t\terror(\"unable to lock %s for update\", newrefname);\n \t\tgoto rollback;\n@@ -2681,7 +2703,7 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms\n \treturn 0;\n \n rollback:\n-\tlock = lock_ref_sha1_basic(oldrefname, NULL, 0, NULL);\n+\tlock = lock_ref_sha1_basic(oldrefname, NULL, 0, NULL, NULL, 0);\n \tif (!lock) {\n \t\terror(\"unable to lock %s for rollback\", oldrefname);\n \t\tgoto rollbacklog;\n@@ -3546,7 +3568,8 @@ int ref_transaction_commit(struct ref_transaction *transaction,\n \t\t\t\t\t\t update->old_sha1 :\n \t\t\t\t\t\t NULL),\n \t\t\t\t\t\t update->flags,\n-\t\t\t\t\t\t &update->type);\n+\t\t\t\t\t\t &update->type,\n+\t\t\t\t\t\t delnames, delnum);\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.0.rc3.477.gffe78a2\n"},{"id":"241824","messageId":"1400195741-22996-44-git-send-email-sahlberg@google.com","threadId":"36672","inReplyTo":"1400195741-22996-1-git-send-email-sahlberg@google.com","subject":"[PATCH v9 43/44] refs.c: make rename_ref use a transaction","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-15T23:15:40Z","receivedAt":"2014-05-15T23:15:40Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Change rename_ref to use a single transaction to perform the ref rename.\n\nSigned-off-by: Ronnie Sahlberg \n---\n refs.c | 73 ++++++++++++++++++------------------------------------------------\n 1 file changed, 20 insertions(+), 53 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex c471811..7ea3f42 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2630,9 +2630,10 @@ static int rename_tmp_log(const char *newrefname)\n \n int rename_ref(const char *oldrefname, const char *newrefname, const char *logmsg)\n {\n-\tunsigned char sha1[20], orig_sha1[20];\n-\tint flag = 0, logmoved = 0;\n-\tstruct ref_lock *lock;\n+\tunsigned char sha1[20];\n+\tint flag = 0;\n+\tstruct ref_transaction *transaction;\n+\tstruct strbuf err = STRBUF_INIT;\n \tstruct stat loginfo;\n \tint log = !lstat(git_path(\"logs/%s\", oldrefname), &loginfo);\n \tconst char *symref = NULL;\n@@ -2643,7 +2644,7 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms\n \tif (log && S_ISLNK(loginfo.st_mode))\n \t\treturn error(\"reflog for %s is a symlink\", oldrefname);\n \n-\tsymref = resolve_ref_unsafe(oldrefname, orig_sha1, 1, &flag);\n+\tsymref = resolve_ref_unsafe(oldrefname, sha1, 1, &flag);\n \tif (flag & REF_ISSYMREF)\n \t\treturn error(\"refname %s is a symbolic ref, renaming it is not supported\",\n \t\t\toldrefname);\n@@ -2665,62 +2666,28 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms\n \tif (pack_refs(PACK_REFS_ALL | PACK_REFS_PRUNE))\n \t\treturn error(\"unable to pack refs\");\n \n-\tif (delete_ref(oldrefname, orig_sha1, REF_NODEREF)) {\n-\t\terror(\"unable to delete old %s\", oldrefname);\n-\t\tgoto rollback;\n-\t}\n-\n-\tif (!read_ref_full(newrefname, sha1, 1, NULL) &&\n-\t delete_ref(newrefname, sha1, REF_NODEREF)) {\n-\t\tif (errno==EISDIR) {\n-\t\t\tif (remove_empty_directories(git_path(\"%s\", newrefname))) {\n-\t\t\t\terror(\"Directory not empty: %s\", newrefname);\n-\t\t\t\tgoto rollback;\n-\t\t\t}\n-\t\t} else {\n-\t\t\terror(\"unable to delete existing %s\", newrefname);\n-\t\t\tgoto rollback;\n-\t\t}\n+\ttransaction = ref_transaction_begin();\n+\tif (!transaction ||\n+\t ref_transaction_delete(transaction, oldrefname, sha1,\n+\t\t\t\t REF_NODEREF | REF_ISPACKONLY,\n+\t\t\t\t 1, NULL, &err) ||\n+\t ref_transaction_update(transaction, newrefname, sha1,\n+\t\t\t\t NULL, 0, 0, logmsg, &err) ||\n+\t ref_transaction_commit(transaction, &err)) {\n+\t\tref_transaction_rollback(transaction);\n+\t\terror(\"rename_ref failed: %s\", err.buf);\n+\t\tstrbuf_release(&err);\n+\t\tgoto rollbacklog;\n \t}\n+\tref_transaction_free(transaction);\n \n \tif (log && rename_tmp_log(newrefname))\n-\t\tgoto rollback;\n-\n-\tlogmoved = log;\n-\n-\tlock = lock_ref_sha1_basic(newrefname, NULL, 0, NULL, NULL, 0);\n-\tif (!lock) {\n-\t\terror(\"unable to lock %s for update\", newrefname);\n-\t\tgoto rollback;\n-\t}\n-\tlock->force_write = 1;\n-\thashcpy(lock->old_sha1, orig_sha1);\n-\tif (write_ref_sha1(lock, orig_sha1, logmsg)) {\n-\t\terror(\"unable to write current sha1 into %s\", newrefname);\n-\t\tgoto rollback;\n-\t}\n-\n-\treturn 0;\n-\n- rollback:\n-\tlock = lock_ref_sha1_basic(oldrefname, NULL, 0, NULL, NULL, 0);\n-\tif (!lock) {\n-\t\terror(\"unable to lock %s for rollback\", oldrefname);\n \t\tgoto rollbacklog;\n-\t}\n \n-\tlock->force_write = 1;\n-\tflag = log_all_ref_updates;\n-\tlog_all_ref_updates = 0;\n-\tif (write_ref_sha1(lock, orig_sha1, NULL))\n-\t\terror(\"unable to write current sha1 into %s\", oldrefname);\n-\tlog_all_ref_updates = flag;\n+\treturn 0;\n \n rollbacklog:\n-\tif (logmoved && rename(git_path(\"logs/%s\", newrefname), git_path(\"logs/%s\", oldrefname)))\n-\t\terror(\"unable to restore logfile %s from %s: %s\",\n-\t\t\toldrefname, newrefname, strerror(errno));\n-\tif (!logmoved && log &&\n+\tif (log &&\n \t rename(git_path(TMP_RENAMED_LOG), git_path(\"logs/%s\", oldrefname)))\n \t\terror(\"unable to restore logfile %s from \"TMP_RENAMED_LOG\": %s\",\n \t\t\toldrefname, strerror(errno));\n-- \n2.0.0.rc3.477.gffe78a2\n"},{"id":"241823","messageId":"1400195741-22996-45-git-send-email-sahlberg@google.com","threadId":"36672","inReplyTo":"1400195741-22996-1-git-send-email-sahlberg@google.com","subject":"[PATCH v9 44/44] refs.c: remove forward declaration of write_ref_sha1","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-15T23:15:41Z","receivedAt":"2014-05-15T23:15:41Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Signed-off-by: Ronnie Sahlberg \n---\n refs.c | 2 --\n 1 file changed, 2 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 7ea3f42..8e30511 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -260,8 +260,6 @@ struct ref_entry {\n };\n \n static void read_loose_refs(const char *dirname, struct ref_dir *dir);\n-static int write_ref_sha1(struct ref_lock *lock,\n-\t\t\t const unsigned char *sha1, const char *logmsg);\n \n static struct ref_dir *get_ref_dir(struct ref_entry *entry)\n {\n-- \n2.0.0.rc3.477.gffe78a2\n"}]}