{"thread":{"id":"36949","subject":"[PATCH v3 00/14] ref-transactions-reflog","startedAt":"2014-06-18T17:08:52Z","lastAt":"2014-07-02T18:27:25Z","messageCount":19,"participants":["Ronnie Sahlberg","Junio C Hamano"],"isPatch":true,"patchVersion":3,"patchTotal":14},"messages":[{"id":"244520","messageId":"1403111346-18466-1-git-send-email-sahlberg@google.com","threadId":"36949","inReplyTo":null,"subject":"[PATCH v3 00/14] ref-transactions-reflog","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-06-18T17:08:52Z","receivedAt":"2014-06-18T17:08:52Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"These patches can also be found at:\nhttps://github.com/rsahlberg/git/tree/ref-transactions-reflog\nThis series is based on, and applies ontop of, the previous 48 patch long\nref-transaction series that is now in origin/pu.\n\n \nThis series introduces support for reflog updates to the transaction framework\nand ends up re-factoring reflog.c to use a single atomic transaction for\nupdating both the ref and its reflog.\n\nWith these changes we also reduce the number of places where we build and write\na reflog entry to a single function which makes maintenance easier.\nSeveral functions that act on reflogs can now be made private to refs.c\nsince we no longer need to export them.\n\n\nThis is version 3:\n - Update and rebased ontop of the current ref-transactions series.\n\nRonnie Sahlberg (14):\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 lockfile.c: make hold_lock_file_for_append preserve meaningful errno\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: rename log_ref_setup to create_reflog\n refs.c: make unlock_ref/close_ref/commit_ref static\n refs.c: remove lock_any_ref_for_update\n\n branch.c | 11 +-\n builtin/checkout.c | 8 +-\n builtin/commit.c | 14 +-\n builtin/fetch.c | 12 +-\n builtin/receive-pack.c | 14 +-\n builtin/reflog.c | 84 +++++------\n builtin/replace.c | 10 +-\n builtin/tag.c | 10 +-\n builtin/update-ref.c | 22 +--\n copy.c | 20 ++-\n fast-import.c | 22 +--\n lockfile.c | 7 +-\n refs.c | 377 ++++++++++++++++++++++++++++++++++---------------\n refs.h | 109 +++++++-------\n sequencer.c | 12 +-\n walker.c | 16 +--\n 16 files changed, 448 insertions(+), 300 deletions(-)\n\n-- \n2.0.0.467.g08c0633\n"},{"id":"244527","messageId":"1403111346-18466-2-git-send-email-sahlberg@google.com","threadId":"36949","inReplyTo":"1403111346-18466-1-git-send-email-sahlberg@google.com","subject":"[PATCH v3 01/14] refs.c make ref_transaction_create a wrapper to ref_transaction_update","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-06-18T17:08:53Z","receivedAt":"2014-06-18T17:08:53Z","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, 6 insertions(+), 14 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex dfbf003..a9f91ab 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3487,23 +3487,14 @@ int ref_transaction_create(struct ref_transaction *transaction,\n \t\t\t int flags, const char *msg,\n \t\t\t struct strbuf *err)\n {\n-\tstruct ref_update *update;\n-\n \tif (transaction->state != REF_TRANSACTION_OPEN)\n \t\tdie(\"BUG: create called for transaction that is not open\");\n \n \tif (!new_sha1 || is_null_sha1(new_sha1))\n \t\tdie(\"BUG: create ref with null new_sha1\");\n \n-\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, err);\n }\n \n int ref_transaction_delete(struct ref_transaction *transaction,\ndiff --git a/refs.h b/refs.h\nindex db463d0..495740d 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -283,9 +283,10 @@ struct ref_transaction *ref_transaction_begin(struct strbuf *err);\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. A failure to update\n * means that the transaction as a whole has failed and will need to be\n * rolled back.\n-- \n2.0.0.467.g08c0633\n"},{"id":"244522","messageId":"1403111346-18466-3-git-send-email-sahlberg@google.com","threadId":"36949","inReplyTo":"1403111346-18466-1-git-send-email-sahlberg@google.com","subject":"[PATCH v3 02/14] refs.c: make ref_transaction_delete a wrapper for ref_transaction_update","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-06-18T17:08:54Z","receivedAt":"2014-06-18T17:08:54Z","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 | 17 +++++------------\n refs.h | 2 +-\n 2 files changed, 6 insertions(+), 13 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex a9f91ab..0eace70 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3503,24 +3503,17 @@ int ref_transaction_delete(struct ref_transaction *transaction,\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 \tif (transaction->state != REF_TRANSACTION_OPEN)\n \t\tdie(\"BUG: delete called for transaction that is not open\");\n \n \tif (have_old && !old_sha1)\n \t\tdie(\"BUG: have_old is true but old_sha1 is NULL\");\n \n-\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+\tif (have_old && is_null_sha1(old_sha1))\n+\t\tdie(\"BUG: have_old is true but old_sha1 is null_sha1\");\n+\n+\treturn ref_transaction_update(transaction, refname, null_sha1,\n+\t\t\t\t old_sha1, flags, have_old, msg, err);\n }\n \n int update_ref(const char *action, const char *refname,\ndiff --git a/refs.h b/refs.h\nindex 495740d..20bb152 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -282,7 +282,7 @@ struct ref_transaction *ref_transaction_begin(struct strbuf *err);\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.467.g08c0633\n"},{"id":"244528","messageId":"1403111346-18466-4-git-send-email-sahlberg@google.com","threadId":"36949","inReplyTo":"1403111346-18466-1-git-send-email-sahlberg@google.com","subject":"[PATCH v3 03/14] refs.c: rename the transaction functions","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-06-18T17:08:55Z","receivedAt":"2014-06-18T17:08:55Z","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\nnames and append _sha1 to the names for functions that create/delete/\nupdate sha1 refs.\n\nSigned-off-by: Ronnie Sahlberg \n---\n branch.c | 11 ++++---\n builtin/commit.c | 14 ++++-----\n builtin/fetch.c | 12 ++++----\n builtin/receive-pack.c | 14 ++++-----\n builtin/replace.c | 10 +++---\n builtin/tag.c | 10 +++---\n builtin/update-ref.c | 22 +++++++-------\n fast-import.c | 22 +++++++-------\n refs.c | 82 +++++++++++++++++++++++++-------------------------\n refs.h | 56 +++++++++++++++++-----------------\n sequencer.c | 12 ++++----\n walker.c | 16 +++++-----\n 12 files changed, 141 insertions(+), 140 deletions(-)\n\ndiff --git a/branch.c b/branch.c\nindex e0439af..6fa6d78 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -298,13 +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(&err);\n+\t\ttransaction = transaction_begin(&err);\n \t\tif (!transaction ||\n-\t\t ref_transaction_update(transaction, ref.buf, sha1,\n-\t\t\t\t\t null_sha1, 0, !forcing, msg, &err) ||\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\t\t\t &err) ||\n+\t\t transaction_commit(transaction, &err))\n \t\t\tdie(\"%s\", 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 c499826..bf8d85a 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1762,17 +1762,17 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \tstrbuf_insert(&sb, 0, reflog_msg, strlen(reflog_msg));\n \tstrbuf_insert(&sb, strlen(reflog_msg), \": \", 2);\n \n-\ttransaction = ref_transaction_begin(&err);\n+\ttransaction = transaction_begin(&err);\n \tif (!transaction ||\n-\t ref_transaction_update(transaction, \"HEAD\", sha1,\n-\t\t\t\t current_head ?\n-\t\t\t\t current_head->object.sha1 : NULL,\n-\t\t\t\t 0, !!current_head, sb.buf, &err) ||\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, &err) ||\n+\t transaction_commit(transaction, &err)) {\n \t\trollback_index_files();\n \t\tdie(\"%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 52f1ebc..baf7929 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -385,22 +385,22 @@ 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-\ttransaction = ref_transaction_begin(&err);\n+\ttransaction = transaction_begin(&err);\n \tif (!transaction ||\n-\t ref_transaction_update(transaction, ref->name, ref->new_sha1,\n-\t\t\t\t ref->old_sha1, 0, check_old, msg, &err))\n+\t transaction_update_sha1(transaction, ref->name, ref->new_sha1,\n+\t\t\t\t ref->old_sha1, 0, check_old, msg, &err))\n \t\tgoto fail;\n \n-\tret = ref_transaction_commit(transaction, &err);\n+\tret = transaction_commit(transaction, &err);\n \tif (ret == UPDATE_REFS_NAME_CONFLICT)\n \t\tdf_conflict = 1;\n \tif (ret)\n \t\tgoto fail;\n \n-\tref_transaction_free(transaction);\n+\ttransaction_free(transaction);\n \treturn 0;\n fail:\n-\tref_transaction_free(transaction);\n+\ttransaction_free(transaction);\n \terror(\"%s\", err.buf);\n \tstrbuf_release(&err);\n \treturn df_conflict ? STORE_REF_ERROR_DF_CONFLICT\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex 0ed1ddd..dcd1862 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -582,20 +582,20 @@ static char *update(struct command *cmd, struct shallow_info *si)\n \t\t update_shallow_ref(cmd, si))\n \t\t\treturn xstrdup(\"shallow error\");\n \n-\t\ttransaction = ref_transaction_begin(&err);\n+\t\ttransaction = transaction_begin(&err);\n \t\tif (!transaction ||\n-\t\t ref_transaction_update(transaction, namespaced_name,\n-\t\t\t\t\t new_sha1, old_sha1, 0, 1, \"push\",\n-\t\t\t\t\t &err) ||\n-\t\t ref_transaction_commit(transaction, &err)) {\n+\t\t 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 &err) ||\n+\t\t transaction_commit(transaction, &err)) {\n \t\t\tchar *str = strbuf_detach(&err, NULL);\n-\t\t\tref_transaction_free(transaction);\n+\t\t\ttransaction_free(transaction);\n \n \t\t\trp_error(\"%s\", str);\n \t\t\treturn str;\n \t\t}\n \n-\t\tref_transaction_free(transaction);\n+\t\ttransaction_free(transaction);\n \t\tstrbuf_release(&err);\n \t\treturn NULL; /* good */\n \t}\ndiff --git a/builtin/replace.c b/builtin/replace.c\nindex 8e699f2..9e3f7e0 100644\n--- a/builtin/replace.c\n+++ b/builtin/replace.c\n@@ -167,14 +167,14 @@ static int replace_object_sha1(const char *object_ref,\n \n \tcheck_ref_valid(object, prev, ref, sizeof(ref), force);\n \n-\ttransaction = ref_transaction_begin(&err);\n+\ttransaction = transaction_begin(&err);\n \tif (!transaction ||\n-\t ref_transaction_update(transaction, ref, repl, prev,\n-\t\t\t\t 0, !is_null_sha1(prev), NULL, &err) ||\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, &err) ||\n+\t transaction_commit(transaction, &err))\n \t\tdie(\"%s\", 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 74af63e..4525c69 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(&err);\n+\ttransaction = transaction_begin(&err);\n \tif (!transaction ||\n-\t ref_transaction_update(transaction, ref.buf, object, prev,\n-\t\t\t\t 0, !is_null_sha1(prev), NULL, &err) ||\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, &err) ||\n+\t transaction_commit(transaction, &err))\n \t\tdie(\"%s\", 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 28b478a..d62871d 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -199,8 +199,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, &err))\n+\tif (transaction_update_sha1(transaction, refname, new_sha1, old_sha1,\n+\t\t\t\t update_flags, have_old, msg, &err))\n \t\tdie(\"%s\", err.buf);\n \n \tupdate_flags = 0;\n@@ -227,8 +227,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, &err))\n+\tif (transaction_create_sha1(transaction, refname, new_sha1,\n+\t\t\t\t update_flags, msg, &err))\n \t\tdie(\"%s\", err.buf);\n \n \tupdate_flags = 0;\n@@ -259,8 +259,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, &err))\n+\tif (transaction_delete_sha1(transaction, refname, old_sha1,\n+\t\t\t\t update_flags, have_old, msg, &err))\n \t\tdie(\"%s\", err.buf);\n \n \tupdate_flags = 0;\n@@ -292,8 +292,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, &err))\n+\tif (transaction_update_sha1(transaction, refname, new_sha1, old_sha1,\n+\t\t\t\t update_flags, have_old, msg, &err))\n \t\tdie(\"%s\", err.buf);\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(&err);\n+\t\ttransaction = transaction_begin(&err);\n \t\tif (delete || no_deref || argc > 0)\n \t\t\tusage_with_options(git_update_ref_usage, options);\n \t\tif (end_null)\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 f5ccf4e..778b6ea 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -1705,17 +1705,17 @@ static int update_branch(struct branch *b)\n \t\t\treturn -1;\n \t\t}\n \t}\n-\ttransaction = ref_transaction_begin(&err);\n+\ttransaction = transaction_begin(&err);\n \tif (!transaction ||\n-\t ref_transaction_update(transaction, b->name, b->sha1, old_sha1,\n-\t\t\t\t 0, 1, msg, &err) ||\n-\t ref_transaction_commit(transaction, &err)) {\n-\t\tref_transaction_free(transaction);\n+\t transaction_update_sha1(transaction, b->name, b->sha1, old_sha1,\n+\t\t\t\t 0, 1, msg, &err) ||\n+\t transaction_commit(transaction, &err)) {\n+\t\ttransaction_free(transaction);\n \t\terror(\"%s\", err.buf);\n \t\tstrbuf_release(&err);\n \t\treturn -1;\n \t}\n-\tref_transaction_free(transaction);\n+\ttransaction_free(transaction);\n \treturn 0;\n }\n \n@@ -1738,7 +1738,7 @@ static void dump_tags(void)\n \tstruct strbuf err = STRBUF_INIT;\n \tstruct ref_transaction *transaction;\n \n-\ttransaction = ref_transaction_begin(&err);\n+\ttransaction = transaction_begin(&err);\n \tif (!transaction) {\n \t\tfailure |= error(\"%s\", err.buf);\n \t\tgoto cleanup;\n@@ -1747,17 +1747,17 @@ static void dump_tags(void)\n \t\tstrbuf_reset(&ref_name);\n \t\tstrbuf_addf(&ref_name, \"refs/tags/%s\", t->name);\n \n-\t\tif (ref_transaction_update(transaction, ref_name.buf, t->sha1,\n-\t\t\t\t\t NULL, 0, 0, msg, &err)) {\n+\t\tif (transaction_update_sha1(transaction, ref_name.buf, t->sha1,\n+\t\t\t\t\t NULL, 0, 0, msg, &err)) {\n \t\t\tfailure |= error(\"%s\", err.buf);\n \t\t\tgoto cleanup;\n \t\t}\n \t}\n-\tif (ref_transaction_commit(transaction, &err))\n+\tif (transaction_commit(transaction, &err))\n \t\tfailure |= error(\"%s\", err.buf);\n \n cleanup:\n-\tref_transaction_free(transaction);\n+\ttransaction_free(transaction);\n \tstrbuf_release(&ref_name);\n \tstrbuf_release(&err);\n }\ndiff --git a/refs.c b/refs.c\nindex 0eace70..4e3d4c3 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -25,7 +25,7 @@ static unsigned char refname_disposition[256] = {\n };\n \n /*\n- * Used as a flag to ref_transaction_delete when a loose ref is being\n+ * Used as a flag to transaction_delete_sha1 when a loose ref is being\n * pruned.\n */\n #define REF_ISPRUNING\t0x0100\n@@ -2402,17 +2402,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(&err);\n+\ttransaction = transaction_begin(&err);\n \tif (!transaction ||\n-\t ref_transaction_delete(transaction, r->name, r->sha1,\n-\t\t\t\t REF_ISPRUNING, 1, NULL, &err) ||\n-\t ref_transaction_commit(transaction, &err)) {\n-\t\tref_transaction_free(transaction);\n+\t transaction_delete_sha1(transaction, r->name, r->sha1,\n+\t\t\t\t REF_ISPRUNING, 1, NULL, &err) ||\n+\t transaction_commit(transaction, &err)) {\n+\t\ttransaction_free(transaction);\n \t\terror(\"%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@@ -2598,17 +2598,17 @@ int delete_ref(const char *refname, const unsigned char *sha1, int delopt)\n \tstruct ref_transaction *transaction;\n \tstruct strbuf err = STRBUF_INIT;\n \n-\ttransaction = ref_transaction_begin(&err);\n+\ttransaction = transaction_begin(&err);\n \tif (!transaction ||\n-\t ref_transaction_delete(transaction, refname, sha1, delopt,\n-\t\t\t\t sha1 && !is_null_sha1(sha1), NULL, &err) ||\n-\t ref_transaction_commit(transaction, &err)) {\n+\t transaction_delete_sha1(transaction, refname, sha1, delopt,\n+\t\t\t\t sha1 && !is_null_sha1(sha1), NULL, &err) ||\n+\t transaction_commit(transaction, &err)) {\n \t\terror(\"%s\", err.buf);\n-\t\tref_transaction_free(transaction);\n+\t\ttransaction_free(transaction);\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@@ -3423,12 +3423,12 @@ struct ref_transaction {\n \tint status;\n };\n \n-struct ref_transaction *ref_transaction_begin(struct strbuf *err)\n+struct ref_transaction *transaction_begin(struct strbuf *err)\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@@ -3455,12 +3455,12 @@ 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-\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 struct strbuf *err)\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 struct strbuf *err)\n {\n \tstruct ref_update *update;\n \n@@ -3481,11 +3481,11 @@ 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-\t\t\t struct strbuf *err)\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 struct strbuf *err)\n {\n \tif (transaction->state != REF_TRANSACTION_OPEN)\n \t\tdie(\"BUG: create called for transaction that is not open\");\n@@ -3493,15 +3493,15 @@ int ref_transaction_create(struct ref_transaction *transaction,\n \tif (!new_sha1 || is_null_sha1(new_sha1))\n \t\tdie(\"BUG: create ref with null new_sha1\");\n \n-\treturn ref_transaction_update(transaction, refname, new_sha1,\n-\t\t\t\t null_sha1, flags, 1, msg, err);\n+\treturn transaction_update_sha1(transaction, refname, new_sha1,\n+\t\t\t\t null_sha1, flags, 1, msg, err);\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-\t\t\t struct strbuf *err)\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 struct strbuf *err)\n {\n \tif (transaction->state != REF_TRANSACTION_OPEN)\n \t\tdie(\"BUG: delete called for transaction that is not open\");\n@@ -3512,7 +3512,7 @@ int ref_transaction_delete(struct ref_transaction *transaction,\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-\treturn ref_transaction_update(transaction, refname, null_sha1,\n+\treturn transaction_update_sha1(transaction, refname, null_sha1,\n \t\t\t\t old_sha1, flags, have_old, msg, err);\n }\n \n@@ -3523,14 +3523,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(&err);\n+\tt = transaction_begin(&err);\n \tif (!t ||\n-\t ref_transaction_update(t, refname, sha1, oldval, flags,\n-\t\t\t\t !!oldval, action, &err) ||\n-\t ref_transaction_commit(t, &err)) {\n+\t transaction_update_sha1(t, refname, sha1, oldval, flags,\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 \n-\t\tref_transaction_free(t);\n+\t\ttransaction_free(t);\n \t\tswitch (onerr) {\n \t\tcase UPDATE_REFS_MSG_ON_ERR:\n \t\t\terror(str, refname, err.buf); break;\n@@ -3567,8 +3567,8 @@ 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-\t\t\t struct strbuf *err)\n+int transaction_commit(struct ref_transaction *transaction,\n+\t\t struct strbuf *err)\n {\n \tint ret = 0, delnum = 0, i, df_conflict = 0;\n \tconst char **delnames;\ndiff --git a/refs.h b/refs.h\nindex 20bb152..cf48351 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -17,16 +17,16 @@ struct ref_lock {\n * Calling sequence\n * ----------------\n * - Allocate and initialize a `struct ref_transaction` by calling\n- * `ref_transaction_begin()`.\n+ * `transaction_begin()`.\n *\n * - List intended ref updates by calling functions like\n- * `ref_transaction_update()` and `ref_transaction_create()`.\n+ * `transaction_update_sha1()` and `transaction_create_sha1()`.\n *\n- * - Call `ref_transaction_commit()` to execute the transaction.\n+ * - Call `transaction_commit()` to execute the transaction.\n * If this succeeds, the ref updates will have taken place and\n * the transaction cannot be rolled back.\n *\n- * - At any time call `ref_transaction_free()` to discard the\n+ * - At any time call `transaction_free()` to discard the\n * transaction and free associated resources. In particular,\n * this rolls back the transaction if it has not been\n * successfully committed.\n@@ -44,7 +44,7 @@ struct ref_lock {\n * error message:\n *\n * strbuf_addf(&err, \"Error while doing foo-bar: \");\n- * if (ref_transaction_update(..., &err)) {\n+ * if (transaction_update_sha1(..., &err)) {\n * ret = error(\"%s\", err.buf);\n * goto cleanup;\n * }\n@@ -178,8 +178,8 @@ extern int ref_exists(const char *);\n extern int peel_ref(const char *refname, unsigned char *sha1);\n \n /*\n- * Flags controlling lock_any_ref_for_update(), ref_transaction_update(),\n- * ref_transaction_create(), etc.\n+ * Flags controlling lock_any_ref_for_update(), transaction_update_sha1(),\n+ * transaction_create_sha1(), etc.\n * REF_NODEREF: act on the ref directly, instead of dereferencing\n * symbolic references.\n *\n@@ -268,9 +268,9 @@ enum action_on_err {\n \n /*\n * Begin a reference transaction. The reference transaction must\n- * be freed by calling ref_transaction_free().\n+ * be freed by calling transaction_free().\n */\n-struct ref_transaction *ref_transaction_begin(struct strbuf *err);\n+struct ref_transaction *transaction_begin(struct strbuf *err);\n \n /*\n * The following functions add a reference check or update to a\n@@ -291,12 +291,12 @@ struct ref_transaction *ref_transaction_begin(struct strbuf *err);\n * means that the transaction as a whole has failed and will need to be\n * rolled back.\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-\t\t\t struct strbuf *err);\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 struct strbuf *err);\n \n /*\n * Add a reference creation to transaction. new_sha1 is the value\n@@ -307,11 +307,11 @@ int ref_transaction_update(struct ref_transaction *transaction,\n * means that the transaction as a whole has failed and will need to be\n * rolled back.\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-\t\t\t struct strbuf *err);\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 struct strbuf *err);\n \n /*\n * Add a reference deletion to transaction. If have_old is true, then\n@@ -321,11 +321,11 @@ int ref_transaction_create(struct ref_transaction *transaction,\n * means that the transaction as a whole has failed and will need to be\n * rolled back.\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-\t\t\t struct strbuf *err);\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 struct strbuf *err);\n \n /*\n * Commit all of the changes that have been queued in transaction, as\n@@ -338,13 +338,13 @@ int ref_transaction_delete(struct ref_transaction *transaction,\n * collision (ENOTDIR).\n */\n #define UPDATE_REFS_NAME_CONFLICT -2\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 and all associated data.\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 f9906ef..5080287 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -282,12 +282,12 @@ 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(&err);\n+\ttransaction = transaction_begin(&err);\n \tif (!transaction ||\n-\t ref_transaction_update(transaction, \"HEAD\", to, from,\n-\t\t\t\t 0, !unborn, sb.buf, &err) ||\n-\t ref_transaction_commit(transaction, &err)) {\n-\t\tref_transaction_free(transaction);\n+\t transaction_update_sha1(transaction, \"HEAD\", to, from,\n+\t\t\t\t 0, !unborn, sb.buf, &err) ||\n+\t transaction_commit(transaction, &err)) {\n+\t\ttransaction_free(transaction);\n \t\terror(\"%s\", err.buf);\n \t\tstrbuf_release(&sb);\n \t\tstrbuf_release(&err);\n@@ -295,7 +295,7 @@ static int fast_forward_to(const unsigned char *to, const unsigned char *from,\n \t}\n \n \tstrbuf_release(&sb);\n-\tref_transaction_free(transaction);\n+\ttransaction_free(transaction);\n \treturn 0;\n }\n \ndiff --git a/walker.c b/walker.c\nindex fd9ef87..d22d07a 100644\n--- a/walker.c\n+++ b/walker.c\n@@ -261,7 +261,7 @@ int walker_fetch(struct walker *walker, int targets, char **target,\n \tsave_commit_buffer = 0;\n \n \tif (write_ref) {\n-\t\ttransaction = ref_transaction_begin(&err);\n+\t\ttransaction = transaction_begin(&err);\n \t\tif (!transaction) {\n \t\t\terror(\"%s\", err.buf);\n \t\t\tgoto rollback_and_fail;\n@@ -293,27 +293,27 @@ int walker_fetch(struct walker *walker, int targets, char **target,\n \t\t\tcontinue;\n \t\tstrbuf_reset(&ref_name);\n \t\tstrbuf_addf(&ref_name, \"refs/%s\", write_ref[i]);\n-\t\tif (ref_transaction_update(transaction, ref_name.buf,\n-\t\t\t\t\t &sha1[20 * i], NULL, 0, 0,\n-\t\t\t\t\t msg ? msg : \"fetch (unknown)\",\n-\t\t\t\t\t &err)) {\n+\t\tif (transaction_update_sha1(transaction, ref_name.buf,\n+\t\t\t\t\t &sha1[20 * i], NULL, 0, 0,\n+\t\t\t\t\t msg ? msg : \"fetch (unknown)\",\n+\t\t\t\t\t &err)) {\n \t\t\terror(\"%s\", err.buf);\n \t\t\tgoto rollback_and_fail;\n \t\t}\n \t}\n \tif (write_ref) {\n-\t\tif (ref_transaction_commit(transaction, &err)) {\n+\t\tif (transaction_commit(transaction, &err)) {\n \t\t\terror(\"%s\", err.buf);\n \t\t\tgoto rollback_and_fail;\n \t\t}\n-\t\tref_transaction_free(transaction);\n+\t\ttransaction_free(transaction);\n \t}\n \n \tfree(msg);\n \treturn 0;\n \n rollback_and_fail:\n-\tref_transaction_free(transaction);\n+\ttransaction_free(transaction);\n \tfree(msg);\n \tstrbuf_release(&err);\n \tstrbuf_release(&ref_name);\n-- \n2.0.0.467.g08c0633\n"},{"id":"244532","messageId":"1403111346-18466-5-git-send-email-sahlberg@google.com","threadId":"36949","inReplyTo":"1403111346-18466-1-git-send-email-sahlberg@google.com","subject":"[PATCH v3 04/14] refs.c: add a new update_type field to ref_update","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-06-18T17:08:56Z","receivedAt":"2014-06-18T17:08:56Z","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 | 25 +++++++++++++++++++++----\n 1 file changed, 21 insertions(+), 4 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 4e3d4c3..4129de6 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3374,6 +3374,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@@ -3381,6 +3385,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@@ -3444,12 +3449,14 @@ void transaction_free(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@@ -3470,7 +3477,7 @@ int transaction_update_sha1(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-\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@@ -3555,7 +3562,10 @@ 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 - 1]->update_type != UPDATE_SHA1 ||\n+\t\t 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@@ -3564,6 +3574,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@@ -3593,10 +3604,12 @@ int transaction_commit(struct ref_transaction *transaction,\n \t\tgoto cleanup;\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\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@@ -3619,6 +3632,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 (!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@@ -3638,6 +3653,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->lock) {\n \t\t\tif (delete_ref_loose(update->lock, update->type, err))\n \t\t\t\tret = -1;\n-- \n2.0.0.467.g08c0633\n"},{"id":"244523","messageId":"1403111346-18466-6-git-send-email-sahlberg@google.com","threadId":"36949","inReplyTo":"1403111346-18466-1-git-send-email-sahlberg@google.com","subject":"[PATCH v3 05/14] refs.c: add a function to append a reflog entry to a fd","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-06-18T17:08:57Z","receivedAt":"2014-06-18T17:08:57Z","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\ndescriptor from log_ref_write and into a dedicated function log_ref_write_fd.\nFor now this is only used from log_ref_write but later on we will call\nthis function from reflog transactions too which means that we will end\nup with only a single place where we write a reflog entry to a file instead\nof the current two places (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 4129de6..f203285 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2865,15 +2865,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+\n+\twritten = len <= maxlen ? write_in_full(fd, logrec, len) : -1;\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@@ -2885,19 +2907,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 (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 (result) {\n \t\tint save_errno = errno;\n \t\tclose(logfd);\n \t\terror(\"Unable to append to %s\", log_file);\n-- \n2.0.0.467.g08c0633\n"},{"id":"244531","messageId":"1403111346-18466-7-git-send-email-sahlberg@google.com","threadId":"36949","inReplyTo":"1403111346-18466-1-git-send-email-sahlberg@google.com","subject":"[PATCH v3 06/14] lockfile.c: make hold_lock_file_for_append preserve meaningful errno","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-06-18T17:08:58Z","receivedAt":"2014-06-18T17:08:58Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Update hold_lock_file_for_append and copy_fd to return a meaningful errno\non failure.\n\nSigned-off-by: Ronnie Sahlberg \n---\n copy.c | 20 +++++++++++++-------\n lockfile.c | 7 ++++++-\n 2 files changed, 19 insertions(+), 8 deletions(-)\n\ndiff --git a/copy.c b/copy.c\nindex a7f58fd..5cb8679 100644\n--- a/copy.c\n+++ b/copy.c\n@@ -9,10 +9,12 @@ int copy_fd(int ifd, int ofd)\n \t\tif (!len)\n \t\t\tbreak;\n \t\tif (len < 0) {\n-\t\t\tint read_error = errno;\n+\t\t\tint save_errno = errno;\n \t\t\tclose(ifd);\n-\t\t\treturn error(\"copy-fd: read returned %s\",\n-\t\t\t\t strerror(read_error));\n+\t\t\terror(\"copy-fd: read returned %s\",\n+\t\t\t strerror(save_errno));\n+\t\t\terrno = save_errno;\n+\t\t\treturn -1;\n \t\t}\n \t\twhile (len) {\n \t\t\tint written = xwrite(ofd, buf, len);\n@@ -22,12 +24,16 @@ int copy_fd(int ifd, int ofd)\n \t\t\t}\n \t\t\telse if (!written) {\n \t\t\t\tclose(ifd);\n-\t\t\t\treturn error(\"copy-fd: write returned 0\");\n+\t\t\t\terror(\"copy-fd: write returned 0\");\n+\t\t\t\terrno = EAGAIN;\n+\t\t\t\treturn -1;\n \t\t\t} else {\n-\t\t\t\tint write_error = errno;\n+\t\t\t\tint save_errno = errno;\n \t\t\t\tclose(ifd);\n-\t\t\t\treturn error(\"copy-fd: write returned %s\",\n-\t\t\t\t\t strerror(write_error));\n+\t\t\t\terror(\"copy-fd: write returned %s\",\n+\t\t\t\t strerror(save_errno));\n+\t\t\t\terrno = save_errno;\n+\t\t\t\treturn -1;\n \t\t\t}\n \t\t}\n \t}\ndiff --git a/lockfile.c b/lockfile.c\nindex a921d77..32f4681 100644\n--- a/lockfile.c\n+++ b/lockfile.c\n@@ -217,15 +217,20 @@ int hold_lock_file_for_append(struct lock_file *lk, const char *path, int flags)\n \torig_fd = open(path, O_RDONLY);\n \tif (orig_fd < 0) {\n \t\tif (errno != ENOENT) {\n+\t\t\tint save_errno = errno;\n \t\t\tif (flags & LOCK_DIE_ON_ERROR)\n \t\t\t\tdie(\"cannot open '%s' for copying\", path);\n \t\t\tclose(fd);\n-\t\t\treturn error(\"cannot open '%s' for copying\", path);\n+\t\t\terror(\"cannot open '%s' for copying\", path);\n+\t\t\terrno = save_errno;\n+\t\t\treturn -1;\n \t\t}\n \t} else if (copy_fd(orig_fd, fd)) {\n+\t\tint save_errno = errno;\n \t\tif (flags & LOCK_DIE_ON_ERROR)\n \t\t\texit(128);\n \t\tclose(fd);\n+\t\terrno = save_errno;\n \t\treturn -1;\n \t}\n \treturn fd;\n-- \n2.0.0.467.g08c0633\n"},{"id":"244534","messageId":"1403111346-18466-8-git-send-email-sahlberg@google.com","threadId":"36949","inReplyTo":"1403111346-18466-1-git-send-email-sahlberg@google.com","subject":"[PATCH v3 07/14] refs.c: add a transaction function to append a reflog entry","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-06-18T17:08:59Z","receivedAt":"2014-06-18T17:08:59Z","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---\n refs.c | 101 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-\n refs.h | 12 ++++++++\n 2 files changed, 112 insertions(+), 1 deletion(-)\n\ndiff --git a/refs.c b/refs.c\nindex f203285..d673a0f 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3388,6 +3388,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@@ -3405,6 +3406,12 @@ struct ref_update {\n \tstruct ref_lock *lock;\n \tint type;\n \tchar *msg;\n+\n+\t/* used by reflog updates */\n+\tint reflog_fd;\n+\tstruct lock_file reflog_lock;\n+\tchar *committer;\n+\n \tconst char refname[FLEX_ARRAY];\n };\n \n@@ -3454,6 +3461,7 @@ void transaction_free(struct ref_transaction *transaction)\n \n \tfor (i = 0; i < transaction->nr; i++) {\n \t\tfree(transaction->updates[i]->msg);\n+\t\tfree(transaction->updates[i]->committer);\n \t\tfree(transaction->updates[i]);\n \t}\n \tfree(transaction->updates);\n@@ -3474,6 +3482,42 @@ 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, int flags,\n+\t\t\t struct strbuf *err)\n+{\n+\tstruct ref_update *update;\n+\n+\tif (transaction->state != REF_TRANSACTION_OPEN)\n+\t\tdie(\"BUG: update_reflog called for transaction that is not \"\n+\t\t \"open\");\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@@ -3640,7 +3684,28 @@ 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'. %s\";\n+\n+\t\t\tret = -1;\n+\t\t\tif (err)\n+\t\t\t\t strbuf_addf(err, str, update->refname,\n+\t\t\t\t\t strerror(errno));\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@@ -3676,6 +3741,40 @@ 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\terror(\"Could write to reflog: %s. %s\",\n+\t\t\t update->refname, strerror(errno));\n+\t\t\trollback_lock_file(&update->reflog_lock);\n+\t\t\tupdate->reflog_fd = -1;\n+\t\t}\n+\t}\n+\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->reflog_fd == -1)\n+\t\t\tcontinue;\n+\t\tif (commit_lock_file(&update->reflog_lock)) {\n+\t\t\terror(\"Could not commit reflog: %s. %s\",\n+\t\t\t update->refname, strerror(errno));\n+\t\t\tupdate->reflog_fd = -1;\n+\t\t}\n+\t}\n+\n \tif (repack_without_refs(delnames, delnum, err))\n \t\tret = -1;\n \tfor (i = 0; i < delnum; i++)\ndiff --git a/refs.h b/refs.h\nindex cf48351..bf52068 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -328,6 +328,18 @@ int transaction_delete_sha1(struct ref_transaction *transaction,\n \t\t\t struct strbuf *err);\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, 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 * atomically as possible. Return a nonzero value if there is a\n * problem.\n-- \n2.0.0.467.g08c0633\n"},{"id":"244524","messageId":"1403111346-18466-9-git-send-email-sahlberg@google.com","threadId":"36949","inReplyTo":"1403111346-18466-1-git-send-email-sahlberg@google.com","subject":"[PATCH v3 08/14] refs.c: add a flag to allow reflog updates to truncate the log","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-06-18T17:09:00Z","receivedAt":"2014-06-18T17:09:00Z","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\nupdate.\n\nSigned-off-by: Ronnie Sahlberg \n---\n refs.c | 17 +++++++++++++++--\n refs.h | 10 +++++++++-\n 2 files changed, 24 insertions(+), 3 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex d673a0f..c33d19e 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3741,7 +3741,12 @@ int transaction_commit(struct ref_transaction *transaction,\n \t\t}\n \t}\n \n-\t/* Update all reflog files */\n+\t/*\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@@ -3749,7 +3754,15 @@ 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\tftruncate(update->reflog_fd, 0)) {\n+\t\t\t\terror(\"Could not truncate reflog: %s. %s\",\n+\t\t\t\t update->refname, strerror(errno));\n+\t\t\t\trollback_lock_file(&update->reflog_lock);\n+\t\t\t\tupdate->reflog_fd = -1;\n+\t\t\t\tcontinue;\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 bf52068..f14c8db 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -328,7 +328,15 @@ int transaction_delete_sha1(struct ref_transaction *transaction,\n \t\t\t struct strbuf *err);\n \n /*\n- * Append a reflog entry for refname.\n+ * Flags controlling transaction_update_reflog().\n+ * REFLOG_TRUNCATE: Truncate the reflog.\n+ *\n+ * Flags >= 0x100 are reserved for internal use.\n+ */\n+#define REFLOG_TRUNCATE 0x01\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 */\n int transaction_update_reflog(struct ref_transaction *transaction,\n \t\t\t const char *refname,\n-- \n2.0.0.467.g08c0633\n"},{"id":"244526","messageId":"1403111346-18466-10-git-send-email-sahlberg@google.com","threadId":"36949","inReplyTo":"1403111346-18466-1-git-send-email-sahlberg@google.com","subject":"[PATCH v3 09/14] refs.c: only write reflog update if msg is non-NULL","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-06-18T17:09:01Z","receivedAt":"2014-06-18T17:09:01Z","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 | 5 +++--\n refs.h | 1 +\n 2 files changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex c33d19e..d6df28d 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3763,8 +3763,9 @@ int transaction_commit(struct ref_transaction *transaction,\n \t\t\t\tupdate->reflog_fd = -1;\n \t\t\t\tcontinue;\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\tif (update->msg &&\n+\t\t log_ref_write_fd(update->reflog_fd,\n+\t\t\t\t update->old_sha1, update->new_sha1,\n \t\t\t\t update->committer, update->msg)) {\n \t\t\terror(\"Could write to reflog: %s. %s\",\n \t\t\t update->refname, strerror(errno));\ndiff --git a/refs.h b/refs.h\nindex f14c8db..1d7906c 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -337,6 +337,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.467.g08c0633\n"},{"id":"244533","messageId":"1403111346-18466-11-git-send-email-sahlberg@google.com","threadId":"36949","inReplyTo":"1403111346-18466-1-git-send-email-sahlberg@google.com","subject":"[PATCH v3 10/14] refs.c: allow multiple reflog updates during a single transaction","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-06-18T17:09:02Z","receivedAt":"2014-06-18T17:09:02Z","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\nthat touches the reflog, and that all further updates can just write the\nreflog entry 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 | 46 +++++++++++++++++++++++++++++++++++++---------\n 1 file changed, 37 insertions(+), 9 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex d6df28d..ad60231 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -30,6 +30,12 @@ static unsigned char refname_disposition[256] = {\n */\n #define REF_ISPRUNING\t0x0100\n /*\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 0x0200\n+\n+/*\n * Try to read one refname component from the front of refname.\n * Return the length of the component found, or -1 if the component is\n * not legal. It is legal if it is something reasonable to have under\n@@ -3391,7 +3397,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 * while locking the ref, set have_old to 1 and set old_sha1 to the\n@@ -3401,7 +3407,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@@ -3409,8 +3415,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 \tchar *committer;\n+\tstruct ref_update *orig_update; /* For UPDATE_REFLOG_NOLOCK */\n \n \tconst char refname[FLEX_ARRAY];\n };\n@@ -3492,12 +3499,27 @@ int transaction_update_reflog(struct ref_transaction *transaction,\n \t\t\t struct strbuf *err)\n {\n \tstruct ref_update *update;\n+\tint i;\n \n \tif (transaction->state != REF_TRANSACTION_OPEN)\n \t\tdie(\"BUG: update_reflog called for transaction that is not \"\n \t\t \"open\");\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\tupdate->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@@ -3513,7 +3535,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@@ -3690,10 +3711,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'. %s\";\n \n@@ -3759,7 +3785,7 @@ int transaction_commit(struct ref_transaction *transaction,\n \t\t\t\tftruncate(update->reflog_fd, 0)) {\n \t\t\t\terror(\"Could not truncate reflog: %s. %s\",\n \t\t\t\t update->refname, strerror(errno));\n-\t\t\t\trollback_lock_file(&update->reflog_lock);\n+\t\t\t\trollback_lock_file(update->reflog_lock);\n \t\t\t\tupdate->reflog_fd = -1;\n \t\t\t\tcontinue;\n \t\t\t}\n@@ -3769,7 +3795,7 @@ int transaction_commit(struct ref_transaction *transaction,\n \t\t\t\t update->committer, update->msg)) {\n \t\t\terror(\"Could write to reflog: %s. %s\",\n \t\t\t update->refname, strerror(errno));\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@@ -3780,9 +3806,11 @@ 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\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\terror(\"Could not commit reflog: %s. %s\",\n \t\t\t update->refname, strerror(errno));\n \t\t\tupdate->reflog_fd = -1;\n-- \n2.0.0.467.g08c0633\n"},{"id":"244529","messageId":"1403111346-18466-12-git-send-email-sahlberg@google.com","threadId":"36949","inReplyTo":"1403111346-18466-1-git-send-email-sahlberg@google.com","subject":"[PATCH v3 11/14] reflog.c: use a reflog transaction when writing during expire","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-06-18T17:09:03Z","receivedAt":"2014-06-18T17:09:03Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Use a transaction for all updates during expire_reflog.\n\nSigned-off-by: Ronnie Sahlberg \n---\n builtin/reflog.c | 84 ++++++++++++++++++++++++--------------------------------\n refs.c | 4 +--\n refs.h | 2 +-\n 3 files changed, 39 insertions(+), 51 deletions(-)\n\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex e8a8fb1..f11fee3 100644\n--- a/builtin/reflog.c\n+++ b/builtin/reflog.c\n@@ -32,8 +32,11 @@ struct cmd_reflog_expire_cb {\n \tint recno;\n };\n \n+static struct strbuf err = STRBUF_INIT;\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 +319,18 @@ 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\tif (transaction_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 &err))\n+\t\t\treturn -1;\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@@ -353,29 +354,26 @@ 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-\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-\t * getting updated.\n-\t */\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+\tcb.t = transaction_begin(&err);\n+\tif (!cb.t) {\n+\t\tstatus |= error(\"%s\", err.buf);\n+\t\tgoto cleanup;\n+\t}\n+\tif (transaction_update_reflog(cb.t, cb.refname, null_sha1, null_sha1,\n+\t\t\t\t NULL, 0, 0, NULL, REFLOG_TRUNCATE,\n+\t\t\t\t &err)) {\n+\t\tstatus |= error(\"%s\", err.buf);\n+\t\tgoto cleanup;\n \t}\n-\n \tcb.cmd = cmd;\n \n \tif (!cmd->expire_unreachable || !strcmp(ref, \"HEAD\")) {\n@@ -407,7 +405,10 @@ static int expire_reflog(const char *ref, const unsigned char *sha1, int unused,\n \t\tmark_reachable(&cb);\n \t}\n \n-\tfor_each_reflog_ent(ref, expire_reflog_ent, &cb);\n+\tif (for_each_reflog_ent(ref, expire_reflog_ent, &cb)) {\n+\t\tstatus |= error(\"%s\", err.buf);\n+\t\tgoto cleanup;\n+\t}\n \n \tif (cb.unreachable_expire_kind != UE_ALWAYS) {\n \t\tif (cb.unreachable_expire_kind == UE_HEAD) {\n@@ -420,32 +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.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-\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} 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+\tif (!cmd->dry_run) {\n+\t\tif (cmd->updateref &&\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, &err)) {\n+\t\t\tstatus |= error(\"%s\", err.buf);\n+\t\t} else if (transaction_commit(cb.t, &err)) {\n+\t\t\tstatus |= error(\"%s\", err.buf);\n \t\t}\n \t}\n-\tfree(newlog_path);\n-\tfree(log_file);\n-\tunlock_ref(lock);\n+ cleanup:\n+\ttransaction_free(cb.t);\n+\tstrbuf_release(&err);\n \treturn status;\n }\n \ndiff --git a/refs.c b/refs.c\nindex ad60231..1288c49 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3493,7 +3493,7 @@ 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 const char *email,\n \t\t\t unsigned long timestamp, int tz,\n \t\t\t const char *msg, int flags,\n \t\t\t struct strbuf *err)\n@@ -3769,7 +3769,7 @@ int transaction_commit(struct ref_transaction *transaction,\n \n \t/*\n \t * Update all reflog files\n-\t * We have already done all ref updates and deletes.\n+\t * We have already committed 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 */\ndiff --git a/refs.h b/refs.h\nindex 1d7906c..0564955 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -343,7 +343,7 @@ 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 const char *email,\n \t\t\t unsigned long timestamp, int tz,\n \t\t\t const char *msg, int flags,\n \t\t\t struct strbuf *err);\n-- \n2.0.0.467.g08c0633\n"},{"id":"244521","messageId":"1403111346-18466-13-git-send-email-sahlberg@google.com","threadId":"36949","inReplyTo":"1403111346-18466-1-git-send-email-sahlberg@google.com","subject":"[PATCH v3 12/14] refs.c: rename log_ref_setup to create_reflog","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-06-18T17:09:04Z","receivedAt":"2014-06-18T17:09:04Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"log_ref_setup is used to do several semi-related things :\n* sometimes it will create a new reflog including missing parent directories\n and cleaning up any conflicting stale directories in the path.\n* fill in a filename buffer for the full path to the reflog.\n* unconditionally re-adjust the permissions for the file.\n\nThis function is only called from two places: checkout.c where it is always\nused to create a reflog and refs.c:log_ref_write where it sometimes are\nused to create a reflog and sometimes just used to fill in the filename.\n\nRename log_ref_setup to create_reflog and change it to only take the\nrefname as argument to make its signature similar to delete_reflog and\nreflog_exists. Change create_reflog to ignore log_all_ref_updates and\n\"unconditionally\" create the reflog when called. Since checkout.c always\nwants to create a reflog we can call create_reflog directly and avoid the\ntemp-and-log_all_ref_update dance.\n\nIn log_ref_write, only call create_reflog iff we want to create a reflog\nand if the reflog does not yet exist. This means that for the common case\nwhere the log already exists we now only need to perform a single lstat()\ninstead of a open(O_CREAT)+lstat()+close().\n\nSigned-off-by: Ronnie Sahlberg \n---\n builtin/checkout.c | 8 +-------\n refs.c | 22 +++++++++++++---------\n refs.h | 8 +++-----\n 3 files changed, 17 insertions(+), 21 deletions(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex f1dc56e..808c58f 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -583,19 +583,13 @@ static void update_refs_for_switch(const struct checkout_opts *opts,\n \tif (opts->new_branch) {\n \t\tif (opts->new_orphan_branch) {\n \t\t\tif (opts->new_branch_log && !log_all_ref_updates) {\n-\t\t\t\tint temp;\n-\t\t\t\tchar log_file[PATH_MAX];\n \t\t\t\tchar *ref_name = mkpath(\"refs/heads/%s\", opts->new_orphan_branch);\n \n-\t\t\t\ttemp = log_all_ref_updates;\n-\t\t\t\tlog_all_ref_updates = 1;\n-\t\t\t\tif (log_ref_setup(ref_name, log_file, sizeof(log_file))) {\n+\t\t\t\tif (create_reflog(ref_name)) {\n \t\t\t\t\tfprintf(stderr, _(\"Can not do reflog for '%s'\\n\"),\n \t\t\t\t\t opts->new_orphan_branch);\n-\t\t\t\t\tlog_all_ref_updates = temp;\n \t\t\t\t\treturn;\n \t\t\t\t}\n-\t\t\t\tlog_all_ref_updates = temp;\n \t\t\t}\n \t\t}\n \t\telse\ndiff --git a/refs.c b/refs.c\nindex 1288c49..9653a01 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2822,16 +2822,16 @@ static int copy_msg(char *buf, const char *msg)\n }\n \n /* This function must set a meaningful errno on failure */\n-int log_ref_setup(const char *refname, char *logfile, int bufsize)\n+int create_reflog(const char *refname)\n {\n \tint logfd, oflags = O_APPEND | O_WRONLY;\n+\tchar logfile[PATH_MAX];\n \n-\tgit_snpath(logfile, bufsize, \"logs/%s\", refname);\n-\tif (log_all_ref_updates &&\n-\t (starts_with(refname, \"refs/heads/\") ||\n-\t starts_with(refname, \"refs/remotes/\") ||\n-\t starts_with(refname, \"refs/notes/\") ||\n-\t !strcmp(refname, \"HEAD\"))) {\n+\tgit_snpath(logfile, sizeof(logfile), \"logs/%s\", refname);\n+\tif (starts_with(refname, \"refs/heads/\") ||\n+\t starts_with(refname, \"refs/remotes/\") ||\n+\t starts_with(refname, \"refs/notes/\") ||\n+\t !strcmp(refname, \"HEAD\")) {\n \t\tif (safe_create_leading_directories(logfile) < 0) {\n \t\t\tint save_errno = errno;\n \t\t\terror(\"unable to create directory for %s\", logfile);\n@@ -2900,16 +2900,20 @@ static int log_ref_write_fd(int fd, const unsigned char *old_sha1,\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, oflags = O_APPEND | O_WRONLY;\n+\tint logfd, result = 0, oflags = O_APPEND | O_WRONLY;\n \tchar log_file[PATH_MAX];\n \n \tif (log_all_ref_updates < 0)\n \t\tlog_all_ref_updates = !is_bare_repository();\n \n-\tresult = log_ref_setup(refname, log_file, sizeof(log_file));\n+\tif (log_all_ref_updates && !reflog_exists(refname))\n+\t\tresult = create_reflog(refname);\n+\n \tif (result)\n \t\treturn result;\n \n+\tgit_snpath(log_file, sizeof(log_file), \"logs/%s\", refname);\n+\n \tlogfd = open(log_file, oflags);\n \tif (logfd < 0)\n \t\treturn 0;\ndiff --git a/refs.h b/refs.h\nindex 0564955..e7892fc 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -203,11 +203,6 @@ extern int commit_ref(struct ref_lock *lock);\n /** Release any lock taken but not written. **/\n extern void unlock_ref(struct ref_lock *lock);\n \n-/*\n- * Setup reflog before using. Set errno to something meaningful on failure.\n- */\n-int log_ref_setup(const char *refname, char *logfile, int bufsize);\n-\n /** Reads log for the value of ref during at_time. **/\n extern int read_ref_at(const char *refname, unsigned long at_time, int cnt,\n \t\t unsigned char *sha1, char **msg,\n@@ -216,6 +211,9 @@ extern int read_ref_at(const char *refname, unsigned long at_time, int cnt,\n /** Check if a particular reflog exists */\n extern int reflog_exists(const char *refname);\n \n+/** Create reflog. Set errno to something meaningful on failure. */\n+extern int create_reflog(const char *refname);\n+\n /** Delete a reflog */\n extern int delete_reflog(const char *refname);\n \n-- \n2.0.0.467.g08c0633\n"},{"id":"244530","messageId":"1403111346-18466-14-git-send-email-sahlberg@google.com","threadId":"36949","inReplyTo":"1403111346-18466-1-git-send-email-sahlberg@google.com","subject":"[PATCH v3 13/14] refs.c: make unlock_ref/close_ref/commit_ref static","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-06-18T17:09:05Z","receivedAt":"2014-06-18T17:09:05Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"unlock|close|commit_ref can be made static since there are no more external\ncallers.\n\nSigned-off-by: Ronnie Sahlberg \n---\n refs.c | 24 ++++++++++++------------\n refs.h | 9 ---------\n 2 files changed, 12 insertions(+), 21 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 9653a01..ff98682 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1960,6 +1960,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 /* This function should make sure errno is meaningful on error */\n static struct ref_lock *verify_lock(struct ref_lock *lock,\n \tconst unsigned char *old_sha1, int mustexist)\n@@ -2769,7 +2779,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;\n@@ -2777,7 +2787,7 @@ 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;\n@@ -2785,16 +2795,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 e7892fc..5054388 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -194,15 +194,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-/** Release any lock taken but not written. **/\n-extern void unlock_ref(struct ref_lock *lock);\n-\n /** Reads log for the value of ref during at_time. **/\n extern int read_ref_at(const char *refname, unsigned long at_time, int cnt,\n \t\t unsigned char *sha1, char **msg,\n-- \n2.0.0.467.g08c0633\n"},{"id":"244525","messageId":"1403111346-18466-15-git-send-email-sahlberg@google.com","threadId":"36949","inReplyTo":"1403111346-18466-1-git-send-email-sahlberg@google.com","subject":"[PATCH v3 14/14] refs.c: remove lock_any_ref_for_update","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-06-18T17:09:06Z","receivedAt":"2014-06-18T17:09:06Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"No one is using this function so we can delete it.\n\nSigned-off-by: Ronnie Sahlberg \n---\n refs.c | 7 -------\n refs.h | 10 +---------\n 2 files changed, 1 insertion(+), 16 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex ff98682..95c3eb8 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2205,13 +2205,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 5054388..b674c20 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -178,21 +178,13 @@ extern int ref_exists(const char *);\n extern int peel_ref(const char *refname, unsigned char *sha1);\n \n /*\n- * Flags controlling lock_any_ref_for_update(), transaction_update_sha1(),\n- * transaction_create_sha1(), etc.\n+ * Flags controlling transaction_update_sha1(), transaction_create_sha1(), etc.\n * REF_NODEREF: act on the ref directly, instead of dereferencing\n * symbolic references.\n *\n * Flags >= 0x100 are reserved for internal use.\n */\n #define REF_NODEREF\t0x01\n-/*\n- * Locks any ref (for 'HEAD' type refs) and sets errno to something\n- * meaningful on failure.\n- */\n-extern struct ref_lock *lock_any_ref_for_update(const char *refname,\n-\t\t\t\t\t\tconst unsigned char *old_sha1,\n-\t\t\t\t\t\tint flags, int *type_p);\n \n /** Reads log for the value of ref during at_time. **/\n extern int read_ref_at(const char *refname, unsigned long at_time, int cnt,\n-- \n2.0.0.467.g08c0633\n"},{"id":"244591","messageId":"xmqqha3ihs7y.fsf@gitster.dls.corp.google.com","threadId":"36949","inReplyTo":"1403111346-18466-2-git-send-email-sahlberg@google.com","subject":"Re: [PATCH v3 01/14] refs.c make ref_transaction_create a wrapper to ref_transaction_update","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-06-18T20:34:41Z","receivedAt":"2014-06-18T20:34:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ronnie Sahlberg writes:\n\n> Signed-off-by: Ronnie Sahlberg \n> ---\n> refs.c | 13 ++-----------\n> refs.h | 7 ++++---\n> 2 files changed, 6 insertions(+), 14 deletions(-)\n>\n> diff --git a/refs.c b/refs.c\n> index dfbf003..a9f91ab 100644\n> --- a/refs.c\n> +++ b/refs.c\n> @@ -3487,23 +3487,14 @@ int ref_transaction_create(struct ref_transaction *transaction,\n> \t\t\t int flags, const char *msg,\n> \t\t\t struct strbuf *err)\n> {\n> -\tstruct ref_update *update;\n> -\n> \tif (transaction->state != REF_TRANSACTION_OPEN)\n> \t\tdie(\"BUG: create called for transaction that is not open\");\n> \n> \tif (!new_sha1 || is_null_sha1(new_sha1))\n> \t\tdie(\"BUG: create ref with null new_sha1\");\n> \n> -\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, err);\n> }\n\nHmmm.\n\nThis probably logically belongs to the end of the previous series\nand not necessarily tied to reflog transaction, no?\n\nAt the beginning of ref_transaction_update() there also is the same\nguard on transaction->state, and having both feels somewhat iffy.\nOf course it will give a wrong BUG message if we removed the check\nfrom this function, so perhaps the code is OK as-is.\n\n> int ref_transaction_delete(struct ref_transaction *transaction,\n> diff --git a/refs.h b/refs.h\n> index db463d0..495740d 100644\n> --- a/refs.h\n> +++ b/refs.h\n> @@ -283,9 +283,10 @@ struct ref_transaction *ref_transaction_begin(struct strbuf *err);\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. A failure to update\n> * means that the transaction as a whole has failed and will need to be\n> * rolled back.\n"},{"id":"244594","messageId":"xmqqfvj2hs5g.fsf@gitster.dls.corp.google.com","threadId":"36949","inReplyTo":"1403111346-18466-5-git-send-email-sahlberg@google.com","subject":"Re: [PATCH v3 04/14] refs.c: add a new update_type field to ref_update","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-06-18T20:36:11Z","receivedAt":"2014-06-18T20:36:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ronnie Sahlberg writes:\n\n> Add a field that describes what type of update this refers to. For now\n> the only type is UPDATE_SHA1 but we will soon add more types.\n>\n> Signed-off-by: Ronnie Sahlberg \n> ---\n> refs.c | 25 +++++++++++++++++++++----\n> 1 file changed, 21 insertions(+), 4 deletions(-)\n>\n> diff --git a/refs.c b/refs.c\n> index 4e3d4c3..4129de6 100644\n> --- a/refs.c\n> +++ b/refs.c\n> @@ -3374,6 +3374,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\nindent with SP?\n\nUnlike an array initialisation, e.g.\n\n\tint foo[] = { 1,2,3,4,5, };\n\nsome compilers we support complain if enum definition ends with a\ntrailing comma.\n"},{"id":"244600","messageId":"xmqqbntpj54t.fsf_-_@gitster.dls.corp.google.com","threadId":"36949","inReplyTo":"xmqqfvj2hs5g.fsf@gitster.dls.corp.google.com","subject":"Re* [PATCH v3 04/14] refs.c: add a new update_type field to ref_update","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-06-18T21:10:26Z","receivedAt":"2014-06-18T21:10:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Ronnie Sahlberg writes:\n>\n>> Add a field that describes what type of update this refers to. For now\n>> the only type is UPDATE_SHA1 but we will soon add more types.\n>>\n>> Signed-off-by: Ronnie Sahlberg \n>> ---\n>> refs.c | 25 +++++++++++++++++++++----\n>> 1 file changed, 21 insertions(+), 4 deletions(-)\n>>\n>> diff --git a/refs.c b/refs.c\n>> index 4e3d4c3..4129de6 100644\n>> --- a/refs.c\n>> +++ b/refs.c\n>> @@ -3374,6 +3374,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> indent with SP?\n>\n> Unlike an array initialisation, e.g.\n>\n> \tint foo[] = { 1,2,3,4,5, };\n>\n> some compilers we support complain if enum definition ends with a\n> trailing comma.\n\nI do recall we had fixes to drop the comma after the last element in\nenum definition in the past, in response real compilation breakages\non some platforms. But there is a curious thing:\n\n git grep -A 'enum ' master -- \\*.c\n\ntells me that builtin/clean.c would fail to compile for those folks.\n\nHere is an off-topic \"fix\" that may no longer be needed.\n\n builtin/clean.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/clean.c b/builtin/clean.c\nindex 9a91515..27701d2 100644\n--- a/builtin/clean.c\n+++ b/builtin/clean.c\n@@ -48,7 +48,7 @@ enum color_clean {\n \tCLEAN_COLOR_PROMPT = 2,\n \tCLEAN_COLOR_HEADER = 3,\n \tCLEAN_COLOR_HELP = 4,\n-\tCLEAN_COLOR_ERROR = 5,\n+\tCLEAN_COLOR_ERROR = 5\n };\n \n #define MENU_OPTS_SINGLETON\t\t01\n"},{"id":"245339","messageId":"CAL=YDWngL7mhWs601zEfiCFbZrHffW7sN7pB_dUaoE8DaPedUA@mail.gmail.com","threadId":"36949","inReplyTo":"xmqqbntpj54t.fsf_-_@gitster.dls.corp.google.com","subject":"Re: Re* [PATCH v3 04/14] refs.c: add a new update_type field to ref_update","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-07-02T18:27:25Z","receivedAt":"2014-07-02T18:27:25Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"On Wed, Jun 18, 2014 at 2:10 PM, Junio C Hamano wrote:\n> Junio C Hamano writes:\n>\n>> Ronnie Sahlberg writes:\n>>\n>>> Add a field that describes what type of update this refers to. For now\n>>> the only type is UPDATE_SHA1 but we will soon add more types.\n>>>\n>>> Signed-off-by: Ronnie Sahlberg \n>>> ---\n>>> refs.c | 25 +++++++++++++++++++++----\n>>> 1 file changed, 21 insertions(+), 4 deletions(-)\n>>>\n>>> diff --git a/refs.c b/refs.c\n>>> index 4e3d4c3..4129de6 100644\n>>> --- a/refs.c\n>>> +++ b/refs.c\n>>> @@ -3374,6 +3374,10 @@ int for_each_reflog(each_ref_fn fn, void *cb_data)\n>>> return retval;\n>>> }\n>>>\n>>> +enum transaction_update_type {\n>>> + UPDATE_SHA1 = 0,\n>>\n>> indent with SP?\n>>\n>> Unlike an array initialisation, e.g.\n>>\n>> int foo[] = { 1,2,3,4,5, };\n>>\n>> some compilers we support complain if enum definition ends with a\n>> trailing comma.\n>\n> I do recall we had fixes to drop the comma after the last element in\n> enum definition in the past, in response real compilation breakages\n> on some platforms.\n\nYou are right. I think I recall this too on old c compilers on legacy\nunix boxens.\n\nI have fixed this and will resend the series.\n\n\nI have also checked additional enums in the sources and found 3 more places.\nI sent this as a single patch to the list with the subject :\n\n\"Subject: [PATCH] enums: remove trailing ',' after last item in enum\"\n\n\n\n> But there is a curious thing:\n>\n> git grep -A 'enum ' master -- \\*.c\n>\n> tells me that builtin/clean.c would fail to compile for those folks.\n\nMost likely. It might be that there are less and less people on\nreally old c-compilers these days.\n\n\n>\n> Here is an off-topic \"fix\" that may no longer be needed.\n>\n> builtin/clean.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/builtin/clean.c b/builtin/clean.c\n> index 9a91515..27701d2 100644\n> --- a/builtin/clean.c\n> +++ b/builtin/clean.c\n> @@ -48,7 +48,7 @@ enum color_clean {\n> CLEAN_COLOR_PROMPT = 2,\n> CLEAN_COLOR_HEADER = 3,\n> CLEAN_COLOR_HELP = 4,\n> - CLEAN_COLOR_ERROR = 5,\n> + CLEAN_COLOR_ERROR = 5\n> };\n>\n> #define MENU_OPTS_SINGLETON 01\n>\n\nThanks\nronnie sahlberg\n"}]}