{"thread":{"id":"36660","subject":"[PATCH 01/31] refs.c make ref_transaction_create a wrapper to ref_transaction_update","startedAt":"2014-05-14T22:12:59Z","lastAt":"2014-05-19T23:27:05Z","messageCount":43,"participants":["Ronnie Sahlberg","Junio C Hamano","Eric Sunshine"],"isPatch":true,"patchVersion":1,"patchTotal":31},"messages":[{"id":"241621","messageId":"1400105610-21194-1-git-send-email-sahlberg@google.com","threadId":"36660","inReplyTo":null,"subject":"[PATCH 00/31] Finish implementing ref and reflog transactions","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-14T22:12:59Z","receivedAt":"2014-05-14T22:12:59Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"This patch series is available also from\nhttps://github.com/rsahlberg/git/tree/ref-transactions-next\nand is based on\nhttps://github.com/rsahlberg/git/tree/ref-transactions\n\nThis is a preview of the direction for the transaction work and finished\nconverting also reflog handling to transactions. This greatly simplifies\nboth the implementation of rename_ref in refs.c as well as reflog.c\n\nThis patch series is not fully finished and can not be applied until\nthe previous series is merged. Once this series is finished we will be\nin pretty good shape to start experimenting with alternative refs backends\nsuch as a TDB database.\n\n\n\nRonnie Sahlberg (31):\n refs.c make ref_transaction_create a wrapper to ref_transaction_update\n refs.c: make ref_transaction_delete a wrapper for\n ref_transaction_update\n refs.c: rename the transaction functions\n refs.c: add a new update_type field to ref_update\n refs.c: add a function to append a reflog entry to a fd\n refs.c: add a transaction function to append a reflog entry\n refs.c: add a flag to allow reflog updates to truncate the log\n refs.c: only write reflog update if msg is non-NULL\n refs.c: allow multiple reflog updates during a single transaction\n reflog.c: use a reflog transaction when writing during expire\n refs.c: null terminate the string in copy_msg\n refs.c: track the refnames we are deleting in the transaction\n structure\n refs.c: update the list of deleted refs during _update instead of\n _commit\n refs.c: return error instead of dying when locking fails during\n transaction\n refs.c: lock the ref during _update instead of during _commit\n refs.c: add an error argument to create/delete/update just like commit\n refs.c: make _update_reflog take an error argument\n refs.c: return immediately from _commit if the transaction has an\n error\n tests: move tests for -z update/delete/verify to after the ref is\n created\n refs.c: check for lock conflicts already in _update\n refs.c allow multiple updates of the same ref in a transaction\n refs.c: release all remaining locks during transaction_free\n reflog.c: use the existing transaction to also lock and update the ref\n refs.c: make unlock_ref static\n refs.c: make close_ref static\n refs.c: make commit_ref static\n refs.c: remove the function lock_any_ref_for_update\n refs.c: make struct ref_lock private to refs.c\n refs.c: allow passing raw git_committer_info as email to\n _update_reflog\n refs.c: move ref_update and other definitions to earlier in the file\n refs.c: use the transaction to manage the reflog in rename_refs\n\n branch.c | 11 +-\n builtin/commit.c | 14 +-\n builtin/fetch.c | 10 +-\n builtin/receive-pack.c | 11 +-\n builtin/reflog.c | 76 +++---\n builtin/replace.c | 10 +-\n builtin/tag.c | 10 +-\n builtin/update-ref.c | 34 +--\n fast-import.c | 26 ++-\n refs.c | 611 +++++++++++++++++++++++++++++++++----------------\n refs.h | 86 +++----\n sequencer.c | 12 +-\n t/t1400-update-ref.sh | 59 +++--\n walker.c | 15 +-\n 14 files changed, 610 insertions(+), 375 deletions(-)\n\n-- \n2.0.0.rc3.506.g3739a35\n"},{"id":"241593","messageId":"1400105610-21194-2-git-send-email-sahlberg@google.com","threadId":"36660","inReplyTo":"1400105610-21194-1-git-send-email-sahlberg@google.com","subject":"[PATCH 01/31] refs.c make ref_transaction_create a wrapper to ref_transaction_update","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-14T22:13:00Z","receivedAt":"2014-05-14T22:13:00Z","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 | 13 +++----------\n refs.h | 7 ++++---\n 2 files changed, 7 insertions(+), 13 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex aed700b..fe195dd 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3354,17 +3354,10 @@ int ref_transaction_create(struct ref_transaction *transaction,\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+\t\tdie(\"BUG: REF_ISPACKONLY can not be used with create\");\n \n-\tupdate = add_update(transaction, refname);\n-\n-\thashcpy(update->new_sha1, new_sha1);\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+\treturn ref_transaction_update(transaction, refname, new_sha1,\n+\t\t\t\t null_sha1, flags, 1, msg);\n }\n \n int ref_transaction_delete(struct ref_transaction *transaction,\ndiff --git a/refs.h b/refs.h\nindex 2d2362c..d615871 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -233,9 +233,10 @@ void ref_transaction_rollback(struct ref_transaction *transaction);\n /*\n * Add a reference update to transaction. new_sha1 is the value that\n * the reference should have after the update, or zeros if it should\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+ * be deleted. If have_old is true and old_sha is not the null_sha1\n+ * then the previous value of the ref must match or the update will fail.\n+ * If have_old is true and old_sha1 is the null_sha1 then the ref must not\n+ * already exist and a new ref will be created with new_sha1.\n * Function returns 0 on success and non-zero on failure.\n */\n int ref_transaction_update(struct ref_transaction *transaction,\n-- \n2.0.0.rc3.506.g3739a35\n"},{"id":"241622","messageId":"1400105610-21194-3-git-send-email-sahlberg@google.com","threadId":"36660","inReplyTo":"1400105610-21194-1-git-send-email-sahlberg@google.com","subject":"[PATCH 02/31] refs.c: make ref_transaction_delete a wrapper for ref_transaction_update","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-14T22:13:01Z","receivedAt":"2014-05-14T22:13:01Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"We want to allow to delete a ref even if it only exists as a packed ref\nso we must tweak the REF_ISPACKONLY test in _update so it only applies\nto the non-delete case.\n\nSigned-off-by: Ronnie Sahlberg \n---\n refs.c | 17 ++++++-----------\n refs.h | 2 +-\n 2 files changed, 7 insertions(+), 12 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex fe195dd..c180fa2 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3326,7 +3326,7 @@ 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+\tif (flags & REF_ISPACKONLY && !is_null_sha1(new_sha1))\n \t\tdie(\"BUG: REF_ISPACKONLY can not be used with updates\");\n \n \tupdate = add_update(transaction, refname);\n@@ -3370,19 +3370,14 @@ 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 (have_old && is_null_sha1(old_sha1))\n+\t\tdie(\"BUG: have_old is true but old_sha1 is null_sha1\");\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-\tif (have_old) {\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+\treturn ref_transaction_update(transaction, refname, null_sha1,\n+\t\t\t\t old_sha1, flags, have_old, msg);\n }\n \n int update_ref(const char *action, const char *refname,\ndiff --git a/refs.h b/refs.h\nindex d615871..8d56edf 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -232,7 +232,7 @@ void ref_transaction_rollback(struct ref_transaction *transaction);\n \n /*\n * Add a reference update to transaction. new_sha1 is the value that\n- * the reference should have after the update, or zeros if it should\n+ * the reference should have after the update, or null_sha1 if it should\n * be deleted. If have_old is true and old_sha is not the null_sha1\n * then the previous value of the ref must match or the update will fail.\n * If have_old is true and old_sha1 is the null_sha1 then the ref must not\n-- \n2.0.0.rc3.506.g3739a35\n"},{"id":"241624","messageId":"1400105610-21194-4-git-send-email-sahlberg@google.com","threadId":"36660","inReplyTo":"1400105610-21194-1-git-send-email-sahlberg@google.com","subject":"[PATCH 03/31] refs.c: rename the transaction functions","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-14T22:13:02Z","receivedAt":"2014-05-14T22:13:02Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Rename the transaction functions. Remove the leading ref_ from the names\nand append _sha1 to the names for functions that create/delete/update sha1\nrefs.\n\nSigned-off-by: Ronnie Sahlberg \n---\n branch.c | 10 +++---\n builtin/commit.c | 14 ++++----\n builtin/fetch.c | 10 +++---\n builtin/receive-pack.c | 10 +++---\n builtin/replace.c | 10 +++---\n builtin/tag.c | 10 +++---\n builtin/update-ref.c | 22 ++++++------\n fast-import.c | 22 ++++++------\n refs.c | 92 +++++++++++++++++++++++++-------------------------\n refs.h | 36 ++++++++++----------\n sequencer.c | 12 +++----\n walker.c | 14 ++++----\n 12 files changed, 131 insertions(+), 131 deletions(-)\n\ndiff --git a/branch.c b/branch.c\nindex 74d55e7..0a4d4f3 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -298,14 +298,14 @@ void create_branch(const char *head,\n \t\tstruct ref_transaction *transaction;\n \t\tstruct strbuf err = STRBUF_INIT;\n \n-\t\ttransaction = ref_transaction_begin();\n+\t\ttransaction = 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, msg) ||\n-\t\t ref_transaction_commit(transaction, &err))\n+\t\t transaction_update_sha1(transaction, ref.buf, sha1,\n+\t\t\t\t\t null_sha1, 0, !forcing, msg) ||\n+\t\t transaction_commit(transaction, &err))\n \t\t\t\tdie_errno(_(\"%s: failed to write ref: %s\"),\n \t\t\t\t\t ref.buf, err.buf);\n-\t\tref_transaction_free(transaction);\n+\t\ttransaction_free(transaction);\n \t}\n \n \tif (real_ref && track)\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex e6bb7b3..386dfb1 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1716,17 +1716,17 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \tstrbuf_insert(&sb, 0, reflog_msg, strlen(reflog_msg));\n \tstrbuf_insert(&sb, strlen(reflog_msg), \": \", 2);\n \n-\ttransaction = ref_transaction_begin();\n+\ttransaction = 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, sb.buf) ||\n-\t ref_transaction_commit(transaction, &err)) {\n+\t transaction_update_sha1(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, sb.buf) ||\n+\t transaction_commit(transaction, &err)) {\n \t\trollback_index_files();\n \t\tdie(_(\"HEAD: cannot update ref: %s\"), err.buf);\n \t}\n-\tref_transaction_free(transaction);\n+\ttransaction_free(transaction);\n \n \tunlink(git_path(\"CHERRY_PICK_HEAD\"));\n \tunlink(git_path(\"REVERT_HEAD\"));\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex 3a849b0..f74a267 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -383,8 +383,8 @@ static int s_update_ref(const char *action,\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, msg))\n+\tif (transaction_update_sha1(transaction, ref->name, ref->new_sha1,\n+\t\t\t\t ref->old_sha1, 0, check_old, msg))\n \t\treturn STORE_REF_ERROR_OTHER;\n \n \treturn 0;\n@@ -559,7 +559,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,\n \t}\n \n \terrno = 0;\n-\ttransaction = ref_transaction_begin();\n+\ttransaction = transaction_begin();\n \tif (!transaction) {\n \t\trc = error(_(\"cannot start ref transaction\\n\"));\n \t\tgoto abort;\n@@ -676,10 +676,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, NULL))\n+\tif (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);\n+\ttransaction_free(transaction);\n \n \tif (rc & STORE_REF_ERROR_DF_CONFLICT)\n \t\terror(_(\"some local refs could not be updated; try running\\n\"\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex 991c659..0461f93 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -581,8 +581,8 @@ 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\tif (ref_transaction_update(transaction, namespaced_name,\n-\t\t\t\t\t new_sha1, old_sha1, 0, 1, \"push\"))\n+\t\tif (transaction_update_sha1(transaction, namespaced_name,\n+\t\t\t\t\t new_sha1, old_sha1, 0, 1, \"push\"))\n \t\t\treturn \"failed to update\";\n \t\treturn NULL; /* good */\n \t}\n@@ -807,7 +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+\ttransaction = transaction_begin();\n \tfor (cmd = commands; cmd; cmd = cmd->next) {\n \t\tif (cmd->error_string)\n \t\t\tcontinue;\n@@ -823,9 +823,9 @@ static void execute_commands(struct command *commands,\n \t\t\tchecked_connectivity = 0;\n \t\t}\n \t}\n-\tif (ref_transaction_commit(transaction, &err))\n+\tif (transaction_commit(transaction, &err))\n \t\terror(\"%s\", err.buf);\n-\tref_transaction_free(transaction);\n+\ttransaction_free(transaction);\n \tstrbuf_release(&err);\n \n \tif (shallow_update && !checked_connectivity)\ndiff --git a/builtin/replace.c b/builtin/replace.c\nindex 820d703..2587a06 100644\n--- a/builtin/replace.c\n+++ b/builtin/replace.c\n@@ -157,14 +157,14 @@ static int replace_object_sha1(const char *object_ref,\n \telse if (!force)\n \t\tdie(\"replace ref '%s' already exists\", ref);\n \n-\ttransaction = ref_transaction_begin();\n+\ttransaction = transaction_begin();\n \tif (!transaction ||\n-\t ref_transaction_update(transaction, ref, repl, prev,\n-\t\t\t\t 0, !is_null_sha1(prev), NULL) ||\n-\t ref_transaction_commit(transaction, &err))\n+\t transaction_update_sha1(transaction, ref, repl, prev,\n+\t\t\t\t 0, !is_null_sha1(prev), NULL) ||\n+\t transaction_commit(transaction, &err))\n \t\tdie(_(\"%s: failed to replace ref: %s\"), ref, err.buf);\n \n-\tref_transaction_free(transaction);\n+\ttransaction_free(transaction);\n \treturn 0;\n }\n \ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 2cc260f..3739e23 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -702,13 +702,13 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \tif (annotate)\n \t\tcreate_tag(object, tag, &buf, &opt, prev, object);\n \n-\ttransaction = ref_transaction_begin();\n+\ttransaction = 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), NULL) ||\n-\t ref_transaction_commit(transaction, &err))\n+\t transaction_update_sha1(transaction, ref.buf, object, prev,\n+\t\t\t\t 0, !is_null_sha1(prev), NULL) ||\n+\t transaction_commit(transaction, &err))\n \t\tdie(_(\"%s: cannot update the ref: %s\"), ref.buf, err.buf);\n-\tref_transaction_free(transaction);\n+\ttransaction_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 c5aff92..f7e33bd 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -198,8 +198,8 @@ 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-\tif (ref_transaction_update(transaction, refname, new_sha1, old_sha1,\n-\t\t\t\t update_flags, have_old, msg))\n+\tif (transaction_update_sha1(transaction, refname, new_sha1, old_sha1,\n+\t\t\t\t update_flags, have_old, msg))\n \t\tdie(\"update %s: failed\", refname);\n \n \tupdate_flags = 0;\n@@ -226,8 +226,8 @@ 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-\tif (ref_transaction_create(transaction, refname, new_sha1,\n-\t\t\t\t update_flags, msg))\n+\tif (transaction_create_sha1(transaction, refname, new_sha1,\n+\t\t\t\t update_flags, msg))\n \t\tdie(\"failed transaction create for %s\", refname);\n \n \tupdate_flags = 0;\n@@ -258,8 +258,8 @@ 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-\tif (ref_transaction_delete(transaction, refname, old_sha1,\n-\t\t\t\t update_flags, have_old, msg))\n+\tif (transaction_delete_sha1(transaction, refname, old_sha1,\n+\t\t\t\t update_flags, have_old, msg))\n \t\tdie(\"failed transaction delete for %s\", refname);\n \n \tupdate_flags = 0;\n@@ -291,8 +291,8 @@ 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-\tif (ref_transaction_update(transaction, refname, new_sha1, old_sha1,\n-\t\t\t\t update_flags, have_old, msg))\n+\tif (transaction_update_sha1(transaction, refname, new_sha1, old_sha1,\n+\t\t\t\t update_flags, have_old, msg))\n \t\tdie(\"failed transaction update for %s\", refname);\n \n \tupdate_flags = 0;\n@@ -366,15 +366,15 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)\n \t\tdie(\"Refusing to perform update with empty message.\");\n \n \tif (read_stdin) {\n-\t\ttransaction = ref_transaction_begin();\n+\t\ttransaction = transaction_begin();\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\tif (ref_transaction_commit(transaction, &err))\n+\t\tif (transaction_commit(transaction, &err))\n \t\t\tdie(\"%s\", err.buf);\n-\t\tref_transaction_free(transaction);\n+\t\ttransaction_free(transaction);\n \t\treturn 0;\n \t}\n \ndiff --git a/fast-import.c b/fast-import.c\nindex 099e71b..2fa1d29 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -1707,17 +1707,17 @@ static int update_branch(struct branch *b)\n \t\t\treturn -1;\n \t\t}\n \t}\n-\ttransaction = ref_transaction_begin();\n+\ttransaction = transaction_begin();\n \tif (!transaction ||\n-\t ref_transaction_update(transaction, b->name, b->sha1, old_sha1,\n-\t\t\t\t 0, 1, msg) ||\n-\t ref_transaction_commit(transaction, &err)) {\n-\t\tref_transaction_rollback(transaction);\n+\t transaction_update_sha1(transaction, b->name, b->sha1, old_sha1,\n+\t\t\t\t 0, 1, msg) ||\n+\t transaction_commit(transaction, &err)) {\n+\t\ttransaction_rollback(transaction);\n \t\terror(\"Unable to update branch %s: %s\", b->name, err.buf);\n \t\tstrbuf_release(&err);\n \t\treturn -1;\n \t}\n-\tref_transaction_free(transaction);\n+\ttransaction_free(transaction);\n \treturn 0;\n }\n \n@@ -1740,17 +1740,17 @@ static void dump_tags(void)\n \tstruct strbuf err = STRBUF_INIT;\n \tstruct ref_transaction *transaction;\n \n-\ttransaction = ref_transaction_begin();\n+\ttransaction = transaction_begin();\n \tfor (t = first_tag; t; t = t->next_tag) {\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, msg))\n+\t\tif (transaction_update_sha1(transaction, ref_name, t->sha1,\n+\t\t\t\t\t NULL, 0, 0, msg))\n \t\t\tfailure |= error(\"Unable to update %s\", err.buf);\n \t}\n-\tif (failure || ref_transaction_commit(transaction, &err))\n+\tif (failure || transaction_commit(transaction, &err))\n \t\tfailure |= error(\"Unable to update %s\", err.buf);\n-\tref_transaction_free(transaction);\n+\ttransaction_free(transaction);\n \tstrbuf_release(&err);\n }\n \ndiff --git a/refs.c b/refs.c\nindex c180fa2..6785fa6 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2357,17 +2357,17 @@ static void prune_ref(struct ref_to_prune *r)\n \tif (check_refname_format(r->name + 5, 0))\n \t\treturn;\n \n-\ttransaction = ref_transaction_begin();\n+\ttransaction = transaction_begin();\n \tif (!transaction ||\n-\t ref_transaction_delete(transaction, r->name, r->sha1,\n-\t\t\t\t REF_ISPRUNING, 1, NULL) ||\n-\t ref_transaction_commit(transaction, &err)) {\n-\t\tref_transaction_rollback(transaction);\n+\t transaction_delete_sha1(transaction, r->name, r->sha1,\n+\t\t\t\t REF_ISPRUNING, 1, NULL) ||\n+\t transaction_commit(transaction, &err)) {\n+\t\ttransaction_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+\ttransaction_free(transaction);\n \ttry_remove_empty_parents(r->name);\n }\n \n@@ -2521,15 +2521,15 @@ int delete_ref(const char *refname, const unsigned char *sha1, int delopt)\n {\n \tstruct ref_transaction *transaction;\n \n-\ttransaction = ref_transaction_begin();\n+\ttransaction = 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)) {\n-\t\tref_transaction_rollback(transaction);\n+\t transaction_delete_sha1(transaction, refname, sha1, delopt,\n+\t\t\t\t sha1 && !is_null_sha1(sha1), NULL) ||\n+\t transaction_commit(transaction, NULL)) {\n+\t\ttransaction_rollback(transaction);\n \t\treturn 1;\n \t}\n-\tref_transaction_free(transaction);\n+\ttransaction_free(transaction);\n \treturn 0;\n }\n \n@@ -2625,20 +2625,20 @@ 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-\ttransaction = ref_transaction_begin();\n+\ttransaction = 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) ||\n-\t ref_transaction_update(transaction, newrefname, sha1,\n-\t\t\t\t NULL, 0, 0, logmsg) ||\n-\t ref_transaction_commit(transaction, &err)) {\n-\t\tref_transaction_rollback(transaction);\n+\t transaction_delete_sha1(transaction, oldrefname, sha1,\n+\t\t\t\t REF_NODEREF | REF_ISPACKONLY,\n+\t\t\t\t 1, NULL) ||\n+\t transaction_update_sha1(transaction, newrefname, sha1,\n+\t\t\t\t NULL, 0, 0, logmsg) ||\n+\t transaction_commit(transaction, &err)) {\n+\t\ttransaction_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+\ttransaction_free(transaction);\n \n \tif (log && rename_tmp_log(newrefname))\n \t\tgoto rollbacklog;\n@@ -3270,12 +3270,12 @@ struct ref_transaction {\n \tenum ref_transaction_status status;\n };\n \n-struct ref_transaction *ref_transaction_begin(void)\n+struct ref_transaction *transaction_begin(void)\n {\n \treturn xcalloc(1, sizeof(struct ref_transaction));\n }\n \n-void ref_transaction_free(struct ref_transaction *transaction)\n+void transaction_free(struct ref_transaction *transaction)\n {\n \tint i;\n \n@@ -3290,14 +3290,14 @@ void ref_transaction_free(struct ref_transaction *transaction)\n \tfree(transaction);\n }\n \n-void ref_transaction_rollback(struct ref_transaction *transaction)\n+void 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+\ttransaction_free(transaction);\n }\n \n static struct ref_update *add_update(struct ref_transaction *transaction,\n@@ -3312,11 +3312,11 @@ static struct ref_update *add_update(struct ref_transaction *transaction,\n \treturn update;\n }\n \n-int ref_transaction_update(struct ref_transaction *transaction,\n+int transaction_update_sha1(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, const char *msg)\n+\t\t\t int flags, int have_old, const char *msg)\n {\n \tstruct ref_update *update;\n \n@@ -3340,10 +3340,10 @@ int ref_transaction_update(struct ref_transaction *transaction,\n \treturn 0;\n }\n \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, const char *msg)\n+int transaction_create_sha1(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, const char *msg)\n {\n \tstruct ref_update *update;\n \n@@ -3356,14 +3356,14 @@ int ref_transaction_create(struct ref_transaction *transaction,\n \tif (flags & REF_ISPACKONLY)\n \t\tdie(\"BUG: REF_ISPACKONLY can not be used with create\");\n \n-\treturn ref_transaction_update(transaction, refname, new_sha1,\n-\t\t\t\t null_sha1, flags, 1, msg);\n+\treturn transaction_update_sha1(transaction, refname, new_sha1,\n+\t\t\t\t null_sha1, flags, 1, msg);\n }\n \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, const char *msg)\n+int transaction_delete_sha1(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, const char *msg)\n {\n \tstruct ref_update *update;\n \n@@ -3376,8 +3376,8 @@ int ref_transaction_delete(struct ref_transaction *transaction,\n \tif (transaction->status != REF_TRANSACTION_OPEN)\n \t\tdie(\"BUG: delete on transaction that is not open\");\n \n-\treturn ref_transaction_update(transaction, refname, null_sha1,\n-\t\t\t\t old_sha1, flags, have_old, msg);\n+\treturn transaction_update_sha1(transaction, refname, null_sha1,\n+\t\t\t\t old_sha1, flags, have_old, msg);\n }\n \n int update_ref(const char *action, const char *refname,\n@@ -3387,14 +3387,14 @@ int update_ref(const char *action, const char *refname,\n \tstruct ref_transaction *t;\n \tstruct strbuf err = STRBUF_INIT;\n \n-\tt = ref_transaction_begin();\n+\tt = transaction_begin();\n \tif (!t ||\n-\t ref_transaction_update(t, refname, sha1, oldval, flags,\n-\t\t\t\t !!oldval, action) ||\n-\t ref_transaction_commit(t, &err)) {\n+\t transaction_update_sha1(t, refname, sha1, oldval, flags,\n+\t\t\t\t !!oldval, action) ||\n+\t transaction_commit(t, &err)) {\n \t\tconst char *str = \"update_ref failed for ref '%s': %s\";\n \n-\t\tref_transaction_rollback(t);\n+\t\ttransaction_rollback(t);\n \t\tswitch (onerr) {\n \t\tcase UPDATE_REFS_MSG_ON_ERR:\n \t\t\terror(str, refname, err.buf); break;\n@@ -3405,7 +3405,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+\ttransaction_free(t);\n \treturn 0;\n }\n \n@@ -3432,7 +3432,7 @@ static int ref_update_reject_duplicates(struct ref_update **updates, int n,\n \treturn 0;\n }\n \n-int ref_transaction_commit(struct ref_transaction *transaction,\n+int transaction_commit(struct ref_transaction *transaction,\n \t\t\t struct strbuf *err)\n {\n \tint ret = 0, delnum = 0, i;\ndiff --git a/refs.h b/refs.h\nindex 8d56edf..127c12f 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -213,12 +213,12 @@ enum action_on_err {\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+struct ref_transaction *transaction_begin(void);\n \n /*\n * Roll back a ref_transaction and free all associated data.\n */\n-void ref_transaction_rollback(struct ref_transaction *transaction);\n+void transaction_rollback(struct ref_transaction *transaction);\n \n \n /*\n@@ -239,11 +239,11 @@ void ref_transaction_rollback(struct ref_transaction *transaction);\n * already exist and a new ref will be created with new_sha1.\n * Function returns 0 on success and non-zero on failure.\n */\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, const char *msg);\n+int transaction_update_sha1(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, const char *msg);\n \n /*\n * Add a reference creation to transaction. new_sha1 is the value\n@@ -252,10 +252,10 @@ int ref_transaction_update(struct ref_transaction *transaction,\n * already.\n * Function returns 0 on success and non-zero on failure.\n */\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, const char *msg);\n+int transaction_create_sha1(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, const char *msg);\n \n /*\n * Add a reference deletion to transaction. If have_old is true, then\n@@ -263,10 +263,10 @@ int ref_transaction_create(struct ref_transaction *transaction,\n * the update (which must not be the null SHA-1).\n * Function returns 0 on success and non-zero on failure.\n */\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, const char *msg);\n+int transaction_delete_sha1(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, const char *msg);\n \n /*\n * Commit all of the changes that have been queued in transaction, as\n@@ -274,13 +274,13 @@ int ref_transaction_delete(struct ref_transaction *transaction,\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 struct strbuf *err);\n+int transaction_commit(struct ref_transaction *transaction,\n+\t\t struct strbuf *err);\n \n /*\n * Free an existing transaction.\n */\n-void ref_transaction_free(struct ref_transaction *transaction);\n+void 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,\ndiff --git a/sequencer.c b/sequencer.c\nindex 3a0ee09..3f7996f 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -282,18 +282,18 @@ static int fast_forward_to(const unsigned char *to, const unsigned char *from,\n \n \tstrbuf_addf(&sb, \"%s: fast-forward\", action_name(opts));\n \n-\ttransaction = ref_transaction_begin();\n+\ttransaction = transaction_begin();\n \tif (!transaction ||\n-\t ref_transaction_update(transaction, \"HEAD\", to, from,\n-\t\t\t\t 0, !unborn, sb.buf) ||\n-\t ref_transaction_commit(transaction, &err)) {\n-\t\tref_transaction_rollback(transaction);\n+\t transaction_update_sha1(transaction, \"HEAD\", to, from,\n+\t\t\t\t 0, !unborn, sb.buf) ||\n+\t transaction_commit(transaction, &err)) {\n+\t\ttransaction_rollback(transaction);\n \t\terror(_(\"HEAD: Could not fast-forward: %s\\n\"), err.buf);\n \t\tstrbuf_release(&sb);\n \t\tstrbuf_release(&err);\n \t\treturn -1;\n \t}\n-\tref_transaction_free(transaction);\n+\ttransaction_free(transaction);\n \n \tstrbuf_release(&sb);\n \treturn 0;\ndiff --git a/walker.c b/walker.c\nindex c2a1266..3701c78 100644\n--- a/walker.c\n+++ b/walker.c\n@@ -260,7 +260,7 @@ int walker_fetch(struct walker *walker, int targets, char **target,\n \n \tsave_commit_buffer = 0;\n \n-\ttransaction = ref_transaction_begin();\n+\ttransaction = transaction_begin();\n \tif (!transaction)\n \t\treturn -1;\n \n@@ -289,14 +289,14 @@ int walker_fetch(struct walker *walker, int targets, char **target,\n \t\tif (!write_ref || !write_ref[i])\n \t\t\tcontinue;\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,\n-\t\t\t\t\t msg ? msg : \"fetch (unknown)\"))\n+\t\tif (transaction_update_sha1(transaction, ref_name,\n+\t\t\t\t\t &sha1[20 * i], NULL,\n+\t\t\t\t\t 0, 0,\n+\t\t\t\t\t msg ? msg : \"fetch (unknown)\"))\n \t\t\tgoto rollback_and_fail;\n \t}\n \n-\tif (ref_transaction_commit(transaction, &err)) {\n+\tif (transaction_commit(transaction, &err)) {\n \t\terror(\"%s\", err.buf);\n \t\tgoto rollback_and_fail;\n \t}\n@@ -307,7 +307,7 @@ int walker_fetch(struct walker *walker, int targets, char **target,\n rollback_and_fail:\n \tfree(msg);\n \tstrbuf_release(&err);\n-\tref_transaction_free(transaction);\n+\ttransaction_free(transaction);\n \n \treturn -1;\n }\n-- \n2.0.0.rc3.506.g3739a35\n"},{"id":"241623","messageId":"1400105610-21194-5-git-send-email-sahlberg@google.com","threadId":"36660","inReplyTo":"1400105610-21194-1-git-send-email-sahlberg@google.com","subject":"[PATCH 04/31] refs.c: add a new update_type field to ref_update","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-14T22:13:03Z","receivedAt":"2014-05-14T22:13:03Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Add a field that describes what type of update this refers to. For now\nthe only type is UPDATE_SHA1 but we will soon add more types.\n\nSigned-off-by: Ronnie Sahlberg \n---\n refs.c | 11 +++++++++--\n 1 file changed, 9 insertions(+), 2 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 6785fa6..018062d 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3235,6 +3235,10 @@ int for_each_reflog(each_ref_fn fn, void *cb_data)\n \treturn retval;\n }\n \n+enum transaction_update_type {\n+ UPDATE_SHA1 = 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@@ -3242,6 +3246,7 @@ int for_each_reflog(each_ref_fn fn, void *cb_data)\n * value or to zero to ensure the ref does not exist before update.\n */\n struct ref_update {\n+\tenum transaction_update_type update_type;\n \tunsigned char new_sha1[20];\n \tunsigned char old_sha1[20];\n \tint flags; /* REF_NODEREF? */\n@@ -3301,12 +3306,14 @@ void transaction_rollback(struct ref_transaction *transaction)\n }\n \n static struct ref_update *add_update(struct ref_transaction *transaction,\n-\t\t\t\t const char *refname)\n+\t\t\t\t const char *refname,\n+\t\t\t\t enum transaction_update_type update_type)\n {\n \tsize_t len = strlen(refname);\n \tstruct ref_update *update = xcalloc(1, sizeof(*update) + len + 1);\n \n \tstrcpy((char *)update->refname, refname);\n+\tupdate->update_type = update_type;\n \tALLOC_GROW(transaction->updates, transaction->nr + 1, transaction->alloc);\n \ttransaction->updates[transaction->nr++] = update;\n \treturn update;\n@@ -3329,7 +3336,7 @@ int transaction_update_sha1(struct ref_transaction *transaction,\n \tif (flags & REF_ISPACKONLY && !is_null_sha1(new_sha1))\n \t\tdie(\"BUG: REF_ISPACKONLY can not be used with updates\");\n \n-\tupdate = add_update(transaction, refname);\n+\tupdate = add_update(transaction, refname, UPDATE_SHA1);\n \thashcpy(update->new_sha1, new_sha1);\n \tupdate->flags = flags;\n \tupdate->have_old = have_old;\n-- \n2.0.0.rc3.506.g3739a35\n"},{"id":"241595","messageId":"1400105610-21194-6-git-send-email-sahlberg@google.com","threadId":"36660","inReplyTo":"1400105610-21194-1-git-send-email-sahlberg@google.com","subject":"[PATCH 05/31] refs.c: add a function to append a reflog entry to a fd","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-14T22:13:04Z","receivedAt":"2014-05-14T22:13:04Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Break out the code to create the string and writing it to the file descriptor\nfrom log_ref_write and into a dedicated function log_ref_write_fd. For now\nthis is only used from log_ref_write but later on we will call this function\nfrom reflog transactions too which means that we will end up with only a single\nplace where we write a reflog entry to a file instead of the current two places\n(log_ref_write and builtin/reflog.c).\n\nSigned-off-by: Ronnie Sahlberg \n---\n refs.c | 48 ++++++++++++++++++++++++++++++------------------\n 1 file changed, 30 insertions(+), 18 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 018062d..98c6c00 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2745,15 +2745,37 @@ int log_ref_setup(const char *refname, char *logfile, int bufsize)\n \treturn 0;\n }\n \n+static int log_ref_write_fd(int fd, const unsigned char *old_sha1,\n+\t\t\t const unsigned char *new_sha1,\n+\t\t\t const char *committer, const char *msg)\n+{\n+\tint msglen, written;\n+\tunsigned maxlen, len;\n+\tchar *logrec;\n+\n+\tmsglen = msg ? strlen(msg) : 0;\n+\tmaxlen = strlen(committer) + msglen + 100;\n+\tlogrec = xmalloc(maxlen);\n+\tlen = sprintf(logrec, \"%s %s %s\\n\",\n+\t\t sha1_to_hex(old_sha1),\n+\t\t sha1_to_hex(new_sha1),\n+\t\t committer);\n+\tif (msglen)\n+\t\tlen += copy_msg(logrec + len - 1, msg) - 1;\n+\twritten = len <= maxlen ? write_in_full(fd, logrec, len) : -1;\n+\n+\tfree(logrec);\n+\tif (written != len)\n+\t\treturn -1;\n+\n+\treturn 0;\n+}\n+\n static int log_ref_write(const char *refname, const unsigned char *old_sha1,\n \t\t\t const unsigned char *new_sha1, const char *msg)\n {\n-\tint logfd, result, written, oflags = O_APPEND | O_WRONLY;\n-\tunsigned maxlen, len;\n-\tint msglen;\n+\tint logfd, result, oflags = O_APPEND | O_WRONLY;\n \tchar log_file[PATH_MAX];\n-\tchar *logrec;\n-\tconst char *committer;\n \n \tif (log_all_ref_updates < 0)\n \t\tlog_all_ref_updates = !is_bare_repository();\n@@ -2765,19 +2787,9 @@ static int log_ref_write(const char *refname, const unsigned char *old_sha1,\n \tlogfd = open(log_file, oflags);\n \tif (logfd < 0)\n \t\treturn 0;\n-\tmsglen = msg ? strlen(msg) : 0;\n-\tcommitter = git_committer_info(0);\n-\tmaxlen = strlen(committer) + msglen + 100;\n-\tlogrec = xmalloc(maxlen);\n-\tlen = sprintf(logrec, \"%s %s %s\\n\",\n-\t\t sha1_to_hex(old_sha1),\n-\t\t sha1_to_hex(new_sha1),\n-\t\t committer);\n-\tif (msglen)\n-\t\tlen += copy_msg(logrec + len - 1, msg) - 1;\n-\twritten = len <= maxlen ? write_in_full(logfd, logrec, len) : -1;\n-\tfree(logrec);\n-\tif (close(logfd) != 0 || written != len)\n+\tresult = log_ref_write_fd(logfd, old_sha1, new_sha1,\n+\t\t\t\t git_committer_info(0), msg);\n+\tif (close(logfd) != 0 || result)\n \t\treturn error(\"Unable to append to %s\", log_file);\n \treturn 0;\n }\n-- \n2.0.0.rc3.506.g3739a35\n"},{"id":"241619","messageId":"1400105610-21194-7-git-send-email-sahlberg@google.com","threadId":"36660","inReplyTo":"1400105610-21194-1-git-send-email-sahlberg@google.com","subject":"[PATCH 06/31] refs.c: add a transaction function to append a reflog entry","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-14T22:13:05Z","receivedAt":"2014-05-14T22:13:05Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Define a new transaction update type, UPDATE_LOG, and a new function\ntransaction_update_reflog. This function will lock the reflog and append\nan entry to it during transaction commit.\n\nSigned-off-by: Ronnie Sahlberg \n\nConflicts:\n\trefs.h\n---\n refs.c | 107 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--\n refs.h | 13 ++++++++\n 2 files changed, 117 insertions(+), 3 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 98c6c00..d8a1568 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3249,6 +3249,7 @@ int for_each_reflog(each_ref_fn fn, void *cb_data)\n \n enum transaction_update_type {\n UPDATE_SHA1 = 0,\n+ UPDATE_LOG = 1,\n };\n \n /**\n@@ -3266,6 +3267,12 @@ struct ref_update {\n \tstruct ref_lock *lock;\n \tint type;\n \tconst char *msg;\n+\n+\t/* used by reflog updates */\n+\tint reflog_fd;\n+\tstruct lock_file reflog_lock;\n+\tconst char *committer;\n+\n \tconst char refname[FLEX_ARRAY];\n };\n \n@@ -3300,7 +3307,8 @@ void transaction_free(struct ref_transaction *transaction)\n \t\treturn;\n \n \tfor (i = 0; i < transaction->nr; i++) {\n-\t free((char *)transaction->updates[i]->msg);\n+\t\tfree((char *)transaction->updates[i]->committer);\n+\t\tfree((char *)transaction->updates[i]->msg);\n \t\tfree(transaction->updates[i]);\n \t}\n \tfree(transaction->updates);\n@@ -3331,6 +3339,38 @@ static struct ref_update *add_update(struct ref_transaction *transaction,\n \treturn update;\n }\n \n+int transaction_update_reflog(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 const unsigned char *email,\n+\t\t\t unsigned long timestamp, int tz,\n+\t\t\t const char *msg,\n+\t\t\t int flags)\n+{\n+\tstruct ref_update *update;\n+\n+\tupdate = add_update(transaction, refname, UPDATE_LOG);\n+\thashcpy(update->new_sha1, new_sha1);\n+\thashcpy(update->old_sha1, old_sha1);\n+\tupdate->reflog_fd = -1;\n+\tif (email) {\n+\t\tstruct strbuf buf = STRBUF_INIT;\n+\t\tchar sign = (tz < 0) ? '-' : '+';\n+\t\tint zone = (tz < 0) ? (-tz) : tz;\n+\n+\t\tstrbuf_addf(&buf, \"%s %lu %c%04d\", email, timestamp, sign,\n+\t\t\t zone);\n+\t\tupdate->committer = xstrdup(buf.buf);\n+\t\tstrbuf_release(&buf);\n+\t}\n+\tif (msg)\n+\t\tupdate->msg = xstrdup(msg);\n+\tupdate->flags = flags;\n+\n+\treturn 0;\n+}\n+\n int transaction_update_sha1(struct ref_transaction *transaction,\n \t\t\t const char *refname,\n \t\t\t const unsigned char *new_sha1,\n@@ -3440,6 +3480,8 @@ static int ref_update_reject_duplicates(struct ref_update **updates, int n,\n {\n \tint i;\n \tfor (i = 1; i < n; i++)\n+\t\tif (updates[i]->update_type != UPDATE_SHA1)\n+\t\t\tcontinue;\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@@ -3479,14 +3521,18 @@ int 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->update_type != UPDATE_SHA1)\n+\t\t\tcontinue;\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+\t/* Acquire all ref 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->update_type != UPDATE_SHA1)\n+\t\t\tcontinue;\n \t\tif (update->flags & REF_ISPACKONLY)\n \t\t\tcontinue;\n \n@@ -3506,10 +3552,32 @@ int transaction_commit(struct ref_transaction *transaction,\n \t\t}\n \t}\n \n-\t/* Perform updates first so live commits remain referenced */\n+\t/* Lock all reflog files */\n+\tfor (i = 0; i < n; i++) {\n+\t\tstruct ref_update *update = updates[i];\n+\n+\t\tif (update->update_type != UPDATE_LOG)\n+\t\t\tcontinue;\n+\t\tupdate->reflog_fd = hold_lock_file_for_append(\n+\t\t\t\t\t&update->reflog_lock,\n+\t\t\t\t\tgit_path(\"logs/%s\", update->refname),\n+\t\t\t\t\t0);\n+\t\tif (update->reflog_fd < 0) {\n+\t\t\tconst char *str = \"Cannot lock reflog for '%s'.\";\n+\n+\t\t\tret = -1;\n+\t\t\tif (err)\n+\t\t\t\tstrbuf_addf(err, str, update->refname);\n+\t\t\tgoto cleanup;\n+\t\t}\n+\t}\n+\n+\t/* Perform ref updates first so live commits remain referenced */\n \tfor (i = 0; i < n; i++) {\n \t\tstruct ref_update *update = updates[i];\n \n+\t\tif (update->update_type != UPDATE_SHA1)\n+\t\t\tcontinue;\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 update->msg);\n@@ -3528,6 +3596,8 @@ int 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->update_type != UPDATE_SHA1)\n+\t\t\tcontinue;\n \t\tif (update->flags & REF_ISPACKONLY)\n \t\t\tcontinue;\n \n@@ -3538,6 +3608,36 @@ int transaction_commit(struct ref_transaction *transaction,\n \t\t}\n \t}\n \n+\t/* Update all reflog files */\n+\tfor (i = 0; i < n; i++) {\n+\t\tstruct ref_update *update = updates[i];\n+\n+\t\tif (update->update_type != UPDATE_LOG)\n+\t\t\tcontinue;\n+\t\tif (update->reflog_fd == -1)\n+\t\t\tcontinue;\n+\n+\t\tif (log_ref_write_fd(update->reflog_fd, update->old_sha1,\n+\t\t\t\t update->new_sha1,\n+\t\t\t\t update->committer, update->msg)) {\n+\t\t\trollback_lock_file(&update->reflog_lock);\n+\t\t\tupdate->reflog_fd = -1;\n+\t\t}\n+\t}\n+\n+\t/* Unock all reflog files */\n+\tfor (i = 0; i < n; i++) {\n+\t\tstruct ref_update *update = updates[i];\n+\n+\t\tif (update->update_type != UPDATE_LOG)\n+\t\t\tcontinue;\n+\t\tif (update->reflog_fd == -1)\n+\t\t\tcontinue;\n+\t\tif (commit_lock_file(&update->reflog_lock)) {\n+\t\t\tupdate->reflog_fd = -1;\n+\t\t}\n+\t}\n+\n \tret |= repack_without_refs(delnames, delnum);\n \tfor (i = 0; i < delnum; i++)\n \t\tunlink_or_warn(git_path(\"logs/%s\", delnames[i]));\n@@ -3688,4 +3788,5 @@ int ref_is_hidden(const char *refname)\n \t\t\treturn 1;\n \t}\n \treturn 0;\n+\n }\ndiff --git a/refs.h b/refs.h\nindex 127c12f..2e22a26 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -134,6 +134,7 @@ extern int peel_ref(const char *refname, unsigned char *sha1);\n \n /** Locks any ref (for 'HEAD' type refs). */\n #define REF_NODEREF\t0x01\n+\n extern struct ref_lock *lock_any_ref_for_update(const char *refname,\n \t\t\t\t\t\tconst unsigned char *old_sha1,\n \t\t\t\t\t\tint flags, int *type_p);\n@@ -269,6 +270,18 @@ int transaction_delete_sha1(struct ref_transaction *transaction,\n \t\t\t int flags, int have_old, const char *msg);\n \n /*\n+ * Append a reflog entry for refname.\n+ */\n+int transaction_update_reflog(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 const unsigned char *email,\n+\t\t\t unsigned long timestamp, int tz,\n+\t\t\t const char *msg,\n+\t\t\t int flags);\n+\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. If err is non-NULL we will add an error string to it to explain\n-- \n2.0.0.rc3.506.g3739a35\n"},{"id":"241615","messageId":"1400105610-21194-8-git-send-email-sahlberg@google.com","threadId":"36660","inReplyTo":"1400105610-21194-1-git-send-email-sahlberg@google.com","subject":"[PATCH 07/31] refs.c: add a flag to allow reflog updates to truncate the log","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-14T22:13:06Z","receivedAt":"2014-05-14T22:13:06Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Add a flag that allows us to truncate the reflog before we write the update.\n\nSigned-off-by: Ronnie Sahlberg \n---\n refs.c | 12 ++++++++++--\n refs.h | 4 +++-\n 2 files changed, 13 insertions(+), 3 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex d8a1568..a8b583a 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3608,7 +3608,11 @@ int transaction_commit(struct ref_transaction *transaction,\n \t\t}\n \t}\n \n-\t/* Update all reflog files */\n+\t/* Update all reflog files\n+\t We have already done all ref updates and deletes.\n+\t There is not much we can do here if there are any reflog\n+\t update errors, other than complain.\n+\t*/\n \tfor (i = 0; i < n; i++) {\n \t\tstruct ref_update *update = updates[i];\n \n@@ -3616,7 +3620,11 @@ int transaction_commit(struct ref_transaction *transaction,\n \t\t\tcontinue;\n \t\tif (update->reflog_fd == -1)\n \t\t\tcontinue;\n-\n+\t\tif (update->flags & REFLOG_TRUNCATE)\n+\t\t\tif (lseek(update->reflog_fd, 0, SEEK_SET) < 0 ||\n+\t\t\t ftruncate(update->reflog_fd, 0)) {\n+\t\t\t\terror(\"Could not truncate reflog\");\n+\t\t\t}\n \t\tif (log_ref_write_fd(update->reflog_fd, update->old_sha1,\n \t\t\t\t update->new_sha1,\n \t\t\t\t update->committer, update->msg)) {\ndiff --git a/refs.h b/refs.h\nindex 2e22a26..b1b97e7 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -269,8 +269,10 @@ int transaction_delete_sha1(struct ref_transaction *transaction,\n \t\t\t const unsigned char *old_sha1,\n \t\t\t int flags, int have_old, const char *msg);\n \n+#define REFLOG_TRUNCATE 0x01\n /*\n- * Append a reflog entry for refname.\n+ * Append a reflog entry for refname. If the REFLOG_TRUNCATE flag is set\n+ * this update will first truncate the reflog before writing the entry.\n */\n int transaction_update_reflog(struct ref_transaction *transaction,\n \t\t\t const char *refname,\n-- \n2.0.0.rc3.506.g3739a35\n"},{"id":"241594","messageId":"1400105610-21194-9-git-send-email-sahlberg@google.com","threadId":"36660","inReplyTo":"1400105610-21194-1-git-send-email-sahlberg@google.com","subject":"[PATCH 08/31] refs.c: only write reflog update if msg is non-NULL","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-14T22:13:07Z","receivedAt":"2014-05-14T22:13:07Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"When performing a reflog transaction update, only write to the reflog iff\nmsg is non-NULL. This can then be combined with REFLOG_TRUNCATE to perform\nan update that only truncates but does not write.\n\nSigned-off-by: Ronnie Sahlberg \n---\n refs.c | 8 +++++---\n refs.h | 1 +\n 2 files changed, 6 insertions(+), 3 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex a8b583a..a3f60ad 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3625,9 +3625,11 @@ int transaction_commit(struct ref_transaction *transaction,\n \t\t\t ftruncate(update->reflog_fd, 0)) {\n \t\t\t\terror(\"Could not truncate reflog\");\n \t\t\t}\n-\t\tif (log_ref_write_fd(update->reflog_fd, update->old_sha1,\n-\t\t\t\t update->new_sha1,\n-\t\t\t\t update->committer, update->msg)) {\n+\t\tif (update->msg && log_ref_write_fd(update->reflog_fd,\n+\t\t\t\t\t\t update->old_sha1,\n+\t\t\t\t\t\t update->new_sha1,\n+\t\t\t\t\t\t update->committer,\n+\t\t\t\t\t\t update->msg)) {\n \t\t\trollback_lock_file(&update->reflog_lock);\n \t\t\tupdate->reflog_fd = -1;\n \t\t}\ndiff --git a/refs.h b/refs.h\nindex b1b97e7..c552f04 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -273,6 +273,7 @@ int transaction_delete_sha1(struct ref_transaction *transaction,\n /*\n * Append a reflog entry for refname. If the REFLOG_TRUNCATE flag is set\n * this update will first truncate the reflog before writing the entry.\n+ * If msg is NULL no update will be written to the log.\n */\n int transaction_update_reflog(struct ref_transaction *transaction,\n \t\t\t const char *refname,\n-- \n2.0.0.rc3.506.g3739a35\n"},{"id":"241612","messageId":"1400105610-21194-10-git-send-email-sahlberg@google.com","threadId":"36660","inReplyTo":"1400105610-21194-1-git-send-email-sahlberg@google.com","subject":"[PATCH 09/31] refs.c: allow multiple reflog updates during a single transaction","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-14T22:13:08Z","receivedAt":"2014-05-14T22:13:08Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Allow to make multiple reflog updates to the same ref during a transaction.\nThis means we only need to lock the reflog once, during the first update that\ntouches the reflog, and that all further updates can just write the reflog\nentry since the reflog is already locked.\n\nThis allows us to write code such as:\n\nt = transaction_begin()\ntransaction_reflog_update(t, \"foo\", REFLOG_TRUNCATE, NULL);\nloop-over-somehting...\n transaction_reflog_update(t, \"foo\", 0, );\ntransaction_commit(t)\n\nwhere we first truncate the reflog and then build the new content one line at a\ntime.\n\nSigned-off-by: Ronnie Sahlberg \n---\n refs.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++---------\n 1 file changed, 49 insertions(+), 9 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex a3f60ad..e7ede03 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -37,6 +37,10 @@ static inline int bad_ref_char(int ch)\n * need to lock the loose ref during the transaction.\n */\n #define REF_ISPACKONLY\t0x0200\n+/** Only the first reflog update needs to lock the reflog file. Further updates\n+ * just use the lock taken by the first update.\n+ */\n+#define UPDATE_REFLOG_NOLOCK 0x0300\n \n /*\n * Try to read one refname component from the front of refname. Return\n@@ -3252,6 +3256,7 @@ enum transaction_update_type {\n UPDATE_LOG = 1,\n };\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@@ -3262,7 +3267,7 @@ struct ref_update {\n \tenum transaction_update_type update_type;\n \tunsigned char new_sha1[20];\n \tunsigned char old_sha1[20];\n-\tint flags; /* REF_NODEREF? */\n+\tint flags; /* REF_NODEREF? or private flags */\n \tint have_old; /* 1 if old_sha1 is valid, 0 otherwise */\n \tstruct ref_lock *lock;\n \tint type;\n@@ -3270,8 +3275,9 @@ struct ref_update {\n \n \t/* used by reflog updates */\n \tint reflog_fd;\n-\tstruct lock_file reflog_lock;\n+\tstruct lock_file *reflog_lock;\n \tconst char *committer;\n+\tstruct ref_update *orig_update; /* For UPDATE_REFLOG_NOLOCK */\n \n \tconst char refname[FLEX_ARRAY];\n };\n@@ -3349,8 +3355,23 @@ int transaction_update_reflog(struct ref_transaction *transaction,\n \t\t\t int flags)\n {\n \tstruct ref_update *update;\n+\tint i;\n \n \tupdate = add_update(transaction, refname, UPDATE_LOG);\n+\tupdate->flags = flags;\n+\tfor (i = 0; i < transaction->nr - 1; i++) {\n+\t\tif (transaction->updates[i]->update_type != UPDATE_LOG)\n+\t\t\tcontinue;\n+\t\tif (!strcmp(transaction->updates[i]->refname,\n+\t\t\t update->refname)) {\n+\t\t\tupdate->flags |= UPDATE_REFLOG_NOLOCK;\n+\t\t\tupdate->orig_update = transaction->updates[i];\n+\t\t\tbreak;\n+\t\t}\n+\t}\n+\tif (!(update->flags & UPDATE_REFLOG_NOLOCK))\n+\t update->reflog_lock = xcalloc(1, sizeof(struct lock_file));\n+\n \thashcpy(update->new_sha1, new_sha1);\n \thashcpy(update->old_sha1, old_sha1);\n \tupdate->reflog_fd = -1;\n@@ -3366,7 +3387,6 @@ int transaction_update_reflog(struct ref_transaction *transaction,\n \t}\n \tif (msg)\n \t\tupdate->msg = xstrdup(msg);\n-\tupdate->flags = flags;\n \n \treturn 0;\n }\n@@ -3479,7 +3499,7 @@ static int ref_update_reject_duplicates(struct ref_update **updates, int n,\n \t\t\t\t\tstruct strbuf *err)\n {\n \tint i;\n-\tfor (i = 1; i < n; i++)\n+\tfor (i = 1; i < n; i++) {\n \t\tif (updates[i]->update_type != UPDATE_SHA1)\n \t\t\tcontinue;\n \t\tif (!strcmp(updates[i - 1]->refname, updates[i]->refname)) {\n@@ -3490,6 +3510,7 @@ static int ref_update_reject_duplicates(struct ref_update **updates, int n,\n \n \t\t\treturn 1;\n \t\t}\n+\t}\n \treturn 0;\n }\n \n@@ -3558,10 +3579,15 @@ int transaction_commit(struct ref_transaction *transaction,\n \n \t\tif (update->update_type != UPDATE_LOG)\n \t\t\tcontinue;\n+\t\tif (update->flags & UPDATE_REFLOG_NOLOCK) {\n+\t\t\tupdate->reflog_fd = update->orig_update->reflog_fd;\n+\t\t\tupdate->reflog_lock = update->orig_update->reflog_lock;\n+\t\t\tcontinue;\n+\t\t}\n \t\tupdate->reflog_fd = hold_lock_file_for_append(\n-\t\t\t\t\t&update->reflog_lock,\n+\t\t\t\t\tupdate->reflog_lock,\n \t\t\t\t\tgit_path(\"logs/%s\", update->refname),\n-\t\t\t\t\t0);\n+\t\t\t\t\tLOCK_NODEREF);\n \t\tif (update->reflog_fd < 0) {\n \t\t\tconst char *str = \"Cannot lock reflog for '%s'.\";\n \n@@ -3630,20 +3656,22 @@ int transaction_commit(struct ref_transaction *transaction,\n \t\t\t\t\t\t update->new_sha1,\n \t\t\t\t\t\t update->committer,\n \t\t\t\t\t\t update->msg)) {\n-\t\t\trollback_lock_file(&update->reflog_lock);\n+\t\t\trollback_lock_file(update->reflog_lock);\n \t\t\tupdate->reflog_fd = -1;\n \t\t}\n \t}\n \n-\t/* Unock all reflog files */\n+\t/* Commit all reflog files */\n \tfor (i = 0; i < n; i++) {\n \t\tstruct ref_update *update = updates[i];\n \n \t\tif (update->update_type != UPDATE_LOG)\n \t\t\tcontinue;\n+\t\tif (update->flags & UPDATE_REFLOG_NOLOCK)\n+\t\t\tcontinue;\n \t\tif (update->reflog_fd == -1)\n \t\t\tcontinue;\n-\t\tif (commit_lock_file(&update->reflog_lock)) {\n+\t\tif (commit_lock_file(update->reflog_lock)) {\n \t\t\tupdate->reflog_fd = -1;\n \t\t}\n \t}\n@@ -3654,6 +3682,18 @@ int transaction_commit(struct ref_transaction *transaction,\n \tclear_loose_ref_cache(&ref_cache);\n \n cleanup:\n+\t/* Rollback any reflog files that are still open */\n+\tfor (i = 0; i < n; i++) {\n+\t\tstruct ref_update *update = updates[i];\n+\n+\t\tif (update->update_type != UPDATE_LOG)\n+\t\t\tcontinue;\n+\t\tif (update->flags & UPDATE_REFLOG_NOLOCK)\n+\t\t\tcontinue;\n+\t\tif (update->reflog_fd == -1)\n+\t\t\tcontinue;\n+\t\trollback_lock_file(update->reflog_lock);\n+\t}\n \ttransaction->status = ret ? REF_TRANSACTION_ERROR\n \t : REF_TRANSACTION_CLOSED;\n \n-- \n2.0.0.rc3.506.g3739a35\n"},{"id":"241620","messageId":"1400105610-21194-11-git-send-email-sahlberg@google.com","threadId":"36660","inReplyTo":"1400105610-21194-1-git-send-email-sahlberg@google.com","subject":"[PATCH 10/31] reflog.c: use a reflog transaction when writing during expire","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-14T22:13:09Z","receivedAt":"2014-05-14T22:13:09Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Use a transaction when doing the updates of the reflog during expire_reflog.\n\nSigned-off-by: Ronnie Sahlberg \n---\n builtin/reflog.c | 54 ++++++++++++++++++++++--------------------------------\n refs.c | 4 ++--\n 2 files changed, 24 insertions(+), 34 deletions(-)\n\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex e8a8fb1..5f07647 100644\n--- a/builtin/reflog.c\n+++ b/builtin/reflog.c\n@@ -33,7 +33,8 @@ struct cmd_reflog_expire_cb {\n };\n \n struct expire_reflog_cb {\n-\tFILE *newlog;\n+\tstruct ref_transaction *t;\n+\tconst char *refname;\n \tenum {\n \t\tUE_NORMAL,\n \t\tUE_ALWAYS,\n@@ -316,20 +317,16 @@ static int expire_reflog_ent(unsigned char *osha1, unsigned char *nsha1,\n \tif (cb->cmd->recno && --(cb->cmd->recno) == 0)\n \t\tgoto prune;\n \n-\tif (cb->newlog) {\n-\t\tchar sign = (tz < 0) ? '-' : '+';\n-\t\tint zone = (tz < 0) ? (-tz) : tz;\n-\t\tfprintf(cb->newlog, \"%s %s %s %lu %c%04d\\t%s\",\n-\t\t\tsha1_to_hex(osha1), sha1_to_hex(nsha1),\n-\t\t\temail, timestamp, sign, zone,\n-\t\t\tmessage);\n+\tif (cb->t) {\n+\t\ttransaction_update_reflog(cb->t, cb->refname, nsha1, osha1,\n+\t\t\t\t\t email, timestamp, tz, message, 0);\n \t\thashcpy(cb->last_kept_sha1, nsha1);\n \t}\n \tif (cb->cmd->verbose)\n \t\tprintf(\"keep %s\", message);\n \treturn 0;\n prune:\n-\tif (!cb->newlog)\n+\tif (!cb->t)\n \t\tprintf(\"would prune %s\", message);\n \telse if (cb->cmd->verbose)\n \t\tprintf(\"prune %s\", message);\n@@ -354,12 +351,12 @@ static int expire_reflog(const char *ref, const unsigned char *sha1, int unused,\n \tstruct cmd_reflog_expire_cb *cmd = cb_data;\n \tstruct expire_reflog_cb cb;\n \tstruct ref_lock *lock;\n-\tchar *log_file, *newlog_path = NULL;\n \tstruct commit *tip_commit;\n \tstruct commit_list *tips;\n \tint status = 0;\n \n \tmemset(&cb, 0, sizeof(cb));\n+\tcb.refname = ref;\n \n \t/*\n \t * we take the lock for the ref itself to prevent it from\n@@ -368,12 +365,14 @@ static int expire_reflog(const char *ref, const unsigned char *sha1, int unused,\n \tlock = lock_any_ref_for_update(ref, sha1, 0, NULL);\n \tif (!lock)\n \t\treturn error(\"cannot lock ref '%s'\", ref);\n-\tlog_file = git_pathdup(\"logs/%s\", ref);\n \tif (!reflog_exists(ref))\n \t\tgoto finish;\n \tif (!cmd->dry_run) {\n-\t\tnewlog_path = git_pathdup(\"logs/%s.lock\", ref);\n-\t\tcb.newlog = fopen(newlog_path, \"w\");\n+\t\tcb.t = transaction_begin();\n+\t\ttransaction_update_reflog(cb.t, cb.refname,\n+\t\t\t\t\t null_sha1, null_sha1, NULL,\n+\t\t\t\t\t 0, 0, NULL,\n+\t\t\t\t\t REFLOG_TRUNCATE);\n \t}\n \n \tcb.cmd = cmd;\n@@ -420,31 +419,22 @@ static int expire_reflog(const char *ref, const unsigned char *sha1, int unused,\n \t\t}\n \t}\n finish:\n-\tif (cb.newlog) {\n-\t\tif (fclose(cb.newlog)) {\n-\t\t\tstatus |= error(\"%s: %s\", strerror(errno),\n-\t\t\t\t\tnewlog_path);\n-\t\t\tunlink(newlog_path);\n-\t\t} else if (cmd->updateref &&\n-\t\t\t(write_in_full(lock->lock_fd,\n-\t\t\t\tsha1_to_hex(cb.last_kept_sha1), 40) != 40 ||\n-\t\t\t write_str_in_full(lock->lock_fd, \"\\n\") != 1 ||\n-\t\t\t close_ref(lock) < 0)) {\n+\tif (cb.t) {\n+\t\tif (cmd->updateref &&\n+\t\t (write_in_full(lock->lock_fd,\n+\t\t\t\t sha1_to_hex(cb.last_kept_sha1), 40) != 40 ||\n+\t\t write_str_in_full(lock->lock_fd, \"\\n\") != 1 ||\n+\t\t close_ref(lock) < 0)) {\n \t\t\tstatus |= error(\"Couldn't write %s\",\n \t\t\t\tlock->lk->filename);\n-\t\t\tunlink(newlog_path);\n-\t\t} else if (rename(newlog_path, log_file)) {\n-\t\t\tstatus |= error(\"cannot rename %s to %s\",\n-\t\t\t\t\tnewlog_path, log_file);\n-\t\t\tunlink(newlog_path);\n+\t\t\ttransaction_rollback(cb.t);\n+\t\t} else if (transaction_commit(cb.t, NULL)) {\n+\t\t\tstatus |= error(\"cannot commit reflog for %s\", ref);\n \t\t} else if (cmd->updateref && commit_ref(lock)) {\n \t\t\tstatus |= error(\"Couldn't set %s\", lock->ref_name);\n-\t\t} else {\n-\t\t\tadjust_shared_perm(log_file);\n \t\t}\n+\t\ttransaction_free(cb.t);\n \t}\n-\tfree(newlog_path);\n-\tfree(log_file);\n \tunlock_ref(lock);\n \treturn status;\n }\ndiff --git a/refs.c b/refs.c\nindex e7ede03..5a5f9df 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2766,8 +2766,8 @@ static int log_ref_write_fd(int fd, const unsigned char *old_sha1,\n \t\t committer);\n \tif (msglen)\n \t\tlen += copy_msg(logrec + len - 1, msg) - 1;\n-\twritten = len <= maxlen ? write_in_full(fd, logrec, len) : -1;\n \n+\twritten = len <= maxlen ? write_in_full(fd, logrec, len) : -1;\n \tfree(logrec);\n \tif (written != len)\n \t\treturn -1;\n@@ -3634,7 +3634,7 @@ int transaction_commit(struct ref_transaction *transaction,\n \t\t}\n \t}\n \n-\t/* Update all reflog files\n+\t/* Update all reflog files.\n \t We have already done all ref updates and deletes.\n \t There is not much we can do here if there are any reflog\n \t update errors, other than complain.\n-- \n2.0.0.rc3.506.g3739a35\n"},{"id":"241618","messageId":"1400105610-21194-12-git-send-email-sahlberg@google.com","threadId":"36660","inReplyTo":"1400105610-21194-1-git-send-email-sahlberg@google.com","subject":"[PATCH 11/31] refs.c: null terminate the string in copy_msg","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-14T22:13:10Z","receivedAt":"2014-05-14T22:13:10Z","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 | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/refs.c b/refs.c\nindex 5a5f9df..58bbf1b 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2707,6 +2707,7 @@ static int copy_msg(char *buf, const char *msg)\n \twhile (buf < cp && isspace(cp[-1]))\n \t\tcp--;\n \t*cp++ = '\\n';\n+\t*cp = 0;\n \treturn cp - buf;\n }\n \n-- \n2.0.0.rc3.506.g3739a35\n"},{"id":"241606","messageId":"1400105610-21194-13-git-send-email-sahlberg@google.com","threadId":"36660","inReplyTo":"1400105610-21194-1-git-send-email-sahlberg@google.com","subject":"[PATCH 12/31] refs.c: track the refnames we are deleting in the transaction structure","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-14T22:13:11Z","receivedAt":"2014-05-14T22:13:11Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Track the names of the refs we are deleting in the ref_transaction structure\ninstead of building an array of names during _commit.\n\nSigned-off-by: Ronnie Sahlberg \n---\n refs.c | 35 +++++++++++++++++++++++------------\n 1 file changed, 23 insertions(+), 12 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 58bbf1b..cc31efe 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3299,6 +3299,8 @@ struct ref_transaction {\n \tsize_t alloc;\n \tsize_t nr;\n \tenum ref_transaction_status status;\n+\tconst char **delnames;\n+\tint delnum;\n };\n \n struct ref_transaction *transaction_begin(void)\n@@ -3306,6 +3308,15 @@ struct ref_transaction *transaction_begin(void)\n \treturn xcalloc(1, sizeof(struct ref_transaction));\n }\n \n+static void add_delname(struct ref_transaction *transaction,\n+\t\t\tconst char *refname)\n+{\n+\ttransaction->delnames = xrealloc(transaction->delnames,\n+\t\t\t\t\t ++transaction->delnum *\n+\t\t\t\t\t sizeof(const char *));\n+\ttransaction->delnames[transaction->delnum - 1] = xstrdup(refname);\n+}\n+\n void transaction_free(struct ref_transaction *transaction)\n {\n \tint i;\n@@ -3319,6 +3330,9 @@ void transaction_free(struct ref_transaction *transaction)\n \t\tfree(transaction->updates[i]);\n \t}\n \tfree(transaction->updates);\n+\tfor (i = 0; i < transaction->delnum; i++)\n+\t\tfree((char *)transaction->delnames[i]);\n+\tfree((char *)transaction->delnames);\n \tfree(transaction);\n }\n \n@@ -3518,8 +3532,7 @@ static int ref_update_reject_duplicates(struct ref_update **updates, int n,\n int transaction_commit(struct ref_transaction *transaction,\n \t\t\t struct strbuf *err)\n {\n-\tint ret = 0, delnum = 0, i;\n-\tconst char **delnames;\n+\tint ret = 0, i;\n \tint n = transaction->nr;\n \tstruct ref_update **updates = transaction->updates;\n \n@@ -3531,9 +3544,6 @@ int transaction_commit(struct ref_transaction *transaction,\n \t\treturn 0;\n \t}\n \n-\t/* Allocate work space */\n-\tdelnames = xmalloc(sizeof(*delnames) * n);\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);\n@@ -3546,7 +3556,7 @@ int transaction_commit(struct ref_transaction *transaction,\n \t\tif (update->update_type != UPDATE_SHA1)\n \t\t\tcontinue;\n \t\tif (update->flags & REF_ISPACKONLY)\n-\t\t\tdelnames[delnum++] = update->refname;\n+\t\t\tadd_delname(transaction, update->refname);\n \t}\n \n \t/* Acquire all ref locks while verifying old values */\n@@ -3564,7 +3574,8 @@ int transaction_commit(struct ref_transaction *transaction,\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 delnames, delnum);\n+\t\t\t\t\t\t transaction->delnames,\n+\t\t\t\t\t\t transaction->delnum);\n \t\tif (!update->lock) {\n \t\t\tif (err)\n \t\t\t\tstrbuf_addf(err, \"Cannot lock the ref '%s'.\",\n@@ -3631,7 +3642,8 @@ int transaction_commit(struct ref_transaction *transaction,\n \t\tif (update->lock) {\n \t\t\tret |= delete_ref_loose(update->lock, update->type);\n \t\t\tif (!(update->flags & REF_ISPRUNING))\n-\t\t\t\tdelnames[delnum++] = update->lock->ref_name;\n+\t\t\t\tadd_delname(transaction,\n+\t\t\t\t\t update->lock->ref_name);\n \t\t}\n \t}\n \n@@ -3677,9 +3689,9 @@ int transaction_commit(struct ref_transaction *transaction,\n \t\t}\n \t}\n \n-\tret |= repack_without_refs(delnames, delnum);\n-\tfor (i = 0; i < delnum; i++)\n-\t\tunlink_or_warn(git_path(\"logs/%s\", delnames[i]));\n+\tret |= repack_without_refs(transaction->delnames, transaction->delnum);\n+\tfor (i = 0; i < transaction->delnum; i++)\n+\t\tunlink_or_warn(git_path(\"logs/%s\", transaction->delnames[i]));\n \tclear_loose_ref_cache(&ref_cache);\n \n cleanup:\n@@ -3701,7 +3713,6 @@ cleanup:\n \tfor (i = 0; i < n; i++)\n \t\tif (updates[i]->lock)\n \t\t\tunlock_ref(updates[i]->lock);\n-\tfree(delnames);\n \treturn ret;\n }\n \n-- \n2.0.0.rc3.506.g3739a35\n"},{"id":"241605","messageId":"1400105610-21194-14-git-send-email-sahlberg@google.com","threadId":"36660","inReplyTo":"1400105610-21194-1-git-send-email-sahlberg@google.com","subject":"[PATCH 13/31] refs.c: update the list of deleted refs during _update instead of _commit","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-14T22:13:12Z","receivedAt":"2014-05-14T22:13:12Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"When deleting refs during a transaction, update the list of the refs to be\ndeleted already during _update instead of waiting until the _commit stage.\n\nSigned-off-by: Ronnie Sahlberg \n---\n refs.c | 16 +++++++---------\n 1 file changed, 7 insertions(+), 9 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex cc31efe..3249576 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3431,6 +3431,13 @@ int transaction_update_sha1(struct ref_transaction *transaction,\n \t\thashcpy(update->old_sha1, old_sha1);\n \tif (msg)\n \t\tupdate->msg = xstrdup(msg);\n+\n+\t/* This is a deletion of a ref that exists as a packed ref which means\n+\t * we do not need to check against this ref for name collissions\n+\t * during locking.\n+\t */\n+\tif (update->flags & REF_ISPACKONLY)\n+\t\tadd_delname(transaction, update->refname);\n \treturn 0;\n }\n \n@@ -3550,15 +3557,6 @@ int 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->update_type != UPDATE_SHA1)\n-\t\t\tcontinue;\n-\t\tif (update->flags & REF_ISPACKONLY)\n-\t\t\tadd_delname(transaction, update->refname);\n-\t}\n-\n \t/* Acquire all ref locks while verifying old values */\n \tfor (i = 0; i < n; i++) {\n \t\tstruct ref_update *update = updates[i];\n-- \n2.0.0.rc3.506.g3739a35\n"},{"id":"241607","messageId":"1400105610-21194-15-git-send-email-sahlberg@google.com","threadId":"36660","inReplyTo":"1400105610-21194-1-git-send-email-sahlberg@google.com","subject":"[PATCH 14/31] refs.c: return error instead of dying when locking fails during transaction","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-14T22:13:13Z","receivedAt":"2014-05-14T22:13:13Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Change lock_ref_sha1_basic to return an error instead of dying when\nwe fail to lock a file during a transaction.\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 3249576..8212d77 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2147,7 +2147,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,\n \t\t\t */\n \t\t\tgoto retry;\n \t\telse\n-\t\t\tunable_to_lock_index_die(ref_file, errno);\n+\t\t\tgoto error_return;\n \t}\n \treturn old_sha1 ? verify_lock(lock, old_sha1, mustexist) : lock;\n \n-- \n2.0.0.rc3.506.g3739a35\n"},{"id":"241596","messageId":"1400105610-21194-16-git-send-email-sahlberg@google.com","threadId":"36660","inReplyTo":"1400105610-21194-1-git-send-email-sahlberg@google.com","subject":"[PATCH 15/31] refs.c: lock the ref during _update instead of during _commit","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-14T22:13:14Z","receivedAt":"2014-05-14T22:13:14Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Try to lock the ref during the transaction update instead of later during the\ncommit phase. This allows us to do things such as :\n\nt = transaction_begin()\ntransaction_update_sha1(t, ref, ...) /* locking ref */\n...\ndo stuff while we know that ref is locked\n...\ntransaction_commit(t)\n\nSigned-off-by: Ronnie Sahlberg \n---\n refs.c | 18 ++++++++++--------\n 1 file changed, 10 insertions(+), 8 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 8212d77..b006238 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3438,6 +3438,16 @@ int transaction_update_sha1(struct ref_transaction *transaction,\n \t */\n \tif (update->flags & REF_ISPACKONLY)\n \t\tadd_delname(transaction, update->refname);\n+\telse {\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\t\t\t\t\t transaction->delnames,\n+\t\t\t\t\t\t transaction->delnum);\n+\t}\n \treturn 0;\n }\n \n@@ -3566,14 +3576,6 @@ int transaction_commit(struct ref_transaction *transaction,\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-\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 transaction->delnames,\n-\t\t\t\t\t\t transaction->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.506.g3739a35\n"},{"id":"241604","messageId":"1400105610-21194-17-git-send-email-sahlberg@google.com","threadId":"36660","inReplyTo":"1400105610-21194-1-git-send-email-sahlberg@google.com","subject":"[PATCH 16/31] refs.c: add an error argument to create/delete/update just like commit","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-14T22:13:15Z","receivedAt":"2014-05-14T22:13:15Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Update the signatures for transaction create/delete/update and add\na strbuf err argument we can use to pass an error back to the caller.\nWe will need this later once we start moving checks from _commit to _update.\nChange all callers that would die() on _commit failures to handle _update\nfailures similarly.\n\nSigned-off-by: Ronnie Sahlberg \n---\n branch.c | 3 ++-\n builtin/commit.c | 2 +-\n builtin/fetch.c | 2 +-\n builtin/receive-pack.c | 3 ++-\n builtin/replace.c | 2 +-\n builtin/tag.c | 2 +-\n builtin/update-ref.c | 20 ++++++++++++--------\n fast-import.c | 8 +++++---\n refs.c | 24 ++++++++++++++----------\n refs.h | 9 ++++++---\n sequencer.c | 2 +-\n walker.c | 3 ++-\n 12 files changed, 48 insertions(+), 32 deletions(-)\n\ndiff --git a/branch.c b/branch.c\nindex 0a4d4f3..0b9fd61 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -301,7 +301,8 @@ void create_branch(const char *head,\n \t\ttransaction = transaction_begin();\n \t\tif (!transaction ||\n \t\t transaction_update_sha1(transaction, ref.buf, sha1,\n-\t\t\t\t\t null_sha1, 0, !forcing, msg) ||\n+\t\t\t\t\t null_sha1, 0, !forcing, msg,\n+\t\t\t\t\t &err) ||\n \t\t transaction_commit(transaction, &err))\n \t\t\t\tdie_errno(_(\"%s: failed to write ref: %s\"),\n \t\t\t\t\t ref.buf, err.buf);\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 386dfb1..fd61a8e 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1721,7 +1721,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t transaction_update_sha1(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, sb.buf) ||\n+\t\t\t\t 0, !!current_head, sb.buf, &err) ||\n \t transaction_commit(transaction, &err)) {\n \t\trollback_index_files();\n \t\tdie(_(\"HEAD: cannot update ref: %s\"), err.buf);\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex f74a267..e5e891f 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -384,7 +384,7 @@ static int s_update_ref(const char *action,\n \tsnprintf(msg, sizeof(msg), \"%s: %s\", rla, action);\n \n \tif (transaction_update_sha1(transaction, ref->name, ref->new_sha1,\n-\t\t\t\t ref->old_sha1, 0, check_old, msg))\n+\t\t\t\t ref->old_sha1, 0, check_old, msg, NULL))\n \t\treturn STORE_REF_ERROR_OTHER;\n \n \treturn 0;\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex 0461f93..777ecf8 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 (transaction_update_sha1(transaction, namespaced_name,\n-\t\t\t\t\t new_sha1, old_sha1, 0, 1, \"push\"))\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}\ndiff --git a/builtin/replace.c b/builtin/replace.c\nindex 2587a06..1598d69 100644\n--- a/builtin/replace.c\n+++ b/builtin/replace.c\n@@ -160,7 +160,7 @@ static int replace_object_sha1(const char *object_ref,\n \ttransaction = transaction_begin();\n \tif (!transaction ||\n \t transaction_update_sha1(transaction, ref, repl, prev,\n-\t\t\t\t 0, !is_null_sha1(prev), NULL) ||\n+\t\t\t\t 0, !is_null_sha1(prev), NULL, &err) ||\n \t transaction_commit(transaction, &err))\n \t\tdie(_(\"%s: failed to replace ref: %s\"), ref, err.buf);\n \ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 3739e23..9b6da6c 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -705,7 +705,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \ttransaction = transaction_begin();\n \tif (!transaction ||\n \t transaction_update_sha1(transaction, ref.buf, object, prev,\n-\t\t\t\t 0, !is_null_sha1(prev), NULL) ||\n+\t\t\t\t 0, !is_null_sha1(prev), NULL, &err) ||\n \t transaction_commit(transaction, &err))\n \t\tdie(_(\"%s: cannot update the ref: %s\"), ref.buf, err.buf);\n \ttransaction_free(transaction);\ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex f7e33bd..1b0c889 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -183,6 +183,7 @@ static const char *parse_cmd_update(struct strbuf *input, const char *next)\n \tunsigned char new_sha1[20];\n \tunsigned char old_sha1[20];\n \tint have_old;\n+\tstruct strbuf err = STRBUF_INIT;\n \n \trefname = parse_refname(input, &next);\n \tif (!refname)\n@@ -199,8 +200,8 @@ 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 (transaction_update_sha1(transaction, refname, new_sha1, old_sha1,\n-\t\t\t\t update_flags, have_old, msg))\n-\t\tdie(\"update %s: failed\", refname);\n+\t\t\t\t update_flags, have_old, msg, &err))\n+\t\tdie(\"%s\", err.buf);\n \n \tupdate_flags = 0;\n \tfree(refname);\n@@ -212,6 +213,7 @@ static const char *parse_cmd_create(struct strbuf *input, const char *next)\n {\n \tchar *refname;\n \tunsigned char new_sha1[20];\n+\tstruct strbuf err = STRBUF_INIT;\n \n \trefname = parse_refname(input, &next);\n \tif (!refname)\n@@ -227,8 +229,8 @@ 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 (transaction_create_sha1(transaction, refname, new_sha1,\n-\t\t\t\t update_flags, msg))\n-\t\tdie(\"failed transaction create for %s\", refname);\n+\t\t\t\t update_flags, msg, &err))\n+\t\tdie(\"%s\", err.buf);\n \n \tupdate_flags = 0;\n \tfree(refname);\n@@ -241,6 +243,7 @@ static const char *parse_cmd_delete(struct strbuf *input, const char *next)\n \tchar *refname;\n \tunsigned char old_sha1[20];\n \tint have_old;\n+\tstruct strbuf err = STRBUF_INIT;\n \n \trefname = parse_refname(input, &next);\n \tif (!refname)\n@@ -259,8 +262,8 @@ 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 (transaction_delete_sha1(transaction, refname, old_sha1,\n-\t\t\t\t update_flags, have_old, msg))\n-\t\tdie(\"failed transaction delete for %s\", refname);\n+\t\t\t\t update_flags, have_old, msg, &err))\n+\t\tdie(\"%s\", err.buf);\n \n \tupdate_flags = 0;\n \tfree(refname);\n@@ -274,6 +277,7 @@ static const char *parse_cmd_verify(struct strbuf *input, const char *next)\n \tunsigned char new_sha1[20];\n \tunsigned char old_sha1[20];\n \tint have_old;\n+\tstruct strbuf err = STRBUF_INIT;\n \n \trefname = parse_refname(input, &next);\n \tif (!refname)\n@@ -292,8 +296,8 @@ 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 (transaction_update_sha1(transaction, refname, new_sha1, old_sha1,\n-\t\t\t\t update_flags, have_old, msg))\n-\t\tdie(\"failed transaction update for %s\", refname);\n+\t\t\t\t update_flags, have_old, msg, &err))\n+\t\tdie(\"%s\", err.buf);\n \n \tupdate_flags = 0;\n \tfree(refname);\ndiff --git a/fast-import.c b/fast-import.c\nindex 2fa1d29..24378d1 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -1710,7 +1710,7 @@ static int update_branch(struct branch *b)\n \ttransaction = transaction_begin();\n \tif (!transaction ||\n \t transaction_update_sha1(transaction, b->name, b->sha1, old_sha1,\n-\t\t\t\t 0, 1, msg) ||\n+\t\t\t\t 0, 1, msg, &err) ||\n \t transaction_commit(transaction, &err)) {\n \t\ttransaction_rollback(transaction);\n \t\terror(\"Unable to update branch %s: %s\", b->name, err.buf);\n@@ -1745,8 +1745,10 @@ static void dump_tags(void)\n \t\tsprintf(ref_name, \"refs/tags/%s\", t->name);\n \n \t\tif (transaction_update_sha1(transaction, ref_name, t->sha1,\n-\t\t\t\t\t NULL, 0, 0, msg))\n-\t\t\tfailure |= error(\"Unable to update %s\", err.buf);\n+\t\t\t\t\t NULL, 0, 0, msg, &err)) {\n+\t\t\tfailure |= 1;\n+\t\t\tbreak;\n+\t\t}\n \t}\n \tif (failure || transaction_commit(transaction, &err))\n \t\tfailure |= error(\"Unable to update %s\", err.buf);\ndiff --git a/refs.c b/refs.c\nindex b006238..59f1ca1 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2364,7 +2364,7 @@ static void prune_ref(struct ref_to_prune *r)\n \ttransaction = transaction_begin();\n \tif (!transaction ||\n \t transaction_delete_sha1(transaction, r->name, r->sha1,\n-\t\t\t\t REF_ISPRUNING, 1, NULL) ||\n+\t\t\t\t REF_ISPRUNING, 1, NULL, &err) ||\n \t transaction_commit(transaction, &err)) {\n \t\ttransaction_rollback(transaction);\n \t\twarning(\"prune_ref: %s\", err.buf);\n@@ -2528,7 +2528,7 @@ int delete_ref(const char *refname, const unsigned char *sha1, int delopt)\n \ttransaction = transaction_begin();\n \tif (!transaction ||\n \t transaction_delete_sha1(transaction, refname, sha1, delopt,\n-\t\t\t\t sha1 && !is_null_sha1(sha1), NULL) ||\n+\t\t\t\t sha1 && !is_null_sha1(sha1), NULL, NULL) ||\n \t transaction_commit(transaction, NULL)) {\n \t\ttransaction_rollback(transaction);\n \t\treturn 1;\n@@ -2633,9 +2633,9 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms\n \tif (!transaction ||\n \t transaction_delete_sha1(transaction, oldrefname, sha1,\n \t\t\t\t REF_NODEREF | REF_ISPACKONLY,\n-\t\t\t\t 1, NULL) ||\n+\t\t\t\t 1, NULL, &err) ||\n \t transaction_update_sha1(transaction, newrefname, sha1,\n-\t\t\t\t NULL, 0, 0, logmsg) ||\n+\t\t\t\t NULL, 0, 0, logmsg, &err) ||\n \t transaction_commit(transaction, &err)) {\n \t\ttransaction_rollback(transaction);\n \t\terror(\"rename_ref failed: %s\", err.buf);\n@@ -3410,7 +3410,8 @@ int transaction_update_sha1(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, const char *msg)\n+\t\t\t int flags, int have_old, const char *msg,\n+\t\t\t struct strbuf *err)\n {\n \tstruct ref_update *update;\n \n@@ -3454,7 +3455,8 @@ int transaction_update_sha1(struct ref_transaction *transaction,\n int transaction_create_sha1(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, const char *msg)\n+\t\t\t int flags, const char *msg,\n+\t\t\t struct strbuf *err)\n {\n \tstruct ref_update *update;\n \n@@ -3468,13 +3470,14 @@ int transaction_create_sha1(struct ref_transaction *transaction,\n \t\tdie(\"BUG: REF_ISPACKONLY can not be used with create\");\n \n \treturn transaction_update_sha1(transaction, refname, new_sha1,\n-\t\t\t\t null_sha1, flags, 1, msg);\n+\t\t\t\t null_sha1, flags, 1, msg, err);\n }\n \n int transaction_delete_sha1(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, const char *msg)\n+\t\t\t int flags, int have_old, const char *msg,\n+\t\t\t struct strbuf *err)\n {\n \tstruct ref_update *update;\n \n@@ -3488,7 +3491,8 @@ int transaction_delete_sha1(struct ref_transaction *transaction,\n \t\tdie(\"BUG: delete on transaction that is not open\");\n \n \treturn transaction_update_sha1(transaction, refname, null_sha1,\n-\t\t\t\t old_sha1, flags, have_old, msg);\n+\t\t\t\t old_sha1, flags, have_old, msg,\n+\t\t\t\t err);\n }\n \n int update_ref(const char *action, const char *refname,\n@@ -3501,7 +3505,7 @@ int update_ref(const char *action, const char *refname,\n \tt = transaction_begin();\n \tif (!t ||\n \t transaction_update_sha1(t, refname, sha1, oldval, flags,\n-\t\t\t\t !!oldval, action) ||\n+\t\t\t\t !!oldval, action, &err) ||\n \t transaction_commit(t, &err)) {\n \t\tconst char *str = \"update_ref failed for ref '%s': %s\";\n \ndiff --git a/refs.h b/refs.h\nindex c552f04..ebe7368 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -244,7 +244,8 @@ int transaction_update_sha1(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, const char *msg);\n+\t\t\t int flags, int have_old, const char *msg,\n+\t\t\t struct strbuf *err);\n \n /*\n * Add a reference creation to transaction. new_sha1 is the value\n@@ -256,7 +257,8 @@ int transaction_update_sha1(struct ref_transaction *transaction,\n int transaction_create_sha1(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, const char *msg);\n+\t\t\t int flags, const char *msg,\n+\t\t\t struct strbuf *err);\n \n /*\n * Add a reference deletion to transaction. If have_old is true, then\n@@ -267,7 +269,8 @@ int transaction_create_sha1(struct ref_transaction *transaction,\n int transaction_delete_sha1(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, const char *msg);\n+\t\t\t int flags, int have_old, const char *msg,\n+\t\t\t struct strbuf *err);\n \n #define REFLOG_TRUNCATE 0x01\n /*\ndiff --git a/sequencer.c b/sequencer.c\nindex 3f7996f..604df39 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -285,7 +285,7 @@ static int fast_forward_to(const unsigned char *to, const unsigned char *from,\n \ttransaction = transaction_begin();\n \tif (!transaction ||\n \t transaction_update_sha1(transaction, \"HEAD\", to, from,\n-\t\t\t\t 0, !unborn, sb.buf) ||\n+\t\t\t\t 0, !unborn, sb.buf, &err) ||\n \t transaction_commit(transaction, &err)) {\n \t\ttransaction_rollback(transaction);\n \t\terror(_(\"HEAD: Could not fast-forward: %s\\n\"), err.buf);\ndiff --git a/walker.c b/walker.c\nindex 3701c78..8630fcb 100644\n--- a/walker.c\n+++ b/walker.c\n@@ -292,7 +292,8 @@ int walker_fetch(struct walker *walker, int targets, char **target,\n \t\tif (transaction_update_sha1(transaction, ref_name,\n \t\t\t\t\t &sha1[20 * i], NULL,\n \t\t\t\t\t 0, 0,\n-\t\t\t\t\t msg ? msg : \"fetch (unknown)\"))\n+\t\t\t\t\t msg ? msg : \"fetch (unknown)\",\n+\t\t\t\t\t &err))\n \t\t\tgoto rollback_and_fail;\n \t}\n \n-- \n2.0.0.rc3.506.g3739a35\n"},{"id":"241609","messageId":"1400105610-21194-18-git-send-email-sahlberg@google.com","threadId":"36660","inReplyTo":"1400105610-21194-1-git-send-email-sahlberg@google.com","subject":"[PATCH 17/31] refs.c: make _update_reflog take an error argument","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-14T22:13:16Z","receivedAt":"2014-05-14T22:13:16Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Add a strbug argument to the update reflog transaction call so we can return\nan error string back to the caller on failure.\n\nSigned-off-by: Ronnie Sahlberg \n---\n builtin/reflog.c | 6 ++++--\n refs.c | 3 ++-\n refs.h | 3 ++-\n 3 files changed, 8 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex 5f07647..2239249 100644\n--- a/builtin/reflog.c\n+++ b/builtin/reflog.c\n@@ -319,7 +319,8 @@ static int expire_reflog_ent(unsigned char *osha1, unsigned char *nsha1,\n \n \tif (cb->t) {\n \t\ttransaction_update_reflog(cb->t, cb->refname, nsha1, osha1,\n-\t\t\t\t\t email, timestamp, tz, message, 0);\n+\t\t\t\t\t email, timestamp, tz, message, 0,\n+\t\t\t\t\t NULL);\n \t\thashcpy(cb->last_kept_sha1, nsha1);\n \t}\n \tif (cb->cmd->verbose)\n@@ -372,7 +373,8 @@ static int expire_reflog(const char *ref, const unsigned char *sha1, int unused,\n \t\ttransaction_update_reflog(cb.t, cb.refname,\n \t\t\t\t\t null_sha1, null_sha1, NULL,\n \t\t\t\t\t 0, 0, NULL,\n-\t\t\t\t\t REFLOG_TRUNCATE);\n+\t\t\t\t\t REFLOG_TRUNCATE,\n+\t\t\t\t\t NULL);\n \t}\n \n \tcb.cmd = cmd;\ndiff --git a/refs.c b/refs.c\nindex 59f1ca1..8b09258 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3367,7 +3367,8 @@ int transaction_update_reflog(struct ref_transaction *transaction,\n \t\t\t const unsigned char *email,\n \t\t\t unsigned long timestamp, int tz,\n \t\t\t const char *msg,\n-\t\t\t int flags)\n+\t\t\t int flags,\n+\t\t\t struct strbuf *err)\n {\n \tstruct ref_update *update;\n \tint i;\ndiff --git a/refs.h b/refs.h\nindex ebe7368..6c628bb 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -285,7 +285,8 @@ int transaction_update_reflog(struct ref_transaction *transaction,\n \t\t\t const unsigned char *email,\n \t\t\t unsigned long timestamp, int tz,\n \t\t\t const char *msg,\n-\t\t\t int flags);\n+\t\t\t int flags,\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.506.g3739a35\n"},{"id":"241597","messageId":"1400105610-21194-19-git-send-email-sahlberg@google.com","threadId":"36660","inReplyTo":"1400105610-21194-1-git-send-email-sahlberg@google.com","subject":"[PATCH 18/31] refs.c: return immediately from _commit if the transaction has an error","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-14T22:13:17Z","receivedAt":"2014-05-14T22:13:17Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"If a previous transaction operation has failed and flagged a transaction\nas having an error, then return immediately from commit and indicate failure.\nOnce we move checks from _commit to _update, _update will already have updated\nthe error string so the caller will know what failed. Thus we don't need to do\nanything at all in _commit than return failure.\n\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 8b09258..93f01e8 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3558,6 +3558,9 @@ int transaction_commit(struct ref_transaction *transaction,\n \tint n = transaction->nr;\n \tstruct ref_update **updates = transaction->updates;\n \n+\tif (transaction->status == REF_TRANSACTION_ERROR)\n+\t\treturn 1;\n+\n \tif (transaction->status != REF_TRANSACTION_OPEN)\n \t\tdie(\"BUG: commit on transaction that is not open\");\n \n-- \n2.0.0.rc3.506.g3739a35\n"},{"id":"241608","messageId":"1400105610-21194-20-git-send-email-sahlberg@google.com","threadId":"36660","inReplyTo":"1400105610-21194-1-git-send-email-sahlberg@google.com","subject":"[PATCH 19/31] tests: move tests for -z update/delete/verify to after the ref is created","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-14T22:13:18Z","receivedAt":"2014-05-14T22:13:18Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"The tests for stdin -z fails for update/delete/verify are of the form\nupdate refs/heads/a $m $m $m\nand rely on the fact that when parsed we will first parse everything except\nthe trailing $m. And once that command has completed it will start parsing\nthe next command, find the trailing $m from before and fail because this\nwould not look like a proper command. I.e. we do not detect that there are\ntoo many arguments so much as detect that the next command looks like\ngarbage.\n\nBut since the ref refs/heads/a does not exist at this point in the test we\nactually have two failures that trigger. We have one error that is due to\nupdate refs/heads/a $m $m would be invalid because the ref does not exist\nand we have a second error that is due to the trailing $m causing a \"too\nmany arguments\". The current test depends on the order of check for these two\nwhich makes the test fragile (and would break after the next patch).\n\nMove these tests further down in the test to occur after we have created\nrefs/heads/a so that there is only one error condition in the test and that\nwe no longer depend on in which order the checks for error occur.\n\nSigned-off-by: Ronnie Sahlberg \n---\n t/t1400-update-ref.sh | 36 ++++++++++++++++++------------------\n 1 file changed, 18 insertions(+), 18 deletions(-)\n\ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex 4e2459a..f9b7bef 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -756,12 +756,6 @@ test_expect_success 'stdin -z fails update with no old value' '\n \tgrep \"fatal: update $a: unexpected end of input when reading \" err\n '\n \n-test_expect_success 'stdin -z fails update with too many arguments' '\n-\tprintf $F \"update $a\" \"$m\" \"$m\" \"$m\" >stdin &&\n-\ttest_must_fail git update-ref -z --stdin err &&\n-\tgrep \"fatal: unknown command: $m\" err\n-'\n-\n test_expect_success 'stdin -z fails delete with no ref' '\n \tprintf $F \"delete \" >stdin &&\n \ttest_must_fail git update-ref -z --stdin err &&\n@@ -780,18 +774,6 @@ test_expect_success 'stdin -z fails delete with no old value' '\n \tgrep \"fatal: delete $a: unexpected end of input when reading \" err\n '\n \n-test_expect_success 'stdin -z fails delete with too many arguments' '\n-\tprintf $F \"delete $a\" \"$m\" \"$m\" >stdin &&\n-\ttest_must_fail git update-ref -z --stdin err &&\n-\tgrep \"fatal: unknown command: $m\" err\n-'\n-\n-test_expect_success 'stdin -z fails verify with too many arguments' '\n-\tprintf $F \"verify $a\" \"$m\" \"$m\" >stdin &&\n-\ttest_must_fail git update-ref -z --stdin err &&\n-\tgrep \"fatal: unknown command: $m\" err\n-'\n-\n test_expect_success 'stdin -z fails verify with no old value' '\n \tprintf $F \"verify $a\" >stdin &&\n \ttest_must_fail git update-ref -z --stdin err &&\n@@ -818,6 +800,24 @@ test_expect_success 'stdin -z create ref works' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'stdin -z fails update with too many arguments' '\n+\tprintf $F \"update $a\" \"$m\" \"$m\" \"$m\" >stdin &&\n+\ttest_must_fail git update-ref -z --stdin err &&\n+\tgrep \"fatal: unknown command: $m\" err\n+'\n+\n+test_expect_success 'stdin -z fails delete with too many arguments' '\n+\tprintf $F \"delete $a\" \"$m\" \"$m\" >stdin &&\n+\ttest_must_fail git update-ref -z --stdin err &&\n+\tgrep \"fatal: unknown command: $m\" err\n+'\n+\n+test_expect_success 'stdin -z fails verify with too many arguments' '\n+\tprintf $F \"verify $a\" \"$m\" \"$m\" >stdin &&\n+\ttest_must_fail git update-ref -z --stdin err &&\n+\tgrep \"fatal: unknown command: $m\" err\n+'\n+\n test_expect_success 'stdin -z update ref creates with zero old value' '\n \tprintf $F \"update $b\" \"$m\" \"$Z\" >stdin &&\n \tgit update-ref -z --stdin \n---\n refs.c | 85 +++++++++++++++++++++++++++++++++---------------------------------\n 1 file changed, 42 insertions(+), 43 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 93f01e8..76cab6e 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3415,6 +3415,7 @@ int transaction_update_sha1(struct ref_transaction *transaction,\n \t\t\t struct strbuf *err)\n {\n \tstruct ref_update *update;\n+\tint i;\n \n \tif (have_old && !old_sha1)\n \t\tdie(\"BUG: have_old is true but old_sha1 is NULL\");\n@@ -3438,19 +3439,49 @@ int transaction_update_sha1(struct ref_transaction *transaction,\n \t * we do not need to check against this ref for name collissions\n \t * during locking.\n \t */\n-\tif (update->flags & REF_ISPACKONLY)\n+\tif (update->flags & REF_ISPACKONLY) {\n \t\tadd_delname(transaction, update->refname);\n-\telse {\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\t\t\t\t\t transaction->delnames,\n-\t\t\t\t\t\t transaction->delnum);\n+\t\treturn 0;\n \t}\n-\treturn 0;\n+\n+\tupdate->lock = lock_ref_sha1_basic(update->refname,\n+\t\t\t\t\t (update->have_old ?\n+\t\t\t\t\t update->old_sha1 :\n+\t\t\t\t\t NULL),\n+\t\t\t\t\t update->flags,\n+\t\t\t\t\t &update->type,\n+\t\t\t\t\t transaction->delnames,\n+\t\t\t\t\t transaction->delnum);\n+\tif (update->lock)\n+\t\treturn 0;\n+\n+\t/* If we could not lock the ref it means we either collided with a\n+\t different command or that we tried to perform a second update to\n+\t the same ref from within the same transaction.\n+\t*/\n+\ttransaction->status = REF_TRANSACTION_ERROR;\n+\n+\t/* -1 is the update we just added. Start at -2 and find the most recent\n+\t previous update for this ref.\n+\t*/\n+\tfor (i = transaction->nr - 2; i >= 0; i--) {\n+\t\tif (transaction->updates[i]->update_type != UPDATE_SHA1) {\n+\t\t\tcontinue;\n+\t\t}\n+\t\tif (!strcmp(transaction->updates[i]->refname,\n+\t\t\t update->refname))\n+\t\t\tbreak;\n+\t}\n+\tif (err)\n+\t\tif (i >= 0) {\n+\t\t\tconst char *str =\n+\t\t\t\t\"Multiple updates for ref '%s' not allowed.\";\n+\t\t\tstrbuf_addf(err, str, update->refname);\n+\t\t} else {\n+\t\t\tconst char *str = \"Cannot lock the ref '%s'.\";\n+\t\t\tstrbuf_addf(err, str, update->refname);\n+\t\t}\n+\treturn 1;\n }\n \n int transaction_create_sha1(struct ref_transaction *transaction,\n@@ -3525,32 +3556,6 @@ int update_ref(const char *action, const char *refname,\n \treturn 0;\n }\n \n-static int ref_update_compare(const void *r1, const void *r2)\n-{\n-\tconst struct ref_update * const *u1 = r1;\n-\tconst struct ref_update * const *u2 = r2;\n-\treturn strcmp((*u1)->refname, (*u2)->refname);\n-}\n-\n-static int ref_update_reject_duplicates(struct ref_update **updates, int n,\n-\t\t\t\t\tstruct strbuf *err)\n-{\n-\tint i;\n-\tfor (i = 1; i < n; i++) {\n-\t\tif (updates[i]->update_type != UPDATE_SHA1)\n-\t\t\tcontinue;\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\treturn 1;\n-\t\t}\n-\t}\n-\treturn 0;\n-}\n-\n int transaction_commit(struct ref_transaction *transaction,\n \t\t\t struct strbuf *err)\n {\n@@ -3569,12 +3574,6 @@ int transaction_commit(struct ref_transaction *transaction,\n \t\treturn 0;\n \t}\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);\n-\tif (ret)\n-\t\tgoto cleanup;\n-\n \t/* Acquire all ref locks while verifying old values */\n \tfor (i = 0; i < n; i++) {\n \t\tstruct ref_update *update = updates[i];\n-- \n2.0.0.rc3.506.g3739a35\n"},{"id":"241611","messageId":"1400105610-21194-22-git-send-email-sahlberg@google.com","threadId":"36660","inReplyTo":"1400105610-21194-1-git-send-email-sahlberg@google.com","subject":"[PATCH 21/31] refs.c allow multiple updates of the same ref in a transaction","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-14T22:13:20Z","receivedAt":"2014-05-14T22:13:20Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Allow multiple updates of a ref in the same transaction as long as\neach update has have_old and old_sha1 matches the new_sha1 of the\nprevious update.\n\nAdd a test that verifies that a valid sequence such as\n create ref a\n update ref b a\n update ref c b\nworks and a test that an invalid sequence such as this still fails:\n update ref a c\n update ref b b\n update ref c c\n\nSigned-off-by: Ronnie Sahlberg \n---\n refs.c | 23 +++++++++++++++++------\n t/t1400-update-ref.sh | 23 +++++++++++++++++++++--\n 2 files changed, 38 insertions(+), 8 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 76cab6e..87cdd91 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3455,12 +3455,6 @@ int transaction_update_sha1(struct ref_transaction *transaction,\n \tif (update->lock)\n \t\treturn 0;\n \n-\t/* If we could not lock the ref it means we either collided with a\n-\t different command or that we tried to perform a second update to\n-\t the same ref from within the same transaction.\n-\t*/\n-\ttransaction->status = REF_TRANSACTION_ERROR;\n-\n \t/* -1 is the update we just added. Start at -2 and find the most recent\n \t previous update for this ref.\n \t*/\n@@ -3472,6 +3466,23 @@ int transaction_update_sha1(struct ref_transaction *transaction,\n \t\t\t update->refname))\n \t\t\tbreak;\n \t}\n+\t/* If the current update has_old==1 and old_sha1 matches the new_sha1\n+\t * of the previous update then merge the two updates into one.\n+\t */\n+\tif (i >= 0 && update->have_old && !hashcmp(update->old_sha1,\n+\t\t\t transaction->updates[i]->new_sha1)) {\n+\t\thashcpy(transaction->updates[i]->new_sha1, update->new_sha1);\n+\t\ttransaction->nr--;\n+\t\tfree((char *)transaction->updates[transaction->nr]->msg);\n+\t\tfree(transaction->updates[transaction->nr]);\n+\t\treturn 0;\n+\t}\n+\t/* If we could not lock the ref it means we either collided with a\n+\t different command or that we tried to perform a second update to\n+\t the same ref from within the same transaction.\n+\t*/\n+\ttransaction->status = REF_TRANSACTION_ERROR;\n+\n \tif (err)\n \t\tif (i >= 0) {\n \t\t\tconst char *str =\ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex f9b7bef..078cd4b 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -446,7 +446,7 @@ test_expect_success 'stdin fails option with unknown name' '\n \tgrep \"fatal: option unknown: unknown\" err\n '\n \n-test_expect_success 'stdin fails with duplicate refs' '\n+test_expect_success 'stdin fails with duplicate create refs' '\n \tcat >stdin <<-EOF &&\n \tcreate $a $m\n \tcreate $b $m\n@@ -464,6 +464,25 @@ test_expect_success 'stdin create ref works' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'stdin succeeds with correctly chained update refs' '\n+\tcat >stdin <<-EOF &&\n+\tupdate $a $A $m\n+\tupdate $a $B $A\n+\tupdate $a $C $B\n+\tEOF\n+\tgit update-ref --stdin stdin <<-EOF &&\n+\tupdate $a $A $C\n+\tupdate $a $B $B\n+\tupdate $a $B $B\n+\tEOF\n+\ttest_must_fail git update-ref --stdin stdin &&\n@@ -786,7 +805,7 @@ test_expect_success 'stdin -z fails option with unknown name' '\n \tgrep \"fatal: option unknown: unknown\" err\n '\n \n-test_expect_success 'stdin -z fails with duplicate refs' '\n+test_expect_success 'stdin -z fails with duplicate create refs' '\n \tprintf $F \"create $a\" \"$m\" \"create $b\" \"$m\" \"create $a\" \"$m\" >stdin &&\n \ttest_must_fail git update-ref -z --stdin err &&\n \tgrep \"fatal: Multiple updates for ref '\"'\"'$a'\"'\"' not allowed.\" err\n-- \n2.0.0.rc3.506.g3739a35\n"},{"id":"241603","messageId":"1400105610-21194-23-git-send-email-sahlberg@google.com","threadId":"36660","inReplyTo":"1400105610-21194-1-git-send-email-sahlberg@google.com","subject":"[PATCH 22/31] refs.c: release all remaining locks during transaction_free","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-14T22:13:21Z","receivedAt":"2014-05-14T22:13:21Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"If there are remaining locks held when we free a transaction, make sure that\nwe release them instead of relying on the atexit magic to release them for\nus. Otherwise, if we perform two transactions on the same ref and the first\ntransaction is aborted then the second transaction will fail to lock the ref.\nThis can currently not trigger since there are no commands that use multiple\ntransactions.\n\nThis sequence illustrates the bug:\n\nt = transaction_begin() /* first transaction */\ntransaction_update_sha1(t, ref, ...) /* ref is locked */\ntransaction_free(t)\n\nt = transaction_begin() /* second transaction */\ntransaction_update_sha1(t, ref, ...) /* will fail since ref was locked above */\n\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 87cdd91..17feec5 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3325,6 +3325,8 @@ void transaction_free(struct ref_transaction *transaction)\n \t\treturn;\n \n \tfor (i = 0; i < transaction->nr; i++) {\n+\t\tif (transaction->updates[i]->lock)\n+\t\t\tunlock_ref(transaction->updates[i]->lock);\n \t\tfree((char *)transaction->updates[i]->committer);\n \t\tfree((char *)transaction->updates[i]->msg);\n \t\tfree(transaction->updates[i]);\n@@ -3729,8 +3731,10 @@ cleanup:\n \t : REF_TRANSACTION_CLOSED;\n \n \tfor (i = 0; i < n; i++)\n-\t\tif (updates[i]->lock)\n+\t\tif (updates[i]->lock) {\n \t\t\tunlock_ref(updates[i]->lock);\n+\t\t\tupdates[i]->lock = NULL;\n+\t\t}\n \treturn ret;\n }\n \n-- \n2.0.0.rc3.506.g3739a35\n"},{"id":"241617","messageId":"1400105610-21194-24-git-send-email-sahlberg@google.com","threadId":"36660","inReplyTo":"1400105610-21194-1-git-send-email-sahlberg@google.com","subject":"[PATCH 23/31] reflog.c: use the existing transaction to also lock and update the ref","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-14T22:13:22Z","receivedAt":"2014-05-14T22:13:22Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"reflog.c has a transaction for updating the reflog. Use this transaction\nalso for locking the ref itself and updating the ref instead of\nlocking the file directly and writing to it. This now completely eliminates\nthe refs and reflog implementation from reflog.c.\n\nSigned-off-by: Ronnie Sahlberg \n---\n builtin/reflog.c | 44 ++++++++++++++++++++------------------------\n 1 file changed, 20 insertions(+), 24 deletions(-)\n\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex 2239249..78fd442 100644\n--- a/builtin/reflog.c\n+++ b/builtin/reflog.c\n@@ -351,7 +351,6 @@ static int expire_reflog(const char *ref, const unsigned char *sha1, int unused,\n {\n \tstruct cmd_reflog_expire_cb *cmd = cb_data;\n \tstruct expire_reflog_cb cb;\n-\tstruct ref_lock *lock;\n \tstruct commit *tip_commit;\n \tstruct commit_list *tips;\n \tint status = 0;\n@@ -359,23 +358,24 @@ static int expire_reflog(const char *ref, const unsigned char *sha1, int unused,\n \tmemset(&cb, 0, sizeof(cb));\n \tcb.refname = ref;\n \n+\tif (!reflog_exists(ref))\n+\t\tgoto finish;\n+\n+\tcb.t = transaction_begin();\n \t/*\n-\t * we take the lock for the ref itself to prevent it from\n-\t * getting updated.\n+\t * This locks the ref itself, preventing it from getting\n+\t * updated.\n \t */\n-\tlock = lock_any_ref_for_update(ref, sha1, 0, NULL);\n-\tif (!lock)\n+\tif (transaction_update_sha1(cb.t, cb.refname,\n+\t\t\t\t sha1, sha1, 0, 1, NULL, NULL)) {\n+\t\ttransaction_rollback(cb.t);\n \t\treturn error(\"cannot lock ref '%s'\", ref);\n-\tif (!reflog_exists(ref))\n-\t\tgoto finish;\n-\tif (!cmd->dry_run) {\n-\t\tcb.t = transaction_begin();\n-\t\ttransaction_update_reflog(cb.t, cb.refname,\n-\t\t\t\t\t null_sha1, null_sha1, NULL,\n-\t\t\t\t\t 0, 0, NULL,\n-\t\t\t\t\t REFLOG_TRUNCATE,\n-\t\t\t\t\t NULL);\n \t}\n+\ttransaction_update_reflog(cb.t, cb.refname,\n+\t\t\t\t null_sha1, null_sha1, NULL,\n+\t\t\t\t 0, 0, NULL,\n+\t\t\t\t REFLOG_TRUNCATE,\n+\t\t\t\t NULL);\n \n \tcb.cmd = cmd;\n \n@@ -421,23 +421,19 @@ static int expire_reflog(const char *ref, const unsigned char *sha1, int unused,\n \t\t}\n \t}\n finish:\n-\tif (cb.t) {\n+\tif (!cmd->dry_run) {\n \t\tif (cmd->updateref &&\n-\t\t (write_in_full(lock->lock_fd,\n-\t\t\t\t sha1_to_hex(cb.last_kept_sha1), 40) != 40 ||\n-\t\t write_str_in_full(lock->lock_fd, \"\\n\") != 1 ||\n-\t\t close_ref(lock) < 0)) {\n+\t\t transaction_update_sha1(cb.t, cb.refname,\n+\t\t\t\t\t cb.last_kept_sha1, sha1,\n+\t\t\t\t\t 0, 1, NULL, NULL)) {\n \t\t\tstatus |= error(\"Couldn't write %s\",\n-\t\t\t\tlock->lk->filename);\n+\t\t\t\tcb.refname);\n \t\t\ttransaction_rollback(cb.t);\n \t\t} else if (transaction_commit(cb.t, NULL)) {\n \t\t\tstatus |= error(\"cannot commit reflog for %s\", ref);\n-\t\t} else if (cmd->updateref && commit_ref(lock)) {\n-\t\t\tstatus |= error(\"Couldn't set %s\", lock->ref_name);\n \t\t}\n-\t\ttransaction_free(cb.t);\n \t}\n-\tunlock_ref(lock);\n+\ttransaction_free(cb.t);\n \treturn status;\n }\n \n-- \n2.0.0.rc3.506.g3739a35\n"},{"id":"241616","messageId":"1400105610-21194-25-git-send-email-sahlberg@google.com","threadId":"36660","inReplyTo":"1400105610-21194-1-git-send-email-sahlberg@google.com","subject":"[PATCH 24/31] refs.c: make unlock_ref static","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-14T22:13:23Z","receivedAt":"2014-05-14T22:13:23Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"We no longer need to export unlock_ref. Make the function static.\n\nSigned-off-by: Ronnie Sahlberg \n---\n refs.c | 20 ++++++++++----------\n refs.h | 3 ---\n 2 files changed, 10 insertions(+), 13 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 17feec5..6933036 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1931,6 +1931,16 @@ int refname_match(const char *abbrev_name, const char *full_name)\n \treturn 0;\n }\n \n+static void unlock_ref(struct ref_lock *lock)\n+{\n+\t/* Do not free lock->lk -- atexit() still looks at them */\n+\tif (lock->lk)\n+\t\trollback_lock_file(lock->lk);\n+\tfree(lock->ref_name);\n+\tfree(lock->orig_ref_name);\n+\tfree(lock);\n+}\n+\n static struct ref_lock *verify_lock(struct ref_lock *lock,\n \tconst unsigned char *old_sha1, int mustexist)\n {\n@@ -2674,16 +2684,6 @@ int commit_ref(struct ref_lock *lock)\n \treturn 0;\n }\n \n-void unlock_ref(struct ref_lock *lock)\n-{\n-\t/* Do not free lock->lk -- atexit() still looks at them */\n-\tif (lock->lk)\n-\t\trollback_lock_file(lock->lk);\n-\tfree(lock->ref_name);\n-\tfree(lock->orig_ref_name);\n-\tfree(lock);\n-}\n-\n /*\n * copy the reflog message msg to buf, which has been allocated sufficiently\n * large, while cleaning up the whitespaces. Especially, convert LF to space,\ndiff --git a/refs.h b/refs.h\nindex 6c628bb..47982ca 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -145,9 +145,6 @@ extern int close_ref(struct ref_lock *lock);\n /** Close and commit the ref locked by the lock */\n extern int commit_ref(struct ref_lock *lock);\n \n-/** Release any lock taken but not written. **/\n-extern void unlock_ref(struct ref_lock *lock);\n-\n /** Setup reflog before using. **/\n int log_ref_setup(const char *refname, char *logfile, int bufsize);\n \n-- \n2.0.0.rc3.506.g3739a35\n"},{"id":"241613","messageId":"1400105610-21194-26-git-send-email-sahlberg@google.com","threadId":"36660","inReplyTo":"1400105610-21194-1-git-send-email-sahlberg@google.com","subject":"[PATCH 25/31] refs.c: make close_ref static","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-14T22:13:24Z","receivedAt":"2014-05-14T22:13:24Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"We no longer need to export close_ref. Make the function 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 6933036..a430b22 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2668,7 +2668,7 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms\n \treturn 1;\n }\n \n-int close_ref(struct ref_lock *lock)\n+static int close_ref(struct ref_lock *lock)\n {\n \tif (close_lock_file(lock->lk))\n \t\treturn -1;\ndiff --git a/refs.h b/refs.h\nindex 47982ca..f211e13 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -139,9 +139,6 @@ extern struct ref_lock *lock_any_ref_for_update(const char *refname,\n \t\t\t\t\t\tconst unsigned char *old_sha1,\n \t\t\t\t\t\tint flags, int *type_p);\n \n-/** Close the file descriptor owned by a lock and return the status */\n-extern int close_ref(struct ref_lock *lock);\n-\n /** Close and commit the ref locked by the lock */\n extern int commit_ref(struct ref_lock *lock);\n \n-- \n2.0.0.rc3.506.g3739a35\n"},{"id":"241614","messageId":"1400105610-21194-27-git-send-email-sahlberg@google.com","threadId":"36660","inReplyTo":"1400105610-21194-1-git-send-email-sahlberg@google.com","subject":"[PATCH 26/31] refs.c: make commit_ref static","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-14T22:13:25Z","receivedAt":"2014-05-14T22:13:25Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"We no longer need to export commit_ref. Make the function 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 a430b22..0cb4ae8 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2676,7 +2676,7 @@ static int close_ref(struct ref_lock *lock)\n \treturn 0;\n }\n \n-int commit_ref(struct ref_lock *lock)\n+static int commit_ref(struct ref_lock *lock)\n {\n \tif (commit_lock_file(lock->lk))\n \t\treturn -1;\ndiff --git a/refs.h b/refs.h\nindex f211e13..289558c 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -139,9 +139,6 @@ extern struct ref_lock *lock_any_ref_for_update(const char *refname,\n \t\t\t\t\t\tconst unsigned char *old_sha1,\n \t\t\t\t\t\tint flags, int *type_p);\n \n-/** Close and commit the ref locked by the lock */\n-extern int commit_ref(struct ref_lock *lock);\n-\n /** Setup reflog before using. **/\n int log_ref_setup(const char *refname, char *logfile, int bufsize);\n \n-- \n2.0.0.rc3.506.g3739a35\n"},{"id":"241602","messageId":"1400105610-21194-28-git-send-email-sahlberg@google.com","threadId":"36660","inReplyTo":"1400105610-21194-1-git-send-email-sahlberg@google.com","subject":"[PATCH 27/31] refs.c: remove the function lock_any_ref_for_update","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-14T22:13:26Z","receivedAt":"2014-05-14T22:13:26Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"We no longer need the function lock_any_ref_for_update so lets remove it.\n\nSigned-off-by: Ronnie Sahlberg \n---\n refs.c | 7 -------\n refs.h | 4 ----\n 2 files changed, 11 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 0cb4ae8..918cda3 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2167,13 +2167,6 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,\n \treturn 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-{\n-\treturn lock_ref_sha1_basic(refname, old_sha1, flags, type_p, NULL, 0);\n-}\n-\n /*\n * Write an entry to the packed-refs file for the specified refname.\n * If peeled is non-NULL, write it as the entry's peeled value.\ndiff --git a/refs.h b/refs.h\nindex 289558c..1e89187 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -135,10 +135,6 @@ extern int peel_ref(const char *refname, unsigned char *sha1);\n /** Locks any ref (for 'HEAD' type refs). */\n #define REF_NODEREF\t0x01\n \n-extern struct ref_lock *lock_any_ref_for_update(const char *refname,\n-\t\t\t\t\t\tconst unsigned char *old_sha1,\n-\t\t\t\t\t\tint flags, int *type_p);\n-\n /** Setup reflog before using. **/\n int log_ref_setup(const char *refname, char *logfile, int bufsize);\n \n-- \n2.0.0.rc3.506.g3739a35\n"},{"id":"241599","messageId":"1400105610-21194-29-git-send-email-sahlberg@google.com","threadId":"36660","inReplyTo":"1400105610-21194-1-git-send-email-sahlberg@google.com","subject":"[PATCH 28/31] refs.c: make struct ref_lock private to refs.c","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-14T22:13:27Z","receivedAt":"2014-05-14T22:13:27Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"We no longer need to expose the ref_lock structure to any external callers.\nMake it private to refs.c.\n\nSigned-off-by: Ronnie Sahlberg \n---\n refs.c | 9 +++++++++\n refs.h | 9 ---------\n 2 files changed, 9 insertions(+), 9 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 918cda3..8129b20 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -5,6 +5,15 @@\n #include \"dir.h\"\n #include \"string-list.h\"\n \n+struct ref_lock {\n+\tchar *ref_name;\n+\tchar *orig_ref_name;\n+\tstruct lock_file *lk;\n+\tunsigned char old_sha1[20];\n+\tint lock_fd;\n+\tint force_write;\n+};\n+\n /*\n * Make sure \"ref\" is something reasonable to have under \".git/refs/\";\n * We do not like it if:\ndiff --git a/refs.h b/refs.h\nindex 1e89187..558a711 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -1,15 +1,6 @@\n #ifndef REFS_H\n #define REFS_H\n \n-struct ref_lock {\n-\tchar *ref_name;\n-\tchar *orig_ref_name;\n-\tstruct lock_file *lk;\n-\tunsigned char old_sha1[20];\n-\tint lock_fd;\n-\tint force_write;\n-};\n-\n struct ref_transaction;\n \n /*\n-- \n2.0.0.rc3.506.g3739a35\n"},{"id":"241600","messageId":"1400105610-21194-30-git-send-email-sahlberg@google.com","threadId":"36660","inReplyTo":"1400105610-21194-1-git-send-email-sahlberg@google.com","subject":"[PATCH 29/31] refs.c: allow passing raw git_committer_info as email to _update_reflog","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-14T22:13:28Z","receivedAt":"2014-05-14T22:13:28Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"In many places in the code we do not have access to the individual fields\nin the committer data. Instead we might only have access to prebaked data\nsuch as what is returned by git_committer_info() containing a string\nthat consists of email, timestamp, zone etc.\n\nThis makes it inconvenient to use transaction_update_reflog since it means you\nwould have to first parse git_committer_info before you can call update_reflog.\n\nAdd a new flag REFLOG_EMAIL_IS_COMMITTER to _update_reflog to tell it\nthat what we pass in as email is already the fully baked committer string\nwe can use as-is.\n\nSigned-off-by: Ronnie Sahlberg \n\nConflicts:\n\trefs.c\n---\n refs.c | 20 ++++++++++++--------\n refs.h | 1 +\n 2 files changed, 13 insertions(+), 8 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 8129b20..46a31c1 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3396,14 +3396,18 @@ int transaction_update_reflog(struct ref_transaction *transaction,\n \thashcpy(update->old_sha1, old_sha1);\n \tupdate->reflog_fd = -1;\n \tif (email) {\n-\t\tstruct strbuf buf = STRBUF_INIT;\n-\t\tchar sign = (tz < 0) ? '-' : '+';\n-\t\tint zone = (tz < 0) ? (-tz) : tz;\n-\n-\t\tstrbuf_addf(&buf, \"%s %lu %c%04d\", email, timestamp, sign,\n-\t\t\t zone);\n-\t\tupdate->committer = xstrdup(buf.buf);\n-\t\tstrbuf_release(&buf);\n+\t\tif (flags & REFLOG_EMAIL_IS_COMMITTER)\n+\t\t\tupdate->committer = xstrdup(email);\n+\t\telse {\n+\t\t\tstruct strbuf buf = STRBUF_INIT;\n+\t\t\tchar sign = (tz < 0) ? '-' : '+';\n+\t\t\tint zone = (tz < 0) ? (-tz) : tz;\n+\n+\t\t\tstrbuf_addf(&buf, \"%s %lu %c%04d\", email, timestamp,\n+\t\t\t\t sign, zone);\n+\t\t\tupdate->committer = xstrdup(buf.buf);\n+\t\t\tstrbuf_release(&buf);\n+\t\t}\n \t}\n \tif (msg)\n \t\tupdate->msg = xstrdup(msg);\ndiff --git a/refs.h b/refs.h\nindex 558a711..933cae7 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -251,6 +251,7 @@ int transaction_delete_sha1(struct ref_transaction *transaction,\n \t\t\t struct strbuf *err);\n \n #define REFLOG_TRUNCATE 0x01\n+#define REFLOG_EMAIL_IS_COMMITTER 0x02\n /*\n * Append a reflog entry for refname. If the REFLOG_TRUNCATE flag is set\n * this update will first truncate the reflog before writing the entry.\n-- \n2.0.0.rc3.506.g3739a35\n"},{"id":"241598","messageId":"1400105610-21194-31-git-send-email-sahlberg@google.com","threadId":"36660","inReplyTo":"1400105610-21194-1-git-send-email-sahlberg@google.com","subject":"[PATCH 30/31] refs.c: move ref_update and other definitions to earlier in the file","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-14T22:13:29Z","receivedAt":"2014-05-14T22:13:29Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Move the struct ref_update definition and some enums to earlier in the file.\nThis does not change any actual code or definitions but just moves them\nto a different place in the code since we will soon reference some of these\nfrom the rename_ref function.\n\nSigned-off-by: Ronnie Sahlberg \n\nConflicts:\n\trefs.c\n---\n refs.c | 61 ++++++++++++++++++++++++++++++-------------------------------\n 1 file changed, 30 insertions(+), 31 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 46a31c1..bcce8fe 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2603,6 +2603,36 @@ static int rename_tmp_log(const char *newrefname)\n \treturn 0;\n }\n \n+enum transaction_update_type {\n+ UPDATE_SHA1 = 0,\n+ UPDATE_LOG = 1,\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+ * while locking the ref, set have_old to 1 and set old_sha1 to the\n+ * value or to zero to ensure the ref does not exist before update.\n+ */\n+struct ref_update {\n+\tenum transaction_update_type update_type;\n+\tunsigned char new_sha1[20];\n+\tunsigned char old_sha1[20];\n+\tint flags; /* REF_NODEREF? */\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+\n+\t/* used by reflog updates */\n+\tint reflog_fd;\n+\tstruct lock_file *reflog_lock;\n+\tconst char *committer;\n+\tstruct ref_update *orig_update; /* For UPDATE_REFLOG_NOLOCK */\n+\n+\tconst char refname[FLEX_ARRAY];\n+};\n+\n int rename_ref(const char *oldrefname, const char *newrefname, const char *logmsg)\n {\n \tunsigned char sha1[20];\n@@ -3254,37 +3284,6 @@ int for_each_reflog(each_ref_fn fn, void *cb_data)\n \treturn retval;\n }\n \n-enum transaction_update_type {\n- UPDATE_SHA1 = 0,\n- UPDATE_LOG = 1,\n-};\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- * while locking the ref, set have_old to 1 and set old_sha1 to the\n- * value or to zero to ensure the ref does not exist before update.\n- */\n-struct ref_update {\n-\tenum transaction_update_type update_type;\n-\tunsigned char new_sha1[20];\n-\tunsigned char old_sha1[20];\n-\tint flags; /* REF_NODEREF? or private flags */\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-\n-\t/* used by reflog updates */\n-\tint reflog_fd;\n-\tstruct lock_file *reflog_lock;\n-\tconst char *committer;\n-\tstruct ref_update *orig_update; /* For UPDATE_REFLOG_NOLOCK */\n-\n-\tconst char refname[FLEX_ARRAY];\n-};\n-\n enum ref_transaction_status {\n \tREF_TRANSACTION_OPEN = 0,\n \tREF_TRANSACTION_CLOSED = 1,\n-- \n2.0.0.rc3.506.g3739a35\n"},{"id":"241601","messageId":"1400105610-21194-32-git-send-email-sahlberg@google.com","threadId":"36660","inReplyTo":"1400105610-21194-1-git-send-email-sahlberg@google.com","subject":"[PATCH 31/31] refs.c: use the transaction to manage the reflog in rename_refs","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-14T22:13:30Z","receivedAt":"2014-05-14T22:13:30Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"We have a transaction in rename_ref to managing the ref during the rename.\nUse this transaction to manage the changes to the reflog too.\nThis now means that rename_ref is almost completely agnostic about how refs\nstorage is implemented and should be easier to migrate if/when we allow a\ndifferent store for refs, such as a database.\n\nSigned-off-by: Ronnie Sahlberg \n---\n refs.c | 62 ++++++++++++++++++++++++++++++++++++++++++--------------------\n 1 file changed, 42 insertions(+), 20 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex bcce8fe..829c89a 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2633,6 +2633,24 @@ struct ref_update {\n \tconst char refname[FLEX_ARRAY];\n };\n \n+struct rename_reflog_cb {\n+ struct ref_transaction *t;\n+ const char *refname;\n+};\n+\n+static int rename_reflog_ent(unsigned char *osha1, unsigned char *nsha1,\n+\t\t\t const char *email, unsigned long timestamp, int tz,\n+\t\t\t const char *message, void *cb_data)\n+{\n+ struct rename_reflog_cb *cb = cb_data;\n+\n+ transaction_update_reflog(cb->t, cb->refname, nsha1, osha1,\n+\t\t\t\t email, timestamp, tz, message, 0,\n+\t\t\t\t NULL);\n+\n+ return 0;\n+}\n+\n int rename_ref(const char *oldrefname, const char *newrefname, const char *logmsg)\n {\n \tunsigned char sha1[20];\n@@ -2664,40 +2682,44 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms\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-\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 \ttransaction = transaction_begin();\n-\tif (!transaction ||\n-\t transaction_delete_sha1(transaction, oldrefname, sha1,\n+\tif (log) {\n+\t\tstruct rename_reflog_cb cb;\n+\n+\t\ttransaction_update_reflog(transaction, newrefname,\n+\t\t\t\t\t null_sha1, null_sha1, NULL,\n+\t\t\t\t\t 0, 0, NULL,\n+\t\t\t\t\t REFLOG_TRUNCATE,\n+\t\t\t\t\t NULL);\n+\n+\t\tcb.t = transaction;\n+\t\tcb.refname = newrefname;\n+\t\tfor_each_reflog_ent(oldrefname, rename_reflog_ent, &cb);\n+\n+\t\ttransaction_update_reflog(transaction, newrefname,\n+\t\t\t\t\t sha1, sha1,\n+\t\t\t\t\t git_committer_info(0),\n+\t\t\t\t\t 0, 0, logmsg,\n+\t\t\t\t\t REFLOG_EMAIL_IS_COMMITTER,\n+\t\t\t\t\t &err);\n+\t}\n+\n+\tif (transaction_delete_sha1(transaction, oldrefname, sha1,\n \t\t\t\t REF_NODEREF | REF_ISPACKONLY,\n \t\t\t\t 1, NULL, &err) ||\n \t transaction_update_sha1(transaction, newrefname, sha1,\n-\t\t\t\t NULL, 0, 0, logmsg, &err) ||\n+\t\t\t\t NULL, 0, 0, NULL, &err) ||\n \t transaction_commit(transaction, &err)) {\n \t\ttransaction_rollback(transaction);\n \t\terror(\"rename_ref failed: %s\", err.buf);\n \t\tstrbuf_release(&err);\n-\t\tgoto rollbacklog;\n+\t\treturn 1;\n \t}\n \ttransaction_free(transaction);\n-\n-\tif (log && rename_tmp_log(newrefname))\n-\t\tgoto rollbacklog;\n-\n \treturn 0;\n-\n- rollbacklog:\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-\n-\treturn 1;\n }\n \n static int close_ref(struct ref_lock *lock)\n-- \n2.0.0.rc3.506.g3739a35\n"},{"id":"241998","messageId":"xmqq38g9a0i0.fsf@gitster.dls.corp.google.com","threadId":"36660","inReplyTo":"1400105610-21194-4-git-send-email-sahlberg@google.com","subject":"Re: [PATCH 03/31] refs.c: rename the transaction functions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-16T21:15:35Z","receivedAt":"2014-05-16T21:15:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ronnie Sahlberg writes:\n\n> Rename the transaction functions. Remove the leading ref_ from the names\n> and append _sha1 to the names for functions that create/delete/update sha1\n> refs.\n> ...\n> -\t\ttransaction = ref_transaction_begin();\n> +\t\ttransaction = transaction_begin();\n\nWhy? Do we forsee that there will never be other kinds of\ntransaction, and whenever we say a \"transaction\" that will be always\nabout updating the refs and nothing else?\n"},{"id":"241999","messageId":"xmqqy4y18lol.fsf@gitster.dls.corp.google.com","threadId":"36660","inReplyTo":"1400105610-21194-8-git-send-email-sahlberg@google.com","subject":"Re: [PATCH 07/31] refs.c: add a flag to allow reflog updates to truncate the log","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-16T21:20:58Z","receivedAt":"2014-05-16T21:20:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ronnie Sahlberg writes:\n\n> Add a flag that allows us to truncate the reflog before we write the update.\n>\n> Signed-off-by: Ronnie Sahlberg \n> ---\n\nUntil we read the callers it is hard to see how such a feature is\nuseful, though.\n\n(style) The two multi-line comments are formatted differently.\n\n> refs.c | 12 ++++++++++--\n> refs.h | 4 +++-\n> 2 files changed, 13 insertions(+), 3 deletions(-)\n>\n> diff --git a/refs.c b/refs.c\n> index d8a1568..a8b583a 100644\n> --- a/refs.c\n> +++ b/refs.c\n> @@ -3608,7 +3608,11 @@ int transaction_commit(struct ref_transaction *transaction,\n> \t\t}\n> \t}\n> \n> -\t/* Update all reflog files */\n> +\t/* Update all reflog files\n> +\t We have already done all ref updates and deletes.\n> +\t There is not much we can do here if there are any reflog\n> +\t update errors, other than complain.\n> +\t*/\n> \tfor (i = 0; i < n; i++) {\n> \t\tstruct ref_update *update = updates[i];\n> \n> @@ -3616,7 +3620,11 @@ int transaction_commit(struct ref_transaction *transaction,\n> \t\t\tcontinue;\n> \t\tif (update->reflog_fd == -1)\n> \t\t\tcontinue;\n> -\n> +\t\tif (update->flags & REFLOG_TRUNCATE)\n> +\t\t\tif (lseek(update->reflog_fd, 0, SEEK_SET) < 0 ||\n> +\t\t\t ftruncate(update->reflog_fd, 0)) {\n> +\t\t\t\terror(\"Could not truncate reflog\");\n> +\t\t\t}\n\nThe {} looks somewhat curious here. If you seeked to the beginning,\nand failed to truncate, do you still want to go on without \"return\"\nin front of the error()? Would that overwrite existing entries?\n\n> \t\tif (log_ref_write_fd(update->reflog_fd, update->old_sha1,\n> \t\t\t\t update->new_sha1,\n> \t\t\t\t update->committer, update->msg)) {\n> diff --git a/refs.h b/refs.h\n> index 2e22a26..b1b97e7 100644\n> --- a/refs.h\n> +++ b/refs.h\n> @@ -269,8 +269,10 @@ int transaction_delete_sha1(struct ref_transaction *transaction,\n> \t\t\t const unsigned char *old_sha1,\n> \t\t\t int flags, int have_old, const char *msg);\n> \n> +#define REFLOG_TRUNCATE 0x01\n> /*\n> - * Append a reflog entry for refname.\n> + * Append a reflog entry for refname. If the REFLOG_TRUNCATE flag is set\n> + * this update will first truncate the reflog before writing the entry.\n> */\n> int transaction_update_reflog(struct ref_transaction *transaction,\n> \t\t\t const char *refname,\n"},{"id":"242001","messageId":"xmqqtx8p8lj2.fsf@gitster.dls.corp.google.com","threadId":"36660","inReplyTo":"1400105610-21194-9-git-send-email-sahlberg@google.com","subject":"Re: [PATCH 08/31] refs.c: only write reflog update if msg is non-NULL","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-16T21:24:17Z","receivedAt":"2014-05-16T21:24:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ronnie Sahlberg writes:\n\n> When performing a reflog transaction update, only write to the reflog iff\n> msg is non-NULL. This can then be combined with REFLOG_TRUNCATE to perform\n> an update that only truncates but does not write.\n>\n> Signed-off-by: Ronnie Sahlberg \n> ---\n> refs.c | 8 +++++---\n> refs.h | 1 +\n> 2 files changed, 6 insertions(+), 3 deletions(-)\n>\n> diff --git a/refs.c b/refs.c\n> index a8b583a..a3f60ad 100644\n> --- a/refs.c\n> +++ b/refs.c\n> @@ -3625,9 +3625,11 @@ int transaction_commit(struct ref_transaction *transaction,\n> \t\t\t ftruncate(update->reflog_fd, 0)) {\n> \t\t\t\terror(\"Could not truncate reflog\");\n> \t\t\t}\n> -\t\tif (log_ref_write_fd(update->reflog_fd, update->old_sha1,\n> -\t\t\t\t update->new_sha1,\n> -\t\t\t\t update->committer, update->msg)) {\n> +\t\tif (update->msg && log_ref_write_fd(update->reflog_fd,\n> +\t\t\t\t\t\t update->old_sha1,\n> +\t\t\t\t\t\t update->new_sha1,\n> +\t\t\t\t\t\t update->committer,\n> +\t\t\t\t\t\t update->msg)) {\n\nWouldn't it make it easier to read if you chopped immediately after\nthe &&, i.e. chopping at a gap at a higher-level in the parse tree?\n\n if (update->msg &&\n log_ref_write_fd(update->reflog_fd,\n update->old_sha1, update->old_sha1,\n update->committer, update->msg)) {\n"},{"id":"242002","messageId":"xmqqppjd8l13.fsf@gitster.dls.corp.google.com","threadId":"36660","inReplyTo":"1400105610-21194-10-git-send-email-sahlberg@google.com","subject":"Re: [PATCH 09/31] refs.c: allow multiple reflog updates during a single transaction","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-16T21:35:04Z","receivedAt":"2014-05-16T21:35:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ronnie Sahlberg writes:\n\n> Allow to make multiple reflog updates to the same ref during a transaction.\n> This means we only need to lock the reflog once, during the first update that\n> touches the reflog, and that all further updates can just write the reflog\n> entry since the reflog is already locked.\n>\n> This allows us to write code such as:\n>\n> t = transaction_begin()\n> transaction_reflog_update(t, \"foo\", REFLOG_TRUNCATE, NULL);\n> loop-over-somehting...\n> transaction_reflog_update(t, \"foo\", 0, );\n> transaction_commit(t)\n\nOK, so you are now doing not just \"refs\" but also \"reflogs\", you\nfelt that \"ref_transaction()\" does not cover the latter. Is that\nthe reason for the rename in the earlier step?\n\nI am sort-of on the fence.\n\nCalling the begin \"ref_transaction_begin\" and then calling the new\nfunction \"ref_transaction_log_update\" would still allow us to\ndifferentiate transactions on refs and reflogs, while allowing other\nkinds of transactions that are not related to refs at all to employ\na mechanism that is different from the one that is used to implement\nthe transactions on refs and reflogs you are building here.\n\nBut I think I am OK with the generic \"transaction-begin\" now.\nHaving one mechanism for refs and reflogs, and then having another\ncompletely different mechanism for other things, will not let us\ncoordinate between the two easily, so \"allow transactions that are\nnot related to refs at all to be built on a different mechanism\" may\nnot be a worthwhile goal to pursue in the first place. Please\nconsider the question on the naming in the earlier one dropped.\n\n>\n> where we first truncate the reflog and then build the new content one line at a\n> time.\n>\n> Signed-off-by: Ronnie Sahlberg \n> ---\n> refs.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++---------\n> 1 file changed, 49 insertions(+), 9 deletions(-)\n>\n> diff --git a/refs.c b/refs.c\n> index a3f60ad..e7ede03 100644\n> --- a/refs.c\n> +++ b/refs.c\n> @@ -37,6 +37,10 @@ static inline int bad_ref_char(int ch)\n> * need to lock the loose ref during the transaction.\n> */\n> #define REF_ISPACKONLY\t0x0200\n> +/** Only the first reflog update needs to lock the reflog file. Further updates\n> + * just use the lock taken by the first update.\n> + */\n\nStyle.\n\n> @@ -3349,8 +3355,23 @@ int transaction_update_reflog(struct ref_transaction *transaction,\n> \t\t\t int flags)\n> {\n> \tstruct ref_update *update;\n> +\tint i;\n> \n> \tupdate = add_update(transaction, refname, UPDATE_LOG);\n> +\tupdate->flags = flags;\n> +\tfor (i = 0; i < transaction->nr - 1; i++) {\n> +\t\tif (transaction->updates[i]->update_type != UPDATE_LOG)\n> +\t\t\tcontinue;\n> +\t\tif (!strcmp(transaction->updates[i]->refname,\n> +\t\t\t update->refname)) {\n> +\t\t\tupdate->flags |= UPDATE_REFLOG_NOLOCK;\n> +\t\t\tupdate->orig_update = transaction->updates[i];\n> +\t\t\tbreak;\n> +\t\t}\n> +\t}\n> +\tif (!(update->flags & UPDATE_REFLOG_NOLOCK))\n> +\t update->reflog_lock = xcalloc(1, sizeof(struct lock_file));\n\nSo with two calls to transaction-update-reflog, we make two calls to\nadd-update, and each holds a separate lock? If we write two entries\nto record two updates of the same ref, would we still want to do so?\n\n> +\t/* Rollback any reflog files that are still open */\n> +\tfor (i = 0; i < n; i++) {\n> +\t\tstruct ref_update *update = updates[i];\n> +\n> +\t\tif (update->update_type != UPDATE_LOG)\n> +\t\t\tcontinue;\n> +\t\tif (update->flags & UPDATE_REFLOG_NOLOCK)\n> +\t\t\tcontinue;\n> +\t\tif (update->reflog_fd == -1)\n> +\t\t\tcontinue;\n> +\t\trollback_lock_file(update->reflog_lock);\n> +\t}\n> \ttransaction->status = ret ? REF_TRANSACTION_ERROR\n> \t : REF_TRANSACTION_CLOSED;\n"},{"id":"242003","messageId":"CAPig+cSXt3FDS1XtHBq2y5NLQxKrVM4sp=YbRa2skJX=RrOyTw@mail.gmail.com","threadId":"36660","inReplyTo":"xmqqppjd8l13.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 09/31] refs.c: allow multiple reflog updates during a single transaction","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-05-16T22:01:01Z","receivedAt":"2014-05-16T22:01:01Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, May 16, 2014 at 5:35 PM, Junio C Hamano wrote:\n> Ronnie Sahlberg writes:\n>\n>> Allow to make multiple reflog updates to the same ref during a transaction.\n>> This means we only need to lock the reflog once, during the first update that\n>> touches the reflog, and that all further updates can just write the reflog\n>> entry since the reflog is already locked.\n>>\n>> This allows us to write code such as:\n>>\n>> t = transaction_begin()\n>> transaction_reflog_update(t, \"foo\", REFLOG_TRUNCATE, NULL);\n>> loop-over-somehting...\n>> transaction_reflog_update(t, \"foo\", 0, );\n>> transaction_commit(t)\n>\n> OK, so you are now doing not just \"refs\" but also \"reflogs\", you\n> felt that \"ref_transaction()\" does not cover the latter. Is that\n> the reason for the rename in the earlier step?\n>\n> I am sort-of on the fence.\n>\n> Calling the begin \"ref_transaction_begin\" and then calling the new\n> function \"ref_transaction_log_update\" would still allow us to\n> differentiate transactions on refs and reflogs, while allowing other\n> kinds of transactions that are not related to refs at all to employ\n> a mechanism that is different from the one that is used to implement\n> the transactions on refs and reflogs you are building here.\n>\n> But I think I am OK with the generic \"transaction-begin\" now.\n> Having one mechanism for refs and reflogs, and then having another\n> completely different mechanism for other things, will not let us\n> coordinate between the two easily, so \"allow transactions that are\n> not related to refs at all to be built on a different mechanism\" may\n> not be a worthwhile goal to pursue in the first place. Please\n> consider the question on the naming in the earlier one dropped.\n>\n>>\n>> where we first truncate the reflog and then build the new content one line at a\n>> time.\n>>\n>> Signed-off-by: Ronnie Sahlberg \n>> ---\n>> refs.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++---------\n>> 1 file changed, 49 insertions(+), 9 deletions(-)\n>>\n>> diff --git a/refs.c b/refs.c\n>> index a3f60ad..e7ede03 100644\n>> --- a/refs.c\n>> +++ b/refs.c\n>> @@ -37,6 +37,10 @@ static inline int bad_ref_char(int ch)\n>> * need to lock the loose ref during the transaction.\n>> */\n>> #define REF_ISPACKONLY 0x0200\n>> +/** Only the first reflog update needs to lock the reflog file. Further updates\n>> + * just use the lock taken by the first update.\n>> + */\n>\n> Style.\n>\n>> @@ -3349,8 +3355,23 @@ int transaction_update_reflog(struct ref_transaction *transaction,\n>> int flags)\n>> {\n>> struct ref_update *update;\n>> + int i;\n>>\n>> update = add_update(transaction, refname, UPDATE_LOG);\n>> + update->flags = flags;\n>> + for (i = 0; i < transaction->nr - 1; i++) {\n>> + if (transaction->updates[i]->update_type != UPDATE_LOG)\n>> + continue;\n>> + if (!strcmp(transaction->updates[i]->refname,\n>> + update->refname)) {\n>> + update->flags |= UPDATE_REFLOG_NOLOCK;\n>> + update->orig_update = transaction->updates[i];\n>> + break;\n>> + }\n>> + }\n>> + if (!(update->flags & UPDATE_REFLOG_NOLOCK))\n>> + update->reflog_lock = xcalloc(1, sizeof(struct lock_file));\n>\n> So with two calls to transaction-update-reflog, we make two calls to\n> add-update, and each holds a separate lock? If we write two entries\n> to record two updates of the same ref, would we still want to do so?\n\nAlso, indent with tabs rather than spaces (the line following the 'if').\n\n>> + /* Rollback any reflog files that are still open */\n>> + for (i = 0; i < n; i++) {\n>> + struct ref_update *update = updates[i];\n>> +\n>> + if (update->update_type != UPDATE_LOG)\n>> + continue;\n>> + if (update->flags & UPDATE_REFLOG_NOLOCK)\n>> + continue;\n>> + if (update->reflog_fd == -1)\n>> + continue;\n>> + rollback_lock_file(update->reflog_lock);\n>> + }\n>> transaction->status = ret ? REF_TRANSACTION_ERROR\n>> : REF_TRANSACTION_CLOSED;\n\nIndent with tabs.\n"},{"id":"242212","messageId":"CAL=YDWm3N28jMmdWwWLrseK9bG0kwAmYDZ6+yRoa8cfA3S_kxQ@mail.gmail.com","threadId":"36660","inReplyTo":"xmqqtx8p8lj2.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 08/31] refs.c: only write reflog update if msg is non-NULL","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-19T22:55:52Z","receivedAt":"2014-05-19T22:55:52Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"On Fri, May 16, 2014 at 2:24 PM, Junio C Hamano wrote:\n> Ronnie Sahlberg writes:\n>\n>> When performing a reflog transaction update, only write to the reflog iff\n>> msg is non-NULL. This can then be combined with REFLOG_TRUNCATE to perform\n>> an update that only truncates but does not write.\n>>\n>> Signed-off-by: Ronnie Sahlberg \n>> ---\n>> refs.c | 8 +++++---\n>> refs.h | 1 +\n>> 2 files changed, 6 insertions(+), 3 deletions(-)\n>>\n>> diff --git a/refs.c b/refs.c\n>> index a8b583a..a3f60ad 100644\n>> --- a/refs.c\n>> +++ b/refs.c\n>> @@ -3625,9 +3625,11 @@ int transaction_commit(struct ref_transaction *transaction,\n>> ftruncate(update->reflog_fd, 0)) {\n>> error(\"Could not truncate reflog\");\n>> }\n>> - if (log_ref_write_fd(update->reflog_fd, update->old_sha1,\n>> - update->new_sha1,\n>> - update->committer, update->msg)) {\n>> + if (update->msg && log_ref_write_fd(update->reflog_fd,\n>> + update->old_sha1,\n>> + update->new_sha1,\n>> + update->committer,\n>> + update->msg)) {\n>\n> Wouldn't it make it easier to read if you chopped immediately after\n> the &&, i.e. chopping at a gap at a higher-level in the parse tree?\n\nYepp it does.\nChanged. Thanks.\n>\n> if (update->msg &&\n> log_ref_write_fd(update->reflog_fd,\n> update->old_sha1, update->old_sha1,\n> update->committer, update->msg)) {\n"},{"id":"242214","messageId":"CAL=YDWmuOy=1ghkW6CZcywGGiFJZbptF-EycYsLomcr9m2=Xyw@mail.gmail.com","threadId":"36660","inReplyTo":"CAPig+cSXt3FDS1XtHBq2y5NLQxKrVM4sp=YbRa2skJX=RrOyTw@mail.gmail.com","subject":"Re: [PATCH 09/31] refs.c: allow multiple reflog updates during a single transaction","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-19T22:58:11Z","receivedAt":"2014-05-19T22:58:11Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"On Fri, May 16, 2014 at 3:01 PM, Eric Sunshine wrote:\n> On Fri, May 16, 2014 at 5:35 PM, Junio C Hamano wrote:\n>> Ronnie Sahlberg writes:\n>>\n>>> Allow to make multiple reflog updates to the same ref during a transaction.\n>>> This means we only need to lock the reflog once, during the first update that\n>>> touches the reflog, and that all further updates can just write the reflog\n>>> entry since the reflog is already locked.\n>>>\n>>> This allows us to write code such as:\n>>>\n>>> t = transaction_begin()\n>>> transaction_reflog_update(t, \"foo\", REFLOG_TRUNCATE, NULL);\n>>> loop-over-somehting...\n>>> transaction_reflog_update(t, \"foo\", 0, );\n>>> transaction_commit(t)\n>>\n>> OK, so you are now doing not just \"refs\" but also \"reflogs\", you\n>> felt that \"ref_transaction()\" does not cover the latter. Is that\n>> the reason for the rename in the earlier step?\n>>\n>> I am sort-of on the fence.\n>>\n>> Calling the begin \"ref_transaction_begin\" and then calling the new\n>> function \"ref_transaction_log_update\" would still allow us to\n>> differentiate transactions on refs and reflogs, while allowing other\n>> kinds of transactions that are not related to refs at all to employ\n>> a mechanism that is different from the one that is used to implement\n>> the transactions on refs and reflogs you are building here.\n>>\n>> But I think I am OK with the generic \"transaction-begin\" now.\n>> Having one mechanism for refs and reflogs, and then having another\n>> completely different mechanism for other things, will not let us\n>> coordinate between the two easily, so \"allow transactions that are\n>> not related to refs at all to be built on a different mechanism\" may\n>> not be a worthwhile goal to pursue in the first place. Please\n>> consider the question on the naming in the earlier one dropped.\n>>\n>>>\n>>> where we first truncate the reflog and then build the new content one line at a\n>>> time.\n>>>\n>>> Signed-off-by: Ronnie Sahlberg \n>>> ---\n>>> refs.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++---------\n>>> 1 file changed, 49 insertions(+), 9 deletions(-)\n>>>\n>>> diff --git a/refs.c b/refs.c\n>>> index a3f60ad..e7ede03 100644\n>>> --- a/refs.c\n>>> +++ b/refs.c\n>>> @@ -37,6 +37,10 @@ static inline int bad_ref_char(int ch)\n>>> * need to lock the loose ref during the transaction.\n>>> */\n>>> #define REF_ISPACKONLY 0x0200\n>>> +/** Only the first reflog update needs to lock the reflog file. Further updates\n>>> + * just use the lock taken by the first update.\n>>> + */\n>>\n>> Style.\n>>\n>>> @@ -3349,8 +3355,23 @@ int transaction_update_reflog(struct ref_transaction *transaction,\n>>> int flags)\n>>> {\n>>> struct ref_update *update;\n>>> + int i;\n>>>\n>>> update = add_update(transaction, refname, UPDATE_LOG);\n>>> + update->flags = flags;\n>>> + for (i = 0; i < transaction->nr - 1; i++) {\n>>> + if (transaction->updates[i]->update_type != UPDATE_LOG)\n>>> + continue;\n>>> + if (!strcmp(transaction->updates[i]->refname,\n>>> + update->refname)) {\n>>> + update->flags |= UPDATE_REFLOG_NOLOCK;\n>>> + update->orig_update = transaction->updates[i];\n>>> + break;\n>>> + }\n>>> + }\n>>> + if (!(update->flags & UPDATE_REFLOG_NOLOCK))\n>>> + update->reflog_lock = xcalloc(1, sizeof(struct lock_file));\n>>\n>> So with two calls to transaction-update-reflog, we make two calls to\n>> add-update, and each holds a separate lock? If we write two entries\n>> to record two updates of the same ref, would we still want to do so?\n>\n> Also, indent with tabs rather than spaces (the line following the 'if').\n\nDone.\n\n>\n>>> + /* Rollback any reflog files that are still open */\n>>> + for (i = 0; i < n; i++) {\n>>> + struct ref_update *update = updates[i];\n>>> +\n>>> + if (update->update_type != UPDATE_LOG)\n>>> + continue;\n>>> + if (update->flags & UPDATE_REFLOG_NOLOCK)\n>>> + continue;\n>>> + if (update->reflog_fd == -1)\n>>> + continue;\n>>> + rollback_lock_file(update->reflog_lock);\n>>> + }\n>>> transaction->status = ret ? REF_TRANSACTION_ERROR\n>>> : REF_TRANSACTION_CLOSED;\n>\n> Indent with tabs.\n\nDone. Thanks.\n"},{"id":"242215","messageId":"CAL=YDWn5GXR8ADt8a0dJBS-t62xrkuMz_0h=HTqLkpCawoV=xw@mail.gmail.com","threadId":"36660","inReplyTo":"xmqqppjd8l13.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 09/31] refs.c: allow multiple reflog updates during a single transaction","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-19T23:06:54Z","receivedAt":"2014-05-19T23:06:54Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"On Fri, May 16, 2014 at 2:35 PM, Junio C Hamano wrote:\n> Ronnie Sahlberg writes:\n>\n>> Allow to make multiple reflog updates to the same ref during a transaction.\n>> This means we only need to lock the reflog once, during the first update that\n>> touches the reflog, and that all further updates can just write the reflog\n>> entry since the reflog is already locked.\n>>\n>> This allows us to write code such as:\n>>\n>> t = transaction_begin()\n>> transaction_reflog_update(t, \"foo\", REFLOG_TRUNCATE, NULL);\n>> loop-over-somehting...\n>> transaction_reflog_update(t, \"foo\", 0, );\n>> transaction_commit(t)\n>\n> OK, so you are now doing not just \"refs\" but also \"reflogs\", you\n> felt that \"ref_transaction()\" does not cover the latter. Is that\n> the reason for the rename in the earlier step?\n>\n> I am sort-of on the fence.\n>\n> Calling the begin \"ref_transaction_begin\" and then calling the new\n> function \"ref_transaction_log_update\" would still allow us to\n> differentiate transactions on refs and reflogs, while allowing other\n> kinds of transactions that are not related to refs at all to employ\n> a mechanism that is different from the one that is used to implement\n> the transactions on refs and reflogs you are building here.\n>\n> But I think I am OK with the generic \"transaction-begin\" now.\n> Having one mechanism for refs and reflogs, and then having another\n> completely different mechanism for other things, will not let us\n> coordinate between the two easily, so \"allow transactions that are\n> not related to refs at all to be built on a different mechanism\" may\n> not be a worthwhile goal to pursue in the first place. Please\n> consider the question on the naming in the earlier one dropped.\n>\n>>\n>> where we first truncate the reflog and then build the new content one line at a\n>> time.\n>>\n>> Signed-off-by: Ronnie Sahlberg \n>> ---\n>> refs.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++---------\n>> 1 file changed, 49 insertions(+), 9 deletions(-)\n>>\n>> diff --git a/refs.c b/refs.c\n>> index a3f60ad..e7ede03 100644\n>> --- a/refs.c\n>> +++ b/refs.c\n>> @@ -37,6 +37,10 @@ static inline int bad_ref_char(int ch)\n>> * need to lock the loose ref during the transaction.\n>> */\n>> #define REF_ISPACKONLY 0x0200\n>> +/** Only the first reflog update needs to lock the reflog file. Further updates\n>> + * just use the lock taken by the first update.\n>> + */\n>\n> Style.\n\nDone.\n\n>\n>> @@ -3349,8 +3355,23 @@ int transaction_update_reflog(struct ref_transaction *transaction,\n>> int flags)\n>> {\n>> struct ref_update *update;\n>> + int i;\n>>\n>> update = add_update(transaction, refname, UPDATE_LOG);\n>> + update->flags = flags;\n>> + for (i = 0; i < transaction->nr - 1; i++) {\n>> + if (transaction->updates[i]->update_type != UPDATE_LOG)\n>> + continue;\n>> + if (!strcmp(transaction->updates[i]->refname,\n>> + update->refname)) {\n>> + update->flags |= UPDATE_REFLOG_NOLOCK;\n>> + update->orig_update = transaction->updates[i];\n>> + break;\n>> + }\n>> + }\n>> + if (!(update->flags & UPDATE_REFLOG_NOLOCK))\n>> + update->reflog_lock = xcalloc(1, sizeof(struct lock_file));\n>\n> So with two calls to transaction-update-reflog, we make two calls to\n> add-update, and each holds a separate lock? If we write two entries\n> to record two updates of the same ref, would we still want to do so?\n\nIf we call transaction_update_reflog twice we will call add_update\ntwice since we are doing two updates.\nBut if the two calls are for the same ref then we only lock the reflog\nfor the first update and then in the second update we re-use the lock\nstructure from the first.\n\nLater updates to reflog.c will show how to loop over the old reflog,\nusing the existing iterator, and then create a new expired reflog one\nentry at a time.\n\n\n>\n>> + /* Rollback any reflog files that are still open */\n>> + for (i = 0; i < n; i++) {\n>> + struct ref_update *update = updates[i];\n>> +\n>> + if (update->update_type != UPDATE_LOG)\n>> + continue;\n>> + if (update->flags & UPDATE_REFLOG_NOLOCK)\n>> + continue;\n>> + if (update->reflog_fd == -1)\n>> + continue;\n>> + rollback_lock_file(update->reflog_lock);\n>> + }\n>> transaction->status = ret ? REF_TRANSACTION_ERROR\n>> : REF_TRANSACTION_CLOSED;\n"},{"id":"242216","messageId":"CAL=YDW==tVrtonoDmBrucWgnTGbqEs70tq5zCp6xjDPgg4G16A@mail.gmail.com","threadId":"36660","inReplyTo":"xmqq38g9a0i0.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 03/31] refs.c: rename the transaction functions","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-19T23:11:37Z","receivedAt":"2014-05-19T23:11:37Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"On Fri, May 16, 2014 at 2:15 PM, Junio C Hamano wrote:\n> Ronnie Sahlberg writes:\n>\n>> Rename the transaction functions. Remove the leading ref_ from the names\n>> and append _sha1 to the names for functions that create/delete/update sha1\n>> refs.\n>> ...\n>> - transaction = ref_transaction_begin();\n>> + transaction = transaction_begin();\n>\n> Why? Do we forsee that there will never be other kinds of\n> transaction, and whenever we say a \"transaction\" that will be always\n> about updating the refs and nothing else?\n\nref_transaction_... is a bit of a mouthful.\nI think initially we will only need a transaction framework for normal\nsha1 refs, for symrefs and for reflog updates and these should all be\nhandled by the same transaction system so a single\n_begin/_update*/_commit should be able to atomically commit changes\nthat involve updates to all three types.\n\nI am not sure if we need transactions for other types of data, such as\nsha1 objects, but if it turns out we do in the future we can rename\nthese functions again.\n"},{"id":"242218","messageId":"xmqqlhtxuzaf.fsf@gitster.dls.corp.google.com","threadId":"36660","inReplyTo":"CAL=YDW==tVrtonoDmBrucWgnTGbqEs70tq5zCp6xjDPgg4G16A@mail.gmail.com","subject":"Re: [PATCH 03/31] refs.c: rename the transaction functions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-19T23:25:12Z","receivedAt":"2014-05-19T23:25:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ronnie Sahlberg writes:\n\n> I am not sure if we need transactions for other types of data, such as\n> sha1 objects, but if it turns out we do in the future we can rename\n> these functions again.\n\nI was wrong (and I think you read it in the later patch review).\n\nIf we need transaction for other types of data, and we will\neventually need to coordinate the transaction semantics over refs\nand those other types of data. It would be far cleaner to express\nthat coordination within the same transaction framework.\n\nIn other words, we do not want to be in a situation like this:\n\n other_transaction_begin();\n ref_transaction_begin();\n ref_transaction_update();\n other_transaction_update();\n ref_transaction_commit();\n if (other_transaction_commit() != SUCCESS) {\n ... oops it is too late to roll back the ref_transaction ...\n other_transaction_rollback();\t\n }\n\nand force us doing 3-phase commit inside ourselves to work it\naround.\n"},{"id":"242219","messageId":"CAL=YDWn=t_+0Tt7--k1wAfUFXwTpsaEUDgYwdKbDPjmca5xdVA@mail.gmail.com","threadId":"36660","inReplyTo":"xmqqy4y18lol.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 07/31] refs.c: add a flag to allow reflog updates to truncate the log","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-05-19T23:27:05Z","receivedAt":"2014-05-19T23:27:05Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"On Fri, May 16, 2014 at 2:20 PM, Junio C Hamano wrote:\n> Ronnie Sahlberg writes:\n>\n>> Add a flag that allows us to truncate the reflog before we write the update.\n>>\n>> Signed-off-by: Ronnie Sahlberg \n>> ---\n>\n> Until we read the callers it is hard to see how such a feature is\n> useful, though.\n\nShortly later in the series comes the update to builtin/reflog.c which\nuses this feature.\nEven later in the series I also use this in builtin/reflog.c to both\ndo the ref update as well as the corresponding reflog updates all in\none single transaction.\n\n\n>\n> (style) The two multi-line comments are formatted differently.\n\nFixed. Thanks.\n\n>\n>> refs.c | 12 ++++++++++--\n>> refs.h | 4 +++-\n>> 2 files changed, 13 insertions(+), 3 deletions(-)\n>>\n>> diff --git a/refs.c b/refs.c\n>> index d8a1568..a8b583a 100644\n>> --- a/refs.c\n>> +++ b/refs.c\n>> @@ -3608,7 +3608,11 @@ int transaction_commit(struct ref_transaction *transaction,\n>> }\n>> }\n>>\n>> - /* Update all reflog files */\n>> + /* Update all reflog files\n>> + We have already done all ref updates and deletes.\n>> + There is not much we can do here if there are any reflog\n>> + update errors, other than complain.\n>> + */\n>> for (i = 0; i < n; i++) {\n>> struct ref_update *update = updates[i];\n>>\n>> @@ -3616,7 +3620,11 @@ int transaction_commit(struct ref_transaction *transaction,\n>> continue;\n>> if (update->reflog_fd == -1)\n>> continue;\n>> -\n>> + if (update->flags & REFLOG_TRUNCATE)\n>> + if (lseek(update->reflog_fd, 0, SEEK_SET) < 0 ||\n>> + ftruncate(update->reflog_fd, 0)) {\n>> + error(\"Could not truncate reflog\");\n>> + }\n>\n> The {} looks somewhat curious here. If you seeked to the beginning,\n> and failed to truncate, do you still want to go on without \"return\"\n> in front of the error()? Would that overwrite existing entries?\n\nI have changed it to do this :\n+ if (update->flags & REFLOG_TRUNCATE)\n+ if (lseek(update->reflog_fd, 0, SEEK_SET) < 0 ||\n+ ftruncate(update->reflog_fd, 0)) {\n+ error(\"Could not truncate reflog: %s\",\n+ update->refname);\n+ rollback_lock_file(&update->reflog_lock);\n+ update->reflog_fd = -1;\n+ }\n\n\nAt this point we have already committed all the changes to refs and\nalso performed all unlinks for refs we are deleting. So if things\nstart to go wrong here during the reflog updates it is too late to do\nmuch more than complain.\n\n\n>\n>> if (log_ref_write_fd(update->reflog_fd, update->old_sha1,\n>> update->new_sha1,\n>> update->committer, update->msg)) {\n>> diff --git a/refs.h b/refs.h\n>> index 2e22a26..b1b97e7 100644\n>> --- a/refs.h\n>> +++ b/refs.h\n>> @@ -269,8 +269,10 @@ int transaction_delete_sha1(struct ref_transaction *transaction,\n>> const unsigned char *old_sha1,\n>> int flags, int have_old, const char *msg);\n>>\n>> +#define REFLOG_TRUNCATE 0x01\n>> /*\n>> - * Append a reflog entry for refname.\n>> + * Append a reflog entry for refname. If the REFLOG_TRUNCATE flag is set\n>> + * this update will first truncate the reflog before writing the entry.\n>> */\n>> int transaction_update_reflog(struct ref_transaction *transaction,\n>> const char *refname,\n"}]}